Bug 1425450 - Ensure that we call Drop/HoldJSObjects when a preserved wrapper is changed for one in a different zone r=mccr8

So that we can update the holders table correctly we will need to remove the wrapper from its original vector and add it to the one associated with the new zone.

I tried to make SetPreservingWrapper private but there's still a use in layout/style/Rule.cpp that I couldn't see an obvious fix for.

Differential Revision: https://phabricator.services.mozilla.com/D68521

dom/base/nsWrapperCache.cpp

@@ -100,6 +100,11 @@ void nsWrapperCache::CheckCCWrapperTraversal(void* aScriptObjectHolder,
     return;
   }
 
+  // Temporarily make this a preserving wrapper so that TraceWrapper() traces
+  // it.
+  bool wasPreservingWrapper = PreservingWrapper();
+  SetPreservingWrapper(true);
+
   DebugWrapperTraversalCallback callback(wrapper);
 
   // The CC traversal machinery cannot trigger GC; however, the analysis cannot
@@ -117,6 +122,8 @@ void nsWrapperCache::CheckCCWrapperTraversal(void* aScriptObjectHolder,
   MOZ_ASSERT(callback.mFound,
              "Cycle collection participant didn't trace preserved wrapper! "
              "This will probably crash.");
+
+  SetPreservingWrapper(wasPreservingWrapper);
 }
 
 #endif  // DEBUG

dom/base/nsWrapperCache.h

@@ -178,6 +178,12 @@ class nsWrapperCache {
     }
   }
 
+  /**
+   * Update the wrapper when the object moves between globals.
+   */
+  template <typename T>
+  void UpdateWrapperForNewGlobal(T* aScriptObjectHolder, JSObject* aNewWrapper);
+
   /**
    * Update the wrapper if the object it contains is moved.
    *

dom/base/nsWrapperCacheInlines.h

@@ -68,4 +68,30 @@ inline void nsWrapperCache::MarkWrapperLive() {
   GetWrapper();
 }
 
+template <typename T>
+inline void nsWrapperCache::UpdateWrapperForNewGlobal(T* aScriptObjectHolder,
+                                                      JSObject* aNewWrapper) {
+  // If the new wrapper is in a different zone we must ensure the
+  // DropJSObjects/HoldJSObjects are called to move the holder to the new zone.
+
+  bool preserving = PreservingWrapper();
+  bool zoneChanged =
+      preserving && (JS::GetObjectZone(GetWrapperPreserveColor()) !=
+                     JS::GetObjectZone(aNewWrapper));
+
+  if (zoneChanged) {
+    ReleaseWrapper(aScriptObjectHolder);
+  } else if (preserving) {
+    SetPreservingWrapper(false);
+  }
+
+  SetWrapper(aNewWrapper);
+
+  if (zoneChanged) {
+    PreserveWrapper(aScriptObjectHolder);
+  } else if (preserving) {
+    SetPreservingWrapper(true);
+  }
+}
+
 #endif /* nsWrapperCache_h___ */

dom/bindings/BindingUtils.cpp

@@ -41,6 +41,7 @@
 #include "nsPrintfCString.h"
 #include "mozilla/Sprintf.h"
 #include "nsReadableUtils.h"
+#include "nsWrapperCacheInlines.h"
 
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/CustomElementRegistry.h"
@@ -2204,10 +2205,7 @@ void UpdateReflectorGlobal(JSContext* aCx, JS::Handle<JSObject*> aObjArg,
 
   nsWrapperCache* cache = nullptr;
   CallQueryInterface(native, &cache);
-  bool preserving = cache->PreservingWrapper();
-  cache->SetPreservingWrapper(false);
-  cache->SetWrapper(aObj);
-  cache->SetPreservingWrapper(preserving);
+  cache->UpdateWrapperForNewGlobal(native, aObj);
 
   if (propertyHolder) {
     JS::Rooted<JSObject*> copyTo(aCx);

dom/bindings/BindingUtils.h

@@ -1027,10 +1027,7 @@ struct CheckWrapperCacheTracing<T, true> {
     CallQueryInterface(ccISupports, &participant);
     MOZ_ASSERT(participant, "Can't QI to CycleCollectionParticipant?");
 
-    bool wasPreservingWrapper = wrapperCacheFromQI->PreservingWrapper();
-    wrapperCacheFromQI->SetPreservingWrapper(true);
     wrapperCacheFromQI->CheckCCWrapperTraversal(ccISupports, participant);
-    wrapperCacheFromQI->SetPreservingWrapper(wasPreservingWrapper);
   }
 };