Bug 925269 - Enable/disable individual debugger statements. r=loganfsmyth
authorJason Laster <jlaster@mozilla.com>
Wed, 14 Aug 2019 23:01:26 +0000
changeset 488178 6a2dcba5612bf6f00e0740137c04ea739bdf4d91
parent 488177 9406ccf380bbc14f54eeecd3eec671750b22f53f
child 488179 2d3159992a617c16ec8472479c46a4e3ab4dc147
push id113904
push userncsoregi@mozilla.com
push dateThu, 15 Aug 2019 19:41:00 +0000
treeherdermozilla-inbound@b283a7ef186c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersloganfsmyth
bugs925269
milestone70.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 925269 - Enable/disable individual debugger statements. r=loganfsmyth Differential Revision: https://phabricator.services.mozilla.com/D41838
devtools/server/actors/breakpoint.js
devtools/server/actors/common.js
devtools/server/actors/thread.js
devtools/server/actors/utils/breakpoint-actor-map.js
devtools/server/tests/unit/test_breakpoint-13.js
devtools/server/tests/unit/test_breakpoint-24.js
devtools/server/tests/unit/test_breakpoint-26.js
devtools/server/tests/unit/xpcshell.ini
--- a/devtools/server/actors/breakpoint.js
+++ b/devtools/server/actors/breakpoint.js
@@ -186,18 +186,17 @@ BreakpointActor.prototype = {
   hit: function(frame) {
     // Don't pause if we are currently stepping (in or over) or the frame is
     // black-boxed.
     const location = this.threadActor.sources.getFrameLocation(frame);
     const { sourceActor, line, column } = location;
 
     if (
       this.threadActor.sources.isBlackBoxed(sourceActor.url, line, column) ||
-      this.threadActor.skipBreakpoints ||
-      frame.onStep
+      this.threadActor.skipBreakpoints
     ) {
       return undefined;
     }
 
     // If we're trying to pop this frame, and we see a breakpoint at
     // the spot at which popping started, ignore it.  See bug 970469.
     const locationAtFinish = frame.onPop && frame.onPop.location;
     if (
--- a/devtools/server/actors/common.js
+++ b/devtools/server/actors/common.js
@@ -140,16 +140,20 @@ SourceLocation.prototype = {
   get column() {
     return this._column;
   },
 
   get lastColumn() {
     return this._lastColumn;
   },
 
+  get sourceUrl() {
+    return this.sourceActor.url;
+  },
+
   equals: function(other) {
     return (
       this.sourceActor.url == other.sourceActor.url &&
       this.line === other.line &&
       (this.column === undefined ||
         other.column === undefined ||
         this.column === other.column)
     );
--- a/devtools/server/actors/thread.js
+++ b/devtools/server/actors/thread.js
@@ -924,31 +924,36 @@ const ThreadActor = ActorClassWithSpec(t
   },
 
   _validFrameStepOffset: function(frame, startFrame, offset) {
     const meta = frame.script.getOffsetMetadata(offset);
 
     // Continue if:
     // 1. the location is not a valid breakpoint position
     // 2. the source is not blackboxed
-    // 3. has not moved
+    // 3. we have not moved since the last pause
     if (
       !meta.isBreakpoint ||
       this.sources.isFrameBlackBoxed(frame) ||
       !this.hasMoved(frame)
     ) {
       return false;
     }
 
     // Pause if:
     // 1. the frame has changed
     // 2. the location is a step position.
     return frame !== startFrame || meta.isStepStart;
   },
 
+  atBreakpointLocation(frame) {
+    const location = this.sources.getFrameLocation(frame);
+    return !!this.breakpointActorMap.get(location);
+  },
+
   createCompletionGrip: function(packet, completion) {
     if (!completion) {
       return packet;
     }
 
     const createGrip = value =>
       createValueGrip(value, this._pausePool, this.objectGrip);
     packet.why.frameFinished = {};
@@ -1755,26 +1760,27 @@ const ThreadActor = ActorClassWithSpec(t
    * A function that the engine calls when a debugger statement has been
    * executed in the specified frame.
    *
    * @param frame Debugger.Frame
    *        The stack frame that contained the debugger statement.
    */
   onDebuggerStatement: function(frame) {
     const location = this.sources.getFrameLocation(frame);
-    const url = location.sourceActor.url;
 
     // Don't pause if
-    // 1. the debugger is in the same position
+    // 1. we have not moved since the last pause
     // 2. breakpoints are disabled
     // 3. the source is blackboxed
+    // 4. there is a breakpoint at the same location
     if (
       !this.hasMoved(frame, "debuggerStatement") ||
       this.skipBreakpoints ||
-      this.sources.isBlackBoxed(url)
+      this.sources.isBlackBoxed(location.sourceUrl) ||
+      this.atBreakpointLocation(frame)
     ) {
       return undefined;
     }
 
     return this._pauseAndRespond(frame, { type: "debuggerStatement" });
   },
 
   onSkipBreakpoints: function({ skip }) {
--- a/devtools/server/actors/utils/breakpoint-actor-map.js
+++ b/devtools/server/actors/utils/breakpoint-actor-map.js
@@ -48,16 +48,21 @@ BreakpointActorMap.prototype = {
   getOrCreateBreakpointActor(location) {
     const key = this._locationKey(location);
     if (!this._actors[key]) {
       this._actors[key] = new BreakpointActor(this._threadActor, location);
     }
     return this._actors[key];
   },
 
+  get(location) {
+    const key = this._locationKey(location);
+    return this._actors[key];
+  },
+
   /**
    * Delete the BreakpointActor from the given location in this
    * BreakpointActorMap.
    *
    * @param BreakpointLocation location
    *        The location from which the BreakpointActor should be deleted.
    */
   deleteActor(location) {
--- a/devtools/server/tests/unit/test_breakpoint-13.js
+++ b/devtools/server/tests/unit/test_breakpoint-13.js
@@ -12,17 +12,20 @@
 add_task(
   threadFrontTest(async ({ threadFront, debuggee }) => {
     const packet = await executeOnNextTickAndWaitForPause(
       () => evaluateTestCode(debuggee),
       threadFront
     );
 
     const source = await getSourceById(threadFront, packet.frame.where.actor);
-    await threadFront.setBreakpoint({ sourceUrl: source.url, line: 3 }, {});
+    await threadFront.setBreakpoint(
+      { sourceUrl: source.url, line: 3, column: 6 },
+      {}
+    );
 
     info("Check that the stepping worked.");
     const packet1 = await stepIn(threadFront);
     Assert.equal(packet1.frame.where.line, 6);
     Assert.equal(packet1.why.type, "resumeLimit");
 
     info("Entered the foo function call frame.");
     const packet2 = await stepIn(threadFront);
--- a/devtools/server/tests/unit/test_breakpoint-24.js
+++ b/devtools/server/tests/unit/test_breakpoint-24.js
@@ -105,17 +105,17 @@ async function testBreakpoints({ threadF
       debugger;
     }
     function a() {}
     foo();
     //# sourceURL=http://example.com/testBreakpoints.js`
   );
 
   threadFront.setBreakpoint(
-    { sourceUrl: "http://example.com/testBreakpoints.js", line: 3 },
+    { sourceUrl: "http://example.com/testBreakpoints.js", line: 3, column: 6 },
     {}
   );
 
   await performActions(threadFront, [
     [
       "paused at first debugger statement",
       { line: 2, type: "debuggerStatement" },
       "stepOver",
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_breakpoint-26.js
@@ -0,0 +1,67 @@
+/* eslint-disable max-nested-callbacks */
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Bug 925269 - Verify that debugger statements are skipped
+ * if there is a falsey conditional breakpoint at the same location.
+ */
+add_task(
+  threadFrontTest(async props => {
+    await testBreakpointsAndDebuggerStatements(props);
+  })
+);
+
+async function testBreakpointsAndDebuggerStatements({
+  threadFront,
+  targetFront,
+}) {
+  const consoleFront = await targetFront.getFront("console");
+  consoleFront.evaluateJSAsync(
+    `function foo(stop) {
+      debugger;
+      debugger;
+      debugger;
+    }
+    foo();
+    //# sourceURL=http://example.com/testBreakpointsAndDebuggerStatements.js`
+  );
+
+  threadFront.setBreakpoint(
+    {
+      sourceUrl: "http://example.com/testBreakpointsAndDebuggerStatements.js",
+      line: 3,
+      column: 6,
+    },
+    { condition: "false" }
+  );
+
+  await performActions(threadFront, [
+    [
+      "paused at first debugger statement",
+      { line: 2, type: "debuggerStatement" },
+      "resume",
+    ],
+    [
+      "pause at the third debugger statement",
+      { line: 4, type: "debuggerStatement" },
+      "resume",
+    ],
+  ]);
+}
+
+async function performActions(threadFront, actions) {
+  for (const action of actions) {
+    await performAction(threadFront, action);
+  }
+}
+
+async function performAction(threadFront, [description, result, action]) {
+  info(description);
+  const packet = await waitForEvent(threadFront, "paused");
+  Assert.equal(packet.frame.where.line, result.line);
+  Assert.equal(packet.why.type, result.type);
+  await threadFront[action]();
+}
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -131,16 +131,17 @@ skip-if = true
 reason = bug 1104838
 [test_breakpoint-20.js]
 [test_breakpoint-21.js]
 [test_breakpoint-22.js]
 skip-if = true # breakpoint sliding is not supported bug 1525685
 [test_breakpoint-23.js]
 [test_breakpoint-24.js]
 [test_breakpoint-25.js]
+[test_breakpoint-26.js]
 [test_conditional_breakpoint-01.js]
 [test_conditional_breakpoint-02.js]
 [test_conditional_breakpoint-03.js]
 [test_conditional_breakpoint-04.js]
 [test_logpoint-01.js]
 [test_logpoint-02.js]
 [test_logpoint-03.js]
 [test_listsources-01.js]