Diff
Modified: trunk/JSTests/ChangeLog (259799 => 259800)
--- trunk/JSTests/ChangeLog 2020-04-09 14:48:31 UTC (rev 259799)
+++ trunk/JSTests/ChangeLog 2020-04-09 15:13:07 UTC (rev 259800)
@@ -1,3 +1,12 @@
+2020-04-09 Alexey Shvayka <shvaikal...@gmail.com>
+
+ getOwnPropertyDescriptor() is incorrect with Proxy of exotic object
+ https://bugs.webkit.org/show_bug.cgi?id=200560
+
+ Reviewed by Yusuke Suzuki.
+
+ * test262/expectations.yaml: Mark 14 test cases as passing.
+
2020-04-08 Yusuke Suzuki <ysuz...@apple.com>
[JSC] MultiDeleteByOffset should tell correct result AbstractValue in AI
Modified: trunk/JSTests/test262/expectations.yaml (259799 => 259800)
--- trunk/JSTests/test262/expectations.yaml 2020-04-09 14:48:31 UTC (rev 259799)
+++ trunk/JSTests/test262/expectations.yaml 2020-04-09 15:13:07 UTC (rev 259800)
@@ -672,9 +672,6 @@
test/built-ins/Array/prototype/splice/clamps-length-to-integer-limit.js:
default: 'Test262Error: Length is 2**53 - 1 Expected SameValue(«4294967295», «9007199254740991») to be true'
strict mode: 'Test262Error: Length is 2**53 - 1 Expected SameValue(«4294967295», «9007199254740991») to be true'
-test/built-ins/Array/prototype/splice/create-proxy.js:
- default: 'TypeError: Attempted to assign to readonly property.'
- strict mode: 'TypeError: Attempted to assign to readonly property.'
test/built-ins/Array/prototype/splice/create-species-length-exceeding-integer-limit.js:
default: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
strict mode: 'Test262Error: length and deleteCount were correctly clamped to 2^53-1 Expected SameValue(«4294967295», «9007199254740991») to be true'
@@ -1255,15 +1252,6 @@
test/built-ins/Proxy/defineProperty/trap-is-undefined-target-is-proxy.js:
default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all'
-test/built-ins/Proxy/getOwnPropertyDescriptor/trap-is-missing-target-is-proxy.js:
- default: "TypeError: undefined is not an object (evaluating 'originalDesc.enumerable')"
- strict mode: "TypeError: undefined is not an object (evaluating 'originalDesc.enumerable')"
-test/built-ins/Proxy/getOwnPropertyDescriptor/trap-is-null-target-is-proxy.js:
- default: "TypeError: undefined is not an object (evaluating 'originalDesc.value')"
- strict mode: "TypeError: undefined is not an object (evaluating 'originalDesc.value')"
-test/built-ins/Proxy/getOwnPropertyDescriptor/trap-is-undefined-target-is-proxy.js:
- default: "TypeError: undefined is not an object (evaluating 'originalDesc.value')"
- strict mode: "TypeError: undefined is not an object (evaluating 'originalDesc.value')"
test/built-ins/Proxy/ownKeys/trap-is-undefined-target-is-proxy.js:
default: 'Test262Error: Expected [length, foo, 0, Symbol()] and [Symbol(), length, foo, 0] to have the same contents. '
strict mode: 'Test262Error: Expected [length, foo, 0, Symbol()] and [Symbol(), length, foo, 0] to have the same contents. '
@@ -1273,15 +1261,6 @@
test/built-ins/Proxy/set/trap-is-missing-receiver-multiple-calls.js:
default: 'Test262Error: Expected [foo] and [foo, foo, foo] to have the same contents. getOwnPropertyDescriptor: key present on [[ProxyTarget]]'
strict mode: 'TypeError: Attempted to assign to readonly property.'
-test/built-ins/Proxy/set/trap-is-missing-target-is-proxy.js:
- default: 'Test262Error: Expected SameValue(«0», «1») to be true'
- strict mode: 'TypeError: Attempting to change configurable attribute of unconfigurable property.'
-test/built-ins/Proxy/set/trap-is-null-target-is-proxy.js:
- default: 'Test262Error: Expected [1, 2, 3] and [] to have the same contents. '
- strict mode: 'TypeError: Attempting to change configurable attribute of unconfigurable property.'
-test/built-ins/Proxy/set/trap-is-undefined-target-is-proxy.js:
- default: 'Test262Error: Expected true but got false'
- strict mode: 'Test262Error: Expected true but got false'
test/built-ins/Reflect/ownKeys/order-after-define-property.js:
default: 'Test262Error: Expected [Symbol(b), Symbol(a)] and [Symbol(a), Symbol(b)] to have the same contents. '
strict mode: 'Test262Error: Expected [Symbol(b), Symbol(a)] and [Symbol(a), Symbol(b)] to have the same contents. '
Modified: trunk/LayoutTests/ChangeLog (259799 => 259800)
--- trunk/LayoutTests/ChangeLog 2020-04-09 14:48:31 UTC (rev 259799)
+++ trunk/LayoutTests/ChangeLog 2020-04-09 15:13:07 UTC (rev 259800)
@@ -1,3 +1,13 @@
+2020-04-09 Alexey Shvayka <shvaikal...@gmail.com>
+
+ getOwnPropertyDescriptor() is incorrect with Proxy of exotic object
+ https://bugs.webkit.org/show_bug.cgi?id=200560
+
+ Reviewed by Yusuke Suzuki.
+
+ * js/getOwnPropertyDescriptor-host-object-proxy-expected.txt: Added.
+ * js/getOwnPropertyDescriptor-host-object-proxy.html: Added.
+
2020-04-09 Diego Pino Garcia <dp...@igalia.com>
[GTK] Gardening, update TestExpectations
Added: trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy-expected.txt (0 => 259800)
--- trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy-expected.txt (rev 0)
+++ trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy-expected.txt 2020-04-09 15:13:07 UTC (rev 259800)
@@ -0,0 +1,9 @@
+
+PASS Window target, no trap, "name" attribute
+PASS Window target, forwarding trap, [Unforgeable] "document" attribute
+PASS Window proxy target, custom trap, "onclick" event handler attribute
+PASS Document target, forwarding trap, [Unforgeable] "location" attribute
+PASS Document proxy target, custom trap, non-existent value attribute
+PASS Location target, no trap, [Unforgeable] "hash" attribute
+PASS Location proxy target, forwarding trap, [Unforgeable] "reload" method
+
Added: trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy.html (0 => 259800)
--- trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy.html (rev 0)
+++ trunk/LayoutTests/js/getOwnPropertyDescriptor-host-object-proxy.html 2020-04-09 15:13:07 UTC (rev 259800)
@@ -0,0 +1,92 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>getOwnPropertyDescriptor() is correct for Proxy with host object target</title>
+ <script src=""
+ <script src=""
+</head>
+<body>
+<script>
+'use strict';
+
+test(() => {
+ let windowProxy = new Proxy(window, {});
+ name = 'old_name';
+ let descriptor = Object.getOwnPropertyDescriptor(windowProxy, 'name');
+
+ assert_equals(descriptor.get.call(window), 'old_name');
+ descriptor.set.call(window, 'new_name');
+ assert_equals(name, 'new_name');
+ assert_true(descriptor.enumerable);
+ assert_true(descriptor.configurable);
+}, 'Window target, no trap, "name" attribute');
+
+test(() => {
+ let windowProxy = new Proxy(window, {});
+
+ assert_object_equals(
+ Object.getOwnPropertyDescriptor(windowProxy, 'document'),
+ Object.getOwnPropertyDescriptor(window, 'document')
+ );
+}, 'Window target, forwarding trap, [Unforgeable] "document" attribute');
+
+test(() => {
+ let trapResult = {get() {}, set(_val) {}, enumerable: false, configurable: true};
+ let windowProxy = new Proxy(new Proxy(window, {}), {
+ getOwnPropertyDescriptor: () => trapResult,
+ });
+
+ assert_object_equals(
+ Object.getOwnPropertyDescriptor(windowProxy, 'onclick'),
+ trapResult
+ );
+}, 'Window proxy target, custom trap, "onclick" event handler attribute');
+
+test(() => {
+ let documentProxy = new Proxy(document, {
+ getOwnPropertyDescriptor: Reflect.getOwnPropertyDescriptor,
+ });
+
+ assert_object_equals(
+ Object.getOwnPropertyDescriptor(documentProxy, 'location'),
+ Object.getOwnPropertyDescriptor(document, 'location')
+ );
+}, 'Document target, forwarding trap, [Unforgeable] "location" attribute');
+
+test(() => {
+ let trapResult = {value: 4, writable: false, enumerable: true, configurable: true};
+ let documentProxy = new Proxy(new Proxy(document, {}), {
+ getOwnPropertyDescriptor: () => trapResult,
+ });
+
+ assert_object_equals(
+ Object.getOwnPropertyDescriptor(documentProxy, 'foo'),
+ trapResult
+ );
+}, 'Document proxy target, custom trap, non-existent value attribute');
+
+test(() => {
+ let locationProxy = new Proxy(location, {});
+ location.hash = '#old';
+ let descriptor = Object.getOwnPropertyDescriptor(locationProxy, 'hash');
+
+ assert_equals(descriptor.get.call(location), '#old');
+ descriptor.set.call(location, '#new');
+ assert_equals(location.hash, '#new');
+ assert_true(descriptor.enumerable);
+ assert_false(descriptor.configurable);
+}, 'Location target, no trap, [Unforgeable] "hash" attribute');
+
+test(() => {
+ let locationProxy = new Proxy(new Proxy(location, {}), {
+ getOwnPropertyDescriptor: Reflect.getOwnPropertyDescriptor,
+ });
+
+ assert_object_equals(
+ Object.getOwnPropertyDescriptor(locationProxy, 'reload'),
+ Object.getOwnPropertyDescriptor(location, 'reload')
+ );
+}, 'Location proxy target, forwarding trap, [Unforgeable] "reload" method');
+</script>
+</body>
+</html>
Modified: trunk/Source/_javascript_Core/ChangeLog (259799 => 259800)
--- trunk/Source/_javascript_Core/ChangeLog 2020-04-09 14:48:31 UTC (rev 259799)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-04-09 15:13:07 UTC (rev 259800)
@@ -1,3 +1,27 @@
+2020-04-09 Alexey Shvayka <shvaikal...@gmail.com>
+
+ getOwnPropertyDescriptor() is incorrect with Proxy of exotic object
+ https://bugs.webkit.org/show_bug.cgi?id=200560
+
+ Reviewed by Yusuke Suzuki.
+
+ PropertyAttribute::CustomValue path in JSObject::getOwnPropertyDescriptor() needs to perform
+ getDirect() on correct target. A correct target may be different since getOwnPropertySlot()
+ may return not *own* property.
+
+ This change removes a hack that was covering only JSProxy case and invokes getDirect() on
+ slotBase(), ensuring ProxyObject instances with exotic targets return correct descriptors
+ and aligning JSC with V8 and SpiderMonkey.
+
+ getDirect() can be safely called on slotBase(): if getOwnPropertySlot() result is returned
+ from JS code of ProxyObject's trap, it will never be a PropertyAttribute::CustomValue.
+
+ This patch also moves setCustomDescriptor() down below to avoid mutating a descriptor when
+ returning `false`.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::getOwnPropertyDescriptor):
+
2020-04-09 Angelos Oikonomopoulos <ange...@igalia.com>
Fix CLOOP build
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (259799 => 259800)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2020-04-09 14:48:31 UTC (rev 259799)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2020-04-09 15:13:07 UTC (rev 259800)
@@ -3479,7 +3479,7 @@
{
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- JSC::PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
+ PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
bool result = methodTable(vm)->getOwnPropertySlot(this, globalObject, propertyName, slot);
EXCEPTION_ASSERT(!scope.exception() || !result);
@@ -3486,37 +3486,16 @@
if (!result)
return false;
-
- // FIXME: https://bugs.webkit.org/show_bug.cgi?id=200560
- // This breaks the assumption that getOwnPropertySlot should return "own" property.
- // We should fix DebuggerScope, ProxyObject etc. to remove this.
- //
- // DebuggerScope::getOwnPropertySlot() (and possibly others) may return attributes from the prototype chain
- // but getOwnPropertyDescriptor() should only work for 'own' properties so we exit early if we detect that
- // the property is not an own property.
- if (slot.slotBase() != this && slot.slotBase()) {
- JSProxy* jsProxy = jsDynamicCast<JSProxy*>(vm, this);
- if (!jsProxy || jsProxy->target() != slot.slotBase()) {
- // Try ProxyObject.
- ProxyObject* proxyObject = jsDynamicCast<ProxyObject*>(vm, this);
- if (!proxyObject || proxyObject->target() != slot.slotBase())
- return false;
- }
- }
-
if (slot.isAccessor())
descriptor.setAccessorDescriptor(slot.getterSetter(), slot.attributes());
else if (slot.attributes() & PropertyAttribute::CustomAccessor) {
- descriptor.setCustomDescriptor(slot.attributes());
-
- JSObject* thisObject = this;
- if (auto* proxy = jsDynamicCast<JSProxy*>(vm, this))
- thisObject = proxy->target();
-
CustomGetterSetter* getterSetter;
if (slot.isCustomAccessor())
getterSetter = slot.customGetterSetter();
else {
+ ASSERT(slot.slotBase());
+ JSObject* thisObject = slot.slotBase();
+
JSValue maybeGetterSetter = thisObject->getDirect(vm, propertyName);
if (!maybeGetterSetter) {
thisObject->reifyAllStaticProperties(globalObject);
@@ -3530,6 +3509,7 @@
if (!getterSetter)
return false;
+ descriptor.setCustomDescriptor(slot.attributes());
if (getterSetter->getter())
descriptor.setGetter(getCustomGetterSetterFunctionForGetterSetter(globalObject, propertyName, getterSetter, JSCustomGetterSetterFunction::Type::Getter));
if (getterSetter->setter())
