Implement a new addAllGlobalsAsDebuggees method for faster chrome debugging (bug 821701); r=jimb

Also GC only once when going through all compartments in both addAllGlobalsAsDebuggees and removeAllDebuggees, instead of once for every debuggee added or removed.
2013-01-04 21:34:43 +02:00 · 2013-01-04 21:34:43 +02:00 · f07bc4f132
--- a/browser/devtools/debugger/test/Makefile.in
+++ b/browser/devtools/debugger/test/Makefile.in
@ -87,7 +87,7 @@ MOCHITEST_BROWSER_TESTS = \
 	browser_dbg_breakpoint-new-script.js \
 	browser_dbg_bug737803_editor_actual_location.js \
 	browser_dbg_progress-listener-bug.js \
-	$(filter disabled-for-intermittent-crashes--bug-821701, browser_dbg_chrome-debugging.js) \
+	browser_dbg_chrome-debugging.js \
 	$(filter disabled-for-intermittent-failures--bug-753225, browser_dbg_createRemote.js) \
 	head.js \
 	$(NULL)
--- a/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js
+++ b/browser/devtools/debugger/test/browser_dbg_chrome-debugging.js
@ -16,9 +16,6 @@ const DEBUGGER_TAB_URL = EXAMPLE_URL + "browser_dbg_debuggerstatement.html";

 function test()
 {
-  // Make sure there is enough time for findAllGlobals.
-  requestLongerTimeout(3);
-
  let transport = DebuggerServer.connectPipe();
  gClient = new DebuggerClient(transport);
  gClient.connect(function(aType, aTraits) {
--- a/js/src/jit-test/tests/debug/Debugger-debuggees-20.js
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-20.js
@ -0,0 +1,28 @@
+// addAllGlobalsAsDebuggees adds all the globals as debuggees.
+
+var g1 = newGlobal();           // Created before the Debugger; debuggee.
+var g2 = newGlobal();           // Created before the Debugger; not debuggee.
+
+var dbg = new Debugger;
+
+var g3 = newGlobal();           // Created after the Debugger; debuggee.
+var g4 = newGlobal();           // Created after the Debugger; not debuggee.
+
+var g1w = dbg.addDebuggee(g1);
+assertEq(dbg.addAllGlobalsAsDebuggees(), undefined);
+
+// Get Debugger.Objects viewing the globals from their own compartments;
+// this is the sort that findAllGlobals and addDebuggee return.
+var g1w = g1w.makeDebuggeeValue(g1).unwrap();
+var g2w = g1w.makeDebuggeeValue(g2).unwrap();
+var g3w = g1w.makeDebuggeeValue(g3).unwrap();
+var g4w = g1w.makeDebuggeeValue(g4).unwrap();
+var thisw = g1w.makeDebuggeeValue(this).unwrap();
+
+// Check that they're all there.
+assertEq(dbg.hasDebuggee(g1w), true);
+assertEq(dbg.hasDebuggee(g2w), true);
+assertEq(dbg.hasDebuggee(g3w), true);
+assertEq(dbg.hasDebuggee(g4w), true);
+// The debugger's global is not a debuggee.
+assertEq(dbg.hasDebuggee(thisw), false);
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@ -883,6 +883,15 @@ JSCompartment::updateForDebugMode(FreeOp *fop, AutoDebugModeGC &dmgc)

 bool
 JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global)
+{
+    AutoDebugModeGC dmgc(cx->runtime);
+    return addDebuggee(cx, global, dmgc);
+}
+
+bool
+JSCompartment::addDebuggee(JSContext *cx,
+                           js::GlobalObject *global,
+                           AutoDebugModeGC &dmgc)
 {
    bool wasEnabled = debugMode();
    if (!debuggees.put(global)) {
@ -891,7 +900,6 @@ JSCompartment::addDebuggee(JSContext *cx, js::GlobalObject *global)
    }
    debugModeBits |= DebugFromJS;
    if (!wasEnabled) {
-        AutoDebugModeGC dmgc(cx->runtime);
        updateForDebugMode(cx->runtime->defaultFreeOp(), dmgc);
    }
    return true;
@ -901,6 +909,16 @@ void
 JSCompartment::removeDebuggee(FreeOp *fop,
                              js::GlobalObject *global,
                              js::GlobalObjectSet::Enum *debuggeesEnum)
+{
+    AutoDebugModeGC dmgc(rt);
+    return removeDebuggee(fop, global, dmgc, debuggeesEnum);
+}
+
+void
+JSCompartment::removeDebuggee(FreeOp *fop,
+                              js::GlobalObject *global,
+                              AutoDebugModeGC &dmgc,
+                              js::GlobalObjectSet::Enum *debuggeesEnum)
 {
    bool wasEnabled = debugMode();
    JS_ASSERT(debuggees.has(global));
@ -912,7 +930,6 @@ JSCompartment::removeDebuggee(FreeOp *fop,
    if (debuggees.empty()) {
        debugModeBits &= ~DebugFromJS;
        if (wasEnabled && !debugMode()) {
-            AutoDebugModeGC dmgc(rt);
            DebugScopes::onCompartmentLeaveDebugMode(this);
            updateForDebugMode(fop, dmgc);
        }
--- a/js/src/jscompartment.h
+++ b/js/src/jscompartment.h
@ -499,8 +499,13 @@ struct JSCompartment : private JS::shadow::Compartment, public js::gc::GraphNode
  public:
    js::GlobalObjectSet &getDebuggees() { return debuggees; }
    bool addDebuggee(JSContext *cx, js::GlobalObject *global);
+    bool addDebuggee(JSContext *cx, js::GlobalObject *global,
+                     js::AutoDebugModeGC &dmgc);
    void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global,
                        js::GlobalObjectSet::Enum *debuggeesEnum = NULL);
+    void removeDebuggee(js::FreeOp *fop, js::GlobalObject *global,
+                        js::AutoDebugModeGC &dmgc,
+                        js::GlobalObjectSet::Enum *debuggeesEnum = NULL);
    bool setDebugModeFromC(JSContext *cx, bool b, js::AutoDebugModeGC &dmgc);

    void clearBreakpointsIn(js::FreeOp *fop, js::Debugger *dbg, JSObject *handler);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@ -1881,6 +1881,26 @@ Debugger::addDebuggee(JSContext *cx, unsigned argc, Value *vp)
    return true;
 }

+JSBool
+Debugger::addAllGlobalsAsDebuggees(JSContext *cx, unsigned argc, Value *vp)
+{
+    THIS_DEBUGGER(cx, argc, vp, "addAllGlobalsAsDebuggees", args, dbg);
+    AutoDebugModeGC dmgc(cx->runtime);
+    for (CompartmentsIter c(cx->runtime); !c.done(); c.next()) {
+        if (c == dbg->object->compartment())
+            continue;
+        c->scheduledForDestruction = false;
+        GlobalObject *global = c->maybeGlobal();
+        if (global) {
+            Rooted<GlobalObject*> rg(cx, global);
+            dbg->addDebuggeeGlobal(cx, rg, dmgc);
+        }
+    }
+
+    args.rval().setUndefined();
+    return true;
+}
+
 JSBool
 Debugger::removeDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
@ -1899,8 +1919,9 @@ JSBool
 Debugger::removeAllDebuggees(JSContext *cx, unsigned argc, Value *vp)
 {
    THIS_DEBUGGER(cx, argc, vp, "removeAllDebuggees", args, dbg);
+    AutoDebugModeGC dmgc(cx->runtime);
    for (GlobalObjectSet::Enum e(dbg->debuggees); !e.empty(); e.popFront())
-        dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), e.front(), NULL, &e);
+        dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), e.front(), dmgc, NULL, &e);
    args.rval().setUndefined();
    return true;
 }
