Diff
Modified: trunk/JSTests/ChangeLog (274307 => 274308)
--- trunk/JSTests/ChangeLog 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/JSTests/ChangeLog 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,22 @@
+2021-03-11 Alexey Shvayka <[email protected]>
+
+ Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
+ https://bugs.webkit.org/show_bug.cgi?id=203456
+
+ Reviewed by Robin Morisset.
+
+ * microbenchmarks/global-var-put-to-scope.js: Added.
+ * stress/eval-func-decl-in-frozen-global.js:
+ Object.freeze() redefines all global variables as ReadOnly, including hoisted `var error`.
+ Aligns with V8.
+
+ * stress/global-object-define-own-property-put-to-scope.js: Added.
+ * stress/global-object-define-own-property.js: Added.
+ * stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js:
+ Fix unwanted name conflict, which was an error in the original test, not an intended part of it.
+ Also, remove misleading comment on `defineProperty` and assert accessors are created on global object.
+ Aligns with V8.
+
2021-03-11 Commit Queue <[email protected]>
Unreviewed, reverting r274263.
Added: trunk/JSTests/microbenchmarks/global-var-put-to-scope.js (0 => 274308)
--- trunk/JSTests/microbenchmarks/global-var-put-to-scope.js (rev 0)
+++ trunk/JSTests/microbenchmarks/global-var-put-to-scope.js 2021-03-12 00:08:05 UTC (rev 274308)
@@ -0,0 +1,4 @@
+var k0 = 0, k1 = 0, k2 = 0, k3 = 0, k4 = 0, k5 = 0, k6 = 0, k7 = 0, k8 = 0, k9 = 0, k10 = 0, k11 = 0, k12 = 0, k13 = 0, k14 = 0, k15 = 0, k16 = 0, k17 = 0, k18 = 0, k19 = 0, k20 = 0, k21 = 0, k22 = 0, k23 = 0, k24 = 0, k25 = 0, k26 = 0, k27 = 0, k28 = 0, k29 = 0, k30 = 0, k31 = 0, k32 = 0, k33 = 0, k34 = 0, k35 = 0, k36 = 0, k37 = 0, k38 = 0, k39 = 0, k40 = 0, k41 = 0, k42 = 0, k43 = 0, k44 = 0, k45 = 0, k46 = 0, k47 = 0, k48 = 0, k49 = 0;
+
+for (var i = 0; i < 1e7; i++)
+ k0 = i; k1 = i; k2 = i; k3 = i; k4 = i; k5 = i; k6 = i; k7 = i; k8 = i; k9 = i; k10 = i; k11 = i; k12 = i; k13 = i; k14 = i; k15 = i; k16 = i; k17 = i; k18 = i; k19 = i; k20 = i; k21 = i; k22 = i; k23 = i; k24 = i; k25 = i; k26 = i; k27 = i; k28 = i; k29 = i; k30 = i; k31 = i; k32 = i; k33 = i; k34 = i; k35 = i; k36 = i; k37 = i; k38 = i; k39 = i; k40 = i; k41 = i; k42 = i; k43 = i; k44 = i; k45 = i; k46 = i; k47 = i; k48 = i; k49 = i;
Modified: trunk/JSTests/stress/eval-func-decl-in-frozen-global.js (274307 => 274308)
--- trunk/JSTests/stress/eval-func-decl-in-frozen-global.js 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/JSTests/stress/eval-func-decl-in-frozen-global.js 2021-03-12 00:08:05 UTC (rev 274308)
@@ -36,7 +36,7 @@
Object.freeze(this);
{
- var error = false;
+ let error = false;
try {
eval('{ function boo() {} }');
} catch (e) {
Added: trunk/JSTests/stress/global-object-define-own-property-put-to-scope.js (0 => 274308)
--- trunk/JSTests/stress/global-object-define-own-property-put-to-scope.js (rev 0)
+++ trunk/JSTests/stress/global-object-define-own-property-put-to-scope.js 2021-03-12 00:08:05 UTC (rev 274308)
@@ -0,0 +1,85 @@
+function shouldThrow(func, expectedMessage) {
+ var errorThrown = false;
+ try {
+ func();
+ } catch (error) {
+ errorThrown = true;
+ if (String(error) !== expectedMessage)
+ throw new Error(`Bad error: ${error}`)
+ }
+ if (!errorThrown)
+ throw new Error("Didn't throw!");
+}
+
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error(`Bad value: ${actual}!`);
+}
+
+var foo = 1;
+eval("var bar");
+function func() {}
+
+shouldBe(Object.getOwnPropertyDescriptor(globalThis, "foo").value, 1);
+shouldBe(Object.getOwnPropertyDescriptor(globalThis, "bar").value, undefined);
+shouldBe(typeof Object.getOwnPropertyDescriptor(globalThis, "func").value, "function");
+
+foo = 2;
+bar = 2;
+func = 2;
+
+shouldBe(Object.getOwnPropertyDescriptor(globalThis, "foo").value, 2);
+shouldBe(Object.getOwnPropertyDescriptor(globalThis, "bar").value, 2);
+shouldBe(Object.getOwnPropertyDescriptor(globalThis, "func").value, 2);
+
+foo = 3;
+bar = 3;
+func = 3;
+
+Object.defineProperty(globalThis, "foo", { value: 4 });
+Object.defineProperty(globalThis, "bar", { value: 4 });
+Object.defineProperty(globalThis, "func", { value: 4 });
+
+shouldBe(foo, 4);
+shouldBe(bar, 4);
+shouldBe(func, 4);
+
+Object.defineProperty(globalThis, "foo", { writable: false });
+Object.defineProperty(globalThis, "bar", { writable: false });
+Object.defineProperty(globalThis, "func", { writable: false });
+
+foo = 5;
+bar = 5;
+func = 5;
+
+shouldBe(foo, 4);
+shouldBe(bar, 4);
+shouldBe(func, 4);
+
+shouldThrow(() => { "use strict"; foo = 4; }, "TypeError: Attempted to assign to readonly property.");
+shouldThrow(() => { "use strict"; bar = 4; }, "TypeError: Attempted to assign to readonly property.");
+shouldThrow(() => { "use strict"; func = 4; }, "TypeError: Attempted to assign to readonly property.");
+
+eval("var x = 1");
+var y = 1;
+function setVars(val) { x = val; y = val; }
+noInline(setVars);
+const writableThreshold = 9e6;
+
+for (let i = 0; i < 10e6; i++) {
+ setVars(i);
+
+ if (i < writableThreshold) {
+ shouldBe(x, i);
+ shouldBe(y, i);
+ } else if (i === writableThreshold) {
+ Object.defineProperty(globalThis, "x", { writable: false });
+ Object.defineProperty(globalThis, "y", { writable: false });
+ } else {
+ shouldBe(x, writableThreshold);
+ shouldBe(y, writableThreshold);
+ }
+}
+
+shouldThrow(() => { "use strict"; x = writableThreshold; }, "TypeError: Attempted to assign to readonly property.");
+shouldThrow(() => { "use strict"; y = writableThreshold; }, "TypeError: Attempted to assign to readonly property.");
Added: trunk/JSTests/stress/global-object-define-own-property.js (0 => 274308)
--- trunk/JSTests/stress/global-object-define-own-property.js (rev 0)
+++ trunk/JSTests/stress/global-object-define-own-property.js 2021-03-12 00:08:05 UTC (rev 274308)
@@ -0,0 +1,66 @@
+"use strict";
+
+function shouldThrow(func, expectedMessage) {
+ var errorThrown = false;
+ try {
+ func();
+ } catch (error) {
+ errorThrown = true;
+ if (String(error) !== expectedMessage)
+ throw new Error(`Bad error: ${error}`)
+ }
+ if (!errorThrown)
+ throw new Error("Didn't throw!");
+}
+
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error(`Bad value: ${actual}!`);
+}
+
+for (const key of ["Infinity", "NaN", "undefined"]) {
+ const value = globalThis[key];
+ Object.defineProperty(globalThis, key, {});
+ shouldBe(Reflect.defineProperty(globalThis, key, { value }), true);
+ Object.defineProperty(globalThis, key, { value, writable: false });
+ shouldBe(Reflect.defineProperty(globalThis, key, { value, writable: false, enumerable: false }), true);
+ Object.defineProperty(globalThis, key, { value, writable: false, enumerable: false, configurable: false });
+
+ shouldBe(Reflect.defineProperty(globalThis, key, { value: {} }), false);
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { writable: true }); }, "TypeError: Attempting to change writable attribute of unconfigurable property.");
+ shouldBe(Reflect.defineProperty(globalThis, key, { enumerable: true }), false);
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { configurable: true }); }, "TypeError: Attempting to change configurable attribute of unconfigurable property.")
+ shouldBe(Reflect.defineProperty(globalThis, key, { get() {} }), false);
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { get() {}, set() {} }); }, "TypeError: Attempting to change access mechanism for an unconfigurable property.");
+}
+
+var foo = 1;
+var bar;
+function func() {}
+
+for (const key of ["foo", "bar", "func"]) {
+ const value = globalThis[key];
+ Object.defineProperty(globalThis, key, {});
+ shouldBe(Reflect.defineProperty(globalThis, key, { value }), true);
+ Object.defineProperty(globalThis, key, { value, writable: true });
+ shouldBe(Reflect.defineProperty(globalThis, key, { value, writable: true, enumerable: true }), true);
+ Object.defineProperty(globalThis, key, { value, writable: true, enumerable: true, configurable: false });
+
+ shouldBe(Reflect.defineProperty(globalThis, key, { value: key }), true);
+ shouldBe(Object.getOwnPropertyDescriptor(globalThis, key).value, key);
+ shouldBe(globalThis[key], key);
+
+ Object.defineProperty(globalThis, key, { writable: false });
+ shouldBe(Reflect.getOwnPropertyDescriptor(globalThis, key).writable, false);
+ shouldThrow(() => { globalThis[key] = {}; }, "TypeError: Attempted to assign to readonly property.");
+ shouldBe(globalThis[key], key);
+
+ shouldBe(Reflect.defineProperty(globalThis, key, { writable: true }), false);
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { value: {} }); }, "TypeError: Attempting to change value of a readonly property.");
+ shouldBe(globalThis[key], key);
+
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { enumerable: false }); }, "TypeError: Attempting to change enumerable attribute of unconfigurable property.");
+ shouldBe(Reflect.defineProperty(globalThis, key, { configurable: true }), false);
+ shouldThrow(() => { Object.defineProperty(globalThis, key, { get() {}, set() {} }); }, "TypeError: Attempting to change access mechanism for an unconfigurable property.");
+ shouldBe(Reflect.defineProperty(globalThis, key, { get() {}, set() {} }), false);
+}
Modified: trunk/JSTests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js (274307 => 274308)
--- trunk/JSTests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/JSTests/stress/to-this-before-arrow-function-closes-over-this-that-starts-as-lexical-environment.js 2021-03-12 00:08:05 UTC (rev 274308)
@@ -17,8 +17,7 @@
function capture() { return wrapper; }
function wrapper() {
let x = () => {
- // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
- Object.defineProperty(this, "foo", {
+ Object.defineProperty(this, "baz", {
get: function() { },
set: function() { }
});
@@ -37,11 +36,11 @@
function capture() { return wrapper; }
function wrapper() {
let x = () => {
- // This should not defineProperty on a JSLexicalEnvironment! That's a huge bug.
- Object.defineProperty(this, "foo", {
+ Object.defineProperty(this, "baz2", {
get: function() { },
set: function() { }
});
+ assert(this === globalThis);
}
x();
@@ -56,3 +55,6 @@
wrapper();
}
foo2();
+
+assert(this.hasOwnProperty("baz"));
+assert(this.hasOwnProperty("baz2"));
Modified: trunk/LayoutTests/ChangeLog (274307 => 274308)
--- trunk/LayoutTests/ChangeLog 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/ChangeLog 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,17 @@
+2021-03-11 Alexey Shvayka <[email protected]>
+
+ Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
+ https://bugs.webkit.org/show_bug.cgi?id=203456
+
+ Reviewed by Robin Morisset.
+
+ * fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt:
+ * fast/dom/Window/Location/window-override-location-using-defineGetter.html:
+ * fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt:
+ * fast/dom/Window/Location/window-override-window-using-defineGetter.html:
+ * fast/dom/getter-on-window-object2-expected.txt:
+ * fast/dom/getter-on-window-object2.html:
+
2021-03-11 Aditya Keerthi <[email protected]>
[iOS][FCR] Update disabled state for button-like controls
Modified: trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt (274307 => 274308)
--- trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter-expected.txt 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,6 @@
+PASS function () {
+ window.__defineGetter__("location", () => "haxored");
+ } threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
PASS result is correctValue
PASS successfullyParsed is true
Modified: trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter.html (274307 => 274308)
--- trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter.html 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/Window/Location/window-override-location-using-defineGetter.html 2021-03-12 00:08:05 UTC (rev 274308)
@@ -5,7 +5,9 @@
</head>
<body>
<script>
- window.__defineGetter__("location", function() { return "haxored"; });
+ shouldThrowErrorName(function() {
+ window.__defineGetter__("location", () => "haxored");
+ }, "TypeError");
var result = normalizeURL(String(window.location));
var correctValue = normalizeURL(document.URL);
Modified: trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt (274307 => 274308)
--- trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter-expected.txt 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,6 @@
+PASS function () {
+ window.__defineGetter__("window", () => ({ location: "haxored" }));
+ } threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
PASS result is correctValue
PASS successfullyParsed is true
Modified: trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter.html (274307 => 274308)
--- trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter.html 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/Window/Location/window-override-window-using-defineGetter.html 2021-03-12 00:08:05 UTC (rev 274308)
@@ -5,9 +5,9 @@
</head>
<body>
<script>
- window.__defineGetter__("window", function() {
- return { location: "haxored" };
- });
+ shouldThrowErrorName(function() {
+ window.__defineGetter__("window", () => ({ location: "haxored" }));
+ }, "TypeError");
var result = normalizeURL(String(window.location));
var correctValue = normalizeURL(document.URL);
Modified: trunk/LayoutTests/fast/dom/getter-on-window-object2-expected.txt (274307 => 274308)
--- trunk/LayoutTests/fast/dom/getter-on-window-object2-expected.txt 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/getter-on-window-object2-expected.txt 2021-03-12 00:08:05 UTC (rev 274308)
@@ -3,10 +3,16 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS function () {
+ window.__defineGetter__("x", function() { return "window.x __getter__"; });
+} threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
PASS window.x is 1
PASS typeof window.__lookupGetter__('x') is 'undefined'
PASS typeof Object.getOwnPropertyDescriptor(window, 'x').get is 'undefined'
+PASS function () {
+window.__defineSetter__("x", function() { debug("window.x __setter__ called"); });
+} threw exception TypeError: Attempting to change configurable attribute of unconfigurable property..
PASS window.x is 2
PASS typeof window.__lookupGetter__('x') is 'undefined'
PASS typeof Object.getOwnPropertyDescriptor(window, 'x').get is 'undefined'
Modified: trunk/LayoutTests/fast/dom/getter-on-window-object2.html (274307 => 274308)
--- trunk/LayoutTests/fast/dom/getter-on-window-object2.html 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/fast/dom/getter-on-window-object2.html 2021-03-12 00:08:05 UTC (rev 274308)
@@ -4,9 +4,9 @@
description("This page tests what happens when a getter / setter on the window object conflicts with another property or declared variable");
var x = 1;
-try {
+shouldThrowErrorName(function() {
window.__defineGetter__("x", function() { return "window.x __getter__"; });
-} catch(e) { debug(e); }
+}, "TypeError");
shouldBe("window.x", "1");
shouldBe("typeof window.__lookupGetter__('x')", "'undefined'");
@@ -14,9 +14,9 @@
debug("");
-try {
+shouldThrowErrorName(function() {
window.__defineSetter__("x", function() { debug("window.x __setter__ called"); });
-} catch(e) { debug(e); }
+}, "TypeError");
x = 2;
shouldBe("window.x", "2");
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (274307 => 274308)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,13 @@
+2021-03-11 Alexey Shvayka <[email protected]>
+
+ Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
+ https://bugs.webkit.org/show_bug.cgi?id=203456
+
+ Reviewed by Robin Morisset.
+
+ * web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt: Added.
+ * web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html: Added.
+
2021-03-10 Antoine Quint <[email protected]>
Improve font-variation-settings interpolation
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt (0 => 274308)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin-expected.txt 2021-03-12 00:08:05 UTC (rev 274308)
@@ -0,0 +1,10 @@
+
+PASS [[DefineOwnProperty]] success: "window"
+PASS [[DefineOwnProperty]] failure: "window"
+PASS [[DefineOwnProperty]] success: "document"
+PASS [[DefineOwnProperty]] failure: "document"
+PASS [[DefineOwnProperty]] success: "location"
+PASS [[DefineOwnProperty]] failure: "location"
+PASS [[DefineOwnProperty]] success: "top"
+PASS [[DefineOwnProperty]] failure: "top"
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html (0 => 274308)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html 2021-03-12 00:08:05 UTC (rev 274308)
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>[[DefineOwnProperty]] on a WindowProxy forwards to OrdinaryDefineOwnProperty for same-origin objects</title>
+<link rel="help" href=""
+
+<script src=""
+<script src=""
+
+<script>
+"use strict";
+
+for (const key of ["window", "document", "location", "top"]) {
+ const { get, set } = Object.getOwnPropertyDescriptor(window, key);
+
+ test(() => {
+ Object.defineProperty(window, key, {});
+ assert_true(Reflect.defineProperty(window, key, { configurable: false }), "[[Configurable]]: false");
+ Object.defineProperty(window, key, { enumerable: true });
+
+ assert_true(Reflect.defineProperty(window, key, { get }), "[[Get]]: unchanged");
+ Object.defineProperty(window, key, { set });
+ assert_true(Reflect.defineProperty(window, key, { get, set }), "[[Get]]: unchanged, [[Set]]: unchanged");
+
+ Object.defineProperty(window, key, { get, set, enumerable: true, configurable: false });
+ }, `[[DefineOwnProperty]] success: "${key}"`);
+
+ test(() => {
+ assert_throws_js(TypeError, () => {
+ Object.defineProperty(window, key, { configurable: true });
+ }, "[[Configurable]]: true");
+
+ assert_false(Reflect.defineProperty(window, key, { enumerable: false }), "[[Enumerable]]: false");
+
+ assert_throws_js(TypeError, () => {
+ Object.defineProperty(window, key, { get() {}, set });
+ }, "[[Get]]: changed, [[Set]]: unchanged");
+
+ assert_false(Reflect.defineProperty(window, key, { get, set() {} }), "[[Get]]: unchanged, [[Set]]: changed");
+
+ assert_throws_js(TypeError, () => {
+ Object.defineProperty(window, key, { writable: false, configurable: true });
+ }, "[[Writable]]: false, [[Configurable]]: true");
+
+ assert_false(Reflect.defineProperty(window, key, { value: window[key], enumerable: true }), "[[Value]], [[Enumerable]]: true");
+ }, `[[DefineOwnProperty]] failure: "${key}"`);
+}
+</script>
Modified: trunk/Source/_javascript_Core/ChangeLog (274307 => 274308)
--- trunk/Source/_javascript_Core/ChangeLog 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,59 @@
+2021-03-11 Alexey Shvayka <[email protected]>
+
+ Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
+ https://bugs.webkit.org/show_bug.cgi?id=203456
+
+ Reviewed by Robin Morisset.
+
+ Per spec, top-level `var` bindings are non-configurable properties of the global
+ object [1], while `undefined` / `NaN` / `Infinity` are also non-writable [2].
+
+ Prior to this change, redefining global `var` binding with accessor descriptor
+ failed silently (rather than throwing a TypeError); redefining with data or
+ generic descriptor created a structure property, which took precedence over
+ symbol table entry in JSGlobalObject::getOwnPropertySlot(), effectively
+ destroying live binding between `global.foo` and `var foo`.
+
+ This patch re-engineers JSGlobalObject::defineOwnProperty(), fixing both issues
+ mentioned above. If defineOwnProperty() override is removed, there is no way
+ a live binding can be maintained.
+
+ In a follow-up change, JSGlobalObject::getOwnPropertySlot() will be updated to
+ search symbol table first, aligning it with the spec [3], put(), and
+ defineOwnProperty(). Apart from consistency, this will bring a mild speed-up.
+
+ To accomodate global `var` binding reassignment right after it becomes read-only
+ (in the same scope), this patch introduces a watchpoint that can be fired by
+ JSGlobalObject::defineOwnProperty(). put_to_scope performance is neutral.
+
+ Also, this patch removes unused symbolTableGet() overload and orphaned
+ JSGlobalObject::defineGetter() / JSGlobalObject::defineSetter() declarations.
+
+ [1]: https://tc39.es/ecma262/#sec-object-environment-records-createmutablebinding-n-d
+ [2]: https://tc39.es/ecma262/#sec-value-properties-of-the-global-object
+ [3]: https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::needsDynamicLookup):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * jit/JIT.cpp:
+ (JSC::JIT::emitVarReadOnlyCheck):
+ * jit/JIT.h:
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::emit_op_put_to_scope):
+ * jit/JITPropertyAccess32_64.cpp:
+ (JSC::JIT::emit_op_put_to_scope):
+ * llint/LowLevelInterpreter.asm:
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::JSGlobalObject):
+ (JSC::JSGlobalObject::defineOwnProperty):
+ * runtime/JSGlobalObject.h:
+ (JSC::JSGlobalObject::varReadOnlyWatchpoint):
+ * runtime/JSSymbolTableObject.h:
+ (JSC::symbolTableGet):
+
2021-03-11 BJ Burg <[email protected]>
Web Inspector: Occasional crash under RemoteConnectionToTargetCocoa::close()
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (274307 => 274308)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -4215,8 +4215,15 @@
return true;
switch (type) {
- case GlobalProperty:
case GlobalVar:
+ case GlobalVarWithVarInjectionChecks: {
+ if (opcode == op_put_to_scope && globalObject->varReadOnlyWatchpoint()->hasBeenInvalidated())
+ return true;
+
+ return false;
+ }
+
+ case GlobalProperty:
case GlobalLexicalVar:
case ClosureVar:
case LocalClosureVar:
@@ -4251,7 +4258,6 @@
return true;
case GlobalPropertyWithVarInjectionChecks:
- case GlobalVarWithVarInjectionChecks:
case GlobalLexicalVarWithVarInjectionChecks:
case ClosureVarWithVarInjectionChecks:
return false;
@@ -7925,6 +7931,8 @@
Node* value = addToGraph(GetGlobalLexicalVariable, OpInfo(operand), OpInfo(prediction));
addToGraph(CheckNotEmpty, value);
}
+ if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks)
+ m_graph.watchpoints().addLazily(globalObject->varReadOnlyWatchpoint());
JSSegmentedVariableObject* scopeObject = jsCast<JSSegmentedVariableObject*>(JSScope::constantScopeForCodeBlock(resolveType, m_inlineStackTop->m_codeBlock));
if (watchpoints) {
Modified: trunk/Source/_javascript_Core/jit/JIT.cpp (274307 => 274308)
--- trunk/Source/_javascript_Core/jit/JIT.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/jit/JIT.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -117,6 +117,12 @@
addSlowCase(branch8(NotEqual, Address(pointerToSet, WatchpointSet::offsetOfState()), TrustedImm32(IsInvalidated)));
}
+void JIT::emitVarReadOnlyCheck(ResolveType resolveType)
+{
+ if (resolveType == GlobalVar || resolveType == GlobalVarWithVarInjectionChecks)
+ addSlowCase(branch8(Equal, AbsoluteAddress(m_codeBlock->globalObject()->varReadOnlyWatchpoint()->addressOfState()), TrustedImm32(IsInvalidated)));
+}
+
void JIT::assertStackPointerOffset()
{
if (!ASSERT_ENABLED)
Modified: trunk/Source/_javascript_Core/jit/JIT.h (274307 => 274308)
--- trunk/Source/_javascript_Core/jit/JIT.h 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/jit/JIT.h 2021-03-12 00:08:05 UTC (rev 274308)
@@ -752,6 +752,7 @@
template<typename Op>
void emitNewFuncExprCommon(const Instruction*);
void emitVarInjectionCheck(bool needsVarInjectionChecks);
+ void emitVarReadOnlyCheck(ResolveType);
void emitResolveClosure(VirtualRegister dst, VirtualRegister scope, bool needsVarInjectionChecks, unsigned depth);
void emitLoadWithStructureCheck(VirtualRegister scope, Structure** structureSlot);
#if USE(JSVALUE64)
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (274307 => 274308)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1227,6 +1227,7 @@
JSScope* constantScope = JSScope::constantScopeForCodeBlock(resolveType, m_codeBlock);
RELEASE_ASSERT(constantScope);
emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
+ emitVarReadOnlyCheck(resolveType);
if (!isInitialization(getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
// We need to do a TDZ check here because we can't always prove we need to emit TDZ checks statically.
if (indirectLoadForOperand)
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp (274307 => 274308)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1243,6 +1243,7 @@
RELEASE_ASSERT(constantScope);
emitWriteBarrier(constantScope, value, ShouldFilterValue);
emitVarInjectionCheck(needsVarInjectionChecks(resolveType));
+ emitVarReadOnlyCheck(resolveType);
if (!isInitialization(getPutInfo.initializationMode()) && (resolveType == GlobalLexicalVar || resolveType == GlobalLexicalVarWithVarInjectionChecks)) {
// We need to do a TDZ check here because we can't always prove we need to emit TDZ checks statically.
if (indirectLoadForOperand)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (274307 => 274308)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1341,6 +1341,13 @@
bbneq WatchpointSet::m_state[set], IsInvalidated, slow
end
+macro varReadOnlyCheck(slowPath, scratch)
+ loadp CodeBlock[cfr], scratch
+ loadp CodeBlock::m_globalObject[scratch], scratch
+ loadp JSGlobalObject::m_varReadOnlyWatchpoint[scratch], scratch
+ bbeq WatchpointSet::m_state[scratch], IsInvalidated, slowPath
+end
+
macro checkSwitchToJIT(increment, action)
loadp CodeBlock[cfr], t0
baddis increment, CodeBlock::m_llintExecuteCounter + BaselineExecutionCounter::m_counter[t0], .continue
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (274307 => 274308)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2021-03-12 00:08:05 UTC (rev 274308)
@@ -2625,6 +2625,7 @@
.pGlobalVar:
bineq t0, GlobalVar, .pGlobalLexicalVar
+ varReadOnlyCheck(.pDynamic, t2)
putGlobalVariable()
writeBarrierOnGlobalObject(size, get, m_value)
dispatch()
@@ -2652,7 +2653,10 @@
.pGlobalVarWithVarInjectionChecks:
bineq t0, GlobalVarWithVarInjectionChecks, .pGlobalLexicalVarWithVarInjectionChecks
+ # FIXME: Avoid loading m_globalObject twice
+ # https://bugs.webkit.org/show_bug.cgi?id=223097
varInjectionCheck(.pDynamic)
+ varReadOnlyCheck(.pDynamic, t2)
putGlobalVariable()
writeBarrierOnGlobalObject(size, get, m_value)
dispatch()
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (274307 => 274308)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2021-03-12 00:08:05 UTC (rev 274308)
@@ -2727,6 +2727,7 @@
.pGlobalVar:
bineq t0, GlobalVar, .pGlobalLexicalVar
+ varReadOnlyCheck(.pDynamic, t2)
putGlobalVariable()
writeBarrierOnGlobalObject(size, get, m_value)
dispatch()
@@ -2754,7 +2755,10 @@
.pGlobalVarWithVarInjectionChecks:
bineq t0, GlobalVarWithVarInjectionChecks, .pGlobalLexicalVarWithVarInjectionChecks
+ # FIXME: Avoid loading m_globalObject twice
+ # https://bugs.webkit.org/show_bug.cgi?id=223097
varInjectionCheck(.pDynamic, t2)
+ varReadOnlyCheck(.pDynamic, t2)
putGlobalVariable()
writeBarrierOnGlobalObject(size, get, m_value)
dispatch()
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (274307 => 274308)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -495,6 +495,7 @@
, m_masqueradesAsUndefinedWatchpoint(WatchpointSet::create(IsWatched))
, m_havingABadTimeWatchpoint(WatchpointSet::create(IsWatched))
, m_varInjectionWatchpoint(WatchpointSet::create(IsWatched))
+ , m_varReadOnlyWatchpoint(WatchpointSet::create(IsWatched))
, m_weakRandom(Options::forceWeakRandomSeed() ? Options::forcedWeakRandomSeed() : static_cast<unsigned>(randomNumber() * (std::numeric_limits<unsigned>::max() + 1.0)))
, m_arrayIteratorProtocolWatchpointSet(IsWatched)
, m_mapIteratorProtocolWatchpointSet(IsWatched)
@@ -1480,13 +1481,35 @@
bool JSGlobalObject::defineOwnProperty(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
{
VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
JSGlobalObject* thisObject = jsCast<JSGlobalObject*>(object);
- PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry, &vm);
- // silently ignore attempts to add accessors aliasing vars.
- if (descriptor.isAccessorDescriptor() && symbolTableGet(thisObject, propertyName, slot))
- return false;
- slot.disallowVMEntry.reset();
- return Base::defineOwnProperty(thisObject, globalObject, propertyName, descriptor, shouldThrow);
+
+ SymbolTableEntry entry;
+ PropertyDescriptor currentDescriptor;
+ if (symbolTableGet(thisObject, propertyName, entry, currentDescriptor)) {
+ bool isExtensible = false; // ignored since current descriptor is present
+ bool isCurrentDefined = true;
+ bool isCompatibleDescriptor = validateAndApplyPropertyDescriptor(globalObject, nullptr, propertyName, isExtensible, descriptor, isCurrentDefined, currentDescriptor, shouldThrow);
+ EXCEPTION_ASSERT(!!scope.exception() == !isCompatibleDescriptor);
+ if (!isCompatibleDescriptor)
+ return false;
+
+ if (descriptor.value()) {
+ bool ignoreReadOnlyErrors = true;
+ bool putResult = false;
+ if (symbolTablePutTouchWatchpointSet(thisObject, globalObject, propertyName, descriptor.value(), shouldThrow, ignoreReadOnlyErrors, putResult))
+ ASSERT(putResult);
+ scope.assertNoException();
+ }
+ if (descriptor.writablePresent() && !descriptor.writable() && !entry.isReadOnly()) {
+ entry.setAttributes(static_cast<unsigned>(PropertyAttribute::ReadOnly));
+ thisObject->symbolTable()->set(propertyName.uid(), entry);
+ thisObject->varReadOnlyWatchpoint()->fireAll(vm, "GlobalVar was redefined as ReadOnly");
+ }
+ return true;
+ }
+
+ RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, globalObject, propertyName, descriptor, shouldThrow));
}
void JSGlobalObject::addGlobalVar(const Identifier& ident)
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (274307 => 274308)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h 2021-03-12 00:08:05 UTC (rev 274308)
@@ -474,6 +474,7 @@
RefPtr<WatchpointSet> m_masqueradesAsUndefinedWatchpoint;
RefPtr<WatchpointSet> m_havingABadTimeWatchpoint;
RefPtr<WatchpointSet> m_varInjectionWatchpoint;
+ RefPtr<WatchpointSet> m_varReadOnlyWatchpoint;
std::unique_ptr<JSGlobalObjectRareData> m_rareData;
@@ -623,9 +624,6 @@
JS_EXPORT_PRIVATE static bool getOwnPropertySlot(JSObject*, JSGlobalObject*, PropertyName, PropertySlot&);
JS_EXPORT_PRIVATE static bool put(JSCell*, JSGlobalObject*, PropertyName, JSValue, PutPropertySlot&);
-
- JS_EXPORT_PRIVATE static void defineGetter(JSObject*, JSGlobalObject*, PropertyName, JSObject* getterFunc, unsigned attributes);
- JS_EXPORT_PRIVATE static void defineSetter(JSObject*, JSGlobalObject*, PropertyName, JSObject* setterFunc, unsigned attributes);
JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, JSGlobalObject*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
void addVar(JSGlobalObject* globalObject, const Identifier& propertyName)
@@ -998,6 +996,7 @@
WatchpointSet* masqueradesAsUndefinedWatchpoint() { return m_masqueradesAsUndefinedWatchpoint.get(); }
WatchpointSet* havingABadTimeWatchpoint() { return m_havingABadTimeWatchpoint.get(); }
WatchpointSet* varInjectionWatchpoint() { return m_varInjectionWatchpoint.get(); }
+ WatchpointSet* varReadOnlyWatchpoint() { return m_varReadOnlyWatchpoint.get(); }
bool isHavingABadTime() const
{
Modified: trunk/Source/_javascript_Core/runtime/JSSymbolTableObject.h (274307 => 274308)
--- trunk/Source/_javascript_Core/runtime/JSSymbolTableObject.h 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/_javascript_Core/runtime/JSSymbolTableObject.h 2021-03-12 00:08:05 UTC (rev 274308)
@@ -99,7 +99,7 @@
template<typename SymbolTableObjectType>
inline bool symbolTableGet(
- SymbolTableObjectType* object, PropertyName propertyName, PropertyDescriptor& descriptor)
+ SymbolTableObjectType* object, PropertyName propertyName, SymbolTableEntry& entry, PropertyDescriptor& descriptor)
{
SymbolTable& symbolTable = *object->symbolTable();
ConcurrentJSLocker locker(symbolTable.m_lock);
@@ -106,7 +106,7 @@
SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.uid());
if (iter == symbolTable.end(locker))
return false;
- SymbolTableEntry::Fast entry = iter->value;
+ entry = iter->value;
ASSERT(!entry.isNull());
ScopeOffset offset = entry.scopeOffset();
@@ -119,29 +119,6 @@
}
template<typename SymbolTableObjectType>
-inline bool symbolTableGet(
- SymbolTableObjectType* object, PropertyName propertyName, PropertySlot& slot,
- bool& slotIsWriteable)
-{
- SymbolTable& symbolTable = *object->symbolTable();
- ConcurrentJSLocker locker(symbolTable.m_lock);
- SymbolTable::Map::iterator iter = symbolTable.find(locker, propertyName.uid());
- if (iter == symbolTable.end(locker))
- return false;
- SymbolTableEntry::Fast entry = iter->value;
- ASSERT(!entry.isNull());
-
- ScopeOffset offset = entry.scopeOffset();
- // Defend against the inspector asking for a var after it has been optimized out.
- if (!object->isValidScopeOffset(offset))
- return false;
-
- slot.setValue(object, entry.getAttributes() | PropertyAttribute::DontDelete, object->variableAt(offset).get());
- slotIsWriteable = !entry.isReadOnly();
- return true;
-}
-
-template<typename SymbolTableObjectType>
ALWAYS_INLINE void symbolTablePutTouchWatchpointSet(VM& vm, SymbolTableObjectType* object, PropertyName propertyName, JSValue value, WriteBarrierBase<Unknown>* reg, WatchpointSet* set)
{
reg->set(vm, object, value);
Modified: trunk/Source/WebCore/ChangeLog (274307 => 274308)
--- trunk/Source/WebCore/ChangeLog 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/WebCore/ChangeLog 2021-03-12 00:08:05 UTC (rev 274308)
@@ -1,3 +1,23 @@
+2021-03-11 Alexey Shvayka <[email protected]>
+
+ Align JSGlobalObject::defineOwnProperty() with the spec and other runtimes
+ https://bugs.webkit.org/show_bug.cgi?id=203456
+
+ Reviewed by Robin Morisset.
+
+ This patch removes `location` special-casing, which a) incorrectly returned
+ `false` if new descriptor was the same as the current one and b) failed
+ silently otherwise (rather than throwing a TypeError).
+
+ However, this change introduces `window` / `document` special-casing because
+ they exist on the structure and as symbol table entries (for performance reasons).
+ Aligns WebKit with Blink and partly with Gecko.
+
+ Test: imported/w3c/web-platform-tests/html/browsers/the-windowproxy-exotic-object/windowproxy-define-own-property-unforgeable-same-origin.html
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::defineOwnProperty):
+
2021-03-11 Chris Dumez <[email protected]>
Introduce WorkQueue::main() to get the main thread's work queue
Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (274307 => 274308)
--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2021-03-12 00:01:20 UTC (rev 274307)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2021-03-12 00:08:05 UTC (rev 274308)
@@ -475,20 +475,16 @@
bool JSDOMWindow::defineOwnProperty(JSC::JSObject* object, JSC::JSGlobalObject* lexicalGlobalObject, JSC::PropertyName propertyName, const JSC::PropertyDescriptor& descriptor, bool shouldThrow)
{
- JSC::VM& vm = lexicalGlobalObject->vm();
- auto scope = DECLARE_THROW_SCOPE(vm);
-
JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
// Only allow defining properties in this way by frames in the same origin, as it allows setters to be introduced.
if (!BindingSecurity::shouldAllowAccessToDOMWindow(lexicalGlobalObject, thisObject->wrapped(), ThrowSecurityError))
- RELEASE_AND_RETURN(scope, false);
-
- EXCEPTION_ASSERT(!scope.exception());
- // Don't allow shadowing location using accessor properties.
- if (descriptor.isAccessorDescriptor() && propertyName == Identifier::fromString(vm, "location"))
return false;
- RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow));
+ auto& builtinNames = static_cast<JSVMClientData*>(lexicalGlobalObject->vm().clientData)->builtinNames();
+ if (propertyName == builtinNames.documentPublicName() || propertyName == builtinNames.windowPublicName())
+ return JSObject::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow);
+
+ return Base::defineOwnProperty(thisObject, lexicalGlobalObject, propertyName, descriptor, shouldThrow);
}
JSValue JSDOMWindow::getPrototype(JSObject* object, JSGlobalObject* lexicalGlobalObject)
