Backed out 3 changesets (bug 1301301) for crashing
authorPhil Ringnalda <philringnalda@gmail.com>
Tue, 27 Sep 2016 19:49:17 -0700
changeset 315513 d217f8dd78d65ae5c90d98fb7191879d12d9ff14
parent 315512 ccc856b06a2ddbfffd0119522c0cbd95ff88c4fe
child 315514 43a97503d564787a9fb18ff8d9fd0e403ea22169
push id30750
push usercbook@mozilla.com
push dateWed, 28 Sep 2016 13:57:20 +0000
treeherdermozilla-central@b1d60f2f68c7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1301301
milestone52.0a1
backs outda70c5abd73a184954fde0bef6f01f9c0208e01d
6186eae0c2b78f1bbfc9e450dacac6db4bf8e7e0
45fb9c1ce63a643916ccf0b4bcd0364970392cb8
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
Backed out 3 changesets (bug 1301301) for crashing Backed out changeset da70c5abd73a (bug 1301301) Backed out changeset 6186eae0c2b7 (bug 1301301) Backed out changeset 45fb9c1ce63a (bug 1301301)
dom/workers/RuntimeService.cpp
js/xpconnect/src/XPCJSContext.cpp
xpcom/base/CycleCollectedJSContext.cpp
xpcom/base/CycleCollectedJSContext.h
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -1094,16 +1094,26 @@ public:
     return NS_OK;
   }
 
   virtual void
   PrepareForForgetSkippable() override
   {
   }
 
+  virtual void
+  BeginCycleCollectionCallback() override
+  {
+  }
+
+  virtual void
+  EndCycleCollectionCallback(CycleCollectorResults &aResults) override
+  {
+  }
+
   void
   DispatchDeferredDeletion(bool aContinuation, bool aPurge) override
   {
     MOZ_ASSERT(!aContinuation);
 
     // Do it immediately, no need for asynchronous behavior here.
     nsCycleCollector_doDeferredDeletion();
   }
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -660,29 +660,27 @@ XPCJSContext::PrepareForForgetSkippable(
     if (obs) {
         obs->NotifyObservers(nullptr, "cycle-collector-forget-skippable", nullptr);
     }
 }
 
 void
 XPCJSContext::BeginCycleCollectionCallback()
 {
-    CycleCollectedJSContext::BeginCycleCollectionCallback();
     nsJSContext::BeginCycleCollectionCallback();
 
     nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
     if (obs) {
         obs->NotifyObservers(nullptr, "cycle-collector-begin", nullptr);
     }
 }
 
 void
 XPCJSContext::EndCycleCollectionCallback(CycleCollectorResults& aResults)
 {
-    CycleCollectedJSContext::EndCycleCollectionCallback(aResults);
     nsJSContext::EndCycleCollectionCallback(aResults);
 
     nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
     if (obs) {
         obs->NotifyObservers(nullptr, "cycle-collector-end", nullptr);
     }
 }
 
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -122,39 +122,21 @@ public:
 
   void ReleaseNow(bool aLimited);
 
   NS_DECL_NSIRUNNABLE
 };
 
 } // namespace mozilla
 
