Bug 711125 - Make the debugger client's setBreakpoint pause the debuggee automatically; r=dcamp

Makes DC_setBreakpoint send an interrupt request first, if the debuggee is running (in the main loop), and then proceed with a breakpoint request. Also fixes a bug in the client request dispatcher, where unsolicited notifications would be considered as responses to unrelated active requests.

toolkit/devtools/debugger/dbg-client.jsm

@@ -194,6 +194,16 @@ const ThreadStateTypes = {
   "detached": "detached"
 };
 
+/**
+ * Set of protocol messages that are sent by the server without a prior request
+ * by the client.
+ */
+const UnsolicitedNotifications = {
+  "newScript": "newScript",
+  "tabDetached": "tabDetached",
+  "tabNavigated": "tabNavigated"
+};
+
 /**
  * Set of debug protocol request types that specify the protocol request being
  * sent to the server.
@@ -409,12 +419,15 @@ DebuggerClient.prototype = {
 
     try {
       if (!aPacket.from) {
-        dumpn("Server did not specify an actor, dropping packet: " + JSON.stringify(aPacket));
+        Cu.reportError("Server did not specify an actor, dropping packet: " +
+                       JSON.stringify(aPacket));
         return;
       }
 
       let onResponse;
-      if (aPacket.from in this._activeRequests) {
+      // Don't count unsolicited notifications as responses.
+      if (aPacket.from in this._activeRequests &&
+          !(aPacket.type in UnsolicitedNotifications)) {
         onResponse = this._activeRequests[aPacket.from].onResponse;
         delete this._activeRequests[aPacket.from];
       }
@@ -612,24 +625,49 @@ ThreadClient.prototype = {
    * Request to set a breakpoint in the specified location.
    *
    * @param aLocation object
-   *        The source location object where the breakpoint
-   *        will be set.
+   *        The source location object where the breakpoint will be set.
    * @param aOnResponse integer
    *        Called with the thread's response.
    */
   setBreakpoint: function TC_setBreakpoint(aLocation, aOnResponse) {
-    this._assertPaused("setBreakpoint");
+    // A helper function that sets the breakpoint.
+    let doSetBreakpoint = function _doSetBreakpoint(aCallback) {
+      let packet = { to: this._actor, type: DebugProtocolTypes.setBreakpoint,
+                     location: aLocation };
+      this._client.request(packet, function (aResponse) {
+          if (aOnResponse) {
+            if (aResponse.error) {
+              if (aCallback) {
+                aCallback(aOnResponse.bind(undefined, aResponse));
+              } else {
+                aOnResponse(aResponse);
+              }
+              return;
+            }
+            let bpClient = new BreakpointClient(this._client, aResponse.actor);
+            if (aCallback) {
+              aCallback(aOnResponse(aResponse, bpClient));
+            } else {
+              aOnResponse(aResponse, bpClient);
+            }
+          }
+        }.bind(this));
+    }.bind(this);
 
-    let self = this;
-    let packet = { to: this._actor, type: DebugProtocolTypes.setBreakpoint,
-                   location: aLocation };
-    this._client.request(packet, function (aResponse) {
-                         if (aOnResponse) {
-                           let bpClient = new BreakpointClient(self._client,
-                                                               aResponse.actor);
-                           aOnResponse(aResponse, bpClient);
-                         }
-                       });
+    // If the debuggee is paused, just set the breakpoint.
+    if (this.paused) {
+      doSetBreakpoint();
+      return;
+    }
+    // Otherwise, force a pause in order to set the breakpoint.
+    this.interrupt(function(aResponse) {
+      if (aResponse.error) {
+        // Can't set the breakpoint if pausing failed.
+        aOnResponse(aResponse);
+        return;
+      }
+      doSetBreakpoint(this.resume.bind(this));
+    }.bind(this));
   },
 
   /**

toolkit/devtools/debugger/server/dbg-script-actors.js

@@ -49,8 +49,8 @@
  * of debuggees.
  *
  * @param aHooks object
- *        An object with preNest and postNest methods that can be called when
- *        entering and exiting a nested event loop.
+ *        An object with preNest and postNest methods for calling when entering
+ *        and exiting a nested event loop.
  */
 function ThreadActor(aHooks)
 {

toolkit/devtools/debugger/server/dbg-server.js

@@ -432,7 +432,7 @@ DebuggerServerConnection.prototype = {
       try {
         ret = actor.requestTypes[aPacket.type].bind(actor)(aPacket);
       } catch(e) {
-        dumpn(e);
+        Cu.reportError(e);
         ret = { error: "unknownError",
                 message: "An unknown error has occurred while processing request." };
       }

toolkit/devtools/debugger/tests/unit/test_breakpoint-02.js

@@ -0,0 +1,49 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Check that setting breakpoints when the debuggee is running works.
+ */
+
+var gDebuggee;
+var gClient;
+var gThreadClient;
+
+function run_test()
+{
+  initTestDebuggerServer();
+  gDebuggee = addTestGlobal("test-stack");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function () {
+    attachTestGlobalClientAndResume(gClient, "test-stack", function (aResponse, aThreadClient) {
+      gThreadClient = aThreadClient;
+      test_breakpoint_running();
+    });
+  });
+  do_test_pending();
+}
+
+function test_breakpoint_running()
+{
+  let path = getFilePath('test_breakpoint-01.js');
+
+  gDebuggee.eval("var line0 = Error().lineNumber;\n" +
+                 "var a = 1;\n" +  // line0 + 1
+                 "var b = 2;\n");  // line0 + 2
+
+  // Setting the breakpoint later should interrupt the debuggee.
+  gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+    do_check_eq(aPacket.type, "paused");
+    do_check_eq(aPacket.why.type, "interrupted");
+  });
+
+  gThreadClient.setBreakpoint({ url: path, line: gDebuggee.line0 + 3}, function(aResponse) {
+    // Eval scripts don't stick around long enough for the breakpoint to be set,
+    // so just make sure we got the expected response from the actor.
+    do_check_eq(aResponse.error, "noScript");
+
+    do_execute_soon(function() {
+      finishClient(gClient);
+    });
+  });
+}

toolkit/devtools/debugger/tests/unit/xpcshell.ini

@@ -34,6 +34,7 @@ tail =
 [test_eval-04.js]
 [test_eval-05.js]
 [test_breakpoint-01.js]
+[test_breakpoint-02.js]
 [test_listscripts-01.js]
 [test_objectgrips-01.js]
 [test_objectgrips-02.js]