[webkit-changes] [277829] trunk

Thu, 20 May 2021 15:15:32 -0700

Title: [277829] trunk
Revision
277829
Author
shvaikal...@gmail.com
Date
2021-05-20 15:14:40 -0700 (Thu, 20 May 2021)

Log Message

Align internal methods of WindowProperties object with the spec
https://bugs.webkit.org/show_bug.cgi?id=222918

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

* web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window-expected.txt:
* web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window.js:
* web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object-expected.txt: Added.
* web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html: Added.
* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html:
* web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties-expected.txt:
* web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html:

Source/WebCore:

This patch:

1. Implements [[PreventExtensions]], [[DefineOwnProperty]], and [[Delete]] methods that always fail [1],
   ensuring [[Set]] with altered receiver is passed through, indexed properties are rejected as well,
   and an exception is thrown if neccessary.

   Adding a put() override would a) slow down global `var` creation and b) require factoring out most of
   JSObject::putInlineSlow(). Instead, we prevent extensions on a structure while defining isExtensible()
   override to cover that up.

2. Corrects property attributes reported by [[GetOwnProperty]] methods of cross-origin WindowProxy [2]
   and WindowProperties, bringing these objects in compliance with invariants of internal methods [3].

3. Removes comments on cross-browser differences and priority order from [[GetOwnProperty]] because
   it correctly implements the now detailed spec [4]. Browsing contexts precedence is covered by
   wpt/html/browsers/the-window-object/named-access-on-the-window-object/named-objects.html test.

4. Asserts that WindowProperties [[Prototype]] is an object, as guaranteed by its [[SetPrototypeOf]].

This change fully implements the current spec, aligning WebKit with Blink and Gecko.

[1] https://heycam.github.io/webidl/#named-properties-object-defineownproperty
[2] https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-getownproperty (step 6.2)
[3] https://tc39.es/ecma262/#sec-invariants-of-the-essential-internal-methods
[4] https://html.spec.whatwg.org/multipage/window-object.html#dom-window-nameditem (step 2)

Tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html
       imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
       imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
* bindings/js/JSDOMWindowProperties.cpp:
(WebCore::jsDOMWindowPropertiesGetOwnPropertySlotNamedItemGetter):
(WebCore::JSDOMWindowProperties::finishCreation):
(WebCore::JSDOMWindowProperties::getOwnPropertySlot):
(WebCore::JSDOMWindowProperties::deleteProperty):
(WebCore::JSDOMWindowProperties::deletePropertyByIndex):
(WebCore::JSDOMWindowProperties::preventExtensions):
(WebCore::JSDOMWindowProperties::isExtensible):
(WebCore::JSDOMWindowProperties::defineOwnProperty):
* bindings/js/JSDOMWindowProperties.h:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-05-20 22:14:40 UTC (rev 277829)
@@ -1,3 +1,18 @@
+2021-05-20  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        Align internal methods of WindowProperties object with the spec
+        https://bugs.webkit.org/show_bug.cgi?id=222918
+
+        Reviewed by Sam Weinig.
+
+        * web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window-expected.txt:
+        * web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window.js:
+        * web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object-expected.txt: Added.
+        * web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html: Added.
+        * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html:
+        * web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties-expected.txt:
+        * web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html:
+
 2021-05-20  Felipe Erias  <felipeer...@igalia.com>
 
         [css-flexbox] Wrong height of an empty table inside an orthogonal flex parent

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window-expected.txt (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window-expected.txt	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window-expected.txt	2021-05-20 22:14:40 UTC (rev 277829)
@@ -1,5 +1,4 @@
 
 PASS @@toStringTag exists with the appropriate descriptor
 PASS Object.prototype.toString
-PASS Object.prototype.toString applied after modifying @@toStringTag

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window.js (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window.js	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-named-properties-object.window.js	2021-05-20 22:14:40 UTC (rev 277829)
@@ -16,17 +16,6 @@
   assert_equals(Object.prototype.toString.call(namedPropertiesObject), "[object WindowProperties]");
 }, "Object.prototype.toString");
 
