Bug 93307 - JSC should use BindingState to determine the activeDOMWindow
Summary: JSC should use BindingState to determine the activeDOMWindow
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on: 93339
Blocks: 75793
  Show dependency treegraph
 
Reported: 2012-08-06 16:25 PDT by Adam Barth
Modified: 2012-08-07 14:08 PDT (History)
10 users (show)

See Also:


Attachments
Patch (26.24 KB, patch)
2012-08-06 16:28 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
work in progress (30.54 KB, patch)
2012-08-06 16:28 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (31.33 KB, patch)
2012-08-06 16:49 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (31.32 KB, patch)
2012-08-06 16:50 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (31.36 KB, patch)
2012-08-06 17:00 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
resolve project.pbxproj merge conflict (32.29 KB, patch)
2012-08-06 17:22 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2012-08-06 16:25:38 PDT
JSC should use BindingState to determine the activeDOMWindow
Comment 1 Adam Barth 2012-08-06 16:28:17 PDT
Created attachment 156787 [details]
Patch
Comment 2 Adam Barth 2012-08-06 16:28:54 PDT
Created attachment 156789 [details]
work in progress
Comment 3 Gustavo Noronha (kov) 2012-08-06 16:35:05 PDT
Comment on attachment 156789 [details]
work in progress

Attachment 156789 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13436956
Comment 4 Adam Barth 2012-08-06 16:49:50 PDT
Created attachment 156796 [details]
Patch for landing
Comment 5 Adam Barth 2012-08-06 16:50:08 PDT
Created attachment 156797 [details]
Patch for landing
Comment 6 Adam Barth 2012-08-06 17:00:34 PDT
Created attachment 156803 [details]
Patch
Comment 7 Eric Seidel (no email) 2012-08-06 17:09:13 PDT
Comment on attachment 156803 [details]
Patch

LGTM.
Comment 8 Adam Barth 2012-08-06 17:22:49 PDT
Created attachment 156808 [details]
resolve project.pbxproj merge conflict
Comment 9 Adam Barth 2012-08-06 19:04:44 PDT
Committed r124835: <http://trac.webkit.org/changeset/124835>
Comment 10 Darin Adler 2012-08-07 13:45:51 PDT
Comment on attachment 156808 [details]
resolve project.pbxproj merge conflict

View in context: https://bugs.webkit.org/attachment.cgi?id=156808&action=review

> Source/WebCore/bindings/js/JSDOMBinding.h:-414
> -    // FIXME: Remove these functions in favor of activeContext and
> -    // firstContext, which return ScriptExecutionContext*. We prefer to use
> -    // ScriptExecutionContext* as the context object in the bindings.

Adam: Is this comment obsolete? Did you intentionally delete it rather than moving it to BindingState.h?
Comment 11 Adam Barth 2012-08-07 14:08:20 PDT
(In reply to comment #10)
> (From update of attachment 156808 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156808&action=review
> 
> > Source/WebCore/bindings/js/JSDOMBinding.h:-414
> > -    // FIXME: Remove these functions in favor of activeContext and
> > -    // firstContext, which return ScriptExecutionContext*. We prefer to use
> > -    // ScriptExecutionContext* as the context object in the bindings.
> 
> Adam: Is this comment obsolete? Did you intentionally delete it rather than moving it to BindingState.h?

I added these comments the last time I tried to fix this problem (Bug 75793).  I'm not 100% sure which approach will work.  Replacing these functions with activeContext and firstContext is one promising approach.  If I don't succeed this time around, I'll add comments explaining where we ended up.