Bug 1364547 - Call the slice end callback for every slice, r=jonco

Currently, the final slice of an incremental GC only gets a GC_CYCLE_END callback, not a GC_SLICE_END callback. So if you are doing anything that expects to see all of the slices, you will be missing one.

Simplify the setup so that every GC is bracketed with CYCLE_BEGIN/END, and every slice is bracketed with SLICE_BEGIN/END, treating a nonincremental as a GC with a single slice (which it is for everything else.)

--HG--
extra : rebase_source : 8e21300819d517b3e35de14930f53b3ab737a44e

js/public/GCAPI.h

@@ -319,13 +319,9 @@ class GarbageCollectionEvent
 
 enum GCProgress {
     /*
-     * During non-incremental GC, the GC is bracketed by JSGC_CYCLE_BEGIN/END
-     * callbacks. During an incremental GC, the sequence of callbacks is as
-     * follows:
-     *   JSGC_CYCLE_BEGIN, JSGC_SLICE_END  (first slice)
-     *   JSGC_SLICE_BEGIN, JSGC_SLICE_END  (second slice)
-     *   ...
-     *   JSGC_SLICE_BEGIN, JSGC_CYCLE_END  (last slice)
+     * During GC, the GC is bracketed by GC_CYCLE_BEGIN/END callbacks. Each
+     * slice between those (whether an incremental or the sole non-incremental
+     * slice) is bracketed by GC_SLICE_BEGIN/GC_SLICE_END.
      */
 
     GC_CYCLE_BEGIN,

js/src/gc/Statistics.cpp

@@ -959,10 +959,13 @@ Statistics::beginSlice(const ZoneGCStats& zoneStats, JSGCInvocationKind gckind,
 
     // Slice callbacks should only fire for the outermost level.
     bool wasFullGC = zoneStats.isCollectingAllZones();
-    if (sliceCallback)
-        (*sliceCallback)(TlsContext.get(),
-                         first ? JS::GC_CYCLE_BEGIN : JS::GC_SLICE_BEGIN,
-                         JS::GCDescription(!wasFullGC, gckind, reason));
+    if (sliceCallback) {
+        JSContext* cx = TlsContext.get();
+        JS::GCDescription desc(!wasFullGC, gckind, reason);
+        if (first)
+            (*sliceCallback)(cx, JS::GC_CYCLE_BEGIN, desc);
+        (*sliceCallback)(cx, JS::GC_SLICE_BEGIN, desc);
+    }
 }
 
 void
@@ -1006,10 +1009,13 @@ Statistics::endSlice()
     // Slice callbacks should only fire for the outermost level.
     if (!aborted) {
         bool wasFullGC = zoneStats.isCollectingAllZones();
-        if (sliceCallback)
-            (*sliceCallback)(TlsContext.get(),
-                             last ? JS::GC_CYCLE_END : JS::GC_SLICE_END,
-                             JS::GCDescription(!wasFullGC, gckind, slices_.back().reason));
+        if (sliceCallback) {
+            JSContext* cx = TlsContext.get();
+            JS::GCDescription desc(!wasFullGC, gckind, slices_.back().reason);
+            (*sliceCallback)(cx, JS::GC_SLICE_END, desc);
+            if (last)
+                (*sliceCallback)(cx, JS::GC_CYCLE_END, desc);
+        }
     }
 
     // Do this after the slice callback since it uses these values.

js/src/jsapi-tests/testGCHooks.cpp

@@ -14,12 +14,15 @@ static unsigned gSliceCallbackCount = 0;
 static void
 NonIncrementalGCSliceCallback(JSContext* cx, JS::GCProgress progress, const JS::GCDescription& desc)
 {
-    ++gSliceCallbackCount;
-    MOZ_RELEASE_ASSERT(progress == JS::GC_CYCLE_BEGIN || progress == JS::GC_CYCLE_END);
+    using namespace JS;
+    static GCProgress expect[] =
+        { GC_CYCLE_BEGIN, GC_SLICE_BEGIN, GC_SLICE_END, GC_CYCLE_END };
+
+    MOZ_RELEASE_ASSERT(progress == expect[gSliceCallbackCount++]);
     MOZ_RELEASE_ASSERT(desc.isZone_ == false);
     MOZ_RELEASE_ASSERT(desc.invocationKind_ == GC_NORMAL);
     MOZ_RELEASE_ASSERT(desc.reason_ == JS::gcreason::API);
-    if (progress == JS::GC_CYCLE_END) {
+    if (progress == GC_CYCLE_END) {
         mozilla::UniquePtr<char16_t> summary(desc.formatSummaryMessage(cx));
         mozilla::UniquePtr<char16_t> message(desc.formatSliceMessage(cx));
         mozilla::UniquePtr<char16_t> json(desc.formatJSON(cx, 0));
@@ -31,7 +34,7 @@ BEGIN_TEST(testGCSliceCallback)
     JS::SetGCSliceCallback(cx, NonIncrementalGCSliceCallback);
     JS_GC(cx);
     JS::SetGCSliceCallback(cx, nullptr);
-    CHECK(gSliceCallbackCount == 2);
+    CHECK(gSliceCallbackCount == 4);
     return true;
 }
 END_TEST(testGCSliceCallback)

js/xpconnect/src/XPCJSRuntime.cpp

@@ -756,8 +756,7 @@ XPCJSRuntime::GCSliceCallback(JSContext* cx,
         return;
 
 #ifdef MOZ_CRASHREPORTER
-    CrashReporter::SetGarbageCollecting(progress == JS::GC_CYCLE_BEGIN ||
-                                        progress == JS::GC_SLICE_BEGIN);
+    CrashReporter::SetGarbageCollecting(progress == JS::GC_CYCLE_BEGIN);
 #endif
 
     if (self->mPrevGCSliceCallback)