Bug 29605 – [Chromium] Fix the V8 bindings' handling of window.top

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Nate Chapin]

I introduced a regression in the V8 bindings in http://trac.webkit.org/changeset/47130/trunk/WebCore/bindings/scripts/CodeGeneratorV8.pm.  We were special casing window.top, since we have disallowing shadowing on it, but it's also marked as replaceable.  
I believe the solution is:
1. Add back in the special case handling to ensure window.top is not v8::ReadOnly, as this causes a TypeError rather than silently failing.
2. Add in a FIXME to see if we can stop disallowing shadowing at some point in the future.
3. Remove V8ReadOnly from window.top, since in any case other than this that uses Replaceable, V8ReadOnly is redundant.

[Nate Chapin]

Created attachment 39865 [details]
patch

After rereading CodeGeneratorV8.pm, I found a usage of V8ReadOnly that I hadn't noticed before, so I won't be doing #3 as I had originally thought.

[Dimitri Glazkov (Google)]

Adam is your man here.

[Adam Barth]

Comment on attachment 39865 [details]
patch

We can't allow shadowing of window.top without confusing Flash and other plug-ins.  The correct solution here is to convince the JSC port not to mark |top| are replaceable, but that's a longer conversation.  In the meantime, we should fix the LayoutTest.  If this regression is in stable, we should backport this fix.

Thanks for the patch.

[WebKit Commit Bot]

Comment on attachment 39865 [details]
patch

Clearing flags on attachment: 39865

Committed r48598: <http://trac.webkit.org/changeset/48598>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.