[ES6] Make GetProperty(.) inside ArrayPrototype.cpp spec compatible.
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Mar 2016 23:55:09 +0000 (23:55 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 17 Mar 2016 23:55:09 +0000 (23:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155575

Reviewed by Filip Pizlo and Mark Lam.

This patch makes various Array.prototype.(shift | unshift | splice)
spec compliant. Before, they were performing Get and HasProperty as one
operation. Instead, they need to be performed as two distinct operations
when it would be observable.

* runtime/ArrayPrototype.cpp:
(JSC::getProperty):
* runtime/PropertySlot.h:
(JSC::PropertySlot::PropertySlot):
(JSC::PropertySlot::isCacheableValue):
(JSC::PropertySlot::isCacheableGetter):
(JSC::PropertySlot::isCacheableCustom):
(JSC::PropertySlot::setIsTaintedByProxy):
(JSC::PropertySlot::isTaintedByProxy):
(JSC::PropertySlot::internalMethodType):
(JSC::PropertySlot::getValue):
* runtime/ProxyObject.cpp:
(JSC::ProxyObject::getOwnPropertySlotCommon):
* tests/es6.yaml:
* tests/stress/proxy-array-prototype-methods.js: Added.
(assert):
(test):
(shallowEq):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayPrototype.cpp
Source/JavaScriptCore/runtime/PropertySlot.h
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/JavaScriptCore/tests/es6.yaml
Source/JavaScriptCore/tests/stress/proxy-array-prototype-methods.js [new file with mode: 0644]

index 73895dd..b4cb023 100644 (file)
@@ -1,3 +1,34 @@
+2016-03-17  Saam barati  <sbarati@apple.com>
+
+        [ES6] Make GetProperty(.) inside ArrayPrototype.cpp spec compatible.
+        https://bugs.webkit.org/show_bug.cgi?id=155575
+
+        Reviewed by Filip Pizlo and Mark Lam.
+
+        This patch makes various Array.prototype.(shift | unshift | splice)
+        spec compliant. Before, they were performing Get and HasProperty as one 
+        operation. Instead, they need to be performed as two distinct operations
+        when it would be observable.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::getProperty):
+        * runtime/PropertySlot.h:
+        (JSC::PropertySlot::PropertySlot):
+        (JSC::PropertySlot::isCacheableValue):
+        (JSC::PropertySlot::isCacheableGetter):
+        (JSC::PropertySlot::isCacheableCustom):
+        (JSC::PropertySlot::setIsTaintedByProxy):
+        (JSC::PropertySlot::isTaintedByProxy):
+        (JSC::PropertySlot::internalMethodType):
+        (JSC::PropertySlot::getValue):
+        * runtime/ProxyObject.cpp:
+        (JSC::ProxyObject::getOwnPropertySlotCommon):
+        * tests/es6.yaml:
+        * tests/stress/proxy-array-prototype-methods.js: Added.
+        (assert):
+        (test):
+        (shallowEq):
+
 2016-03-17  Mark Lam  <mark.lam@apple.com>
 
         Make FunctionMode an enum class.
index 0681639..0303702 100644 (file)
@@ -149,9 +149,15 @@ static ALWAYS_INLINE JSValue getProperty(ExecState* exec, JSObject* object, unsi
 {
     if (JSValue result = object->tryGetIndexQuickly(index))
         return result;
-    PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
+    // We want to perform get and has in the same operation.
+    // We can only do so when this behavior is not observable. The
+    // only time it is observable is when we encounter a ProxyObject
+    // somewhere in the prototype chain.
+    PropertySlot slot(object, PropertySlot::InternalMethodType::HasProperty);
     if (!object->getPropertySlot(exec, index, slot))
         return JSValue();
+    if (UNLIKELY(slot.isTaintedByProxy()))
+        return object->get(exec, index);
     return slot.getValue(exec, index);
 }
 
index 4049977..d05f729 100644 (file)
@@ -86,6 +86,7 @@ public:
         , m_cacheability(CachingAllowed)
         , m_propertyType(TypeUnset)
         , m_internalMethodType(internalMethodType)
+        , m_isTaintedByProxy(false)
     {
     }
 
@@ -102,6 +103,8 @@ public:
     bool isCacheableValue() const { return isCacheable() && isValue(); }
     bool isCacheableGetter() const { return isCacheable() && isAccessor(); }
     bool isCacheableCustom() const { return isCacheable() && isCustom(); }
+    void setIsTaintedByProxy() { m_isTaintedByProxy = true; }
+    bool isTaintedByProxy() const { return m_isTaintedByProxy; }
 
     InternalMethodType internalMethodType() const { return m_internalMethodType; }
 
@@ -278,6 +281,7 @@ private:
     CacheabilityType m_cacheability;
     PropertyType m_propertyType;
     InternalMethodType m_internalMethodType;
+    bool m_isTaintedByProxy;
 };
 
 ALWAYS_INLINE JSValue PropertySlot::getValue(ExecState* exec, PropertyName propertyName) const
index b62aca7..8b69b7b 100644 (file)
@@ -332,6 +332,8 @@ bool ProxyObject::performHasProperty(ExecState* exec, PropertyName propertyName,
 bool ProxyObject::getOwnPropertySlotCommon(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
     slot.disableCaching();
+    if (slot.internalMethodType() != PropertySlot::InternalMethodType::VMInquiry)
+        slot.setIsTaintedByProxy();
     switch (slot.internalMethodType()) {
     case PropertySlot::InternalMethodType::Get:
         slot.setCustom(this, CustomAccessor, performProxyGet);
index 1eef361..cf258c5 100644 (file)
 - path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.reverse.js
   cmd: runES6 :normal
 - path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.shift.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.splice.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.unshift.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_get_calls_Array.from.js
   cmd: runES6 :normal
 - path: es6/Proxy_internal_get_calls_Array.prototype.concat.js
 - path: es6/Proxy_internal_set_calls_Array.prototype.reverse.js
   cmd: runES6 :normal
 - path: es6/Proxy_internal_set_calls_Array.prototype.shift.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_set_calls_Array.prototype.splice.js
   cmd: runES6 :normal
 - path: es6/Proxy_internal_set_calls_Array.prototype.unshift.js
-  cmd: runES6 :fail
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_set_calls_Object.assign.js
   cmd: runES6 :normal
 - path: es6/Proxy_isExtensible_handler.js
diff --git a/Source/JavaScriptCore/tests/stress/proxy-array-prototype-methods.js b/Source/JavaScriptCore/tests/stress/proxy-array-prototype-methods.js
new file mode 100644 (file)
index 0000000..7ca4a0c
--- /dev/null
@@ -0,0 +1,163 @@
+function assert(b) {
+    if (!b)
+        throw new Error("Bad assertion!");
+}
+
+function test(f) {
+    for (let i = 0; i < 1000; i++)
+        f();
+}
+
+function shallowEq(a, b) {
+    if (a.length !== b.length)
+        return false;
+    for (let i = 0; i < a.length; i++) {
+        if (a[i] !== b[i])
+            return false;
+    }
+    return true;
+}
+
+test(function() {
+    let delProps = new Set;
+    let hasProps = new Set;
+    let getProps = new Set;
+    let target = [ , , 1, , 4];
+    let handler = {
+        get(theTarget, key) {
+            getProps.add(key);
+            return Reflect.get(theTarget, key);
+        },
+        has(theTarget, key) {
+            hasProps.add(key);
+            return Reflect.has(theTarget, key);
+        },
+        deleteProperty(theTarget, key)
+        {
+            delProps.add(key);
+            return Reflect.deleteProperty(theTarget, key);
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    proxy.unshift(20);
+
+    assert(delProps.size === 3);
+    assert(delProps.has("1"));
+    assert(delProps.has("2"));
+    assert(delProps.has("4"));
+
+    assert(hasProps.size === 5);
+    assert(hasProps.has("0"));
+    assert(hasProps.has("1"));
+    assert(hasProps.has("2"));
+    assert(hasProps.has("3"));
+    assert(hasProps.has("4"));
+
+    assert(getProps.size === 4);
+    assert(getProps.has("unshift"));
+    assert(getProps.has("length"));
+    assert(getProps.has("2"));
+    assert(getProps.has("4"));
+});
+
+test(function() {
+    let delProps = new Set;
+    let hasProps = new Set;
+    let getProps = new Set;
+    let target = [ 0, 0, , 1, , 4];
+    let handler = {
+        get(theTarget, key) {
+            getProps.add(key);
+            return Reflect.get(theTarget, key);
+        },
+        has(theTarget, key) {
+            hasProps.add(key);
+            return Reflect.has(theTarget, key);
+        },
+        deleteProperty(theTarget, key)
+        {
+            delProps.add(key);
+            return Reflect.deleteProperty(theTarget, key);
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    proxy.shift();
+    assert(target.length === 5);
+
+    assert(delProps.size === 3);
+    assert(delProps.has("1"));
+    assert(delProps.has("3"));
+    assert(delProps.has("5"));
+
+    assert(hasProps.size === 5);
+    assert(hasProps.has("1"));
+    assert(hasProps.has("2"));
+    assert(hasProps.has("3"));
+    assert(hasProps.has("4"));
+    assert(hasProps.has("5"));
+
+    assert(getProps.size === 6);
+    assert(getProps.has("shift"));
+    assert(getProps.has("length"));
+    assert(getProps.has("0"));
+    assert(getProps.has("1"));
+    assert(getProps.has("3"));
+    assert(getProps.has("5"));
+});
+
+test(function() {
+    let delProps = new Set;
+    let hasProps = new Set;
+    let getProps = new Set;
+    let target = [ 0, , 1, , 2];
+    let handler = {
+        get(theTarget, key) {
+            getProps.add(key);
+            return Reflect.get(theTarget, key);
+        },
+        has(theTarget, key) {
+            hasProps.add(key);
+            return Reflect.has(theTarget, key);
+        },
+        deleteProperty(theTarget, key)
+        {
+            delProps.add(key);
+            return Reflect.deleteProperty(theTarget, key);
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    proxy.splice(2, 2);
+
+    assert(delProps.size === 2);
+    assert(delProps.has("3"));
+    assert(delProps.has("4"));
+
+    assert(hasProps.size === 3);
+    assert(hasProps.has("2"));
+    assert(hasProps.has("3"));
+    assert(hasProps.has("4"));
+
+    assert(getProps.size === 4);
+    assert(getProps.has("splice"));
+    assert(getProps.has("length"));
+    assert(getProps.has("2"));
+    assert(getProps.has("4"));
+});
+
+test(function() {
+    let x = [1,2,3];
+    x.__proto__ = new Proxy([], {
+        get(theTarget, prop, receiver) {
+            assert(prop === "shift");
+            assert(receiver === x);
+            return Reflect.get(theTarget, prop);
+        }
+    });
+    x.shift();
+    assert(x.length === 2);
+    assert(x[0] === 2);
+    assert(x[1] === 3);
+});