Bug 1481467 part 3 - Use JSAutoRealm instead of JSAutoRealmAllowCCW in XrayTraits::attachExpandoObject. r=bz
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 08 Aug 2018 15:14:02 +0200
changeset 430614 dc3ee665001f01a78281fdf2ccf8abf1c7852065
parent 430613 8b38554d067fb3b071b19f630219a385ce6241b3
child 430615 20673c6ceb3459c672b96cf92f71da0e1a3453a2
push id106201
push userjandemooij@gmail.com
push dateWed, 08 Aug 2018 13:14:54 +0000
treeherdermozilla-inbound@dc3ee665001f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1481467
milestone63.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 1481467 part 3 - Use JSAutoRealm instead of JSAutoRealmAllowCCW in XrayTraits::attachExpandoObject. r=bz Because XrayTraits::attachExpandoObject operates in the Xray target realm/compartment and we cannot use the Xray wrapper with JSAutoRealm, we pass the caller's global as exclusiveWrapperGlobal and use that. This also changes XrayWrapper<Base, Traits>::defineProperty to call ensureExpandoObject in the wrapper (instead of target) realm. This didn't matter before, because ensureExpandoObject immediately entered the target realm anyway.
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1166,21 +1166,26 @@ static const JSClass gWrapperHolderClass
     "XrayExpandoWrapperHolder",
     JSCLASS_HAS_RESERVED_SLOTS(1)
 };
 static const size_t JSSLOT_WRAPPER_HOLDER_CONTENTS = 0;
 
 JSObject*
 XrayTraits::attachExpandoObject(JSContext* cx, HandleObject target,
                                 HandleObject exclusiveWrapper,
+                                HandleObject exclusiveWrapperGlobal,
                                 nsIPrincipal* origin)
 {
     // Make sure the compartments are sane.
     MOZ_ASSERT(js::IsObjectInContextCompartment(target, cx));
-    MOZ_ASSERT_IF(exclusiveWrapper, !js::IsObjectInContextCompartment(exclusiveWrapper, cx));
+    if (exclusiveWrapper) {
+        MOZ_ASSERT(!js::IsObjectInContextCompartment(exclusiveWrapper, cx));
+        MOZ_ASSERT(JS_IsGlobalObject(exclusiveWrapperGlobal));
+        js::AssertSameCompartment(exclusiveWrapper, exclusiveWrapperGlobal);
+    }
 
     // No duplicates allowed.
 #ifdef DEBUG
     {
         JSObject* chain = getExpandoChain(target);
         if (chain) {
             RootedObject existingExpandoObject(cx);
             if (getExpandoObjectInternal(cx, chain, exclusiveWrapper, origin, &existingExpandoObject))
@@ -1201,31 +1206,31 @@ XrayTraits::attachExpandoObject(JSContex
 
     // AddRef and store the principal.
     NS_ADDREF(origin);
     JS_SetReservedSlot(expandoObject, JSSLOT_EXPANDO_ORIGIN, JS::PrivateValue(origin));
 
     // Note the exclusive wrapper, if there is one.
     RootedObject wrapperHolder(cx);
     if (exclusiveWrapper) {
-        JSAutoRealmAllowCCW ar(cx, exclusiveWrapper);
+        JSAutoRealm ar(cx, exclusiveWrapperGlobal);
         wrapperHolder = JS_NewObjectWithGivenProto(cx, &gWrapperHolderClass, nullptr);
         if (!wrapperHolder)
             return nullptr;
         JS_SetReservedSlot(wrapperHolder, JSSLOT_WRAPPER_HOLDER_CONTENTS, ObjectValue(*exclusiveWrapper));
     }
     if (!JS_WrapObject(cx, &wrapperHolder))
         return nullptr;
     JS_SetReservedSlot(expandoObject, JSSLOT_EXPANDO_EXCLUSIVE_WRAPPER_HOLDER,
                        ObjectOrNullValue(wrapperHolder));
 
     // Store it on the exclusive wrapper, if there is one.
     if (exclusiveWrapper) {
         RootedObject cachedExpandoObject(cx, expandoObject);
-        JSAutoRealmAllowCCW ar(cx, exclusiveWrapper);
+        JSAutoRealm ar(cx, exclusiveWrapperGlobal);
         if (!JS_WrapObject(cx, &cachedExpandoObject))
             return nullptr;
         JSObject* holder = ensureHolder(cx, exclusiveWrapper);
         if (!holder)
             return nullptr;
         SetCachedXrayExpando(holder, cachedExpandoObject);
     }
 
@@ -1242,24 +1247,28 @@ XrayTraits::attachExpandoObject(JSContex
 
     return expandoObject;
 }
 
 JSObject*
 XrayTraits::ensureExpandoObject(JSContext* cx, HandleObject wrapper,
                                 HandleObject target)
 {
+    MOZ_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
+    RootedObject wrapperGlobal(cx, JS::CurrentGlobalOrNull(cx));
+
     // Expando objects live in the target compartment.
     JSAutoRealm ar(cx, target);
     RootedObject expandoObject(cx);
     if (!getExpandoObject(cx, target, wrapper, &expandoObject))
         return nullptr;
     if (!expandoObject) {
         bool isExclusive = CompartmentHasExclusiveExpandos(wrapper);
         expandoObject = attachExpandoObject(cx, target, isExclusive ? wrapper : nullptr,
+                                            wrapperGlobal,
                                             ObjectPrincipal(wrapper));
     }
     return expandoObject;
 }
 
 bool
 XrayTraits::cloneExpandoChain(JSContext* cx, HandleObject dst, HandleObject srcChain)
 {
@@ -1270,16 +1279,17 @@ XrayTraits::cloneExpandoChain(JSContext*
     while (oldHead) {
         // If movingIntoXrayCompartment is true, then our new reflector is in a
         // compartment that used to have an Xray-with-expandos to the old reflector
         // and we should copy the expandos to the new reflector directly.
         bool movingIntoXrayCompartment;
 
         // exclusiveWrapper is only used if movingIntoXrayCompartment ends up true.
         RootedObject exclusiveWrapper(cx);
+        RootedObject exclusiveWrapperGlobal(cx);
         RootedObject wrapperHolder(cx, JS_GetReservedSlot(oldHead,
                                                           JSSLOT_EXPANDO_EXCLUSIVE_WRAPPER_HOLDER)
                                                          .toObjectOrNull());
         if (wrapperHolder) {
             RootedObject unwrappedHolder(cx, UncheckedUnwrap(wrapperHolder));
             // unwrappedHolder is the compartment of the relevant Xray, so check
             // whether that matches the compartment of cx (which matches the
             // compartment of dst).
@@ -1289,31 +1299,33 @@ XrayTraits::cloneExpandoChain(JSContext*
             if (!movingIntoXrayCompartment) {
                 // The global containing this wrapper holder has an xray for |src|
                 // with expandos. Create an xray in the global for |dst| which
                 // will be associated with a clone of |src|'s expando object.
                 JSAutoRealm ar(cx, unwrappedHolder);
                 exclusiveWrapper = dst;
                 if (!JS_WrapObject(cx, &exclusiveWrapper))
                     return false;
+                exclusiveWrapperGlobal = JS::CurrentGlobalOrNull(cx);
             }
         } else {
             JSAutoRealm ar(cx, oldHead);
             movingIntoXrayCompartment =
                 expandoObjectMatchesConsumer(cx, oldHead, ObjectPrincipal(dst));
         }
 
         if (movingIntoXrayCompartment) {
             // Just copy properties directly onto dst.
             if (!JS_CopyPropertiesFrom(cx, dst, oldHead))
                 return false;
         } else {
             // Create a new expando object in the compartment of dst to replace
             // oldHead.
             RootedObject newHead(cx, attachExpandoObject(cx, dst, exclusiveWrapper,
+                                                         exclusiveWrapperGlobal,
                                                          GetExpandoObjectPrincipal(oldHead)));
             if (!JS_CopyPropertiesFrom(cx, newHead, oldHead))
                 return false;
         }
         oldHead = JS_GetReservedSlot(oldHead, JSSLOT_EXPANDO_NEXT).toObjectOrNull();
     }
     return true;
 }
@@ -1934,28 +1946,28 @@ XrayWrapper<Base, Traits>::definePropert
     }
 
     bool defined = false;
     if (!Traits::singleton.defineProperty(cx, wrapper, id, desc, existing_desc, result, &defined))
         return false;
     if (defined)
         return true;
 
-    // We're placing an expando. The expando objects live in the target
-    // compartment, so we need to enter it.
+    // Grab the relevant expando object.
     RootedObject target(cx, Traits::getTargetObject(wrapper));
-    JSAutoRealm ar(cx, target);
-    JS_MarkCrossZoneId(cx, id);
-
-    // Grab the relevant expando object.
     RootedObject expandoObject(cx, Traits::singleton.ensureExpandoObject(cx, wrapper,
                                                                          target));
     if (!expandoObject)
         return false;
 
+    // We're placing an expando. The expando objects live in the target
+    // compartment, so we need to enter it.
+    JSAutoRealm ar(cx, target);
+    JS_MarkCrossZoneId(cx, id);
+
     // Wrap the property descriptor for the target compartment.
     Rooted<PropertyDescriptor> wrappedDesc(cx, desc);
     if (!JS_WrapPropertyDescriptor(cx, &wrappedDesc))
         return false;
 
     // Fix up Xray waivers.
     if (!RecreateLostWaivers(cx, desc.address(), &wrappedDesc))
         return false;
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -121,19 +121,21 @@ private:
     // |exclusiveWrapper| is any xray that has exclusive use of the expando.
     // |cx| may be in any compartment.
     bool getExpandoObjectInternal(JSContext* cx, JSObject* expandoChain,
                                   JS::HandleObject exclusiveWrapper,
                                   nsIPrincipal* origin,
                                   JS::MutableHandleObject expandoObject);
 
     // |cx| is in the target's compartment, and |exclusiveWrapper| is any xray
-    // that has exclusive use of the expando.
+    // that has exclusive use of the expando. |exclusiveWrapperGlobal| is the
+    // caller's global and must be same-compartment with |exclusiveWrapper|.
     JSObject* attachExpandoObject(JSContext* cx, JS::HandleObject target,
                                   JS::HandleObject exclusiveWrapper,
+                                  JS::HandleObject exclusiveWrapperGlobal,
                                   nsIPrincipal* origin);
 
     XrayTraits(XrayTraits&) = delete;
     const XrayTraits& operator=(XrayTraits&) = delete;
 };
 
 class DOMXrayTraits : public XrayTraits
 {