Changeset 204179 in webkit


Ignore:
Timestamp:
Aug 5, 2016 11:36:21 AM (8 years ago)
Author:
Chris Dumez
Message:

Window's named properties should be exposed on a WindowProperties object in its prototype
​https://bugs.webkit.org/show_bug.cgi?id=160354

Reviewed by Gavin Barraclough.

LayoutTests/imported/w3c:

Rebaseline W3C test now that one more check is passing.

  • web-platform-tests/html/dom/interfaces-expected.txt:

Source/WebCore:

Window's named properties should be exposed on a WindowProperties object
in its prototype:

Firefox and Chrome both comply with the specification. However, WebKit
had no "WindowProperties" object in the Window prototype chain and the
named properties are exposed on the Window object itself.

No new tests, rebaselined existing tests.

  • CMakeLists.txt:
  • WebCore.xcodeproj/project.pbxproj:
  • bindings/js/JSBindingsAllInOne.cpp:
  • bindings/js/JSDOMWindowCustom.cpp:

(WebCore::JSDOMWindow::getOwnPropertySlot):
(WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess): Deleted.
(WebCore::JSDOMWindow::put): Deleted.
(WebCore::JSDOMWindow::putByIndex): Deleted.
(WebCore::JSDOMWindow::getEnumerableLength): Deleted.

  • bindings/js/JSDOMWindowProperties.cpp: Added.

(WebCore::jsDOMWindowPropertiesGetOwnPropertySlotNamedItemGetter):
(WebCore::JSDOMWindowProperties::getOwnPropertySlot):
(WebCore::JSDOMWindowProperties::getOwnPropertySlotByIndex):

  • bindings/js/JSDOMWindowProperties.h: Added.

(WebCore::JSDOMWindowProperties::create):
(WebCore::JSDOMWindowProperties::createStructure):
(WebCore::JSDOMWindowProperties::JSDOMWindowProperties):

  • bindings/js/JSDOMWindowShell.cpp:

(WebCore::JSDOMWindowShell::setWindow):

LayoutTests:

  • fast/dom/Window/es52-globals-expected.txt:

Update / Rebaseline test now that named properties are no longer reported as "own"
properties on the Window object. I have verified that the test gives the
same result in Firefox and Chrome.

  • fast/loader/window-clearing-expected.txt:

Rebaseline test that prints one more line because there is one more
object in Window's prototype chain.

  • http/tests/security/window-named-proto-expected.txt:
  • http/tests/security/window-named-valueOf-expected.txt:

Rebaseline 2 security tests that give slightly different output. The new
output is identical to the one in Firefox and Chrome. The tests are not
failing since they are not alert'ing content from the other frame.
The reason those tests were logging a security error is because we would
previously prevent named property access if the frame name conflicts with
a property name in the Window prototype, and we now no longer
differentiate this case.

