Bug 1087799, part 3 - Do not include any JS things in the list of white nodes. r=smaug

Root() does not actually root JS things, so if some other class's Unlink() method ends
up calling the GC, whiteNodes will end up containing dead pointers. (This is safe right
now because the Unlink and Unroot methods do not do anything to JS things.) It is less
error prone to simply never store those pointers.

Also, add some asserts to enforce that we never call any of the white-object methods
for JS things.

xpcom/base/CycleCollectedJSRuntime.h

@@ -28,20 +28,24 @@ namespace mozilla {
 class JSGCThingParticipant: public nsCycleCollectionParticipant
 {
 public:
-  NS_IMETHOD_(void) Root(void* aPtr)
+  NS_IMETHOD_(void) Root(void*)
   {
+    MOZ_ASSERT(false, "Don't call Root on GC things");
   }
 
-  NS_IMETHOD_(void) Unlink(void* aPtr)
+  NS_IMETHOD_(void) Unlink(void*)
   {
+    MOZ_ASSERT(false, "Don't call Unlink on GC things, as they may be dead");
   }
 
-  NS_IMETHOD_(void) Unroot(void* aPtr)
+  NS_IMETHOD_(void) Unroot(void*)
   {
+    MOZ_ASSERT(false, "Don't call Unroot on GC things, as they may be dead");
   }
 
   NS_IMETHOD_(void) DeleteCycleCollectable(void* aPtr)
   {
+    MOZ_ASSERT(false, "Can't directly delete a cycle collectable GC thing");
   }
 
   NS_IMETHOD Traverse(void* aPtr, nsCycleCollectionTraversalCallback& aCb);
@@ -54,20 +58,24 @@ public:
   {
   }
 
-  NS_IMETHOD_(void) Root(void* aPtr)
+  NS_IMETHOD_(void) Root(void*)
   {
+    MOZ_ASSERT(false, "Don't call Root on GC things");
   }
 
-  NS_IMETHOD_(void) Unlink(void* aPtr)
+  NS_IMETHOD_(void) Unlink(void*)
   {
+    MOZ_ASSERT(false, "Don't call Unlink on GC things, as they may be dead");
   }
 
-  NS_IMETHOD_(void) Unroot(void* aPtr)
+  NS_IMETHOD_(void) Unroot(void*)
   {
+    MOZ_ASSERT(false, "Don't call Unroot on GC things, as they may be dead");
   }
 
-  NS_IMETHOD_(void) DeleteCycleCollectable(void* aPtr)
+  NS_IMETHOD_(void) DeleteCycleCollectable(void*)
   {
+    MOZ_ASSERT(false, "Can't directly delete a cycle collectable GC thing");
   }
 
   NS_IMETHOD Traverse(void* aPtr, nsCycleCollectionTraversalCallback& aCb);

xpcom/base/nsCycleCollector.cpp

@@ -3252,21 +3252,20 @@ nsCycleCollector::CollectWhite()
   while (!etor.IsDone()) {
     PtrInfo* pinfo = etor.GetNext();
     if (pinfo->mColor == white && pinfo->mParticipant) {
-      whiteNodes.AppendElement(pinfo);
-      pinfo->mParticipant->Root(pinfo->mPointer);
       if (pinfo->IsGrayJS()) {
         ++numWhiteGCed;
         if (MOZ_UNLIKELY(pinfo->mParticipant == zoneParticipant)) {
           ++numWhiteJSZones;
         }
+      } else {
+        whiteNodes.AppendElement(pinfo);
+        pinfo->mParticipant->Root(pinfo->mPointer);
       }
     }
   }
 
   uint32_t numWhiteNodes = whiteNodes.Length();
-  MOZ_ASSERT(numWhiteGCed <= numWhiteNodes,
-             "More freed GCed nodes than total freed nodes.");
-  mResults.mFreedRefCounted += numWhiteNodes - numWhiteGCed;
+  mResults.mFreedRefCounted += numWhiteNodes;
   mResults.mFreedGCed += numWhiteGCed;
   mResults.mFreedJSZones += numWhiteJSZones;
 
@@ -3277,6 +3276,9 @@ nsCycleCollector::CollectWhite()
     timeLog.Checkpoint("CollectWhite::BeforeUnlinkCB");
   }
 
+  // Unlink() can trigger a GC, so do not touch any JS or anything
+  // else not in whiteNodes after here.
+
   for (uint32_t i = 0; i < numWhiteNodes; ++i) {
     PtrInfo* pinfo = whiteNodes.ElementAt(i);
     MOZ_ASSERT(pinfo->mParticipant,
@@ -3303,7 +3305,7 @@ nsCycleCollector::CollectWhite()
 
   mIncrementalPhase = CleanupPhase;
 
-  return numWhiteNodes > 0;
+  return numWhiteNodes > 0 || numWhiteGCed > 0 || numWhiteJSZones > 0;
 }