Bug 1174971 - Introduce two variants of getOwnPropertyFromTargetIfSafe. r=gabor, r=arai, a=lizzard
authorBobby Holley <bobbyholley@gmail.com>
Mon, 15 Jun 2015 18:35:12 -0700
changeset 267740 b493f95649f4dcc96656ceafa72a638fcb7cc153
parent 267739 9ca933b3b236e9010a25ce7196837d6a2549cb4a
child 267741 2f8d779efaa1e6bad1e9b962929cf5062be02f60
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor, arai, lizzard
bugs1174971
milestone39.0
Bug 1174971 - Introduce two variants of getOwnPropertyFromTargetIfSafe. r=gabor, r=arai, a=lizzard
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -258,16 +258,31 @@ ReportWrapperDenial(JSContext* cx, Handl
                                                 windowId);
     NS_ENSURE_SUCCESS(rv, true);
     rv = consoleService->LogMessage(errorObject);
     NS_ENSURE_SUCCESS(rv, true);
 
     return true;
 }
 
+bool JSXrayTraits::getOwnPropertyFromWrapperIfSafe(JSContext* cx,
+                                                   HandleObject wrapper,
+                                                   HandleId id,
+                                                   MutableHandle<JSPropertyDescriptor> outDesc)
+{
+    MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
+    RootedObject target(cx, getTargetObject(wrapper));
+    {
+        JSAutoCompartment ac(cx, target);
+        if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, outDesc))
+            return false;
+    }
+    return JS_WrapPropertyDescriptor(cx, outDesc);
+}
+
 bool JSXrayTraits::getOwnPropertyFromTargetIfSafe(JSContext* cx,
                                                   HandleObject target,
                                                   HandleObject wrapper,
                                                   HandleId id,
                                                   MutableHandle<JSPropertyDescriptor> outDesc)
 {
     // Note - This function operates in the target compartment, because it
     // avoids a bunch of back-and-forth wrapping in enumerateNames.
@@ -353,22 +368,17 @@ JSXrayTraits::resolveOwnProperty(JSConte
         //
         // Note that, as far as JS observables go, Arrays are just Objects with
         // a different prototype and a magic (own, non-configurable) |.length| that
         // serves as a non-tight upper bound on |own| indexed properties. So while
         // it's tempting to try to impose some sort of structure on what Arrays
         // "should" look like over Xrays, the underlying object is squishy enough
         // that it makes sense to just treat them like Objects for Xray purposes.
         if (key == JSProto_Object || key == JSProto_Array) {
-            {
-                JSAutoCompartment ac(cx, target);
-                if (!getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, desc))
-                    return false;
-            }
-            return JS_WrapPropertyDescriptor(cx, desc);
+            return getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc);
         } else if (IsTypedArrayKey(key)) {
             if (IsArrayIndex(GetArrayIndexFromId(cx, id))) {
                 JS_ReportError(cx, "Accessing TypedArray data over Xrays is slow, and forbidden "
                                    "in order to encourage performant code. To copy TypedArrays "
                                    "across origin boundaries, consider using Components.utils.cloneInto().");
                 return false;
             }
         } else if (key == JSProto_Function) {
@@ -421,20 +431,18 @@ JSXrayTraits::resolveOwnProperty(JSConte
                     return false;
                 bool valueMatchesType = (isErrorIntProperty && desc.value().isInt32()) ||
                                         (isErrorStringProperty && desc.value().isString());
                 if (desc.hasGetterOrSetter() || !valueMatchesType)
                     FillPropertyDescriptor(desc, nullptr, 0, UndefinedValue());
                 return true;
             }
         } else if (key == JSProto_RegExp) {
-            if (id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_LASTINDEX)) {
-                JSAutoCompartment ac(cx, target);
-                return getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, desc);
-            }
+            if (id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_LASTINDEX))
+                return getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc);
         }
 
         // The rest of this function applies only to prototypes.
         return true;
     }
 
     // The non-HasPrototypes semantics implemented by traditional Xrays are kind
     // of broken with respect to |own|-ness and the holder. The common code
@@ -470,20 +478,18 @@ JSXrayTraits::resolveOwnProperty(JSConte
     if (IsErrorObjectKey(key) && id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME)) {
         RootedId className(cx);
         ProtoKeyToId(cx, key, &className);
         FillPropertyDescriptor(desc, wrapper, 0, UndefinedValue());
         return JS_IdToValue(cx, className, desc.value());
     }
 
     // Handle the 'lastIndex' property for RegExp prototypes.
-    if (key == JSProto_RegExp && id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_LASTINDEX)) {
-        JSAutoCompartment ac(cx, target);
-        return getOwnPropertyFromTargetIfSafe(cx, target, wrapper, id, desc);
-    }
+    if (key == JSProto_RegExp && id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_LASTINDEX))
+        return getOwnPropertyFromWrapperIfSafe(cx, wrapper, id, desc);
 
     // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
     const js::Class* clasp = js::GetObjectClass(target);
     MOZ_ASSERT(clasp->spec.defined());
 
     // Scan through the functions. Indexed array properties are handled above.
     const JSFunctionSpec* fsMatch = nullptr;
     for (const JSFunctionSpec* fs = clasp->spec.prototypeFunctions; fs && fs->name; ++fs) {
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -308,16 +308,23 @@ public:
         return js::GetReservedSlot(holder, SLOT_ISPROTOTYPE).toBoolean();
     }
 
     static JSProtoKey constructorFor(JSObject* holder) {
         int32_t key = js::GetReservedSlot(holder, SLOT_CONSTRUCTOR_FOR).toInt32();
         return static_cast<JSProtoKey>(key);
     }
 
+    // Operates in the wrapper compartment.
+    static bool getOwnPropertyFromWrapperIfSafe(JSContext* cx,
+                                                JS::HandleObject wrapper,
+                                                JS::HandleId id,
+                                                JS::MutableHandle<JSPropertyDescriptor> desc);
+
+    // Like the above, but operates in the target compartment.
     static bool getOwnPropertyFromTargetIfSafe(JSContext* cx,
                                                JS::HandleObject target,
                                                JS::HandleObject wrapper,
                                                JS::HandleId id,
                                                JS::MutableHandle<JSPropertyDescriptor> desc);
 
     static const JSClass HolderClass;
     static JSXrayTraits singleton;