Bug 1790081 - Make XPCVariant into a normal script holder class. r=smaug

The traverse method no longer traverses the JS val, because that will
now be automatically traced by the CC using the trace method.

The purple methods are unused.

I marked a few methods that regular callers shouldn't use protected.

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

js/xpconnect/src/XPCForwards.h

@@ -29,8 +29,6 @@ class XPCWrappedNative;
 class XPCWrappedNativeProto;
 class XPCWrappedNativeTearOff;
 
-class XPCTraceableVariant;
-
 class JSObject2WrappedJSMap;
 class Native2WrappedNativeMap;
 class IID2NativeInterfaceMap;

js/xpconnect/src/XPCInlines.h

@@ -15,10 +15,6 @@
 
 /***************************************************************************/
 
-inline void XPCJSRuntime::AddVariantRoot(XPCTraceableVariant* variant) {
-  variant->AddToRootSet(&mVariantRoots);
-}
-
 inline void XPCJSRuntime::AddWrappedJSRoot(nsXPCWrappedJS* wrappedJS) {
   wrappedJS->AddToRootSet(&mWrappedJSRoots);
 }

js/xpconnect/src/XPCJSRuntime.cpp

@@ -719,10 +719,6 @@ void XPCJSRuntime::TraceNativeBlackRoots(JSTracer* trc) {
 void XPCJSRuntime::TraceAdditionalNativeGrayRoots(JSTracer* trc) {
   XPCWrappedNativeScope::TraceWrappedNativesInAllScopes(this, trc);
 
-  for (XPCRootSetElem* e = mVariantRoots; e; e = e->GetNextRoot()) {
-    static_cast<XPCTraceableVariant*>(e)->TraceJS(trc);
-  }
-
   for (XPCRootSetElem* e = mWrappedJSRoots; e; e = e->GetNextRoot()) {
     static_cast<nsXPCWrappedJS*>(e)->TraceJS(trc);
   }
@@ -732,13 +728,6 @@ void XPCJSRuntime::TraverseAdditionalNativeRoots(
     nsCycleCollectionNoteRootCallback& cb) {
   XPCWrappedNativeScope::SuspectAllWrappers(cb);
 
-  for (XPCRootSetElem* e = mVariantRoots; e; e = e->GetNextRoot()) {
-    XPCTraceableVariant* v = static_cast<XPCTraceableVariant*>(e);
-    cb.NoteXPCOMRoot(
-        v,
-        XPCTraceableVariant::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant());
-  }
-
   for (XPCRootSetElem* e = mWrappedJSRoots; e; e = e->GetNextRoot()) {
     cb.NoteXPCOMRoot(
         ToSupports(static_cast<nsXPCWrappedJS*>(e)),
@@ -2797,7 +2786,6 @@ XPCJSRuntime::XPCJSRuntime(JSContext* aCx)
       mGCIsRunning(false),
       mNativesToReleaseArray(),
       mDoingFinalization(false),
-      mVariantRoots(nullptr),
       mWrappedJSRoots(nullptr),
       mAsyncSnowWhiteFreer(new AsyncFreeSnowWhite()) {
   MOZ_COUNT_CTOR_INHERITED(XPCJSRuntime, CycleCollectedJSRuntime);

js/xpconnect/src/XPCVariant.cpp

@@ -16,6 +16,7 @@
 #include "js/friend/WindowProxy.h"  // js::ToWindowIfWindowProxy
 #include "js/PropertyAndElement.h"  // JS_GetElement
 #include "js/Wrapper.h"
+#include "mozilla/HoldDropJSObjects.h"
 
 using namespace JS;
 using namespace mozilla;
@@ -52,69 +53,49 @@ XPCVariant::XPCVariant(JSContext* cx, const Value& aJSVal) : mJSVal(aJSVal) {
   } else {
     mReturnRawObject = false;
   }
-}
-
-XPCTraceableVariant::~XPCTraceableVariant() {
-  Value val = GetJSValPreserveColor();
-
-  MOZ_ASSERT(val.isGCThing() || val.isNull(), "Must be traceable or unlinked");
-  bool unroot = val.isGCThing();
 
-  mData.Cleanup();
-
-  if (unroot) {
-    RemoveFromRootSet();
+  if (aJSVal.isGCThing()) {
+    mozilla::HoldJSObjects(this);
   }
 }
 
-void XPCTraceableVariant::TraceJS(JSTracer* trc) {
-  MOZ_ASSERT(GetJSValPreserveColor().isGCThing());
-  JS::TraceEdge(trc, &mJSVal, "XPCTraceableVariant::mJSVal");
-}
+XPCVariant::~XPCVariant() { Cleanup(); }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(XPCVariant)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(XPCVariant)
-  JS::Value val = tmp->GetJSValPreserveColor();
-  if (val.isObject()) {
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mJSVal");
-    cb.NoteJSChild(val.toGCCellPtr());
-  }
-
   tmp->mData.Traverse(cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(XPCVariant)
-  JS::Value val = tmp->GetJSValPreserveColor();
-  bool unroot = val.isGCThing();
-
-  tmp->mData.Cleanup();
-
-  if (unroot) {
-    XPCTraceableVariant* v = static_cast<XPCTraceableVariant*>(tmp);
-    v->RemoveFromRootSet();
-  }
-  tmp->mJSVal = JS::NullValue();
+  tmp->Cleanup();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
+NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(XPCVariant)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mJSVal)
+NS_IMPL_CYCLE_COLLECTION_TRACE_END
+
 // static
 already_AddRefed<XPCVariant> XPCVariant::newVariant(JSContext* cx,
                                                     const Value& aJSVal) {
-  RefPtr<XPCVariant> variant;
-
-  if (!aJSVal.isGCThing()) {
-    variant = new XPCVariant(cx, aJSVal);
-  } else {
-    variant = new XPCTraceableVariant(cx, aJSVal);
-  }
-
+  RefPtr<XPCVariant> variant = new XPCVariant(cx, aJSVal);
   if (!variant->InitializeData(cx)) {
     return nullptr;
   }
 
   return variant.forget();
 }
 
+void XPCVariant::Cleanup() {
+  mData.Cleanup();
+
+  if (!GetJSValPreserveColor().isGCThing()) {
+    return;
+  }
+  mJSVal = JS::NullValue();
+  mozilla::DropJSObjects(this);
+}
+
 // Helper class to give us a namespace for the table based code below.
 class XPCArrayHomogenizer {
  private:

js/xpconnect/src/xpcprivate.h

@@ -555,7 +555,6 @@ class XPCJSRuntime final : public mozilla::CycleCollectedJSRuntime {
   static void WeakPointerCompartmentCallback(JSTracer* trc,
                                              JS::Compartment* comp, void* data);
 
-  inline void AddVariantRoot(XPCTraceableVariant* variant);
   inline void AddWrappedJSRoot(nsXPCWrappedJS* wrappedJS);
 
   void DebugDump(int16_t depth);
@@ -637,7 +636,6 @@ class XPCJSRuntime final : public mozilla::CycleCollectedJSRuntime {
   bool mGCIsRunning;
   nsTArray<nsISupports*> mNativesToReleaseArray;
   bool mDoingFinalization;
-  XPCRootSetElem* mVariantRoots;
   XPCRootSetElem* mWrappedJSRoots;
   nsTArray<xpcGCCallback> extraGCCallbacks;
   JS::GCSliceCallback mPrevGCSliceCallback;
@@ -2104,7 +2102,7 @@ using AutoMarkingWrappedNativeProtoPtr =
     TypedAutoMarkingPtr<XPCWrappedNativeProto>;
 
 /***************************************************************************/
-// in xpcvariant.cpp...
+// Definitions in XPCVariant.cpp.
 
 // {1809FD50-91E8-11d5-90F9-0010A4E73D9A}
 #define XPCVARIANT_IID                              \
@@ -2126,7 +2124,7 @@ class XPCVariant : public nsIVariant {
  public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSIVARIANT
-  NS_DECL_CYCLE_COLLECTION_CLASS(XPCVariant)
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(XPCVariant)
 
   // If this class ever implements nsIWritableVariant, take special care with
   // the case when mJSVal is JSVAL_STRING, since we don't own the data in
@@ -2146,6 +2144,7 @@ class XPCVariant : public nsIVariant {
    */
   JS::Value GetJSVal() const { return mJSVal; }
 
+ protected:
   /**
    * This getter does not change the color of the Value (if it represents a
    * JSObject) meaning that the value returned is not guaranteed to be kept
@@ -2159,6 +2158,7 @@ class XPCVariant : public nsIVariant {
 
   XPCVariant(JSContext* cx, const JS::Value& aJSVal);
 
+ public:
   /**
    * Convert a variant into a JS::Value.
    *
@@ -2171,35 +2171,20 @@ class XPCVariant : public nsIVariant {
   static bool VariantDataToJS(JSContext* cx, nsIVariant* variant,
                               nsresult* pErr, JS::MutableHandleValue pJSVal);
 
-  bool IsPurple() { return mRefCnt.IsPurple(); }
-
-  void RemovePurple() { mRefCnt.RemovePurple(); }
-
  protected:
-  virtual ~XPCVariant() = default;
+  virtual ~XPCVariant();
 
   bool InitializeData(JSContext* cx);
 
- protected:
+  void Cleanup();
+
   nsDiscriminatedUnion mData;
   JS::Heap<JS::Value> mJSVal;
   bool mReturnRawObject;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(XPCVariant, XPCVARIANT_IID)
 
-class XPCTraceableVariant : public XPCVariant, public XPCRootSetElem {
- public:
-  XPCTraceableVariant(JSContext* cx, const JS::Value& aJSVal)
-      : XPCVariant(cx, aJSVal) {
-    nsXPConnect::GetRuntimeInstance()->AddVariantRoot(this);
-  }
-
-  virtual ~XPCTraceableVariant();
-
-  void TraceJS(JSTracer* trc);
-};
-
 /***************************************************************************/
 // Utilities