Bug 26006 – Sort out lexical / dynamic global object for window.location

• Status: RESOLVED
• Resolution: FIXED
• Component: Frames

[Adam Barth]

Our choice of lexicalGlobalObject or dynamicGlobalObject not correct.  The correct choices appear to be:

1) Use dynamicGlobalObject to find the user gesture.
2) Use dynamicGlobalObject to complete URLs.
3) Use lexicalGlobalObject to find the referrer.
4) Use lexicalGlobalObject for the frame navigation checks.
5) Use lexicalGlobalObject for the XSS checks.

Patch forthcoming.

[Adam Barth]

Created attachment 30647 [details]
patch

This patch isn't fully vetted yet, but it's bedtime.  Hopefully I'm nominate it for review tomorrow.

[Sam Weinig]

I would prefer we don't continue to use the term activeFrame, as I think it conveys nothing.  In fact, in the code you are changing, it was used to refer to the frame associated with the dynamic global object, but now you are associating it with the lexical global object. 

[Adam Barth]

> I would prefer we don't continue to use the term activeFrame,

Sure.  What term would you like?  How about "calling Frame?"

[Sam Weinig]

How about the lexicalFrame and the dynamicFrame.  Not the best names for sure, but they at least evoke the terms we use.

[Adam Barth]

> How about the lexicalFrame and the dynamicFrame.  Not the best names for sure,
> but they at least evoke the terms we use.

I'd like to use the same term in both the JSC and V8 bindings so they're easier to keep in sync.  The problem with lexicalFrame is that V8 would think that's a different frame because it would give you the lexicalFrame for the *current* function (i.e., the one we're implementing) not the lexical frame for the function that called this one...

[Adam Barth]

Created attachment 30653 [details]
patch

This patch uses the "callingFrame" name.  We can change the name later.  There are still a bunch more of these lexical/dynamic issues to sort out (e.g., window.open).

[Sam Weinig]

While I understand your desire to have a name that works with the terminology the v8 bindings folks decided to use, I don't think we should make it harder for the webkit folks because of that.  CallingFrame doesn't state which global object it is using clearly, and therefore is less than ideal for me.  I would much prefer to go with lexicalFrame and dynamicFrame and have the v8 bindings change to accommodate.

[Adam Barth]

Comment on attachment 30653 [details]
patch

Ok.  I'll spin up a new patch.

[Adam Barth]

Created attachment 30657 [details]
patch with lexicalFrame

I changed the JSC code to lexicalFrame and left the V8 code with callingFrame.  That way the variables match the terminology for each bindings.  (This patch is still compiling, but I can fix any typos before landing.)

[Sam Weinig]

Comment on attachment 30657 [details]
patch with lexicalFrame

Assuming it all compiles and stuff, r=me.  Thanks!

[Adam Barth]

Transmitting file data ..................
Committed revision 44135.