Bug 1404107 - Fix cloning expando chains when reparenting DOM objects. r=bz, r=mrbkap, a=ritu
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 29 Sep 2017 10:33:13 -0500
changeset 432444 41e3144ad0da0c04f03bb867c8b3366459951b92
parent 432443 c664599c64e8aedd257de4eb3d1ff8c9a15cafd7
child 432445 de5bfee27df08b69864905401bc8f9835bb63e1c
push id7955
push userryanvm@gmail.com
push dateThu, 12 Oct 2017 18:26:39 +0000
treeherdermozilla-beta@c8d3b27528f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, mrbkap, ritu
bugs1404107
milestone57.0
Bug 1404107 - Fix cloning expando chains when reparenting DOM objects. r=bz, r=mrbkap, a=ritu
dom/bindings/BindingUtils.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2256,45 +2256,44 @@ ReparentWrapper(JSContext* aCx, JS::Hand
     if (!JS_CopyPropertiesFrom(aCx, propertyHolder, copyFrom)) {
       aError.StealExceptionFromJSContext(aCx);
       return;
     }
   } else {
     propertyHolder = nullptr;
   }
 
-  // Expandos from other compartments are attached to the target JS object.
-  // Copy them over, and let the old ones die a natural death.
-
-  // Note that at this point the DOM_OBJECT_SLOT for |newobj| has not been set.
-  // CloneExpandoChain() will use this property of |newobj| when it calls
-  // preserveWrapper() via attachExpandoObject() if |aObj| has expandos set, and
-  // preserveWrapper() will not do anything in this case.  This is safe because
-  // if expandos are present then the wrapper will already have been preserved
-  // for this native.
-  if (!xpc::XrayUtils::CloneExpandoChain(aCx, newobj, aObj)) {
-    aError.StealExceptionFromJSContext(aCx);
-    return;
-  }
+  // Grab a reference to the chain of objects that carry aObj's Xray expando
+  // properties (from all compartments). Transplanting will blow this away;
+  // we'll restore it manually afterwards.
+  JS::Rooted<JSObject*> expandoChain(aCx, xpc::XrayUtils::GetExpandoChain(aObj));
 
   // We've set up |newobj|, so we make it own the native by setting its reserved
   // slot and nulling out the reserved slot of |obj|.
   //
   // NB: It's important to do this _after_ copying the properties to
   // propertyHolder. Otherwise, an object with |foo.x === foo| will
   // crash when JS_CopyPropertiesFrom tries to call wrap() on foo.x.
   js::SetReservedSlot(newobj, DOM_OBJECT_SLOT,
                       js::GetReservedSlot(aObj, DOM_OBJECT_SLOT));
   js::SetReservedSlot(aObj, DOM_OBJECT_SLOT, JS::PrivateValue(nullptr));
 
   aObj = xpc::TransplantObject(aCx, aObj, newobj);
   if (!aObj) {
     MOZ_CRASH();
   }
 
+  // Copy Xray expando properties to the new wrapper.
+  if (!xpc::XrayUtils::CloneExpandoChain(aCx, aObj, expandoChain)) {
+    // Failure here means some expandos were not copied over. The object graph
+    // and the Xray machinery are left in a consistent state, but mysteriously
+    // losing these expandos is too weird to allow.
+    MOZ_CRASH();
+  }
+
   nsWrapperCache* cache = nullptr;
   CallQueryInterface(native, &cache);
   bool preserving = cache->PreservingWrapper();
   cache->SetPreservingWrapper(false);
   cache->SetWrapper(aObj);
   cache->SetPreservingWrapper(preserving);
 
   if (propertyHolder) {
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1236,40 +1236,22 @@ XrayTraits::ensureExpandoObject(JSContex
             return nullptr;
         expandoObject = attachExpandoObject(cx, target, ObjectPrincipal(wrapper),
                                             isSandbox ? (HandleObject)consumerGlobal : nullptr);
     }
     return expandoObject;
 }
 
 bool
