Split V8Proxy::retrieveActiveFrame() into two methods.

Split V8Proxy::retrieveActiveFrame() into two methods.

We now have RetrieveFrameForCurrentContext() and 
RetrieveFrameForEnteredContext().

These terms means the same thing they do in V8::Context --
'current' is the top of the js stack and 'entered' is the
bottom.

I needed 'entered' to fix a bug in extensions where if you
call an extension API through the web inspector we get
confused and think the web inspector's view is the one who
called.

[Aaron Boodman, 2009-05-07 20:05:26]

I've updated the method names per our offline conversation and added additional
comments.

Note that this change will break files in
third_party\WebKit\WebCore\bindings\v8\custom. Should I submit those changes
first upstream?

[Aaron Boodman, 2009-05-09 20:05:30]

ping

I'll coordinate with whoever's doing the deps roll after my upstream change gets
landed to check this in.

[darin (slow to review), 2009-05-10 04:13:59]

LGTM

http://codereview.chromium.org/113085/diff/37/40
File chrome/renderer/extensions/extension_process_bindings.cc (right):

http://codereview.chromium.org/113085/diff/37/40#newcode72
Line 72: WebFrame* webframe = WebFrame::RetrieveFrameForEnteredContext();
I think it would be very helpful to document why you are using the entered frame
but the current renderview.

http://codereview.chromium.org/113085/diff/37/41
File chrome/renderer/extensions/renderer_extension_bindings.cc (right):

http://codereview.chromium.org/113085/diff/37/41#newcode52
Line 52: RenderView* renderview = GetRenderViewForCurrentContext();
ditto

http://codereview.chromium.org/113085/diff/37/41#newcode68
Line 68: RenderView* renderview = GetRenderViewForCurrentContext();
ditto... maybe there is some place (perhaps at the top of this file) where you
can explain the issue of entered vs current context so that someone reading this
code for the first time would have some, er,.. context.

http://codereview.chromium.org/113085/diff/37/44
File chrome/test/data/extensions/good/extension2/2/manifest.json (right):

http://codereview.chromium.org/113085/diff/37/44#newcode5
Line 5: "plugins_dir": "npapi"
did you mean to include this change in this CL?

http://codereview.chromium.org/113085/diff/37/47
File webkit/glue/webframe.h (right):

http://codereview.chromium.org/113085/diff/37/47#newcode40
Line 40: // in V8Proxy::retrieveFrameForEnteredContext() for more information.
I think you mean retrieveFrameForCurrentContext.

It is a bit fragile to make Chrome code refer to WebCore code for documentation.
 The WebCore code could change radically which could leave these references
stale.  I think it may be wise to document everything here (even if it is a bit
redundant).

http://codereview.chromium.org/113085/diff/37/51
File webkit/port/bindings/v8/ScriptController.h (right):

http://codereview.chromium.org/113085/diff/37/51#newcode206
Line 206: // V8Proxy::retrieveFrameForEnteredContext() for more information.
nit: Entered -> Current

in this case the reference to V8Proxy is totally fine since both of these
classes are (or will be, once we finish upstreaming) part of WebCore.

http://codereview.chromium.org/113085/diff/37/54
File webkit/port/bindings/v8/v8_proxy.h (right):

http://codereview.chromium.org/113085/diff/37/54#newcode304
Line 304: // NOTE: These cannot be declare as inline function, because VS
complains at
'cannot be declare[d] as...'

http://codereview.chromium.org/113085/diff/37/54#newcode307
Line 307: // Returns the frame object for the entered JS context.
nit: these comments seem redundant with the function names..  probably better to
just delete these comments.

[Aaron Boodman, 2009-05-11 17:22:33]

fyi

http://codereview.chromium.org/113085/diff/37/40
File chrome/renderer/extensions/extension_process_bindings.cc (right):

http://codereview.chromium.org/113085/diff/37/40#newcode72
Line 72: WebFrame* webframe = WebFrame::RetrieveFrameForEnteredContext();
On 2009/05/10 04:14:00, darin wrote:
> I think it would be very helpful to document why you are using the entered
frame
> but the current renderview.

This was a mistake, thanks for pointing it out. I added a short comment though.

http://codereview.chromium.org/113085/diff/37/41
File chrome/renderer/extensions/renderer_extension_bindings.cc (right):

http://codereview.chromium.org/113085/diff/37/41#newcode52
Line 52: RenderView* renderview = GetRenderViewForCurrentContext();
On 2009/05/10 04:14:00, darin wrote:
> ditto

Done.

http://codereview.chromium.org/113085/diff/37/44
File chrome/test/data/extensions/good/extension2/2/manifest.json (right):

http://codereview.chromium.org/113085/diff/37/44#newcode5
Line 5: "plugins_dir": "npapi"
On 2009/05/10 04:14:00, darin wrote:
> did you mean to include this change in this CL?

No, fixed.

http://codereview.chromium.org/113085/diff/37/47
File webkit/glue/webframe.h (right):

http://codereview.chromium.org/113085/diff/37/47#newcode40
Line 40: // in V8Proxy::retrieveFrameForEnteredContext() for more information.
On 2009/05/10 04:14:00, darin wrote:
> I think you mean retrieveFrameForCurrentContext.
> 
> It is a bit fragile to make Chrome code refer to WebCore code for
documentation.
>  The WebCore code could change radically which could leave these references
> stale.  I think it may be wise to document everything here (even if it is a
bit
> redundant).

Done.