Bug 25634 – Need a way to get the Frame for the code at the top of the JS stack

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Aaron Boodman]

Right now, in the Chromium port, there is ScriptController::retrieveActiveFrame(), which retrieves the WebCore Frame corresponding to the JS code at the bottom of the execution stack. We sometimes also need to get the Frame corresponding to the code at the top of execution stack, which can be different.

[Aaron Boodman]

Created attachment 30119 [details]
Patch

[Aaron Boodman]

Created attachment 30120 [details]
Fix formatting and other issues in ChangeLog

[Adam Barth]

We almost always want the bottom of the stack.  Why do we want to the top of the stack in these cases?

[Aaron Boodman]

(In reply to comment #3)
> We almost always want the bottom of the stack.  Why do we want to the top of
> the stack in these cases?

You can check out the related Chromium change. When an Extension API is called, we want to know which frame invoked it so that we can be more intelligent with the behavior of APIs based on what RenderView they came from.

For example: we have a createWindow() API. We want the new window be positioned relative to the window the API call originated from. In order to know which window the call originated from, we need to know which renderview it originated from. Since JavaScript calls can traverse frame boundaries, the bottom of the stack can give the wrong answer.

An easy example of this is the inspector. You can call chromium.windows.create() from the inspector. We want the new window to be positioned relative to the window you're inspecting, not the inspector window.

But there are other examples.

[Aaron Boodman]

Here's the related Chromium change:
http://codereview.chromium.org/113085

[Adam Barth]

(In reply to comment #4)
> You can check out the related Chromium change. When an Extension API is called,
> we want to know which frame invoked it so that we can be more intelligent with
> the behavior of APIs based on what RenderView they came from.

I don't understand why the extension case differs from the general web case.

> An easy example of this is the inspector. You can call
> chromium.windows.create() from the inspector. We want the new window to be
> positioned relative to the window you're inspecting, not the inspector window.

The context for evaluating scripts in the web inspect is all screwed up.  I don't think we should bend the web platform to fix the needs of the inspector.

The thing is, a bunch of these changes violate the HTML 5 spec.  For example, the change to postMessage means that the source would be determined via dynamic scope instead of via lexical scope, as required by the spec.

Similarly, in DOMWindowOpen and WindowSetLocation, you're now applying the frame navigation policy (shouldAllowNavigation) based on dynamic scope instead of lexical scope, in violation of the spec and the behavior of other browsers.

The changes to HTMLDocumentWrite and HTMLDocumentWriteln change our behavior when using these APIs.  Have you tested this to see whether this makes us more or less compatible with other browsers?  I suspect it makes use less compatible.

Basically, I don't think its a good idea to subtly change a bunch of behavior for "consistency" without considering the compatibility and compliance issues.

[Adam Barth]

I spoke with Aaron on #webkit.  I mistakenly thought this patch was changing our behavior.  I think we just have lots of bugs in this department.

We should change the V8 bindings to match the JSC bindings in where we use the lexical or dynamic global object, but we can do that in a separate patch.

[Aaron Boodman]

Adding a larger group of Chromium reviewers, hoping that somebody can give this an r+.

To clarify, this change does not change behavior. It's just adding a new method and renaming an existing one.

[Adam Barth]

Yes.  Please disregard my comment #6.  That's a bug for another day.  :)

[Dimitri Glazkov (Google)]

Landed as http://trac.webkit.org/changeset/43512.

[Adam Barth]

Follow up filed as https://bugs.webkit.org/show_bug.cgi?id=25706