-XrayTraits::cloneExpandoChain(JSContext* cx, HandleObject dst, HandleObject src)
+XrayTraits::cloneExpandoChain(JSContext* cx, HandleObject dst, HandleObject srcChain)
 {
     MOZ_ASSERT(js::IsObjectInContextCompartment(dst, cx));
     MOZ_ASSERT(getExpandoChain(dst) == nullptr);
 
-    RootedObject oldHead(cx, getExpandoChain(src));
-
-#ifdef DEBUG
-    // When this is called from dom::ReparentWrapper() there will be no native
-    // set for |dst|. Eventually it will be set to that of |src|.  This will
-    // prevent attachExpandoObject() from preserving the wrapper, but this is
-    // not a problem because in this case the wrapper will already have been
-    // preserved when expandos were originally added to |src|. Assert the
-    // wrapper for |src| has been preserved if it has expandos set.
-    if (oldHead) {
-        nsISupports* identity = mozilla::dom::UnwrapDOMObjectToISupports(src);
-        if (identity) {
-            nsWrapperCache* cache = nullptr;
-            CallQueryInterface(identity, &cache);
-            MOZ_ASSERT_IF(cache, cache->PreservingWrapper());
-        }
-    }
-#endif
-
+    RootedObject oldHead(cx, srcChain);
     while (oldHead) {
         RootedObject exclusive(cx, JS_GetReservedSlot(oldHead,
                                                       JSSLOT_EXPANDO_EXCLUSIVE_GLOBAL)
                                                      .toObjectOrNull());
         if (!JS_WrapObject(cx, &exclusive))
             return false;
         RootedObject newHead(cx, attachExpandoObject(cx, dst, GetExpandoObjectPrincipal(oldHead),
                                                      exclusive));
@@ -1313,21 +1295,24 @@ EnsureXrayExpandoObject(JSContext* cx, J
 
 const JSClass*
 XrayTraits::getExpandoClass(JSContext* cx, HandleObject target) const
 {
     return &DefaultXrayExpandoObjectClass;
 }
 
 namespace XrayUtils {
-bool CloneExpandoChain(JSContext* cx, JSObject* dstArg, JSObject* srcArg)
+JSObject* GetExpandoChain(HandleObject target)
 {
-    RootedObject dst(cx, dstArg);
-    RootedObject src(cx, srcArg);
-    return GetXrayTraits(src)->cloneExpandoChain(cx, dst, src);
+    return GetXrayTraits(target)->getExpandoChain(target);
+}
+
+bool CloneExpandoChain(JSContext* cx, HandleObject dst, HandleObject srcChain)
+{
+    return GetXrayTraits(dst)->cloneExpandoChain(cx, dst, srcChain);
 }
 } // namespace XrayUtils
 
 static JSObject*
 GetHolder(JSObject* obj)
 {
     return &js::GetProxyReservedSlot(obj, 0).toObject();
 }
--- a/js/xpconnect/wrappers/XrayWrapper.h
+++ b/js/xpconnect/wrappers/XrayWrapper.h
@@ -30,19 +30,24 @@
 
 class nsIPrincipal;
 class XPCWrappedNative;
 
 namespace xpc {
 
 namespace XrayUtils {
 
-bool IsXPCWNHolderClass(const JSClass* clasp);
+bool
+IsXPCWNHolderClass(const JSClass* clasp);
 
-bool CloneExpandoChain(JSContext* cx, JSObject* src, JSObject* dst);
+JSObject*
+GetExpandoChain(JS::HandleObject target);
+
+bool
+CloneExpandoChain(JSContext* cx, JS::HandleObject dst, JS::HandleObject srcChain);
 
 bool
 IsTransparent(JSContext* cx, JS::HandleObject wrapper, JS::HandleId id);
 
 JSObject*
 GetNativePropertiesObject(JSContext* cx, JSObject* wrapper);
 
 bool
@@ -109,17 +114,17 @@ public:
     };
 
     JSObject* getHolder(JSObject* wrapper);
     JSObject* ensureHolder(JSContext* cx, JS::HandleObject wrapper);
     virtual JSObject* createHolder(JSContext* cx, JSObject* wrapper) = 0;
 
     JSObject* getExpandoChain(JS::HandleObject obj);
     bool setExpandoChain(JSContext* cx, JS::HandleObject obj, JS::HandleObject chain);
-    bool cloneExpandoChain(JSContext* cx, JS::HandleObject dst, JS::HandleObject src);
+    bool cloneExpandoChain(JSContext* cx, JS::HandleObject dst, JS::HandleObject srcChain);
 
 protected:
     static const JSClass HolderClass;
 
     // Get the JSClass we should use for our expando object.
     virtual const JSClass* getExpandoClass(JSContext* cx,
                                            JS::HandleObject target) const;