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 tree / graph
 
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
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

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.