Bug 812869 - Breakpoint clients sometimes don't know how to remove themselves, r=vporof

browser/devtools/debugger/debugger-controller.js

@@ -1326,11 +1326,10 @@ Breakpoints.prototype = {
     this.activeThread.setBreakpoint(aLocation, function(aResponse, aBreakpointClient) {
       let { url, line } = aResponse.actualLocation || aLocation;
 
-      // Prevent this new breakpoint from being repositioned on top of an
-      // already existing one.
+      // If the response contains a breakpoint that exists in the cache, prevent
+      // it from being shown in the source editor at an incorrect position.
       if (this.getBreakpoint(url, line)) {
         this._hideBreakpoint(aBreakpointClient);
-        aBreakpointClient.remove();
         return;
       }
 

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

@@ -578,13 +578,21 @@ ThreadActor.prototype = {
             inner.setBreakpoint(lines[line][i], bpActor);
             codeFound = true;
           }
+          bpActor.addScript(inner, this);
           actualLocation = {
             url: aLocation.url,
             line: line,
             column: aLocation.column
           };
+          // If there wasn't already a breakpoint at that line, update the cache
+          // as well.
+          if (scriptBreakpoints[line] && scriptBreakpoints[line].actor) {
+            let existing = scriptBreakpoints[line].actor;
+            bpActor.onDelete();
+            delete scriptBreakpoints[oldLine];
+            return { actor: existing.actorID, actualLocation: actualLocation };
+          }
           bpActor.location = actualLocation;
-          // Update the cache as well.
           scriptBreakpoints[line] = scriptBreakpoints[oldLine];
           scriptBreakpoints[line].line = line;
           delete scriptBreakpoints[oldLine];

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

@@ -0,0 +1,101 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Make sure that setting a breakpoint twice in a line without bytecodes works
+ * as expected.
+ */
+
+const NUM_BREAKPOINTS = 10;
+var gDebuggee;
+var gClient;
+var gThreadClient;
+var gPath = getFilePath('test_breakpoint-12.js');
+var gBpActor;
+var gCount = 1;
+
+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_child_skip_breakpoint();
+    });
+  });
+  do_test_pending();
+}
+
+function test_child_skip_breakpoint()
+{
+  gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+    let location = { url: gPath, line: gDebuggee.line0 + 3};
+    gThreadClient.setBreakpoint(location, function (aResponse, bpClient) {
+      // Check that the breakpoint has properly skipped forward one line.
+      do_check_eq(aResponse.actualLocation.url, location.url);
+      do_check_eq(aResponse.actualLocation.line, location.line + 1);
+      gBpActor = aResponse.actor;
+
+      // Set more breakpoints at the same location.
+      set_breakpoints(location);
+    });
+
+  });
+
+  gDebuggee.eval("var line0 = Error().lineNumber;\n" +
+                 "function foo() {\n" + // line0 + 1
+                 "  this.a = 1;\n" +    // line0 + 2
+                 "  // A comment.\n" +  // line0 + 3
+                 "  this.b = 2;\n" +    // line0 + 4
+                 "}\n" +                // line0 + 5
+                 "debugger;\n" +        // line0 + 6
+                 "foo();\n");           // line0 + 7
+}
+
+// Set many breakpoints at the same location.
+function set_breakpoints(location) {
+  do_check_neq(gCount, NUM_BREAKPOINTS);
+  gThreadClient.setBreakpoint(location, function (aResponse, bpClient) {
+    // Check that the breakpoint has properly skipped forward one line.
+    do_check_eq(aResponse.actualLocation.url, location.url);
+    do_check_eq(aResponse.actualLocation.line, location.line + 1);
+    // Check that the same breakpoint actor was returned.
+    do_check_eq(aResponse.actor, gBpActor);
+
+    if (++gCount < NUM_BREAKPOINTS) {
+      set_breakpoints(location);
+      return;
+    }
+
+    // After setting all the breakpoints, check that only one has effectively
+    // remained.
+    gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+      // Check the return value.
+      do_check_eq(aPacket.type, "paused");
+      do_check_eq(aPacket.frame.where.url, gPath);
+      do_check_eq(aPacket.frame.where.line, location.line + 1);
+      do_check_eq(aPacket.why.type, "breakpoint");
+      do_check_eq(aPacket.why.actors[0], bpClient.actor);
+      // Check that the breakpoint worked.
+      do_check_eq(gDebuggee.a, 1);
+      do_check_eq(gDebuggee.b, undefined);
+
+      gThreadClient.addOneTimeListener("paused", function (aEvent, aPacket) {
+        // We don't expect any more pauses after the breakpoint was hit once.
+        do_check_true(false);
+      });
+      gThreadClient.resume(function () {
+        // Give any remaining breakpoints a chance to trigger.
+        do_timeout(1000, finishClient.bind(null, gClient));
+      });
+
+    });
+    // Continue until the breakpoint is hit.
+    gThreadClient.resume();
+  });
+
+}

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

@@ -45,6 +45,7 @@ tail =
 [test_breakpoint-09.js]
 [test_breakpoint-10.js]
 [test_breakpoint-11.js]
+[test_breakpoint-12.js]
 [test_listscripts-01.js]
 [test_objectgrips-01.js]
 [test_objectgrips-02.js]