Change nsXPConnect::CheckForDebugMode to trigger one multi-compartment GC instead of individual GCs for all compartments, to fix GC pauses when switching tabs with Firebug. Bug 754201, r=jorendorff, r=sfink.

js/src/jscompartment.cpp

@@ -599,7 +599,7 @@ JSCompartment::hasScriptsOnStack()
 }
 
 bool
-JSCompartment::setDebugModeFromC(JSContext *cx, bool b)
+JSCompartment::setDebugModeFromC(JSContext *cx, bool b, AutoDebugModeGC &dmgc)
 {
     bool enabledBefore = debugMode();
     bool enabledAfter = (debugModeBits & ~unsigned(DebugFromC)) || b;
@@ -626,12 +626,12 @@ JSCompartment::setDebugModeFromC(JSContext *cx, bool b)
     debugModeBits = (debugModeBits & ~unsigned(DebugFromC)) | (b ? DebugFromC : 0);
     JS_ASSERT(debugMode() == enabledAfter);
     if (enabledBefore != enabledAfter)
-        updateForDebugMode(cx->runtime->defaultFreeOp());
+        updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc);
     return true;
 }
 
 void
-JSCompartment::updateForDebugMode(FreeOp *fop)
+JSCompartment::updateForDebugMode(FreeOp *fop, AutoDebugModeGC &dmgc)
 {
     for (ContextIter acx(rt); !acx.done(); acx.next()) {
         if (acx->compartment == this) 
@@ -656,10 +656,13 @@ JSCompartment::updateForDebugMode(FreeOp *fop)
     // compartment. Because !hasScriptsOnStack(), it suffices to do a garbage
     // collection cycle or to finish the ongoing GC cycle. The necessary
     // cleanup happens in JSCompartment::sweep.
-    if (!rt->gcRunning) {
-        PrepareCompartmentForGC(this);
-        GC(rt, GC_NORMAL, gcreason::DEBUG_MODE_GC);
-    }
+    //
+    // dmgc makes sure we can't forget to GC, but it is also important not
+    // to run any scripts in this compartment until the dmgc is destroyed.
+    // That is the caller's responsibility.
+    //
+    if (!rt->gcRunning)
+        dmgc.scheduleGC(this);
 #endif
 }
 
@@ -672,8 +675,10 @@ JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global)
         return false;
     }
     debugModeBits |= DebugFromJS;
-    if (!wasEnabled)
-        updateForDebugMode(cx->runtime->defaultFreeOp());
+    if (!wasEnabled) {
+        AutoDebugModeGC dmgc(cx->runtime);
+        updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc);
+    }
     return true;
 }
 
@@ -691,8 +696,10 @@ JSCompartment::removeDebuggee(FreeOp *fop,
 
     if (debuggees.empty()) {
         debugModeBits &= ~DebugFromJS;
-        if (wasEnabled && !debugMode())
-            updateForDebugMode(fop);
+        if (wasEnabled && !debugMode()) {
+            AutoDebugModeGC dmgc(rt);
+            updateForDebugMode(fop, dmgc);
+        }
     }
 }
 

js/src/jscompartment.h

@@ -112,6 +112,10 @@ namespace JS {
 struct TypeInferenceSizes;
 }
 
+namespace js {
+class AutoDebugModeGC;
+}
+
 struct JSCompartment
 {
     JSRuntime                    *rt;
@@ -356,14 +360,14 @@ struct JSCompartment
 
   private:
     /* This is called only when debugMode() has just toggled. */
-    void updateForDebugMode(js::FreeOp *fop);
+    void updateForDebugMode(js::FreeOp *fop, js::AutoDebugModeGC &dmgc);
 
   public:
     js::GlobalObjectSet &getDebuggees() { return debuggees; }
     bool addDebuggee(JSContext *cx, js::GlobalObject *global);
     void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global,
                         js::GlobalObjectSet::Enum *debuggeesEnum = NULL);