@ -2026,6 +2047,15 @@ Debugger::construct(JSContext *cx, unsigned argc, Value *vp)

 bool
 Debugger::addDebuggeeGlobal(JSContext *cx, Handle<GlobalObject*> global)
+{
+    AutoDebugModeGC dmgc(cx->runtime);
+    return addDebuggeeGlobal(cx, global, dmgc);
+}
+
+bool
+Debugger::addDebuggeeGlobal(JSContext *cx,
+                            Handle<GlobalObject*> global,
+                            AutoDebugModeGC &dmgc)
 {
    if (debuggees.has(global))
        return true;
@ -2082,7 +2112,7 @@ Debugger::addDebuggeeGlobal(JSContext *cx, Handle<GlobalObject*> global)
        } else {
            if (global->getDebuggers()->length() > 1)
                return true;
-            if (debuggeeCompartment->addDebuggee(cx, global))
+            if (debuggeeCompartment->addDebuggee(cx, global, dmgc))
                return true;

            /* Maintain consistency on error. */
@ -2098,6 +2128,16 @@ void
 Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global,
                               GlobalObjectSet::Enum *compartmentEnum,
                               GlobalObjectSet::Enum *debugEnum)
+{
+    AutoDebugModeGC dmgc(global->compartment()->rt);
+    return removeDebuggeeGlobal(fop, global, dmgc, compartmentEnum, debugEnum);
+}
+
+void
+Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global,
+                               AutoDebugModeGC &dmgc,
+                               GlobalObjectSet::Enum *compartmentEnum,
+                               GlobalObjectSet::Enum *debugEnum)
 {
    /*
     * Each debuggee is in two HashSets: one for its compartment and one for
@ -2151,7 +2191,7 @@ Debugger::removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global,
     * global cannot be rooted on the stack without a cx.
     */
    if (v->empty())
-        global->compartment()->removeDebuggee(fop, global, compartmentEnum);
+        global->compartment()->removeDebuggee(fop, global, dmgc, compartmentEnum);
 }

 /*
@ -2538,6 +2578,7 @@ JSPropertySpec Debugger::properties[] = {

 JSFunctionSpec Debugger::methods[] = {
    JS_FN("addDebuggee", Debugger::addDebuggee, 1, 0),
+    JS_FN("addAllGlobalsAsDebuggees", Debugger::addAllGlobalsAsDebuggees, 0, 0),
    JS_FN("removeDebuggee", Debugger::removeDebuggee, 1, 0),
    JS_FN("removeAllDebuggees", Debugger::removeAllDebuggees, 0, 0),
    JS_FN("hasDebuggee", Debugger::hasDebuggee, 1, 0),
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@ -225,9 +225,15 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
    class ScriptQuery;

    bool addDebuggeeGlobal(JSContext *cx, Handle<GlobalObject*> obj);
+    bool addDebuggeeGlobal(JSContext *cx, Handle<GlobalObject*> obj,
+                           AutoDebugModeGC &dmgc);
    void removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global,
                              GlobalObjectSet::Enum *compartmentEnum,
                              GlobalObjectSet::Enum *debugEnum);
+    void removeDebuggeeGlobal(FreeOp *fop, GlobalObject *global,
+                              AutoDebugModeGC &dmgc,
+                              GlobalObjectSet::Enum *compartmentEnum,
+                              GlobalObjectSet::Enum *debugEnum);

    /*
     * Cope with an error or exception in a debugger hook.
@ -300,6 +306,7 @@ class Debugger : private mozilla::LinkedListElement<Debugger>
    static JSBool getUncaughtExceptionHook(JSContext *cx, unsigned argc, Value *vp);
    static JSBool setUncaughtExceptionHook(JSContext *cx, unsigned argc, Value *vp);
    static JSBool addDebuggee(JSContext *cx, unsigned argc, Value *vp);
+    static JSBool addAllGlobalsAsDebuggees(JSContext *cx, unsigned argc, Value *vp);
    static JSBool removeDebuggee(JSContext *cx, unsigned argc, Value *vp);
    static JSBool removeAllDebuggees(JSContext *cx, unsigned argc, Value *vp);
    static JSBool hasDebuggee(JSContext *cx, unsigned argc, Value *vp);
--- a/toolkit/devtools/debugger/server/dbg-script-actors.js
+++ b/toolkit/devtools/debugger/server/dbg-script-actors.js
@ -2266,10 +2266,8 @@ update(ChromeDebuggerActor.prototype, {
   */
  globalManager: {
    findGlobals: function CDA_findGlobals() {
-      // Fetch the list of globals from the debugger.
-      for (let g of this.dbg.findAllGlobals()) {
-        this.addDebuggee(g);
-      }
+      // Add every global known to the debugger as debuggee.
+      this.dbg.addAllGlobalsAsDebuggees();
    },

    /**