-struct CCJSTracer : public JS::CallbackTracer
+struct NoteWeakMapChildrenTracer : public JS::CallbackTracer
 {
-protected:
-  CCJSTracer(CycleCollectedJSContext* aCx,
-             WeakMapTraceKind aWeakTraceKind = TraceWeakMapValues)
-    : JS::CallbackTracer(aCx->Context(), aWeakTraceKind)
-    , mCx(aCx)
-  {}
-
-public:
-  void TraceChildren(JS::GCCellPtr aThing)
-  {
-    mCx->TraceJSChildren(this, aThing);
-  }
-
-  CycleCollectedJSContext* mCx;
-};
-
-struct NoteWeakMapChildrenTracer : public CCJSTracer
-{
-  NoteWeakMapChildrenTracer(CycleCollectedJSContext* aCx,
+  NoteWeakMapChildrenTracer(JSContext* aCx,
                             nsCycleCollectionNoteRootCallback& aCb)
-    : CCJSTracer(aCx), mCb(aCb), mTracedAny(false), mMap(nullptr),
+    : JS::CallbackTracer(aCx), mCb(aCb), mTracedAny(false), mMap(nullptr),
       mKey(nullptr), mKeyDelegate(nullptr)
   {
   }
   void onChild(const JS::GCCellPtr& aThing) override;
   nsCycleCollectionNoteRootCallback& mCb;
   bool mTracedAny;
   JSObject* mMap;
   JS::GCCellPtr mKey;
@@ -171,25 +153,24 @@ NoteWeakMapChildrenTracer::onChild(const
   if (!JS::GCThingIsMarkedGray(aThing) && !mCb.WantAllTraces()) {
     return;
   }
 
   if (AddToCCKind(aThing.kind())) {
     mCb.NoteWeakMapping(mMap, mKey, mKeyDelegate, aThing);
     mTracedAny = true;
   } else {
-    TraceChildren(aThing);
+    JS::TraceChildren(this, aThing);
   }
 }
 
 struct NoteWeakMapsTracer : public js::WeakMapTracer
 {
-  NoteWeakMapsTracer(CycleCollectedJSContext* aCx,
-                     nsCycleCollectionNoteRootCallback& aCccb)
-    : js::WeakMapTracer(aCx->Context()), mCb(aCccb), mChildTracer(aCx, aCccb)
+  NoteWeakMapsTracer(JSContext* aCx, nsCycleCollectionNoteRootCallback& aCccb)
+    : js::WeakMapTracer(aCx), mCb(aCccb), mChildTracer(aCx, aCccb)
   {
   }
   void trace(JSObject* aMap, JS::GCCellPtr aKey, JS::GCCellPtr aValue) override;
   nsCycleCollectionNoteRootCallback& mCb;
   NoteWeakMapChildrenTracer mChildTracer;
 };
 
 void
@@ -227,17 +208,17 @@ NoteWeakMapsTracer::trace(JSObject* aMap
     mCb.NoteWeakMapping(aMap, aKey, kdelegate, aValue);
   } else {
     mChildTracer.mTracedAny = false;
     mChildTracer.mMap = aMap;
     mChildTracer.mKey = aKey;
     mChildTracer.mKeyDelegate = kdelegate;
 
     if (!aValue.is<JSString>()) {
-      mChildTracer.TraceChildren(aValue);
+      JS::TraceChildren(&mChildTracer, aValue);
     }
 
     // The delegate could hold alive the key, so report something to the CC
     // if we haven't already.
     if (!mChildTracer.mTracedAny &&
         aKey && JS::GCThingIsMarkedGray(aKey) && kdelegate) {
       mCb.NoteWeakMapping(aMap, aKey, kdelegate, nullptr);
     }
@@ -338,23 +319,22 @@ JSZoneParticipant::Traverse(void* aPtr, 
 
   MOZ_ASSERT(!aCb.WantAllTraces());
   JS::Zone* zone = static_cast<JS::Zone*>(aPtr);
 
   runtime->TraverseZone(zone, aCb);
   return NS_OK;
 }
 
-struct TraversalTracer : public CCJSTracer
+struct TraversalTracer : public JS::CallbackTracer
 {
-  TraversalTracer(CycleCollectedJSContext* aCx,
-                  nsCycleCollectionTraversalCallback& aCb)
-    : CCJSTracer(aCx, DoNotTraceWeakMaps)
-    , mCb(aCb)
-  {}
+  TraversalTracer(JSContext* aCx, nsCycleCollectionTraversalCallback& aCb)
+    : JS::CallbackTracer(aCx, DoNotTraceWeakMaps), mCb(aCb)
+  {
+  }
   void onChild(const JS::GCCellPtr& aThing) override;
   nsCycleCollectionTraversalCallback& mCb;
 };
 
 void
 TraversalTracer::onChild(const JS::GCCellPtr& aThing)
 {
   // Don't traverse non-gray objects, unless we want all traces.
@@ -381,17 +361,17 @@ TraversalTracer::onChild(const JS::GCCel
     // the parent pointers on the shape.
     JS_TraceShapeCycleCollectorChildren(this, aThing);
   } else if (aThing.is<js::ObjectGroup>()) {
     // The maximum depth of traversal when tracing an ObjectGroup is unbounded,
     // due to information attached to the groups which can lead other groups to
     // be traced.
     JS_TraceObjectGroupCycleCollectorChildren(this, aThing);
   } else if (!aThing.is<JSString>()) {
-    TraceChildren(aThing);
+    JS::TraceChildren(this, aThing);
   }
 }
 
 static void
 NoteJSChildGrayWrapperShim(void* aData, JS::GCCellPtr aThing)
 {
   TraversalTracer* trc = static_cast<TraversalTracer*>(aData);
   trc->onChild(aThing);
@@ -457,20 +437,16 @@ CycleCollectedJSContext::CycleCollectedJ
   , mJSContext(nullptr)
   , mPrevGCSliceCallback(nullptr)
   , mPrevGCNurseryCollectionCallback(nullptr)
   , mJSHolders(256)
   , mDoingStableStates(false)
   , mDisableMicroTaskCheckpoint(false)
   , mOutOfMemoryState(OOMState::OK)
   , mLargeAllocationFailureState(OOMState::OK)
-#ifdef DEBUG
-  , mNumTraversedGCThings(0)
-  , mNumTraceChildren(0)
-#endif // DEBUG
 {
   nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
   mOwningThread = thread.forget().downcast<nsThread>().take();
   MOZ_RELEASE_ASSERT(mOwningThread);
 }
 
 CycleCollectedJSContext::~CycleCollectedJSContext()
 {
@@ -649,21 +625,21 @@ CycleCollectedJSContext::DescribeGCThing
   }
 
   // Disable printing global for objects while we figure out ObjShrink fallout.
   aCb.DescribeGCedNode(aIsMarked, name, compartmentAddress);
 }
 
 void
 CycleCollectedJSContext::NoteGCThingJSChildren(JS::GCCellPtr aThing,
-                                               nsCycleCollectionTraversalCallback& aCb)
+                                               nsCycleCollectionTraversalCallback& aCb) const
 {
   MOZ_ASSERT(mJSContext);
-  TraversalTracer trc(this, aCb);
-  trc.TraceChildren(aThing);
+  TraversalTracer trc(mJSContext, aCb);
+  JS::TraceChildren(&trc, aThing);
 }
 
 void
 CycleCollectedJSContext::NoteGCThingXPCOMChildren(const js::Class* aClasp,
                                                   JSObject* aObj,
                                                   nsCycleCollectionTraversalCallback& aCb) const
 {
   MOZ_ASSERT(aClasp);
@@ -710,20 +686,16 @@ CycleCollectedJSContext::TraverseGCThing
   // If this object is alive, then all of its children are alive. For JS objects,
   // the black-gray invariant ensures the children are also marked black. For C++
   // objects, the ref count from this object will keep them alive. Thus we don't
   // need to trace our children, unless we are debugging using WantAllTraces.
   if (!isMarkedGray && !aCb.WantAllTraces()) {
     return;
   }
 
-#ifdef DEBUG
-  ++mNumTraversedGCThings;
-#endif
-
   if (aTs == TRAVERSE_FULL) {
     NoteGCThingJSChildren(aThing, aCb);
   }
 
   if (aThing.is<JSObject>()) {
     JSObject* obj = &aThing.as<JSObject>();
     NoteGCThingXPCOMChildren(js::GetObjectClass(obj), obj, aCb);
   }
@@ -756,17 +728,17 @@ CycleCollectedJSContext::TraverseZone(JS
   /*
    * Every JS child of everything in the zone is either in the zone
    * or is a cross-compartment wrapper. In the former case, we don't need to
    * represent these edges in the CC graph because JS objects are not ref counted.
    * In the latter case, the JS engine keeps a map of these wrappers, which we
    * iterate over. Edges between compartments in the same zone will add
    * unnecessary loop edges to the graph (bug 842137).
    */
-  TraversalTracer trc(this, aCb);
+  TraversalTracer trc(mJSContext, aCb);
   js::VisitGrayWrapperTargets(aZone, NoteJSChildGrayWrapperShim, &trc);
 
   /*
    * To find C++ children of things in the zone, we scan every JS Object in
    * the zone. Only JS Objects can have C++ children.
    */
   TraverseObjectShimClosure closure = { aCb, this };
   js::IterateGrayObjects(aZone, TraverseObjectShim, &closure);
@@ -1235,17 +1207,17 @@ CycleCollectedJSContext::ZoneParticipant
 
 nsresult
 CycleCollectedJSContext::TraverseRoots(nsCycleCollectionNoteRootCallback& aCb)
 {
   MOZ_ASSERT(mJSContext);
 
   TraverseNativeRoots(aCb);
 
-  NoteWeakMapsTracer trc(this, aCb);
+  NoteWeakMapsTracer trc(mJSContext, aCb);
   js::TraceWeakMaps(&trc);
 
   return NS_OK;
 }
 
 bool
 CycleCollectedJSContext::UsefulToMergeZones() const
 {
@@ -1354,50 +1326,16 @@ CycleCollectedJSContext::DeferredFinaliz
 
 void
 CycleCollectedJSContext::DumpJSHeap(FILE* aFile)
 {
   js::DumpHeap(Context(), aFile, js::CollectNurseryBeforeDump);
 }
 
 void
-CycleCollectedJSContext::BeginCycleCollectionCallback()
-{
-#ifdef DEBUG
-  mNumTraversedGCThings = 0;
-  mNumTraceChildren = 0;
-#endif
-}
-
-void
-CycleCollectedJSContext::EndCycleCollectionCallback(CycleCollectorResults& aResults)
-{
-#ifdef DEBUG
-
-  // GC things that the cycle collector calls Traverse on (ie things
-  // in the CC graph) will also get JS::TraceChildren() called on
-  // them. If a child of a GC thing in the CC graph does not have an
-  // AddToCCKind(), then the CC calls JS::TraceChildren() on it
-  // without adding it to the graph. If there are many such objects
-  // that are referred to by many objects in the CC graph, then the CC
-  // can spend a lot of time in JS::TraceChildren(). If you add a new
-  // TraceKind and are hitting this assertion, adding it to
-  // AddToCCKind might help. (The check is not done for small CC
-  // graphs to avoid noise.)
-  if (mNumTraceChildren > 1000 && mNumTraversedGCThings > 0) {
-    double traceRatio = ((double)mNumTraceChildren) / ((double)mNumTraversedGCThings);
-    MOZ_ASSERT(traceRatio < 2.2, "Excessive calls to JS::TraceChildren by the cycle collector");
-  }
-
-  mNumTraversedGCThings = 0;
-  mNumTraceChildren = 0;
-#endif
-}
-
-void
 CycleCollectedJSContext::ProcessStableStateQueue()
 {
   MOZ_ASSERT(mJSContext);
   MOZ_RELEASE_ASSERT(!mDoingStableStates);
   mDoingStableStates = true;
 
   for (uint32_t i = 0; i < mStableStateEvents.Length(); ++i) {
     nsCOMPtr<nsIRunnable> event = mStableStateEvents[i].forget();
@@ -1727,25 +1665,16 @@ CycleCollectedJSContext::OnLargeAllocati
   MOZ_ASSERT(mJSContext);
 
   AnnotateAndSetOutOfMemory(&mLargeAllocationFailureState, OOMState::Reporting);
   CustomLargeAllocationFailureCallback();
   AnnotateAndSetOutOfMemory(&mLargeAllocationFailureState, OOMState::Reported);
 }
 
 void
-CycleCollectedJSContext::TraceJSChildren(JSTracer* aTrc, JS::GCCellPtr aThing)
-{
-#ifdef DEBUG
-  ++mNumTraceChildren;
-#endif // DEBUG
-  JS::TraceChildren(aTrc, aThing);
-}
-
-void
 CycleCollectedJSContext::PrepareWaitingZonesForGC()
 {
   if (mZonesWaitingForGC.Count() == 0) {
     JS::PrepareForFullGC(Context());
   } else {
     for (auto iter = mZonesWaitingForGC.Iter(); !iter.Done(); iter.Next()) {
       JS::PrepareZoneForGC(iter.Get()->GetKey());
     }
--- a/xpcom/base/CycleCollectedJSContext.h
+++ b/xpcom/base/CycleCollectedJSContext.h
@@ -171,17 +171,17 @@ private:
   DescribeCustomObjects(JSObject* aObject, const js::Class* aClasp,
                         char (&aName)[72]) const
   {
     return false; // We did nothing.
   }
 
   void
   NoteGCThingJSChildren(JS::GCCellPtr aThing,
-                        nsCycleCollectionTraversalCallback& aCb);
+                        nsCycleCollectionTraversalCallback& aCb) const;
 
   void
   NoteGCThingXPCOMChildren(const js::Class* aClasp, JSObject* aObj,
                            nsCycleCollectionTraversalCallback& aCb) const;
 
   virtual bool
   NoteCustomGCThingXPCOMChildren(const js::Class* aClasp, JSObject* aObj,
                                  nsCycleCollectionTraversalCallback& aCb) const
@@ -316,18 +316,18 @@ public:
   void DeferredFinalize(DeferredFinalizeAppendFunction aAppendFunc,
                         DeferredFinalizeFunction aFunc,
                         void* aThing);
   void DeferredFinalize(nsISupports* aSupports);
 
   void DumpJSHeap(FILE* aFile);
 
   virtual void PrepareForForgetSkippable() = 0;
-  virtual void BeginCycleCollectionCallback();
-  virtual void EndCycleCollectionCallback(CycleCollectorResults& aResults);
+  virtual void BeginCycleCollectionCallback() = 0;
+  virtual void EndCycleCollectionCallback(CycleCollectorResults& aResults) = 0;
   virtual void DispatchDeferredDeletion(bool aContinuation, bool aPurge = false) = 0;
 
   JSContext* Context() const
   {
     MOZ_ASSERT(mJSContext);
     return mJSContext;
   }
 
@@ -390,18 +390,16 @@ public:
   static CycleCollectedJSContext* Get();
 
   // Add aZone to the set of zones waiting for a GC.
   void AddZoneWaitingForGC(JS::Zone* aZone)
   {
     mZonesWaitingForGC.PutEntry(aZone);
   }
 
-  void TraceJSChildren(JSTracer* aTrc, JS::GCCellPtr aThing);
-
   // Prepare any zones for GC that have been passed to AddZoneWaitingForGC()
   // since the last GC or since the last call to PrepareWaitingZonesForGC(),
   // whichever was most recent. If there were no such zones, prepare for a
   // full GC.
   void PrepareWaitingZonesForGC();
 
   // Queue an async microtask to the current main or worker thread.
   virtual void DispatchToMicroTask(already_AddRefed<nsIRunnable> aRunnable);
@@ -473,21 +471,16 @@ private:
     mPreservedNurseryObjects;
 
   nsTHashtable<nsPtrHashKey<JS::Zone>> mZonesWaitingForGC;
 
   struct EnvironmentPreparer : public js::ScriptEnvironmentPreparer {
     void invoke(JS::HandleObject scope, Closure& closure) override;
   };
   EnvironmentPreparer mEnvironmentPreparer;
-
-#ifdef DEBUG
-  uint32_t mNumTraversedGCThings;
-  uint32_t mNumTraceChildren;
-#endif // DEBUG
 };
 
 void TraceScriptHolder(nsISupports* aHolder, JSTracer* aTracer);
 
 // Returns true if the JS::TraceKind is one the cycle collector cares about.
 inline bool AddToCCKind(JS::TraceKind aKind)
 {
   return aKind == JS::TraceKind::Object || aKind == JS::TraceKind::Script || aKind == JS::TraceKind::Scope;