Diff
Modified: trunk/JSTests/ChangeLog (244329 => 244330)
--- trunk/JSTests/ChangeLog 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/JSTests/ChangeLog 2019-04-16 15:58:59 UTC (rev 244330)
@@ -1,3 +1,20 @@
+2019-04-16 Caitlin Potter <ca...@igalia.com>
+
+ [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames()
+ https://bugs.webkit.org/show_bug.cgi?id=176810
+
+ Reviewed by Saam Barati.
+
+ Add tests for the DontEnum filtering, and variations of other tests
+ take the DontEnum-filtering path.
+
+ * stress/proxy-own-keys.js:
+ (i.catch):
+ (set assert):
+ (set add):
+ (let.set new):
+ (get let):
+
2019-04-15 Saam barati <sbar...@apple.com>
Modify how we do SetArgument when we inline varargs calls
Modified: trunk/JSTests/stress/proxy-own-keys.js (244329 => 244330)
--- trunk/JSTests/stress/proxy-own-keys.js 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/JSTests/stress/proxy-own-keys.js 2019-04-16 15:58:59 UTC (rev 244330)
@@ -135,6 +135,22 @@
assert(called);
called = false;
}
+
+ for (let i = 0; i < 500; i++) {
+ let threw = false;
+ let foundKey = false;
+ try {
+ for (let k in proxy)
+ foundKey = true;
+ } catch(e) {
+ threw = true;
+ assert(e.toString() === "TypeError: Proxy object's non-extensible 'target' has configurable property 'x' that was not in the result from the 'ownKeys' trap");
+ assert(!foundKey);
+ }
+ assert(threw);
+ assert(called);
+ called = false;
+ }
}
{
@@ -166,6 +182,22 @@
assert(called);
called = false;
}
+
+ for (let i = 0; i < 500; i++) {
+ let threw = false;
+ let reached = false;
+ try {
+ for (let k in proxy)
+ reached = true;
+ } catch (e) {
+ threw = true;
+ 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);
+ assert(!reached);
+ called = false;
+ }
}
{
@@ -667,3 +699,68 @@
error = null;
}
}
+
+{
+ let error = null;
+ let s1 = Symbol();
+ let s2 = Symbol();
+ let target = Object.defineProperties({}, {
+ x: {
+ value: "X",
+ enumerable: true,
+ configurable: true,
+ },
+ dontEnum1: {
+ value: "dont-enum",
+ enumerable: false,
+ configurable: true,
+ },
+ y: {
+ get() { return "Y"; },
+ enumerable: true,
+ configurable: true,
+ },
+ dontEnum2: {
+ get() { return "dont-enum-accessor" },
+ enumerable: false,
+ configurable: true
+ },
+ [s1]: {
+ value: "s1",
+ enumerable: true,
+ configurable: true,
+ },
+ [s2]: {
+ value: "dont-enum-symbol",
+ enumerable: false,
+ configurable: true,
+ },
+ });
+ let checkedOwnKeys = false;
+ let checkedPropertyDescriptor = false;
+ let handler = {
+ ownKeys() {
+ checkedOwnKeys = true;
+ return ["x", "dontEnum1", "y", "dontEnum2", s1, s2];
+ },
+ getOwnPropertyDescriptor(t, k) {
+ checkedPropertyDescriptors = true;
+ return Reflect.getOwnPropertyDescriptor(t, k);
+ }
+ };
+ let proxy = new Proxy(target, handler);
+ for (let i = 0; i < 500; i++) {
+ checkedPropertyDescriptors = false;
+ assert(shallowEq(["x", "dontEnum1", "y", "dontEnum2", s1, s2], Reflect.ownKeys(proxy)));
+ assert(checkedOwnKeys);
+ assert(!checkedPropertyDescriptors);
+ checkedOwnKeys = false;
+
+ let enumerableStringKeys = [];
+ for (let k in proxy)
+ enumerableStringKeys.push(k);
+ assert(shallowEq(["x", "y"], enumerableStringKeys));
+ assert(checkedOwnKeys);
+ assert(checkedPropertyDescriptors);
+ }
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (244329 => 244330)
--- trunk/Source/_javascript_Core/ChangeLog 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-04-16 15:58:59 UTC (rev 244330)
@@ -1,3 +1,26 @@
+2019-04-16 Caitlin Potter <ca...@igalia.com>
+
+ [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames()
+ https://bugs.webkit.org/show_bug.cgi?id=176810
+
+ Reviewed by Saam Barati.
+
+ This adds conditional logic following the invariant checks, to perform
+ filtering in common uses of getOwnPropertyNames.
+
+ While this would ideally only be done in JSPropertyNameEnumerator, adding
+ the filtering to ProxyObject::performGetOwnPropertyNames maintains the
+ invariant that the EnumerationMode is properly followed.
+
+ This was originally rolled out in r244020, as DontEnum filtering code
+ in ObjectConstructor.cpp's ownPropertyKeys() had not been removed. It's
+ now redundant due to being handled in ProxyObject::getOwnPropertyNames().
+
+ * runtime/PropertyNameArray.h:
+ (JSC::PropertyNameArray::reset):
+ * runtime/ProxyObject.cpp:
+ (JSC::ProxyObject::performGetOwnPropertyNames):
+
2019-04-15 Saam barati <sbar...@apple.com>
Modify how we do SetArgument when we inline varargs calls
Modified: trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp (244329 => 244330)
--- trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp 2019-04-16 15:58:59 UTC (rev 244330)
@@ -920,23 +920,9 @@
object->methodTable(vm)->getOwnPropertyNames(object, exec, properties, EnumerationMode(dontEnumPropertiesMode));
RETURN_IF_EXCEPTION(scope, nullptr);
- // https://tc39.github.io/ecma262/#sec-enumerableownproperties
- // If {object} is a Proxy, an explicit and observable [[GetOwnProperty]] op is required to filter out non-enumerable properties.
- // In other cases, filtering has already been performed.
- const bool mustFilterProperty = dontEnumPropertiesMode == DontEnumPropertiesMode::Exclude && object->type() == ProxyObjectType;
- auto filterPropertyIfNeeded = [exec, object, mustFilterProperty](const Identifier& identifier) {
- if (!mustFilterProperty)
- return true;
- PropertyDescriptor descriptor;
- PropertyName name(identifier);
- return object->getOwnPropertyDescriptor(exec, name, descriptor) && descriptor.enumerable();
- };
-
- // If !mustFilterProperty and PropertyNameMode::Strings mode, we do not need to filter out any entries in PropertyNameArray.
- // We can use fast allocation and initialization.
if (propertyNameMode != PropertyNameMode::StringsAndSymbols) {
ASSERT(propertyNameMode == PropertyNameMode::Strings || propertyNameMode == PropertyNameMode::Symbols);
- if (!mustFilterProperty && properties.size() < MIN_SPARSE_ARRAY_INDEX) {
+ if (properties.size() < MIN_SPARSE_ARRAY_INDEX) {
if (LIKELY(!globalObject->isHavingABadTime())) {
if (isObjectKeys) {
Structure* structure = object->structure(vm);
@@ -993,10 +979,7 @@
for (size_t i = 0; i < numProperties; i++) {
const auto& identifier = properties[i];
ASSERT(!identifier.isSymbol());
- bool hasProperty = filterPropertyIfNeeded(identifier);
- EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
- if (hasProperty)
- pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
+ pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
RETURN_IF_EXCEPTION(scope, nullptr);
}
break;
@@ -1008,10 +991,7 @@
const auto& identifier = properties[i];
ASSERT(identifier.isSymbol());
ASSERT(!identifier.isPrivateName());
- bool hasProperty = filterPropertyIfNeeded(identifier);
- EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
- if (hasProperty)
- pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
+ pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
RETURN_IF_EXCEPTION(scope, nullptr);
}
break;
@@ -1028,19 +1008,13 @@
continue;
}
- bool hasProperty = filterPropertyIfNeeded(identifier);
- EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
- if (hasProperty)
- pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
+ pushDirect(exec, keys, jsOwnedString(exec, identifier.string()));
RETURN_IF_EXCEPTION(scope, nullptr);
}
// To ensure the order defined in the spec (9.1.12), we append symbols at the last elements of keys.
for (const auto& identifier : propertySymbols) {
- bool hasProperty = filterPropertyIfNeeded(identifier);
- EXCEPTION_ASSERT(!scope.exception() || !hasProperty);
- if (hasProperty)
- pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
+ pushDirect(exec, keys, Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl())));
RETURN_IF_EXCEPTION(scope, nullptr);
}
Modified: trunk/Source/_javascript_Core/runtime/PropertyNameArray.h (244329 => 244330)
--- trunk/Source/_javascript_Core/runtime/PropertyNameArray.h 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/_javascript_Core/runtime/PropertyNameArray.h 2019-04-16 15:58:59 UTC (rev 244330)
@@ -55,6 +55,12 @@
{
}
+ void reset()
+ {
+ m_set.clear();
+ m_data = PropertyNameArrayData::create();
+ }
+
VM* vm() { return m_vm; }
void add(uint32_t index)
Modified: trunk/Source/_javascript_Core/runtime/ProxyObject.cpp (244329 => 244330)
--- trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/_javascript_Core/runtime/ProxyObject.cpp 2019-04-16 15:58:59 UTC (rev 244330)
@@ -1006,19 +1006,35 @@
}
}
- if (targetIsExensible)
- return;
+ if (!targetIsExensible) {
+ for (UniquedStringImpl* impl : targetConfigurableKeys) {
+ if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
+ throwVMTypeError(exec, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
+ return;
+ }
+ }
- for (UniquedStringImpl* impl : targetConfigurableKeys) {
- if (removeIfContainedInUncheckedResultKeys(impl) == IsNotContainedIn) {
- throwVMTypeError(exec, scope, makeString("Proxy object's non-extensible 'target' has configurable property '", String(impl), "' that was not in the result from the 'ownKeys' trap"));
+ if (uncheckedResultKeys.size()) {
+ throwVMTypeError(exec, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s);
return;
}
}
- if (uncheckedResultKeys.size()) {
- throwVMTypeError(exec, scope, "Proxy handler's 'ownKeys' method returned a key that was not present in its non-extensible target"_s);
- return;
+ if (!enumerationMode.includeDontEnumProperties()) {
+ // Filtering DontEnum properties is observable in proxies and must occur following the invariant checks above.
+ auto data = ""
+ trapResult.reset();
+
+ for (auto propertyName : data->propertyNameVector()) {
+ PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
+ auto result = getOwnPropertySlotCommon(exec, propertyName, slot);
+ RETURN_IF_EXCEPTION(scope, void());
+ if (!result)
+ continue;
+ if (slot.attributes() & PropertyAttribute::DontEnum)
+ continue;
+ trapResult.addUnchecked(propertyName.impl());
+ }
}
}
Modified: trunk/Source/WebCore/ChangeLog (244329 => 244330)
--- trunk/Source/WebCore/ChangeLog 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/WebCore/ChangeLog 2019-04-16 15:58:59 UTC (rev 244330)
@@ -1,3 +1,21 @@
+2019-04-16 Caitlin Potter <ca...@igalia.com>
+
+ [JSC] Filter DontEnum properties in ProxyObject::getOwnPropertyNames()
+ https://bugs.webkit.org/show_bug.cgi?id=176810
+
+ Reviewed by Saam Barati.
+
+ Previously, there was a comment here indicating uncertainty of whether it
+ was necessary to filter DontEnum properties explicitly or not. It turns
+ out that it was necessary in the case of JSC ProxyObjects.
+
+ This patch adds DontEnum filtering for ProxyObjects, however we continue
+ to explicitly filter them in JSDOMConvertRecord, which needs to use the
+ property descriptor after filtering. This change prevents observably
+ fetching the property descriptor twice per property.
+
+ * bindings/js/JSDOMConvertRecord.h:
+
2019-04-15 Antoine Quint <grao...@apple.com>
[iOS] Redundant pointer events causes material design buttons to flush twice
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h (244329 => 244330)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h 2019-04-16 09:41:00 UTC (rev 244329)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h 2019-04-16 15:58:59 UTC (rev 244330)
@@ -86,7 +86,7 @@
// 4. Let keys be ? O.[[OwnPropertyKeys]]().
JSC::PropertyNameArray keys(&vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude);
- object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode());
+ object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode(JSC::DontEnumPropertiesMode::Include));
RETURN_IF_EXCEPTION(scope, { });
@@ -99,9 +99,8 @@
// 2. If desc is not undefined and desc.[[Enumerable]] is true:
- // FIXME: Do we need to check for enumerable / undefined, or is this handled by the default
- // enumeration mode?
-
+ // It's necessary to filter enumerable here rather than using the default EnumerationMode,
+ // to prevent an observable extra [[GetOwnProperty]] operation in the case of ProxyObject records.
if (didGetDescriptor && descriptor.enumerable()) {
// 1. Let typedKey be key converted to an IDL value of type K.
auto typedKey = Detail::IdentifierConverter<K>::convert(state, key);