-    bool setDebugModeFromC(JSContext *cx, bool b);
+    bool setDebugModeFromC(JSContext *cx, bool b, js::AutoDebugModeGC &dmgc);
 
     void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JSObject *handler);
     void clearTraps(js::FreeOp *fop);
@@ -381,6 +385,29 @@ struct JSCompartment
     js::DebugScriptMap *debugScriptMap;
 };
 
+// For use when changing the debug mode flag on one or more compartments.
+// Do not run scripts in any compartment that is scheduled for GC using this
+// object. See comment in updateForDebugMode.
+//
+class js::AutoDebugModeGC
+{
+    JSRuntime *rt;
+    bool needGC;
+  public:
+    explicit AutoDebugModeGC(JSRuntime *rt) : rt(rt), needGC(false) {}
+
+    ~AutoDebugModeGC() {
+        if (needGC)
+            GC(rt, GC_NORMAL, gcreason::DEBUG_MODE_GC);
+    }
+
+    void scheduleGC(JSCompartment *compartment) {
+        JS_ASSERT(!rt->gcRunning);
+        PrepareCompartmentForGC(compartment);
+        needGC = true;
+    }
+};
+
 #define JS_PROPERTY_TREE(cx)    ((cx)->compartment->propertyTree)
 
 inline void

js/src/jsdbgapi.cpp

@@ -163,10 +163,26 @@ ScriptDebugEpilogue(JSContext *cx, StackFrame *fp, bool okArg)
 
 } /* namespace js */
 
+JS_FRIEND_API(JSBool)
+JS_SetDebugModeForAllCompartments(JSContext *cx, JSBool debug)
+{
+    AutoDebugModeGC dmgc(cx->runtime);
+
+    for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) {
+        // Ignore special compartments (atoms, JSD compartments)
+        if (c->principals) {
+            if (!c->setDebugModeFromC(cx, !!debug, dmgc))
+                return false;
+        }
+    }
+    return true;
+}
+
 JS_FRIEND_API(JSBool)
 JS_SetDebugModeForCompartment(JSContext *cx, JSCompartment *comp, JSBool debug)
 {
-    return comp->setDebugModeFromC(cx, !!debug);
+    AutoDebugModeGC dmgc(cx->runtime);
+    return comp->setDebugModeFromC(cx, !!debug, dmgc);
 }
 
 static JSBool

js/src/jsdbgapi.h

@@ -120,6 +120,13 @@ JS_SetRuntimeDebugMode(JSRuntime *rt, JSBool debug);
 extern JS_PUBLIC_API(JSBool)
 JS_GetDebugMode(JSContext *cx);
 
+/*
+ * Turn on/off debugging mode for all compartments. This returns false if any code
+ * from any of the runtime's compartments is running or on the stack.
+ */
+JS_FRIEND_API(JSBool)
+JS_SetDebugModeForAllCompartments(JSContext *cx, JSBool debug);
+
 /*
  * Turn on/off debugging mode for a single compartment. This should only be
  * used when no code from this compartment is running or on the stack in any

js/xpconnect/src/nsXPConnect.cpp

@@ -2405,17 +2405,8 @@ nsXPConnect::CheckForDebugMode(JSRuntime *rt)
         } adc(cx);
         JSAutoRequest ar(cx);
 
-        const js::CompartmentVector &vector = js::GetRuntimeCompartments(rt);
-        for (JSCompartment * const *p = vector.begin(); p != vector.end(); ++p) {
-            JSCompartment *comp = *p;
-            if (!JS_GetCompartmentPrincipals(comp)) {
-                /* Ignore special compartments (atoms, JSD compartments) */
-                continue;
-            }
-
-            if (!JS_SetDebugModeForCompartment(cx, comp, gDesiredDebugMode))
-                goto fail;
-        }
+        if (!JS_SetDebugModeForAllCompartments(cx, gDesiredDebugMode))
+            goto fail;
     }
 
     if (gDesiredDebugMode) {