Modified: trunk/Source/_javascript_Core/ChangeLog (201671 => 201672)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-04 00:04:52 UTC (rev 201671)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-04 00:34:27 UTC (rev 201672)
@@ -1,5 +1,25 @@
2016-06-03 Saam barati <sbar...@apple.com>
+ Proxy.ownKeys should no longer throw an exception when duplicate keys are returned and the target is non-extensible
+ https://bugs.webkit.org/show_bug.cgi?id=158350
+ <rdar://problem/26626211>
+
+ Reviewed by Michael Saboff.
+
+ The spec was recently changes in Proxy [[OwnPropertyKeys]]
+ to allow for duplicate property names under certain circumstances.
+ This patch fixes our implementation to match the spec.
+ See: https://github.com/tc39/ecma262/pull/594
+
+ * runtime/ProxyObject.cpp:
+ (JSC::ProxyObject::performGetOwnPropertyNames):
+ * tests/stress/proxy-own-keys.js:
+ (i.catch):
+ (ownKeys):
+ (assert):
+
+2016-06-03 Saam barati <sbar...@apple.com>
+
Some shadow chicken code is wrong when run on a big endian CPU
https://bugs.webkit.org/show_bug.cgi?id=158361
Modified: trunk/Source/_javascript_Core/runtime/ProxyObject.cpp (201671 => 201672)
--- trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2016-06-04 00:04:52 UTC (rev 201671)
+++ trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2016-06-04 00:34:27 UTC (rev 201672)
@@ -861,7 +861,7 @@
CallData callData;
CallType callType;
JSValue ownKeysMethod = handler->getMethod(exec, callData, callType, makeIdentifier(vm, "ownKeys"), ASCIILiteral("'ownKeys' property of a Proxy's handler should be callable"));
- if (exec->hadException())
+ if (vm.exception())
return;
JSObject* target = this->target();
if (ownKeysMethod.isUndefined()) {
@@ -872,7 +872,7 @@
MarkedArgumentBuffer arguments;
arguments.append(target);
JSValue arrayLikeObject = call(exec, ownKeysMethod, callType, callData, handler, arguments);
- if (exec->hadException())
+ if (vm.exception())
return;
PropertyNameMode propertyNameMode = trapResult.mode();
@@ -890,8 +890,7 @@
}
ASSERT(resultFilter);
RuntimeTypeMask dontThrowAnExceptionTypeFilter = TypeString | TypeSymbol;
- HashMap<UniquedStringImpl*, unsigned> uncheckedResultKeys;
- unsigned totalSize = 0;
+ HashSet<UniquedStringImpl*> uncheckedResultKeys;
auto addPropName = [&] (JSValue value, RuntimeType type) -> bool {
static const bool doExitEarly = true;
@@ -901,33 +900,30 @@
return dontExitEarly;
Identifier ident = value.toPropertyKey(exec);
- if (exec->hadException())
+ if (vm.exception())
return doExitEarly;
- ++uncheckedResultKeys.add(ident.impl(), 0).iterator->value;
- ++totalSize;
-
+ uncheckedResultKeys.add(ident.impl());
trapResult.addUnchecked(ident.impl());
-
return dontExitEarly;
};
createListFromArrayLike(exec, arrayLikeObject, dontThrowAnExceptionTypeFilter, ASCIILiteral("Proxy handler's 'ownKeys' method must return an array-like object containing only Strings and Symbols"), addPropName);
- if (exec->hadException())
+ if (vm.exception())
return;
bool targetIsExensible = target->isExtensible(exec);
PropertyNameArray targetKeys(&vm, propertyNameMode);
target->methodTable(vm)->getOwnPropertyNames(target, exec, targetKeys, enumerationMode);
- if (exec->hadException())
+ if (vm.exception())
return;
Vector<UniquedStringImpl*> targetConfigurableKeys;
Vector<UniquedStringImpl*> targetNonConfigurableKeys;
for (const Identifier& ident : targetKeys) {
PropertyDescriptor descriptor;
bool isPropertyDefined = target->getOwnPropertyDescriptor(exec, ident.impl(), descriptor);
- if (exec->hadException())
+ if (vm.exception())
return;
if (isPropertyDefined && !descriptor.configurable())
targetNonConfigurableKeys.append(ident.impl());
@@ -935,26 +931,18 @@
targetConfigurableKeys.append(ident.impl());
}
- auto removeIfContainedInUncheckedResultKeys = [&] (UniquedStringImpl* impl) -> bool {
- static const bool isContainedIn = true;
- static const bool isNotContainedIn = false;
-
+ enum ContainedIn { IsContainedIn, IsNotContainedIn };
+ auto removeIfContainedInUncheckedResultKeys = [&] (UniquedStringImpl* impl) -> ContainedIn {
auto iter = uncheckedResultKeys.find(impl);
if (iter == uncheckedResultKeys.end())
- return isNotContainedIn;
+ return IsNotContainedIn;
- unsigned& count = iter->value;
- if (count == 0)
- return isNotContainedIn;
-
- --count;
- --totalSize;
- return isContainedIn;
+ uncheckedResultKeys.remove(iter);
+ return IsContainedIn;
};
for (UniquedStringImpl* impl : targetNonConfigurableKeys) {
- bool contains = removeIfContainedInUncheckedResultKeys(impl);
- if (!contains) {
+ if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
throwVMTypeError(exec, makeString("Proxy object's 'target' has the non-configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
return;
}
@@ -964,22 +952,14 @@
return;
for (UniquedStringImpl* impl : targetConfigurableKeys) {
- bool contains = removeIfContainedInUncheckedResultKeys(impl);
- if (!contains) {
+ if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
throwVMTypeError(exec, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
return;
}
}
-#ifndef NDEBUG
- unsigned sum = 0;
- for (unsigned keyCount : uncheckedResultKeys.values())
- sum += keyCount;
- ASSERT(sum == totalSize);
-#endif
-
- if (totalSize) {
- throwVMTypeError(exec, ASCIILiteral("Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys"));
+ if (uncheckedResultKeys.size()) {
+ throwVMTypeError(exec, ASCIILiteral("Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"));
return;
}
}
Modified: trunk/Source/_javascript_Core/tests/stress/proxy-own-keys.js (201671 => 201672)
--- trunk/Source/_javascript_Core/tests/stress/proxy-own-keys.js 2016-06-04 00:04:52 UTC (rev 201671)
+++ trunk/Source/_javascript_Core/tests/stress/proxy-own-keys.js 2016-06-04 00:34:27 UTC (rev 201672)
@@ -160,7 +160,7 @@
Object.keys(proxy);
} catch(e) {
threw = true;
- assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys");
+ assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target");
}
assert(threw);
assert(called);
@@ -169,6 +169,75 @@
}
{
+ let target = {};
+ let called1 = false;
+ let called2 = false;
+ Object.defineProperty(target, 'a', { value: 42, configurable: false });
+ let p1 = new Proxy(target, {
+ ownKeys() {
+ called1 = true;
+ return ['a', 'a'];
+ }
+ });
+ let p2 = new Proxy(p1, {
+ ownKeys() {
+ called2 = true;
+ return ['a'];
+ }
+ });
+
+ for (let i = 0; i < 500; i++) {
+ // FIXME: we may update the spec to make this test not throw.
+ // see: https://github.com/tc39/ecma262/pull/594
+ let threw = false;
+ try {
+ Reflect.ownKeys(p2);
+ } catch(e) {
+ assert(e.toString() === "TypeError: Proxy object's 'target' has the non-configurable property 'a' that was not in the result from the 'ownKeys' trap");
+ threw = true;
+ }
+ assert(threw);
+ assert(called1);
+ assert(called2);
+ }
+}
+
+{
+ let target = {};
+ let called1 = false;
+ let called2 = false;
+ Object.defineProperty(target, 'a', { value: 42, configurable: true });
+ Object.preventExtensions(target);
+ let p1 = new Proxy(target, {
+ ownKeys() {
+ called1 = true;
+ return ['a', 'a'];
+ }
+ });
+ let p2 = new Proxy(p1, {
+ ownKeys() {
+ called2 = true;
+ return ['a'];
+ }
+ });
+
+ for (let i = 0; i < 500; i++) {
+ // FIXME: we may update the spec to make this test not throw.
+ // see: https://github.com/tc39/ecma262/pull/594
+ let threw = false;
+ try {
+ Reflect.ownKeys(p2);
+ } catch(e) {
+ assert(e.toString() === "TypeError: Proxy object's non-extensible 'target' has configurable property 'a' that was not in the result from the 'ownKeys' trap");
+ threw = true;
+ }
+ assert(threw);
+ assert(called1);
+ assert(called2);
+ }
+}
+
+{
let target = { };
Object.defineProperty(target, "x", {
configurable: true,
@@ -186,14 +255,7 @@
let proxy = new Proxy(target, handler);
for (let i = 0; i < 500; i++) {
- let threw = false;
- try {
- Object.keys(proxy);
- } catch(e) {
- threw = true;
- assert(e.toString() === "TypeError: Proxy handler's 'ownKeys' method returned a key that was not present in its target or it returned duplicate keys");
- }
- assert(threw);
+ Object.keys(proxy);
assert(called);
called = false;
}
