Bug 1613165 - Ensure adding or removing breakpoint does not disable debugger statement.
authorMiriam <bmiriam1230@gmail.com>
Wed, 18 Mar 2020 22:41:20 +0000
changeset 519757 343041f167f1e9f473452e5ce2e2b6d9a4d2e8c6
parent 519756 224a2033ef9e62c08421c1e8a1d5fd9e2f89a1a3
child 519758 cf75cef9d1f669e03b918451e2ec8e8e6d8e7718
push id37232
push usercsabou@mozilla.com
push dateFri, 20 Mar 2020 09:53:53 +0000
treeherdermozilla-central@32d6a3f1f83c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1613165
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1613165 - Ensure adding or removing breakpoint does not disable debugger statement. Differential Revision: https://phabricator.services.mozilla.com/D67172
devtools/server/actors/thread.js
devtools/server/tests/xpcshell/test_breakpoint-24.js
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -491,19 +491,34 @@ const ThreadActor = ActorClassWithSpec(t
   },
 
   _findXHRBreakpointIndex(p, m) {
     return this._xhrBreakpoints.findIndex(
       ({ path, method }) => path === p && method === m
     );
   },
 
+  // We clear the priorPause field when a breakpoint is added or removed
+  // at the same location because we are no longer worried about pausing twice
+  // at that location (e.g. debugger statement, stepping).
+  _maybeClearPriorPause(location) {
+    if (!this._priorPause) {
+      return;
+    }
+
+    const { where } = this._priorPause.frame;
+    if (where.line === location.line && where.column === location.column) {
+      this._priorPause = null;
+    }
+  },
+
   setBreakpoint: async function(location, options) {
     const actor = this.breakpointActorMap.getOrCreateBreakpointActor(location);
     actor.setOptions(options);
+    this._maybeClearPriorPause(location);
 
     if (location.sourceUrl) {
       // There can be multiple source actors for a URL if there are multiple
       // inline sources on an HTML page.
       const sourceActors = this.sources.getSourceActorsByURL(
         location.sourceUrl
       );
       for (const sourceActor of sourceActors) {
@@ -514,16 +529,17 @@ const ThreadActor = ActorClassWithSpec(t
       if (sourceActor) {
         await sourceActor.applyBreakpoint(actor);
       }
     }
   },
 
   removeBreakpoint(location) {
     const actor = this.breakpointActorMap.getOrCreateBreakpointActor(location);
+    this._maybeClearPriorPause(location);
     actor.delete();
   },
 
   removeXHRBreakpoint: function(path, method) {
     const index = this._findXHRBreakpointIndex(path, method);
 
     if (index >= 0) {
       this._xhrBreakpoints.splice(index, 1);
--- a/devtools/server/tests/xpcshell/test_breakpoint-24.js
+++ b/devtools/server/tests/xpcshell/test_breakpoint-24.js
@@ -2,23 +2,28 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 /**
  * Bug 1441183 - Verify that the debugger advances to a new location
  * when encountering debugger statements and brakpoints
+ *
+ * Bug 1613165 - Verify that debugger statement is not disabled by
+ *  adding/removing a breakpoint
  */
 add_task(
   threadFrontTest(async props => {
     await testDebuggerStatements(props);
     await testBreakpoints(props);
     await testBreakpointsAndDebuggerStatements(props);
     await testLoops(props);
+    await testRemovingBreakpoint(props);
+    await testAddingBreakpoint(props);
   })
 );
 
 // Ensure that we advance to the next line when we
 // step to a debugger statement and resume.
 async function testDebuggerStatements({ threadFront, targetFront }) {
   const consoleFront = await targetFront.getFront("console");
   consoleFront.evaluateJSAsync(
@@ -165,16 +170,88 @@ async function testLoops({ threadFront, 
     [
       "pause at the third debugger satement",
       { line: 7, type: "debuggerStatement" },
       "resume",
     ],
   ]);
 }
 
+// Bug 1613165 - ensure that if you pause on a breakpoint on a line with
+// debugger statement, remove the breakpoint, and try to pause on the
+// debugger statement before pausing anywhere else, debugger pauses instead of
+// skipping debugger statement.
+async function testRemovingBreakpoint({ threadFront, targetFront }) {
+  const consoleFront = await targetFront.getFront("console");
+  consoleFront.evaluateJSAsync(
+    `function foo(stop) {
+      debugger;
+    }
+    foo();
+    foo();
+    //# sourceURL=http://example.com/testRemovingBreakpoint.js`
+  );
+
+  const location = {
+    sourceUrl: "http://example.com/testRemovingBreakpoint.js",
+    line: 2,
+    column: 6,
+  };
+
+  threadFront.setBreakpoint(location, {});
+
+  info("paused at the breakpoint at the first debugger statement");
+  const packet = await waitForEvent(threadFront, "paused");
+  Assert.equal(packet.frame.where.line, 2);
+  Assert.equal(packet.why.type, "breakpoint");
+  threadFront.removeBreakpoint(location);
+  await threadFront.resume();
+
+  info("paused at the first debugger statement");
+  const packet2 = await waitForEvent(threadFront, "paused");
+  Assert.equal(packet2.frame.where.line, 2);
+  Assert.equal(packet2.why.type, "debuggerStatement");
+  await threadFront.resume();
+}
+
+// Bug 1613165 - ensure if you pause on a debugger statement, add a
+// breakpoint on the same line, and try to pause on the breakpoint
+// before pausing anywhere else, debugger pauses on that line instead of
+// skipping breakpoint.
+async function testAddingBreakpoint({ threadFront, targetFront }) {
+  const consoleFront = await targetFront.getFront("console");
+  consoleFront.evaluateJSAsync(
+    `function foo(stop) {
+      debugger;
+    }
+    foo();
+    foo();
+    //# sourceURL=http://example.com/testAddingBreakpoint.js`
+  );
+
+  const location = {
+    sourceUrl: "http://example.com/testAddingBreakpoint.js",
+    line: 2,
+    column: 6,
+  };
+
+  info("paused at the first debugger statement");
+  const packet = await waitForEvent(threadFront, "paused");
+  Assert.equal(packet.frame.where.line, 2);
+  Assert.equal(packet.why.type, "debuggerStatement");
+  threadFront.setBreakpoint(location, {});
+  await threadFront.resume();
+
+  info("paused at the breakpoint at the first debugger statement");
+  const packet2 = await waitForEvent(threadFront, "paused");
+  Assert.equal(packet2.frame.where.line, 2);
+  Assert.equal(packet2.why.type, "breakpoint");
+  await threadFront.resume();
+}
+
 async function performActions(threadFront, actions) {
   for (const action of actions) {
     await performAction(threadFront, action);
   }
 }
 
 async function performAction(threadFront, [description, result, action]) {
   info(description);