Bug 1066834 - Don't traverse prototype chain in propertyIsEnumerable. r=jorendorff
authorAndré Bargull <andrebargull@googlemail.com>
Wed, 01 Oct 2014 22:37:51 +0200
changeset 210015 61453e5b990d8e6551e07b8e4790e5201c6a5a03
parent 210014 120d00383f290a312bbeace22050f54de1f58010
child 210016 fe77f9461b9fe0a90b5a02e9dedfcb7dee9b1a30
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjorendorff
bugs1066834
milestone35.0a1
Bug 1066834 - Don't traverse prototype chain in propertyIsEnumerable. r=jorendorff
js/src/builtin/Object.cpp
js/src/tests/ecma_6/Object/propertyIsEnumerable-proxy.js
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -45,49 +45,63 @@ js::obj_construct(JSContext *cx, unsigne
 }
 
 /* ES5 15.2.4.7. */
 static bool
 obj_propertyIsEnumerable(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
+    HandleValue idValue = args.get(0);
+
+    // As an optimization, provide a fast path when rooting is not necessary and
+    // we can safely retrieve the attributes from the object's shape.
+
+    /* Steps 1-2. */
+    jsid id;
+    if (args.thisv().isObject() && ValueToId<NoGC>(cx, idValue, &id)) {
+        JSObject *obj = &args.thisv().toObject(), *pobj;
+
+        /* Step 3. */
+        Shape *shape;
+        if (!obj->is<ProxyObject>() &&
+            HasOwnProperty<NoGC>(cx, obj->getOps()->lookupGeneric, obj, id, &pobj, &shape))
+        {
+            /* Step 4. */
+            if (!shape) {
+                args.rval().setBoolean(false);
+                return true;
+            }
+
+            /* Step 5. */
+            if (pobj->isNative()) {
+                unsigned attrs = GetShapeAttributes(pobj, shape);
+                args.rval().setBoolean((attrs & JSPROP_ENUMERATE) != 0);
+                return true;
+            }
+        }
+    }
+
     /* Step 1. */
-    RootedId id(cx);
-    if (!ValueToId<CanGC>(cx, args.get(0), &id))
+    RootedId idRoot(cx);
+    if (!ValueToId<CanGC>(cx, idValue, &idRoot))
         return false;
 
     /* Step 2. */
     RootedObject obj(cx, ToObject(cx, args.thisv()));
     if (!obj)
         return false;
 
-    /* Steps 3. */
-    RootedObject pobj(cx);
-    RootedShape prop(cx);
-    if (!JSObject::lookupGeneric(cx, obj, id, &pobj, &prop))
+    /* Step 3. */
+    Rooted<PropertyDescriptor> desc(cx);
+    if (!GetOwnPropertyDescriptor(cx, obj, idRoot, &desc))
         return false;
 
-    /* Step 4. */
-    if (!prop) {
-        args.rval().setBoolean(false);
-        return true;
-    }
-
-    if (pobj != obj) {
-        args.rval().setBoolean(false);
-        return true;
-    }
-
-    /* Step 5. */
-    unsigned attrs;
-    if (!JSObject::getGenericAttributes(cx, pobj, id, &attrs))
-        return false;
-
-    args.rval().setBoolean((attrs & JSPROP_ENUMERATE) != 0);
+    /* Steps 4-5. */
+    args.rval().setBoolean(desc.object() && desc.isEnumerable());
     return true;
 }
 
 #if JS_HAS_TOSOURCE
 static bool
 obj_toSource(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Object/propertyIsEnumerable-proxy.js
@@ -0,0 +1,55 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function logProxy(object) {
+    var log = [];
+    var handler = {
+        getOwnPropertyDescriptor(target, propertyKey) {
+            log.push(propertyKey);
+            return Object.getOwnPropertyDescriptor(target, propertyKey);
+        }
+    };
+    var proxy = new Proxy(object, new Proxy(handler, {
+        get(target, propertyKey, receiver) {
+            if (!(propertyKey in target)) {
+                throw new Error(`Unexpected call to trap: "${propertyKey}"`);
+            }
+            return target[propertyKey];
+        }
+    }));
+    return {proxy, log};
+}
+
+for (var property of ["string-property", Symbol("symbol-property")]) {
+    // Test 1: property is not present on object
+    var {proxy, log} = logProxy({});
+    var result = Object.prototype.propertyIsEnumerable.call(proxy, property);
+    assertEq(result, false);
+    assertDeepEq(log, [property]);
+
+    // Test 2: property is present on object and enumerable
+    var {proxy, log} = logProxy({[property]: 0});
+    var result = Object.prototype.propertyIsEnumerable.call(proxy, property);
+    assertEq(result, true);
+    assertDeepEq(log, [property]);
+
+    // Test 3: property is present on object, but not enumerable
+    var {proxy, log} = logProxy(Object.defineProperty({[property]: 0}, property, {enumerable: false}));
+    var result = Object.prototype.propertyIsEnumerable.call(proxy, property);
+    assertEq(result, false);
+    assertDeepEq(log, [property]);
+
+    // Test 4: property is present on prototype object
+    var {proxy, log} = logProxy(Object.create({[property]: 0}));
+    var result = Object.prototype.propertyIsEnumerable.call(proxy, property);
+    assertEq(result, false);
+    assertDeepEq(log, [property]);
+
+    // Test 5: property is present on prototype object, prototype is proxy object
+    var {proxy, log} = logProxy({[property]: 0});
+    var result = Object.prototype.propertyIsEnumerable.call(Object.create(proxy), property);
+    assertEq(result, false);
+    assertDeepEq(log, []);
+}
+
+reportCompare(0, 0);