-test(t => {
-  assert_own_property(namedPropertiesObject, Symbol.toStringTag, "Precondition for this test: @@toStringTag exists");
-
-  t.add_cleanup(() => {
-    Object.defineProperty(namedPropertiesObject, Symbol.toStringTag, { value: "WindowProperties" });
-  });
-
-  Object.defineProperty(namedPropertiesObject, Symbol.toStringTag, { value: "NotWindowProperties" });
-  assert_equals(Object.prototype.toString.call(namedPropertiesObject), "[object NotWindowProperties]");
-}, "Object.prototype.toString applied after modifying @@toStringTag");
-
 // Chrome had a bug (https://bugs.chromium.org/p/chromium/issues/detail?id=793406) where if there
 // was no @@toStringTag, it would fall back to a magic class string. Tests for this are present in
 // the sibling class-string*.any.js tests. However, the named properties object always fails calls

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object-expected.txt (0 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object-expected.txt	2021-05-20 22:14:40 UTC (rev 277829)
@@ -0,0 +1,15 @@
+
+PASS [[SetPrototypeOf]] and [[GetPrototypeOf]]
+PASS [[PreventExtensions]] and [[IsExtensible]]
+PASS [[GetOwnProperty]]
+PASS [[GetOwnProperty]] (named property visibility algorithm)
+PASS [[DefineOwnProperty]]
+PASS [[HasProperty]]
+PASS [[Get]]
+PASS [[Get]] (named property visibility algorithm)
+PASS [[Set]] (direct)
+PASS [[Set]] (prototype chain)
+PASS [[Set]] (Reflect.set)
+PASS [[Delete]]
+PASS [[OwnPropertyKeys]]
+

Added: trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html (0 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html	                        (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html	2021-05-20 22:14:40 UTC (rev 277829)
@@ -0,0 +1,284 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Internal methods of Window's named properties object</title>
+<link rel="help" href=""
+<script src=""
+<script src=""
+<body>
+<script>
+function sloppyModeSet(base, key, value) { base[key] = value; }
+</script>
+<script>
+"use strict";
+
+const supportedNonIndex = "supported non-index property name";
+const supportedIndex = "supported indexed property name";
+const unsupportedNonIndex = "unsupported non-index property name";
+const unsupportedIndex = "unsupported indexed property name";
+const existingSymbol = "existing symbol property name";
+const nonExistingSymbol = "non-existing symbol property name";
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    Object.setPrototypeOf(wp, w.EventTarget.prototype); // Setting current [[Prototype]] value shouldn't throw
+
+    assert_throws_js(TypeError, () => { Object.setPrototypeOf(wp, {}); });
+    assert_throws_js(w.TypeError, () => { wp.__proto__ = null; });
+    assert_false(Reflect.setPrototypeOf(wp, w.Object.prototype));
+
+    assert_equals(Object.getPrototypeOf(wp), w.EventTarget.prototype);
+}, "[[SetPrototypeOf]] and [[GetPrototypeOf]]");
+
+test(t => {
+    const { wp } = createWindowProperties(t);
+
+    assert_throws_js(TypeError, () => { Object.preventExtensions(wp); });
+    assert_false(Reflect.preventExtensions(wp));
+
+    assert_true(Object.isExtensible(wp));
+}, "[[PreventExtensions]] and [[IsExtensible]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    const elA = appendElementWithId(w, "a");
+    const el0 = appendIframeWithName(w, 0);
+
+    assert_prop_desc(Object.getOwnPropertyDescriptor(wp, "a"), elA, supportedNonIndex);
+    assert_prop_desc(Reflect.getOwnPropertyDescriptor(wp, 0), el0, supportedIndex);
+    assert_equals(Reflect.getOwnPropertyDescriptor(wp, "b"), undefined, unsupportedNonIndex);
+    assert_equals(Object.getOwnPropertyDescriptor(wp, 1), undefined, unsupportedIndex);
+}, "[[GetOwnProperty]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    appendIframeWithName(w, "hasOwnProperty");
+    appendFormWithName(w, "addEventListener");
+    appendElementWithId(w, "a");
+    appendIframeWithName(w, 0);
+
+    w.Object.prototype.a = {};
+    w.EventTarget.prototype[0] = {};
+
+    // These are shadowed by properties higher in [[Prototype]] chain. See https://heycam.github.io/webidl/#dfn-named-property-visibility
+    assert_equals(Object.getOwnPropertyDescriptor(wp, "hasOwnProperty"), undefined, supportedNonIndex);
+    assert_equals(Reflect.getOwnPropertyDescriptor(wp, "addEventListener"), undefined, supportedNonIndex);
+    assert_equals(Object.getOwnPropertyDescriptor(wp, "a"), undefined, supportedNonIndex);
+    assert_equals(Reflect.getOwnPropertyDescriptor(wp, 0), undefined, supportedIndex);
+}, "[[GetOwnProperty]] (named property visibility algorithm)");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    appendElementWithId(w, "a");
+    appendFormWithName(w, 0);
+
+    assert_define_own_property_fails(wp, "a", {}, supportedNonIndex);
+    assert_define_own_property_fails(wp, 0, {}, supportedIndex);
+    assert_define_own_property_fails(wp, "b", {}, unsupportedNonIndex);
+    assert_define_own_property_fails(wp, 1, {}, unsupportedIndex);
+    assert_define_own_property_fails(wp, Symbol.toStringTag, {}, existingSymbol);
+    assert_define_own_property_fails(wp, Symbol(), {}, nonExistingSymbol);
+}, "[[DefineOwnProperty]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    appendFormWithName(w, "a");
+    appendElementWithId(w, 0);
+
+    assert_true("a" in wp, supportedNonIndex);
+    assert_true(Reflect.has(wp, "a"), supportedNonIndex);
+    assert_true(0 in wp, supportedIndex);
+    assert_true(Reflect.has(wp, 0), supportedIndex);
+
+    assert_false("b" in wp, unsupportedNonIndex);
+    assert_false(Reflect.has(wp, 1), unsupportedIndex);
+}, "[[HasProperty]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+    const elA = appendFormWithName(w, "a");
+    const el0 = appendIframeWithName(w, 0);
+
+    assert_equals(wp.a, elA, supportedNonIndex);
+    assert_equals(wp[0], el0, supportedIndex);
+    assert_equals(wp[Symbol.toStringTag], "WindowProperties", existingSymbol);
+
+    assert_equals(wp.b, undefined, unsupportedNonIndex);
+    assert_equals(wp[1], undefined, unsupportedIndex);
+    assert_equals(wp[Symbol.iterator], undefined, nonExistingSymbol);
+}, "[[Get]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    appendIframeWithName(w, "isPrototypeOf");
+    appendFormWithName(w, "dispatchEvent");
+    appendElementWithId(w, "a");
+    appendElementWithId(w, 0);
+
+    w.EventTarget.prototype.a = 10;
+    w.Object.prototype[0] = 20;
+
+    // These are shadowed by properties higher in [[Prototype]] chain. See https://heycam.github.io/webidl/#dfn-named-property-visibility
+    assert_equals(wp.isPrototypeOf, w.Object.prototype.isPrototypeOf, supportedNonIndex);
+    assert_equals(wp.dispatchEvent, w.EventTarget.prototype.dispatchEvent, supportedNonIndex);
+    assert_equals(wp.a, 10, supportedNonIndex);
+    assert_equals(wp[0], 20, supportedIndex);
+}, "[[Get]] (named property visibility algorithm)");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+    const elA = appendIframeWithName(w, "a");
+    const el0 = appendFormWithName(w, 0);
+
+    assert_set_fails(wp, "a", supportedNonIndex);
+    assert_set_fails(wp, "b", unsupportedNonIndex);
+    assert_set_fails(wp, 0, supportedIndex);
+    assert_set_fails(wp, 1, unsupportedIndex);
+    assert_set_fails(wp, Symbol.toStringTag, existingSymbol);
+    assert_set_fails(wp, Symbol(), nonExistingSymbol);
+
+    assert_equals(wp.a, elA, supportedNonIndex);
+    assert_equals(wp[0], el0, supportedIndex);
+    assert_equals(wp.b, undefined, unsupportedNonIndex);
+    assert_equals(wp[1], undefined, unsupportedIndex);
+}, "[[Set]] (direct)");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+    const receiver = Object.create(wp);
+
+    appendIframeWithName(w, "a");
+    appendElementWithId(w, 0);
+
+    let setterThisValue;
+    Object.defineProperty(w.Object.prototype, 1, { set() { setterThisValue = this; } });
+    Object.defineProperty(w.EventTarget.prototype, "b", { writable: false });
+
+    receiver.a = 10;
+    assert_throws_js(TypeError, () => { receiver.b = {}; }, unsupportedNonIndex);
+    receiver[0] = 20;
+    receiver[1] = {};
+
+    assert_equals(receiver.a, 10, supportedNonIndex);
+    assert_equals(receiver[0], 20, supportedIndex);
+    assert_false(receiver.hasOwnProperty("b"), unsupportedNonIndex);
+    assert_false(receiver.hasOwnProperty(1), unsupportedIndex);
+    assert_equals(setterThisValue, receiver, "setter |this| value is receiver");
+}, "[[Set]] (prototype chain)");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+    const receiver = {};
+
+    appendFormWithName(w, "a");
+    appendIframeWithName(w, 0);
+
+    let setterThisValue;
+    Object.defineProperty(w.Object.prototype, "b", { set() { setterThisValue = this; } });
+    Object.defineProperty(w.EventTarget.prototype, 1, { writable: false });
+
+    assert_true(Reflect.set(wp, "a", 10, receiver), supportedNonIndex);
+    assert_true(Reflect.set(wp, 0, 20, receiver), supportedIndex);
+    assert_true(Reflect.set(wp, "b", {}, receiver), unsupportedNonIndex);
+    assert_false(Reflect.set(wp, 1, {}, receiver), unsupportedIndex);
+
+    assert_equals(receiver.a, 10, supportedNonIndex);
+    assert_equals(receiver[0], 20, supportedIndex);
+    assert_false(receiver.hasOwnProperty("b"), unsupportedNonIndex);
+    assert_equals(setterThisValue, receiver, "setter |this| value is receiver");
+    assert_false(receiver.hasOwnProperty(1), unsupportedIndex);
+}, "[[Set]] (Reflect.set)");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+    const elA = appendFormWithName(w, "a");
+    const el0 = appendElementWithId(w, 0);
+
+    assert_delete_fails(wp, "a", supportedNonIndex);
+    assert_delete_fails(wp, 0, supportedIndex);
+    assert_delete_fails(wp, "b", unsupportedNonIndex);
+    assert_delete_fails(wp, 1, unsupportedIndex);
+    assert_delete_fails(wp, Symbol.toStringTag, existingSymbol);
+    assert_delete_fails(wp, Symbol("foo"), nonExistingSymbol);
+
+    assert_equals(wp.a, elA, supportedNonIndex);
+    assert_equals(wp[0], el0, supportedIndex);
+    assert_equals(wp[Symbol.toStringTag], "WindowProperties", existingSymbol);
+}, "[[Delete]]");
+
+test(t => {
+    const { w, wp } = createWindowProperties(t);
+
+    appendIframeWithName(w, "a");
+    appendElementWithId(w, 0);
+    appendFormWithName(w, "b");
+
+    const forInKeys = [];
+    for (const key in wp)
+        forInKeys.push(key);
+
+    assert_array_equals(forInKeys, Object.keys(w.EventTarget.prototype));
+    assert_array_equals(Object.getOwnPropertyNames(wp), []);
+    assert_array_equals(Reflect.ownKeys(wp), [Symbol.toStringTag]);
+}, "[[OwnPropertyKeys]]");
+
+function createWindowProperties(t) {
+    const iframe = document.createElement("iframe");
+    document.body.append(iframe);
+    t.add_cleanup(() => { iframe.remove(); });
+
+    const w = iframe.contentWindow;
+    const wp = Object.getPrototypeOf(w.Window.prototype);
+    return { w, wp };
+}
+
+function appendIframeWithName(w, name) {
+    const el = w.document.createElement("iframe");
+    el.name = name;
+    w.document.body.append(el);
+    return el.contentWindow;
+}
+
+function appendFormWithName(w, name) {
+    const el = w.document.createElement("form");
+    el.name = name;
+    w.document.body.append(el);
+    return el;
+}
+
+function appendElementWithId(w, id) {
+    const el = w.document.createElement("div");
+    el.id = id;
+    w.document.body.append(el);
+    return el;
+}
+
+function assert_prop_desc(desc, value, testInfo) {
+    assert_equals(typeof desc, "object", `${testInfo} typeof desc`);
+    assert_equals(desc.value, value, `${testInfo} [[Value]]`);
+    assert_true(desc.writable, `${testInfo} [[Writable]]`);
+    assert_false(desc.enumerable, `${testInfo} [[Enumerable]]`);
+    assert_true(desc.configurable, `${testInfo} [[Configurable]]`);
+}
+
+function assert_define_own_property_fails(object, key, desc, testInfo) {
+    assert_throws_js(TypeError, () => { Object.defineProperty(object, key, desc); }, testInfo);
+    assert_false(Reflect.defineProperty(object, key, desc), testInfo);
+}
+
+function assert_set_fails(object, key, value, testInfo) {
+    sloppyModeSet(object, key, value);
+    assert_throws_js(TypeError, () => { object[key] = value; }, testInfo);
+    assert_false(Reflect.set(object, key, value), testInfo);
+}
+
+function assert_delete_fails(object, key, testInfo) {
+    assert_throws_js(TypeError, () => { delete object[key]; }, testInfo);
+    assert_false(Reflect.deleteProperty(object, key), testInfo);
+}
+</script>

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html	2021-05-20 22:14:40 UTC (rev 277829)
@@ -232,7 +232,10 @@
   }
   for (var prop of windowAllowlists.namedFrames) {
     win[prop]; // Shouldn't throw.
-    Object.getOwnPropertyDescriptor(win, prop); // Shouldn't throw.
+    var desc = Object.getOwnPropertyDescriptor(win, prop);
+    assert_false(desc.writable, "[[Writable]] for named frame " + String(prop));
+    assert_false(desc.enumerable, "[[Enumerable]] for named frame " + String(prop));
+    assert_true(desc.configurable, "[[Configurable]] for named frame " + String(prop));
     assert_true(Object.prototype.hasOwnProperty.call(win, prop), "hasOwnProperty for " + String(prop));
   }
   for (var prop in location) {

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties-expected.txt (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties-expected.txt	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties-expected.txt	2021-05-20 22:14:40 UTC (rev 277829)
@@ -1,9 +1,9 @@
 
 PASS Static name
 PASS Static id
-FAIL Static name on the prototype assert_equals: expected true but got false
+PASS Static name on the prototype
 PASS constructor
-FAIL duplicate property names assert_equals: expected 1 but got 0
+PASS duplicate property names
 PASS Dynamic name
 PASS Ghost name

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html (277828 => 277829)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html	2021-05-20 22:14:40 UTC (rev 277829)
@@ -60,9 +60,7 @@
 }, "constructor");
 test(function() {
   var gsp = Object.getPrototypeOf(Object.getPrototypeOf(window));
-  var names = Object.getOwnPropertyNames(gsp);
-  assert_equals(names.filter((name) => name == "baz").length, 1);
-
+  assert_equals(gsp.baz, document.getElementsByTagName("iframe")[1].contentWindow);
 }, "duplicate property names")
 var t = async_test("Dynamic name")
 var t2 = async_test("Ghost name")

