Unreviewed, rolling out r243943.
authorryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Apr 2019 15:51:19 +0000 (15:51 +0000)
committerryanhaddad@apple.com <ryanhaddad@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Apr 2019 15:51:19 +0000 (15:51 +0000)
Caused test262 failures.

Reverted changeset:

"[JSC] Filter DontEnum properties in
ProxyObject::getOwnPropertyNames()"
https://bugs.webkit.org/show_bug.cgi?id=176810
https://trac.webkit.org/changeset/243943

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244020 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/proxy-own-keys.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/PropertyNameArray.h
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMConvertRecord.h

index 9f2d760..9008027 100644 (file)
@@ -1,3 +1,16 @@
+2019-04-08  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r243943.
+
+        Caused test262 failures.
+
+        Reverted changeset:
+
+        "[JSC] Filter DontEnum properties in
+        ProxyObject::getOwnPropertyNames()"
+        https://bugs.webkit.org/show_bug.cgi?id=176810
+        https://trac.webkit.org/changeset/243943
+
 2019-04-07  Michael Saboff  <msaboff@apple.com>
 
         REGRESSION (r243642): Crash in reddit.com page
index 853d9c1..01756fb 100644 (file)
@@ -135,22 +135,6 @@ function assert(b) {
         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;
-    }
 }
 
 {
@@ -182,22 +166,6 @@ function assert(b) {
         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;
-    }
 }
 
 {
@@ -699,68 +667,3 @@ function shallowEq(a, b) {
         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);
-    }
-}
index 4c0863c..710affb 100644 (file)
@@ -1,3 +1,16 @@
+2019-04-08  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r243943.
+
+        Caused test262 failures.
+
+        Reverted changeset:
+
+        "[JSC] Filter DontEnum properties in
+        ProxyObject::getOwnPropertyNames()"
+        https://bugs.webkit.org/show_bug.cgi?id=176810
+        https://trac.webkit.org/changeset/243943
+
 2019-04-08  Claudio Saavedra  <csaavedra@igalia.com>
 
         [JSC] Partially fix the build with unified builds disabled
index 8ec9ac1..5e93b79 100644 (file)
@@ -55,12 +55,6 @@ public:
     {
     }
 
-    void reset()
-    {
-        m_set.clear();
-        m_data = PropertyNameArrayData::create();
-    }
-
     VM* vm() { return m_vm; }
 
     void add(uint32_t index)
index 414a75e..8cafad5 100644 (file)
@@ -1006,33 +1006,19 @@ void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray&
         }
     }
 
-    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;
-            }
-        }
+    if (targetIsExensible)
+        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);
+    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;
         }
     }
 
-    if (!enumerationMode.includeDontEnumProperties()) {
-        // Filtering DontEnum properties is observable in proxies and must occur following the invariant checks above.
-        auto data = trapResult.releaseData();
-        trapResult.reset();
-
-        for (auto propertyName : data->propertyNameVector()) {
-            PropertySlot slot(this, PropertySlot::InternalMethodType::GetOwnProperty);
-            if (!getOwnPropertySlotCommon(exec, propertyName, slot))
-                continue;
-            if (slot.attributes() & PropertyAttribute::DontEnum)
-                continue;
-            trapResult.addUnchecked(propertyName.impl());
-        }
+    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;
     }
 }
 
index 091713d..6289a88 100644 (file)
@@ -1,3 +1,16 @@
+2019-04-08  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r243943.
+
+        Caused test262 failures.
+
+        Reverted changeset:
+
+        "[JSC] Filter DontEnum properties in
+        ProxyObject::getOwnPropertyNames()"
+        https://bugs.webkit.org/show_bug.cgi?id=176810
+        https://trac.webkit.org/changeset/243943
+
 2019-04-05  Sergio Villar Senin  <svillar@igalia.com>
 
         [GTK][WPE] outlook.live.com displays old-fashioned UI
index df9c3a6..75b2eb6 100644 (file)
@@ -86,7 +86,7 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
     
         // 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(JSC::DontEnumPropertiesMode::Include));
+        object->methodTable(vm)->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode());
 
         RETURN_IF_EXCEPTION(scope, { });
 
@@ -99,8 +99,9 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
 
             // 2. If desc is not undefined and desc.[[Enumerable]] is true:
 
-            // 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.
+            // FIXME: Do we need to check for enumerable / undefined, or is this handled by the default
+            // enumeration mode?
+
             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);