Bug 29605

Summary: [Chromium] Fix the V8 bindings' handling of window.top
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, dglazkov, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch none

Description Nate Chapin 2009-09-21 12:57:49 PDT
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.
Comment 1 Nate Chapin 2009-09-21 13:11:02 PDT
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.
Comment 2 Dimitri Glazkov (Google) 2009-09-21 13:12:08 PDT
Adam is your man here.
Comment 3 Adam Barth 2009-09-21 13:46:35 PDT
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.
Comment 4 WebKit Commit Bot 2009-09-21 14:03:15 PDT
Comment on attachment 39865 [details]
patch

Clearing flags on attachment: 39865

Committed r48598: <http://trac.webkit.org/changeset/48598>
Comment 5 WebKit Commit Bot 2009-09-21 14:03:20 PDT
All reviewed patches have been landed.  Closing bug.