Modified: trunk/Source/WebCore/ChangeLog (277828 => 277829)


--- trunk/Source/WebCore/ChangeLog	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/Source/WebCore/ChangeLog	2021-05-20 22:14:40 UTC (rev 277829)
@@ -1,3 +1,53 @@
+2021-05-20  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        Align internal methods of WindowProperties object with the spec
+        https://bugs.webkit.org/show_bug.cgi?id=222918
+
+        Reviewed by Sam Weinig.
+
+        This patch:
+
+        1. Implements [[PreventExtensions]], [[DefineOwnProperty]], and [[Delete]] methods that always fail [1],
+           ensuring [[Set]] with altered receiver is passed through, indexed properties are rejected as well,
+           and an exception is thrown if neccessary.
+
+           Adding a put() override would a) slow down global `var` creation and b) require factoring out most of
+           JSObject::putInlineSlow(). Instead, we prevent extensions on a structure while defining isExtensible()
+           override to cover that up.
+
+        2. Corrects property attributes reported by [[GetOwnProperty]] methods of cross-origin WindowProxy [2]
+           and WindowProperties, bringing these objects in compliance with invariants of internal methods [3].
+
+        3. Removes comments on cross-browser differences and priority order from [[GetOwnProperty]] because
+           it correctly implements the now detailed spec [4]. Browsing contexts precedence is covered by
+           wpt/html/browsers/the-window-object/named-access-on-the-window-object/named-objects.html test.
+
+        4. Asserts that WindowProperties [[Prototype]] is an object, as guaranteed by its [[SetPrototypeOf]].
+
+        This change fully implements the current spec, aligning WebKit with Blink and Gecko.
+
+        [1] https://heycam.github.io/webidl/#named-properties-object-defineownproperty
+        [2] https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-getownproperty (step 6.2)
+        [3] https://tc39.es/ecma262/#sec-invariants-of-the-essential-internal-methods
+        [4] https://html.spec.whatwg.org/multipage/window-object.html#dom-window-nameditem (step 2)
+
+        Tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/window-named-properties-object.html
+               imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
+               imported/w3c/web-platform-tests/html/browsers/the-window-object/named-access-on-the-window-object/window-named-properties.html
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
+        * bindings/js/JSDOMWindowProperties.cpp:
+        (WebCore::jsDOMWindowPropertiesGetOwnPropertySlotNamedItemGetter):
+        (WebCore::JSDOMWindowProperties::finishCreation):
+        (WebCore::JSDOMWindowProperties::getOwnPropertySlot):
+        (WebCore::JSDOMWindowProperties::deleteProperty):
+        (WebCore::JSDOMWindowProperties::deletePropertyByIndex):
+        (WebCore::JSDOMWindowProperties::preventExtensions):
+        (WebCore::JSDOMWindowProperties::isExtensible):
+        (WebCore::JSDOMWindowProperties::defineOwnProperty):
+        * bindings/js/JSDOMWindowProperties.h:
+
 2021-05-20  Chris Dumez  <cdu...@apple.com>
 
         WebAudioBufferList::setSampleCount() should early return if computeBufferSizes() fails

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (277828 => 277829)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2021-05-20 22:14:40 UTC (rev 277829)
@@ -140,7 +140,7 @@
     auto* frame = window.frame();
     if (frame && is<Frame>(*frame)) {
         if (auto* scopedChild = downcast<Frame>(*frame).tree().scopedChild(propertyNameToAtomString(propertyName))) {
-            slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, toJS(&lexicalGlobalObject, scopedChild->document()->domWindow()));
+            slot.setValue(thisObject, PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum, toJS(&lexicalGlobalObject, scopedChild->document()->domWindow()));
             return true;
         }
     }

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.cpp (277828 => 277829)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.cpp	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.cpp	2021-05-20 22:14:40 UTC (rev 277829)
@@ -42,15 +42,12 @@
 
 const ClassInfo JSDOMWindowProperties::s_info = { "WindowProperties", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDOMWindowProperties) };
 
