Bug 1385474 - Avoid QIing for NoteXPCOMRoot. r=smaug draft
authorAndrew McCreight <continuation@gmail.com>
Fri, 28 Jul 2017 16:11:03 -0700
changeset 618032 fea3468c91c7e53d25f28c1c4d034f56900d3647
parent 618031 ddc4616855f88ebda162fe761db371a0bdd7b8a2
child 639947 0538511c92a4b3ef5960308354cc85fdc89e9809
push id71200
push userbmo:continuation@gmail.com
push dateSat, 29 Jul 2017 15:35:06 +0000
reviewerssmaug
bugs1385474
milestone56.0a1
Bug 1385474 - Avoid QIing for NoteXPCOMRoot. r=smaug This callback is only used in very limited ways, so just require that the caller pass in the canonical supports pointer, plus the participant. This probably won't affect performance much. MozReview-Commit-ID: CsThzFsKyYx
js/xpconnect/src/XPCJSRuntime.cpp
xpcom/base/nsCycleCollectionNoteRootCallback.h
xpcom/base/nsCycleCollector.cpp
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -653,21 +653,23 @@ XPCJSRuntime::TraverseAdditionalNativeRo
     for (XPCRootSetElem* e = mVariantRoots; e ; e = e->GetNextRoot()) {
         XPCTraceableVariant* v = static_cast<XPCTraceableVariant*>(e);
         if (nsCCUncollectableMarker::InGeneration(cb,
                                                   v->CCGeneration())) {
            JS::Value val = v->GetJSValPreserveColor();
            if (val.isObject() && !JS::ObjectIsMarkedGray(&val.toObject()))
                continue;
         }
-        cb.NoteXPCOMRoot(v);
+        cb.NoteXPCOMRoot(v,
+                         XPCTraceableVariant::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant());
     }
 
     for (XPCRootSetElem* e = mWrappedJSRoots; e ; e = e->GetNextRoot()) {
-        cb.NoteXPCOMRoot(ToSupports(static_cast<nsXPCWrappedJS*>(e)));
+        cb.NoteXPCOMRoot(ToSupports(static_cast<nsXPCWrappedJS*>(e)),
+                         nsXPCWrappedJS::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant());
     }
 }
 
 void
 XPCJSRuntime::UnmarkSkippableJSHolders()
 {
     CycleCollectedJSRuntime::UnmarkSkippableJSHolders();
 }
--- a/xpcom/base/nsCycleCollectionNoteRootCallback.h
+++ b/xpcom/base/nsCycleCollectionNoteRootCallback.h
@@ -8,17 +8,20 @@
 #define nsCycleCollectionNoteRootCallback_h__
 
 class nsCycleCollectionParticipant;
 class nsISupports;
 
 class nsCycleCollectionNoteRootCallback
 {
 public:
-  NS_IMETHOD_(void) NoteXPCOMRoot(nsISupports* aRoot) = 0;
+  // aRoot must be canonical (ie the result of QIing to nsCycleCollectionISupports).
+  NS_IMETHOD_(void) NoteXPCOMRoot(nsISupports* aRoot,
+                                  nsCycleCollectionParticipant* aParticipant) = 0;
+
   NS_IMETHOD_(void) NoteJSRoot(JSObject* aRoot) = 0;
   NS_IMETHOD_(void) NoteNativeRoot(void* aRoot,
                                    nsCycleCollectionParticipant* aParticipant) = 0;
 
   NS_IMETHOD_(void) NoteWeakMapping(JSObject* aMap, JS::GCCellPtr aKey,
                                     JSObject* aKeyDelegate, JS::GCCellPtr aVal) = 0;
 
   bool WantAllTraces() const { return mWantAllTraces; }
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -2097,17 +2097,18 @@ private:
 
   void SetLastChild()
   {
     mCurrPi->SetLastChild(mEdgeBuilder.Mark());
   }
 
 public:
   // nsCycleCollectionNoteRootCallback methods.
-  NS_IMETHOD_(void) NoteXPCOMRoot(nsISupports* aRoot);
+  NS_IMETHOD_(void) NoteXPCOMRoot(nsISupports* aRoot,
+                                  nsCycleCollectionParticipant* aParticipant);
   NS_IMETHOD_(void) NoteJSRoot(JSObject* aRoot);
   NS_IMETHOD_(void) NoteNativeRoot(void* aRoot,
                                    nsCycleCollectionParticipant* aParticipant);
   NS_IMETHOD_(void) NoteWeakMapping(JSObject* aMap, JS::GCCellPtr aKey,
                                     JSObject* aKdelegate, JS::GCCellPtr aVal);
 
   // nsCycleCollectionTraversalCallback methods.
   NS_IMETHOD_(void) DescribeRefCountedNode(nsrefcnt aRefCount,
@@ -2290,26 +2291,28 @@ CCGraphBuilder::BuildGraph(SliceBudget& 
   }
 
   mCurrNode = nullptr;
 
   return true;
 }
 
 NS_IMETHODIMP_(void)
-CCGraphBuilder::NoteXPCOMRoot(nsISupports* aRoot)
+CCGraphBuilder::NoteXPCOMRoot(nsISupports* aRoot,
+                              nsCycleCollectionParticipant* aParticipant)
 {
-  aRoot = CanonicalizeXPCOMParticipant(aRoot);
-  NS_ASSERTION(aRoot,
-               "Don't add objects that don't participate in collection!");
-
+  MOZ_ASSERT(aRoot == CanonicalizeXPCOMParticipant(aRoot));
+
+#ifdef DEBUG
   nsXPCOMCycleCollectionParticipant* cp;
   ToParticipant(aRoot, &cp);
-
-  NoteRoot(aRoot, cp);
+  MOZ_ASSERT(aParticipant == cp);
+#endif
+
+  NoteRoot(aRoot, aParticipant);
 }
 
 NS_IMETHODIMP_(void)
 CCGraphBuilder::NoteJSRoot(JSObject* aRoot)
 {
   if (JS::Zone* zone = MergeZone(JS::GCCellPtr(aRoot))) {
     NoteRoot(zone, mJSZoneParticipant);
   } else {