Bug 132922 - Move subframe name getter lookup later in JSDOMWindow::getOwnPropertySlot
Summary: Move subframe name getter lookup later in JSDOMWindow::getOwnPropertySlot
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 132705
  Show dependency treegraph
 
Reported: 2014-05-14 14:34 PDT by Mark Hahnenberg
Modified: 2014-05-18 20:59 PDT (History)
2 users (show)

See Also:


Attachments
Patch (15.65 KB, patch)
2014-05-14 18:07 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2014-05-14 14:34:03 PDT
In JSDOMWindow::getOwnPropertySlot, we currently look for the property on the JSDOMWindow, then we search the window's subframes for name getters, then we look in the window's prototype chain. Apparently we were doing the lookup in this order to be compatible with Mozilla, but Mozilla does not implement this behavior. Instead, they do the lookup on the prototype before looking for subframe name getters. We should change this to match Mozilla. This has the convenient side effect of allowing us to cache lookups in the window's prototype chain.
Comment 1 Mark Hahnenberg 2014-05-14 18:07:38 PDT
Created attachment 231476 [details]
Patch
Comment 2 Geoffrey Garen 2014-05-14 21:18:01 PDT
Comment on attachment 231476 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231476&action=review

r=me

> Source/WebCore/ChangeLog:15
> +        Mozilla no longer implements this behavior. Instead, they do the lookup on the prototype before 
> +        looking for subframe name getters. We should change this to match Mozilla. This has the convenient 
> +        side effect of allowing us to cache lookups in the window's prototype chain.

FWIW, I think Mozilla and WebIDL technically specify that name getters should exist in the prototype chain between the window prototype and the object prototype. So, eventually, we'll want to do that, and verify that a frame named "toString" takes precedence over Object.prototype.toString.

Still, this patch is a step in the right direction. I don't think I'll let <iframe name="toString"> stand in our way.
Comment 3 WebKit Commit Bot 2014-05-15 11:29:20 PDT
Comment on attachment 231476 [details]
Patch

Clearing flags on attachment: 231476

Committed r168902: <http://trac.webkit.org/changeset/168902>
Comment 4 WebKit Commit Bot 2014-05-15 11:29:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2014-05-18 20:59:04 PDT
(In reply to comment #2)
> FWIW, I think Mozilla and WebIDL technically specify that name getters should exist in the prototype chain between the window prototype and the object prototype. So, eventually, we'll want to do that, and verify that a frame named "toString" takes precedence over Object.prototype.toString.
> 
> Still, this patch is a step in the right direction. I don't think I'll let <iframe name="toString"> stand in our way.

Sure would be nice having a test demonstrating this problem that remains.