Bug 1404107 - Fix cloning expando chains when reparenting DOM objects. r=bz,r=mrbkap
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 29 Sep 2017 10:33:13 -0500
changeset 436247 a9c5ab491f2fd0e7f59124ad26a2e74f31f45e94
parent 436246 9f28b146cb887e8a00833ee8a8f18ee4f74cf57d
child 436248 6d427a154d33e0b16bdd0ad01e802047f8930c7f
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, mrbkap
bugs1404107
milestone58.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 1404107 - Fix cloning expando chains when reparenting DOM objects. r=bz,r=mrbkap
dom/bindings/BindingUtils.cpp
js/xpconnect/wrappers/XrayWrapper.cpp
js/xpconnect/wrappers/XrayWrapper.h
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2254,45 +2254,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;