Location:
trunk
Files:
2 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r204174 r204179  
     12016-08-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Window's named properties should be exposed on a WindowProperties object in its prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=160354
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        * fast/dom/Window/es52-globals-expected.txt:
     9        Update / Rebaseline test now that named properties are no longer reported as "own"
     10        properties on the Window object. I have verified that the test gives the
     11        same result in Firefox and Chrome.
     12
     13        * fast/loader/window-clearing-expected.txt:
     14        Rebaseline test that prints one more line because there is one more
     15        object in Window's prototype chain.
     16
     17        * http/tests/security/window-named-proto-expected.txt:
     18        * http/tests/security/window-named-valueOf-expected.txt:
     19        Rebaseline 2 security tests that give slightly different output. The new
     20        output is identical to the one in Firefox and Chrome. The tests are not
     21        failing since they are not alert'ing content from the other frame.
     22        The reason those tests were logging a security error is because we would
     23        previously prevent named property access if the frame name conflicts with
     24        a property name in the Window prototype, and we now no longer
     25        differentiate this case.
     26
    1272016-08-05  John Wilander  <wilander@apple.com>
    228

  • trunk/LayoutTests/fast/dom/Window/es52-globals-expected.txt

    r204166 r204179  
    22PASS window.hasOwnProperty("x") is true
    33PASS window.hasOwnProperty("y") is false
    4 PASS window.hasOwnProperty("f") is true
    5 PASS window.hasOwnProperty("div") is true
    6 FAIL window.hasOwnProperty("a") should be true. Was false.
     4PASS window.hasOwnProperty("f") is false
     5PASS window.__proto__.__proto__.hasOwnProperty("f") is true
     6PASS window.hasOwnProperty("div") is false
     7PASS window.__proto__.__proto__.hasOwnProperty("div") is true
     8PASS window.hasOwnProperty("a") is false
    79PASS Element is not undefined
    810PASS x is 1

  • trunk/LayoutTests/fast/dom/Window/es52-globals.html

    r204166 r204179  
    1515shouldBeTrue('window.hasOwnProperty("x")');
    1616shouldBeFalse('window.hasOwnProperty("y")');
    17 shouldBeTrue('window.hasOwnProperty("f")');
    18 shouldBeTrue('window.hasOwnProperty("div")');
    19 shouldBeTrue('window.hasOwnProperty("a")');
     17shouldBeFalse('window.hasOwnProperty("f")');
     18shouldBeTrue('window.__proto__.__proto__.hasOwnProperty("f")');
     19shouldBeFalse('window.hasOwnProperty("div")');
     20shouldBeTrue('window.__proto__.__proto__.hasOwnProperty("div")');
     21shouldBeFalse('window.hasOwnProperty("a")');
    2022
    2123</script>

  • trunk/LayoutTests/fast/loader/window-clearing-expected.txt

    r204166 r204179  
    1010
    1111PASS: element 3 in the window's prototype chain was cleared
     12
     13PASS: element 4 in the window's prototype chain was cleared

  • trunk/LayoutTests/http/tests/security/window-named-proto-expected.txt

    r204166 r204179  
    1 CONSOLE MESSAGE: line 1: Blocked a frame with origin "null" from accessing a frame with origin "http://localhost:8080".  The frame requesting access has a protocol of "data", the frame being accessed has a protocol of "http". Protocols must match.
     1CONSOLE MESSAGE: line 2: TypeError: null is not an object (evaluating 'document.body.innerHTML')
    22
    3 CONSOLE MESSAGE: line 1: TypeError: undefined is not an object (evaluating 'parent.__proto__.alert')
    4 

  • trunk/LayoutTests/http/tests/security/window-named-valueOf-expected.txt

    r204166 r204179  
    1 CONSOLE MESSAGE: line 1: Blocked a frame with origin "null" from accessing a frame with origin "http://localhost:8080".  The frame requesting access has a protocol of "data", the frame being accessed has a protocol of "http". Protocols must match.
    2 
    3 CONSOLE MESSAGE: line 1: Blocked a frame with origin "null" from accessing a frame with origin "http://localhost:8080".  The frame requesting access has a protocol of "data", the frame being accessed has a protocol of "http". Protocols must match.
    4 
     1CONSOLE MESSAGE: line 1: Threw exception: TypeError: Illegal constructor
    52This passes if it doesn't alert the contents of innocent-victim. 

  • trunk/LayoutTests/http/tests/security/window-named-valueOf.html

    r204166 r204179  
    2424                alert(obj.valueOf.constructor("return document.body.innerHTML")());
    2525            } catch(ex) {
     26               console.log("Threw exception: " + ex)
    2627            }
    2728            if (window.testRunner)

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r204172 r204179  
     12016-08-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Window's named properties should be exposed on a WindowProperties object in its prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=160354
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        Rebaseline W3C test now that one more check is passing.
     9
     10        * web-platform-tests/html/dom/interfaces-expected.txt:
     11
    1122016-08-05  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/dom/interfaces-expected.txt

    r204166 r204179  
    51605160PASS Window interface object length
    51615161PASS Window interface object name
    5162 FAIL Window interface: existence and properties of interface prototype object assert_equals: Class name for prototype of Window.prototype is not "WindowProperties" expected "[object WindowProperties]" but got "[object EventTargetPrototype]"
     5162PASS Window interface: existence and properties of interface prototype object
    51635163PASS Window interface: existence and properties of interface prototype object's "constructor" property
    51645164PASS Window interface: attribute self

  • trunk/Source/WebCore/CMakeLists.txt

    r204178 r204179  
    11371137    bindings/js/JSDOMWindowBase.cpp
    11381138    bindings/js/JSDOMWindowCustom.cpp
     1139    bindings/js/JSDOMWindowProperties.cpp
    11391140    bindings/js/JSDOMWindowShell.cpp
    11401141    bindings/js/JSDOMWrapper.cpp

  • trunk/Source/WebCore/ChangeLog

    r204178 r204179  
     12016-08-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Window's named properties should be exposed on a WindowProperties object in its prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=160354
     5
     6        Reviewed by Gavin Barraclough.
     7
     8        Window's named properties should be exposed on a WindowProperties object
     9        in its prototype:
     10        - http://heycam.github.io/webidl/#named-properties-object
     11
     12        Firefox and Chrome both comply with the specification. However, WebKit
     13        had no "WindowProperties" object in the Window prototype chain and the
     14        named properties are exposed on the Window object itself.
     15
     16        No new tests, rebaselined existing tests.
     17
     18        * CMakeLists.txt:
     19        * WebCore.xcodeproj/project.pbxproj:
     20        * bindings/js/JSBindingsAllInOne.cpp:
     21        * bindings/js/JSDOMWindowCustom.cpp:
     22        (WebCore::JSDOMWindow::getOwnPropertySlot):
     23        (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
     24        (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess): Deleted.
     25        (WebCore::JSDOMWindow::put): Deleted.
     26        (WebCore::JSDOMWindow::putByIndex): Deleted.
     27        (WebCore::JSDOMWindow::getEnumerableLength): Deleted.
     28        * bindings/js/JSDOMWindowProperties.cpp: Added.
     29        (WebCore::jsDOMWindowPropertiesGetOwnPropertySlotNamedItemGetter):
     30        (WebCore::JSDOMWindowProperties::getOwnPropertySlot):
     31        (WebCore::JSDOMWindowProperties::getOwnPropertySlotByIndex):
     32        * bindings/js/JSDOMWindowProperties.h: Added.
     33        (WebCore::JSDOMWindowProperties::create):
     34        (WebCore::JSDOMWindowProperties::createStructure):
     35        (WebCore::JSDOMWindowProperties::JSDOMWindowProperties):
     36        * bindings/js/JSDOMWindowShell.cpp:
     37        (WebCore::JSDOMWindowShell::setWindow):
     38
    1392016-08-05  Brady Eidson  <beidson@apple.com>
    240

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r204178 r204179  
    17861786                460BB6151D0A1BF000221812 /* Base64Utilities.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 460BB6131D0A1BEC00221812 /* Base64Utilities.cpp */; };
    17871787                460BB6161D0A1BF000221812 /* Base64Utilities.h in Headers */ = {isa = PBXBuildFile; fileRef = 460BB6141D0A1BEC00221812 /* Base64Utilities.h */; settings = {ATTRIBUTES = (Private, ); }; };
     1788                460CBF351D4BCD0E0092E88E /* JSDOMWindowProperties.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 460CBF331D4BCCFE0092E88E /* JSDOMWindowProperties.cpp */; };
     1789                460CBF361D4BCD0E0092E88E /* JSDOMWindowProperties.h in Headers */ = {isa = PBXBuildFile; fileRef = 460CBF341D4BCCFE0092E88E /* JSDOMWindowProperties.h */; };
    17881790                4634592C1AC2271000ECB71C /* PowerObserverMac.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4634592B1AC2271000ECB71C /* PowerObserverMac.cpp */; };
    17891791                463EB6221B8789E00096ED51 /* TagCollection.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 463EB6201B8789CB0096ED51 /* TagCollection.cpp */; };
    … …  
    90879089                460BB6131D0A1BEC00221812 /* Base64Utilities.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Base64Utilities.cpp; sourceTree = "<group>"; };
    90889090                460BB6141D0A1BEC00221812 /* Base64Utilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Base64Utilities.h; sourceTree = "<group>"; };
     9091                460CBF331D4BCCFE0092E88E /* JSDOMWindowProperties.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSDOMWindowProperties.cpp; sourceTree = "<group>"; };
     9092                460CBF341D4BCCFE0092E88E /* JSDOMWindowProperties.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSDOMWindowProperties.h; sourceTree = "<group>"; };
    90899093                4634592B1AC2271000ECB71C /* PowerObserverMac.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PowerObserverMac.cpp; sourceTree = "<group>"; };
    90909094                463EB6201B8789CB0096ED51 /* TagCollection.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = TagCollection.cpp; sourceTree = "<group>"; };
    … …  
    1778417788                                9767CE09145ABC12005E64DB /* ExceptionHeaders.h */,
    1778517789                                9767CE0A145ABC13005E64DB /* ExceptionInterfaces.h */,
    17786                                 9908B0FD1BCAD07D00ED0F45 /* FetchInternalsBuiltins.cpp */,
    1778717790                                9B03D8061BB3110D00B764B9 /* FetchInternalsBuiltins.h */,
    1778817791                                A17C81200F2A5CF7005DAAEB /* HTMLElementFactory.cpp */,
    … …  
    2195821961                                BC6932710D7E293900AE44D1 /* JSDOMWindowBase.cpp */,
    2195921962                                BC6932720D7E293900AE44D1 /* JSDOMWindowBase.h */,
     21963                                460CBF331D4BCCFE0092E88E /* JSDOMWindowProperties.cpp */,
     21964                                460CBF341D4BCCFE0092E88E /* JSDOMWindowProperties.h */,
    2196021965                                BCBFB53A0DCD29CF0019B3E5 /* JSDOMWindowShell.cpp */,
    2196121966                                BCBFB53B0DCD29CF0019B3E5 /* JSDOMWindowShell.h */,
    … …  
    2626826273                                81BE20D311F4BC3200915DFA /* JSIDBCursor.h in Headers */,
    2626926274                                C585A68311D4FB08004C3E4B /* JSIDBDatabase.h in Headers */,
     26275                                460CBF361D4BCD0E0092E88E /* JSDOMWindowProperties.h in Headers */,
    2627026276                                C585A69711D4FB13004C3E4B /* JSIDBFactory.h in Headers */,
    2627126277                                C572EE1F1201C9BC007D8F82 /* JSIDBIndex.h in Headers */,
    … …  
    2911529121                                A8C2280E11D4A59700D5A7D3 /* DocumentParser.cpp in Sources */,
    2911629122                                4A4F48A916B0DFC000EDBB29 /* DocumentRuleSets.cpp in Sources */,
     29123                                460CBF351D4BCD0E0092E88E /* JSDOMWindowProperties.cpp in Sources */,
    2911729124                                AD6E71AC1668899D00320C13 /* DocumentSharedObjectPool.cpp in Sources */,
    2911829125                                0B9056190F2578BE0095FF6A /* DocumentThreadableLoader.cpp in Sources */,

  • trunk/Source/WebCore/bindings/js/JSBindingsAllInOne.cpp

    r204166 r204179  
    6363#include "JSDOMWindowBase.cpp"
    6464#include "JSDOMWindowCustom.cpp"
     65#include "JSDOMWindowProperties.cpp"
    6566#include "JSDOMWindowShell.cpp"
    6667#include "JSDOMWrapper.cpp"

  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r204166 r204179  
    147147    }
    148148
    149     // Do prototype lookup early so that functions and attributes in the prototype can have
    150     // precedence over the index and name getters.
    151     // FIXME: This seems like a silly idea. It only serves to suppress named property access
    152     // to frames that happen to have names corresponding to properties on the prototype.
    153     // This seems to only serve to leak some information cross-origin.
    154     JSValue proto = thisObject->getPrototypeDirect();
    155     if (proto.isObject() && asObject(proto)->getPropertySlot(exec, propertyName, slot)) {
    156         thisObject->printErrorMessage(errorMessage);
    157         slot.setUndefined();
    158         return true;
    159     }
    160 
    161149    // Check for child frames by name before built-in properties to match Mozilla. This does
    162150    // not match IE, but some sites end up naming frames things that conflict with window
    … …  
    171159    slot.setUndefined();
    172160    return true;
    173 }
    174 
    175 static bool jsDOMWindowGetOwnPropertySlotNamedItemGetter(JSDOMWindow* thisObject, Frame& frame, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
    176 {
    177     JSValue proto = thisObject->getPrototypeDirect();
    178     if (proto.isObject() && asObject(proto)->hasProperty(exec, propertyName))
    179         return false;
    180 
    181     // Check for child frames by name before built-in properties to match Mozilla. This does
    182     // not match IE, but some sites end up naming frames things that conflict with window
    183     // properties that are in Moz but not IE. Since we have some of these, we have to do it
    184     // the Moz way.
    185     if (auto* scopedChild = frame.tree().scopedChild(propertyNameToAtomicString(propertyName))) {
    186         slot.setValue(thisObject, ReadOnly | DontDelete | DontEnum, toJS(exec, scopedChild->document()->domWindow()));
    187         return true;
    188     }
    189 
    190     // FIXME: Search the whole frame hierarchy somewhere around here.
    191     // We need to test the correct priority order.
    192 
    193     // Allow shortcuts like 'Image1' instead of document.images.Image1
    194     Document* document = frame.document();
    195     if (is<HTMLDocument>(*document)) {
    196         auto& htmlDocument = downcast<HTMLDocument>(*document);
    197         auto* atomicPropertyName = propertyName.publicName();
    198         if (atomicPropertyName && htmlDocument.hasWindowNamedItem(*atomicPropertyName)) {
    199             JSValue namedItem;
    200             if (UNLIKELY(htmlDocument.windowNamedItemContainsMultipleElements(*atomicPropertyName))) {
    201                 Ref<HTMLCollection> collection = document->windowNamedItems(atomicPropertyName);
    202                 ASSERT(collection->length() > 1);
    203                 namedItem = toJS(exec, thisObject->globalObject(), collection);
    204             } else
    205                 namedItem = toJS(exec, thisObject->globalObject(), htmlDocument.windowNamedItem(*atomicPropertyName));
    206             slot.setValue(thisObject, ReadOnly | DontDelete | DontEnum, namedItem);
    207             return true;
    208         }
    209     }
    210 
    211     return false;
    212161}
    213162
    … …  
    255204#endif
    256205
    257     // (3) Finally, named properties.
    258     // Really, this should just be 'return false;' - these should all be on the NPO.
    259     return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, propertyName, slot);
     206    return false;
    260207}
    261208
    … …  
    285232
    286233    // (2) Regular own properties.
    287     if (Base::getOwnPropertySlotByIndex(thisObject, exec, index, slot))
    288         return true;
    289 
    290     // (3) Finally, named properties.
    291     // Really, this should just be 'return false;' - these should all be on the NPO.
    292     return jsDOMWindowGetOwnPropertySlotNamedItemGetter(thisObject, *frame, exec, Identifier::from(exec, index), slot);
     234    return Base::getOwnPropertySlotByIndex(thisObject, exec, index, slot);
    293235}
    294236

  • trunk/Source/WebCore/bindings/js/JSDOMWindowShell.cpp

    r204166 r204179  
    3333#include "GCController.h"
    3434#include "JSDOMWindow.h"
     35#include "JSDOMWindowProperties.h"
    3536#include "JSEventTarget.h"
    3637#include "ScriptController.h"
    … …  
    8687    JSDOMWindow* jsDOMWindow = JSDOMWindow::create(vm, structure, *domWindow, this);
    8788    prototype->structure()->setGlobalObject(vm, jsDOMWindow);
    88     prototype->structure()->setPrototypeWithoutTransition(vm, JSEventTarget::prototype(vm, jsDOMWindow));
     89
     90    Structure* windowPropertiesStructure = JSDOMWindowProperties::createStructure(vm, jsDOMWindow, JSEventTarget::prototype(vm, jsDOMWindow));
     91    JSDOMWindowProperties* windowProperties = JSDOMWindowProperties::create(windowPropertiesStructure, *jsDOMWindow);
     92
     93    prototype->structure()->setPrototypeWithoutTransition(vm, windowProperties);
    8994    setWindow(vm, jsDOMWindow);
    9095    ASSERT(jsDOMWindow->globalObject() == jsDOMWindow);
Note: See TracChangeset for help on using the changeset viewer.