Bug 132922

Summary: Move subframe name getter lookup later in JSDOMWindow::getOwnPropertySlot
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 132705    
Attachments:
Description Flags
Patch none

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.