Bug 1097694 - js::HasOwnProperty sanity surgery. r=efaust.
☠☠ backed out by 2ca8635fe240 ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 11 Nov 2014 11:14:48 -0600
changeset 241598 ff97ac763705dd01700fc605500047a1bd374297
parent 241597 059b2c8bffa8d4b3092bfe68032d624abe28f631
child 241599 c69e27e865656ccd9180a82e5ca761e32140860c
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1097694
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1097694 - js::HasOwnProperty sanity surgery. r=efaust. * Rename the clownshoes js::HasOwnProperty signature to js::NonProxyLookupOwnProperty, which is what it really is. * Change the sane js::HasOwnProperty signature to use the hasOwn handler when the argument is a proxy, as that's what it's there for. * Change the DirectProxyHandler::hasOwn implementation to use js::HasOwnProperty, so that when target is also a proxy, we end up calling its hasOwn handler. Similar changes in ScriptedDirectProxyHandler.cpp.
js/src/builtin/Object.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/vm/Debugger.cpp
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -59,17 +59,18 @@ obj_propertyIsEnumerable(JSContext *cx, 
     /* 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))
+            NonProxyLookupOwnProperty<NoGC>(cx, obj->getOps()->lookupGeneric, obj, id,
+                                            &pobj, &shape))
         {
             /* Step 4. */
             if (!shape) {
                 args.rval().setBoolean(false);
                 return true;
             }
 
             /* Step 5. */
@@ -718,42 +719,34 @@ js::obj_hasOwnProperty(JSContext *cx, un
     HandleValue idValue = args.get(0);
 
     /* Step 1, 2. */
     jsid id;
     if (args.thisv().isObject() && ValueToId<NoGC>(cx, idValue, &id)) {
         JSObject *obj = &args.thisv().toObject(), *obj2;
         Shape *prop;
         if (!obj->is<ProxyObject>() &&
-            HasOwnProperty<NoGC>(cx, obj->getOps()->lookupGeneric, obj, id, &obj2, &prop))
+            NonProxyLookupOwnProperty<NoGC>(cx, obj->getOps()->lookupGeneric, obj, id,
+                                            &obj2, &prop))
         {
             args.rval().setBoolean(!!prop);
             return true;
         }
     }
 
     /* Step 1. */
     RootedId idRoot(cx);
     if (!ValueToId<CanGC>(cx, idValue, &idRoot))
         return false;
 
     /* Step 2. */
     RootedObject obj(cx, ToObject(cx, args.thisv()));
     if (!obj)
         return false;
 
-    /* Non-standard code for proxies. */
-    if (obj->is<ProxyObject>()) {
-        bool has;
-        if (!Proxy::hasOwn(cx, obj, idRoot, &has))
-            return false;
-        args.rval().setBoolean(has);
-        return true;
-    }
-
     /* Step 3. */
     bool found;
     if (!HasOwnProperty(cx, obj, idRoot, &found))
         return false;
 
     /* Step 4,5. */
     args.rval().setBoolean(found);
     return true;
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -366,17 +366,18 @@ bool
 js::GetOwnPropertyDescriptor(JSContext *cx, HandleObject obj, HandleId id,
                              MutableHandle<PropertyDescriptor> desc)
 {
     if (obj->is<ProxyObject>())
         return Proxy::getOwnPropertyDescriptor(cx, obj, id, desc);
 
     RootedObject pobj(cx);
     RootedShape shape(cx);
-    if (!HasOwnProperty<CanGC>(cx, obj->getOps()->lookupGeneric, obj, id, &pobj, &shape))
+    LookupGenericOp lookupOp = obj->getOps()->lookupGeneric;
+    if (!NonProxyLookupOwnProperty<CanGC>(cx, lookupOp, obj, id, &pobj, &shape))
         return false;
     if (!shape) {
         desc.object().set(nullptr);
         return true;
     }
 
     bool doGet = true;
     if (pobj->isNative()) {
@@ -718,17 +719,17 @@ js::CheckDefineProperty(JSContext *cx, H
 static bool
 DefinePropertyOnObject(JSContext *cx, HandleNativeObject obj, HandleId id, const PropDesc &desc,
                        bool throwError, bool *rval)
 {
     /* 8.12.9 step 1. */
     RootedShape shape(cx);
     RootedObject obj2(cx);
     MOZ_ASSERT(!obj->getOps()->lookupGeneric);
-    if (!HasOwnProperty<CanGC>(cx, nullptr, obj, id, &obj2, &shape))
+    if (!NonProxyLookupOwnProperty<CanGC>(cx, nullptr, obj, id, &obj2, &shape))
         return false;
 
     MOZ_ASSERT(!obj->getOps()->defineProperty);
 
     /* 8.12.9 steps 2-4. */
     if (!shape) {
         bool extensible;
         if (!JSObject::isExtensible(cx, obj, &extensible))
@@ -3160,22 +3161,24 @@ js::LookupNameUnqualified(JSContext *cx,
     }
 
     objp.set(scope);
     return true;
 }
 
 template <AllowGC allowGC>
 bool
-js::HasOwnProperty(JSContext *cx, LookupGenericOp lookup,
-                   typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
-                   typename MaybeRooted<jsid, allowGC>::HandleType id,
-                   typename MaybeRooted<JSObject*, allowGC>::MutableHandleType objp,
-                   typename MaybeRooted<Shape*, allowGC>::MutableHandleType propp)
+js::NonProxyLookupOwnProperty(JSContext *cx, LookupGenericOp lookup,
+                              typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
+                              typename MaybeRooted<jsid, allowGC>::HandleType id,
+                              typename MaybeRooted<JSObject*, allowGC>::MutableHandleType objp,
+                              typename MaybeRooted<Shape*, allowGC>::MutableHandleType propp)
 {
+    MOZ_ASSERT(!obj->template is<ProxyObject>());
+
     if (lookup) {
         if (!allowGC)
             return false;
         if (!lookup(cx,
                     MaybeRooted<JSObject*, allowGC>::toHandle(obj),
                     MaybeRooted<jsid, allowGC>::toHandle(id),
                     MaybeRooted<JSObject*, allowGC>::toMutableHandle(objp),
                     MaybeRooted<Shape*, allowGC>::toMutableHandle(propp)))
@@ -3213,31 +3216,36 @@ js::HasOwnProperty(JSContext *cx, Lookup
     }
 
     if (outer != objp)
         propp.set(nullptr);
     return true;
 }
 
 template bool
-js::HasOwnProperty<CanGC>(JSContext *cx, LookupGenericOp lookup,
-                          HandleObject obj, HandleId id,
-                          MutableHandleObject objp, MutableHandleShape propp);
+js::NonProxyLookupOwnProperty<CanGC>(JSContext *cx, LookupGenericOp lookup,
+                                     HandleObject obj, HandleId id,
+                                     MutableHandleObject objp, MutableHandleShape propp);
 
 template bool
-js::HasOwnProperty<NoGC>(JSContext *cx, LookupGenericOp lookup,
-                         JSObject *obj, jsid id,
-                         FakeMutableHandle<JSObject*> objp, FakeMutableHandle<Shape*> propp);
+js::NonProxyLookupOwnProperty<NoGC>(JSContext *cx, LookupGenericOp lookup,
+                                    JSObject *obj, jsid id,
+                                    FakeMutableHandle<JSObject*> objp,
+                                    FakeMutableHandle<Shape*> propp);
 
 bool
 js::HasOwnProperty(JSContext *cx, HandleObject obj, HandleId id, bool *resultp)
 {
+    if (obj->is<ProxyObject>())
+        return Proxy::hasOwn(cx, obj, id, resultp);
+
     RootedObject pobj(cx);
     RootedShape shape(cx);
-    if (!HasOwnProperty<CanGC>(cx, obj->getOps()->lookupGeneric, obj, id, &pobj, &shape))
+    LookupGenericOp lookupOp = obj->getOps()->lookupGeneric;
+    if (!NonProxyLookupOwnProperty<CanGC>(cx, lookupOp, obj, id, &pobj, &shape))
         return false;
     *resultp = (shape != nullptr);
     return true;
 }
 
 static MOZ_ALWAYS_INLINE bool
 LookupPropertyPureInline(ThreadSafeContext *cx, JSObject *obj, jsid id, NativeObject **objp,
                          Shape **propp)
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -919,21 +919,21 @@ class ValueArray {
 namespace js {
 
 /* Set *resultp to tell whether obj has an own property with the given id. */
 bool
 HasOwnProperty(JSContext *cx, HandleObject obj, HandleId id, bool *resultp);
 
 template <AllowGC allowGC>
 extern bool
-HasOwnProperty(JSContext *cx, LookupGenericOp lookup,
-               typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
-               typename MaybeRooted<jsid, allowGC>::HandleType id,
-               typename MaybeRooted<JSObject*, allowGC>::MutableHandleType objp,
-               typename MaybeRooted<Shape*, allowGC>::MutableHandleType propp);
+NonProxyLookupOwnProperty(JSContext *cx, LookupGenericOp lookup,
+                          typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
+                          typename MaybeRooted<jsid, allowGC>::HandleType id,
+                          typename MaybeRooted<JSObject*, allowGC>::MutableHandleType objp,
+                          typename MaybeRooted<Shape*, allowGC>::MutableHandleType propp);
 
 typedef JSObject *(*ClassInitializerOp)(JSContext *cx, JS::HandleObject obj);
 
 /* Fast access to builtin constructors and prototypes. */
 bool
 GetBuiltinConstructor(ExclusiveContext *cx, JSProtoKey key, MutableHandleObject objp);
 
 bool
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -203,21 +203,17 @@ DirectProxyHandler::has(JSContext *cx, H
     return true;
 }
 
 bool
 DirectProxyHandler::hasOwn(JSContext *cx, HandleObject proxy, HandleId id, bool *bp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
-    Rooted<PropertyDescriptor> desc(cx);
-    if (!JS_GetPropertyDescriptorById(cx, target, id, &desc))
-        return false;
-    *bp = (desc.object() == target);
-    return true;
+    return js::HasOwnProperty(cx, target, id, bp);
 }
 
 bool
 DirectProxyHandler::get(JSContext *cx, HandleObject proxy, HandleObject receiver,
                         HandleId id, MutableHandleValue vp) const
 {
     assertEnteredPolicy(cx, proxy, id, GET);
     RootedObject target(cx, proxy->as<ProxyObject>().target());
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -138,26 +138,16 @@ IsSealed(JSContext* cx, HandleObject obj
     if (!GetOwnPropertyDescriptor(cx, obj, id, &desc))
         return false;
 
     // steps 2-3
     *bp = desc.object() && desc.isPermanent();
     return true;
 }
 
-static bool
-HasOwn(JSContext *cx, HandleObject obj, HandleId id, bool *bp)
-{
-    Rooted<PropertyDescriptor> desc(cx);
-    if (!JS_GetPropertyDescriptorById(cx, obj, id, &desc))
-        return false;
-    *bp = (desc.object() == obj);
-    return true;
-}
-
 // Get the [[ProxyHandler]] of a scripted direct proxy.
 static JSObject *
 GetDirectProxyHandlerObject(JSObject *proxy)
 {
     MOZ_ASSERT(proxy->as<ProxyObject>().handler() == &ScriptedDirectProxyHandler::singleton);
     return proxy->as<ProxyObject>().extra(ScriptedDirectProxyHandler::HANDLER_EXTRA).toObjectOrNull();
 }
 
@@ -204,17 +194,17 @@ ArrayToIdVector(JSContext *cx, HandleObj
             if (props[j].get() == id) {
                 ReportInvalidTrapResult(cx, proxy, trapName);
                 return false;
             }
         }
 
         // step iv
         bool isFixed;
-        if (!HasOwn(cx, target, id, &isFixed))
+        if (!HasOwnProperty(cx, target, id, &isFixed))
             return false;
 
         // step v
         bool extensible;
         if (!JSObject::isExtensible(cx, target, &extensible))
             return false;
         if (!extensible && !isFixed) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_NEW);
@@ -251,17 +241,17 @@ ArrayToIdVector(JSContext *cx, HandleObj
             return false;
         if (sealed) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_SKIP_NC);
             return false;
         }
 
         // step ii
         bool isFixed;
-        if (!HasOwn(cx, target, id, &isFixed))
+        if (!HasOwnProperty(cx, target, id, &isFixed))
             return false;
 
         // step iii
         bool extensible;
         if (!JSObject::isExtensible(cx, target, &extensible))
             return false;
         if (!extensible && isFixed) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_CANT_REPORT_E_AS_NE);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -4339,17 +4339,17 @@ DebuggerScript_getAllOffsets(JSContext *
         if (!flowData[offset].hasNoEdges() && flowData[offset].lineno() != lineno) {
             /* Get the offsets array for this line. */
             RootedObject offsets(cx);
             RootedValue offsetsv(cx);
 
             RootedId id(cx, INT_TO_JSID(lineno));
 
             bool found;
-            if (!js::HasOwnProperty(cx, result, id, &found))
+            if (!HasOwnProperty(cx, result, id, &found))
                 return false;
             if (found && !JSObject::getGeneric(cx, result, result, id, &offsetsv))
                 return false;
 
             if (offsetsv.isObject()) {
                 offsets = &offsetsv.toObject();
             } else {
                 MOZ_ASSERT(offsetsv.isUndefined());