+// https://html.spec.whatwg.org/multipage/window-object.html#dom-window-nameditem
 static bool jsDOMWindowPropertiesGetOwnPropertySlotNamedItemGetter(JSDOMWindowProperties* thisObject, DOMWindow& window, JSGlobalObject* lexicalGlobalObject, PropertyName propertyName, PropertySlot& slot)
 {
-    // Check for child frames by name before built-in properties to match Mozilla. This does
-    // not match IE, but some sites end up naming frames things that conflict with window
-    // properties that are in Moz but not IE. Since we have some of these, we have to do it
-    // the Moz way.
     if (auto* frame = window.frame()) {
         if (auto* scopedChild = frame->tree().scopedChild(propertyNameToAtomString(propertyName))) {
-            slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, toJS(lexicalGlobalObject, scopedChild->document()->domWindow()));
+            slot.setValue(thisObject, static_cast<unsigned>(PropertyAttribute::DontEnum), toJS(lexicalGlobalObject, scopedChild->document()->domWindow()));
             return true;
         }
     }
@@ -58,9 +55,6 @@
     if (!BindingSecurity::shouldAllowAccessToDOMWindow(lexicalGlobalObject, window, ThrowSecurityError))
         return false;
 
-    // FIXME: Search the whole frame hierarchy somewhere around here.
-    // We need to test the correct priority order.
-
     // Allow shortcuts like 'Image1' instead of document.images.Image1
     auto* document = window.document();
     if (is<HTMLDocument>(document)) {
@@ -74,7 +68,7 @@
                 namedItem = toJS(lexicalGlobalObject, thisObject->globalObject(), collection);
             } else
                 namedItem = toJS(lexicalGlobalObject, thisObject->globalObject(), htmlDocument.windowNamedItem(*atomicPropertyName));
