Bug 29605 - [Chromium] Fix the V8 bindings' handling of window.top
Summary: [Chromium] Fix the V8 bindings' handling of window.top
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-21 12:57 PDT by Nate Chapin
Modified: 2009-09-21 14:03 PDT (History)
4 users (show)

See Also:


Attachments
patch (1.57 KB, patch)
2009-09-21 13:11 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.