Bug 93049 - V8Proxy::retrieveFrameFor*Context are used only by BindingState and should be removed as separate functions
Summary: V8Proxy::retrieveFrameFor*Context are used only by BindingState and should be...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 75793
  Show dependency tree / graph
 
Reported: 2012-08-02 16:58 PDT by Adam Barth
Modified: 2012-08-02 22:32 PDT (History)
4 users (show)

See Also:

Attachments
Patch (7.12 KB, patch)
2012-08-02 16:59 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
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-02 16:58:40 PDT 
V8Proxy::retrieveFrameFor*Context are used only by BindingState and should be removed as separate functions
Comment 1 Adam Barth 2012-08-02 16:59:43 PDT 
Created attachment 156210 [details]
Patch
Comment 2 Erik Arvidsson 2012-08-02 17:05:21 PDT 
I'm using retrieveFrameForCallingContext in a patch I'm working on. What should I be using after this?
Comment 3 Adam Barth 2012-08-02 17:09:15 PDT 
> I'm using retrieveFrameForCallingContext in a patch I'm working on. What should I be using after this?

Bug 93038 has a bunch of examples of removing calls to retrieveFrameForCallingContext.

The longer answer is that almost all callers of this function are wrong.  This series of patches will result in very few places in the bindings operating in terms of Frames.  Almost every instance of the type "Frame" in the bindings is wrong.
Comment 4 Erik Arvidsson 2012-08-02 18:11:06 PDT 
I have a case where I need to use the calling ScriptExecutionContext for someWindow.focus(). It is needed to determine if focus should be allowed.
Comment 5 Adam Barth 2012-08-02 18:12:35 PDT 
That's a prototypical example.  You want activeScriptExecutionContext(BindingState*), which doesn't yet exist but will when I'm done.  Almost everyone wants the activeScriptExecutionContext rather than anything having to do with a Frame.
Comment 6 Erik Arvidsson 2012-08-02 18:22:25 PDT 
(In reply to comment #5)
> That's a prototypical example.  You want activeScriptExecutionContext(BindingState*), which doesn't yet exist but will when I'm done.  Almost everyone wants the activeScriptExecutionContext rather than anything having to do with a Frame.

Sounds good.
Comment 7 Eric Seidel (no email) 2012-08-02 20:27:15 PDT 
Comment on attachment 156210 [details]
Patch

LGTM.
Comment 8 WebKit Review Bot 2012-08-02 22:32:49 PDT 
Comment on attachment 156210 [details]
Patch

Clearing flags on attachment: 156210

Committed r124563: <http://trac.webkit.org/changeset/124563>
Comment 9 WebKit Review Bot 2012-08-02 22:32:53 PDT 
All reviewed patches have been landed.  Closing bug.