-            slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, namedItem);
+            slot.setValue(thisObject, static_cast<unsigned>(PropertyAttribute::DontEnum), namedItem);
             return true;
         }
     }
@@ -82,25 +76,27 @@
     return false;
 }
 
-void JSDOMWindowProperties::finishCreation(VM& vm)
+void JSDOMWindowProperties::finishCreation(JSGlobalObject& globalObject)
 {
+    VM& vm = globalObject.vm();
     Base::finishCreation(vm);
     ASSERT(inherits(vm, info()));
     JSC_TO_STRING_TAG_WITHOUT_TRANSITION();
+    JSObject::preventExtensions(this, &globalObject);
 }
 
 bool JSDOMWindowProperties::getOwnPropertySlot(JSObject* object, JSGlobalObject* lexicalGlobalObject, PropertyName propertyName, PropertySlot& slot)
 {
+    VM& vm = lexicalGlobalObject->vm();
     auto* thisObject = jsCast<JSDOMWindowProperties*>(object);
     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+
     if (Base::getOwnPropertySlot(thisObject, lexicalGlobalObject, propertyName, slot))
         return true;
-    JSValue proto = thisObject->getPrototypeDirect(lexicalGlobalObject->vm());
-    if (proto.isObject() && jsCast<JSObject*>(proto)->hasProperty(lexicalGlobalObject, propertyName))
+    JSObject* proto = asObject(thisObject->getPrototypeDirect(vm));
+    if (proto->hasProperty(lexicalGlobalObject, propertyName))
         return false;
 
-    auto& vm = lexicalGlobalObject->vm();
-
     // FIXME: We should probably add support for JSRemoteDOMWindowBase too.
     auto* jsWindow = jsDynamicCast<JSDOMWindowBase*>(vm, thisObject->globalObject());
     if (!jsWindow)
@@ -116,6 +112,32 @@
     return getOwnPropertySlot(object, lexicalGlobalObject, Identifier::from(vm, index), slot);
 }
 
+bool JSDOMWindowProperties::deleteProperty(JSCell*, JSGlobalObject*, PropertyName, DeletePropertySlot&)
+{
+    return false;
+}
+
+bool JSDOMWindowProperties::deletePropertyByIndex(JSCell*, JSGlobalObject*, unsigned)
+{
+    return false;
+}
+
+bool JSDOMWindowProperties::preventExtensions(JSObject*, JSGlobalObject*)
+{
+    return false;
+}
+
+bool JSDOMWindowProperties::isExtensible(JSObject*, JSGlobalObject*)
+{
+    return true;
+}
+
+bool JSDOMWindowProperties::defineOwnProperty(JSObject*, JSGlobalObject* lexicalGlobalObject, PropertyName, const PropertyDescriptor&, bool shouldThrow)
+{
+    auto scope = DECLARE_THROW_SCOPE(lexicalGlobalObject->vm());
+    return typeError(lexicalGlobalObject, scope, shouldThrow, "Defining a property on a WindowProperties object is not allowed."_s);
+}
+
 JSC::IsoSubspace* JSDOMWindowProperties::subspaceForImpl(JSC::VM& vm)
 {
     return &static_cast<JSVMClientData*>(vm.clientData)->domWindowPropertiesSpace();

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.h (277828 => 277829)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.h	2021-05-20 22:10:27 UTC (rev 277828)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowProperties.h	2021-05-20 22:14:40 UTC (rev 277829)
@@ -46,7 +46,7 @@
     static JSDOMWindowProperties* create(JSC::Structure* structure, JSC::JSGlobalObject& globalObject)
     {
         JSDOMWindowProperties* ptr = new (NotNull, JSC::allocateCell<JSDOMWindowProperties>(globalObject.vm().heap)) JSDOMWindowProperties(structure, globalObject);
-        ptr->finishCreation(globalObject.vm());
+        ptr->finishCreation(globalObject);
         return ptr;
     }
 
@@ -59,6 +59,11 @@
 
     static bool getOwnPropertySlot(JSC::JSObject*, JSC::JSGlobalObject*, JSC::PropertyName, JSC::PropertySlot&);
     static bool getOwnPropertySlotByIndex(JSC::JSObject*, JSC::JSGlobalObject*, unsigned propertyName, JSC::PropertySlot&);
+    static bool deleteProperty(JSC::JSCell*, JSC::JSGlobalObject*, JSC::PropertyName, JSC::DeletePropertySlot&);
+    static bool deletePropertyByIndex(JSC::JSCell*, JSC::JSGlobalObject*, unsigned propertyName);
+    static bool preventExtensions(JSC::JSObject*, JSC::JSGlobalObject*);
+    static bool isExtensible(JSC::JSObject*, JSC::JSGlobalObject*);
+    static bool defineOwnProperty(JSC::JSObject*, JSC::JSGlobalObject*, JSC::PropertyName, const JSC::PropertyDescriptor&, bool shouldThrow);
 
 private:
     JSDOMWindowProperties(JSC::Structure* structure, JSC::JSGlobalObject& globalObject)
@@ -66,7 +71,7 @@
     {
     }
 
-    void finishCreation(JSC::VM&);
+    void finishCreation(JSC::JSGlobalObject&);
     static JSC::IsoSubspace* subspaceForImpl(JSC::VM&);
 };
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to