Bug 970469 - ignore breakpoints on the current line when stepping out; r?yulia
MozReview-Commit-ID: KTzygRac2D7
--- a/devtools/server/actors/breakpoint.js
+++ b/devtools/server/actors/breakpoint.js
@@ -135,25 +135,38 @@ let BreakpointActor = ActorClassWithSpec
*
* @param frame Debugger.Frame
* The stack frame that contained the breakpoint.
*/
hit: function (frame) {
// Don't pause if we are currently stepping (in or over) or the frame is
// black-boxed.
let generatedLocation = this.threadActor.sources.getFrameLocation(frame);
- let { originalSourceActor } = this.threadActor.unsafeSynchronize(
+ let {
+ originalSourceActor,
+ originalLine,
+ originalColumn
+ } = this.threadActor.unsafeSynchronize(
this.threadActor.sources.getOriginalLocation(generatedLocation));
let url = originalSourceActor.url;
if (this.threadActor.sources.isBlackBoxed(url)
|| frame.onStep) {
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.originalLocation;
+ if (locationAtFinish &&
+ locationAtFinish.originalLine === originalLine &&
+ locationAtFinish.originalColumn === originalColumn) {
+ return undefined;
+ }
+
let reason = {};
if (this.threadActor._hiddenBreakpoints.has(this.actorID)) {
reason.type = "pauseOnDOMEvents";
} else if (!this.condition) {
reason.type = "breakpoint";
// TODO: add the rest of the breakpoints on that line (bug 676602).
reason.actors = [ this.actorID ];
--- a/devtools/server/actors/script.js
+++ b/devtools/server/actors/script.js
@@ -770,19 +770,19 @@ const ThreadActor = ActorClassWithSpec(t
let url = originalSourceActor.url;
return this.sources.isBlackBoxed(url)
? undefined
: pauseAndRespond(frame);
};
},
- _makeOnPop: function (
- { thread, pauseAndRespond, createValueGrip: createValueGripHook }) {
- return function (completion) {
+ _makeOnPop: function ({ thread, pauseAndRespond, createValueGrip: createValueGripHook,
+ startLocation }) {
+ const result = function (completion) {
// onPop is called with 'this' set to the current frame.
const generatedLocation = thread.sources.getFrameLocation(this);
const { originalSourceActor } = thread.unsafeSynchronize(
thread.sources.getOriginalLocation(generatedLocation));
const url = originalSourceActor.url;
if (thread.sources.isBlackBoxed(url)) {
@@ -802,16 +802,27 @@ const ThreadActor = ActorClassWithSpec(t
} else if (completion.hasOwnProperty("yield")) {
packet.why.frameFinished.return = createValueGripHook(completion.yield);
} else {
packet.why.frameFinished.throw = createValueGripHook(completion.throw);
}
return packet;
});
};
+
+ // When stepping out, we don't want to stop at a breakpoint that
+ // happened to be set exactly at the spot where we stepped out.
+ // See bug 970469. We record the original location here and check
+ // it when a breakpoint is hit. Furthermore we store this on the
+ // function because, while we could store it directly on the
+ // frame, if we did we'd also have to find the appropriate spot to
+ // clear it.
+ result.originalLocation = startLocation;
+
+ return result;
},
_makeOnStep: function ({ thread, pauseAndRespond, startFrame,
startLocation, steppingType }) {
// Breaking in place: we should always pause.
if (steppingType === "break") {
return function () {
return pauseAndRespond(this);
--- a/devtools/server/tests/unit/head_dbg.js
+++ b/devtools/server/tests/unit/head_dbg.js
@@ -762,16 +762,30 @@ function stepIn(client, threadClient) {
*/
function stepOver(client, threadClient) {
dumpn("Stepping over.");
return threadClient.stepOver()
.then(() => waitForPause(client));
}
/**
+ * Resume JS execution for a step out and wait for the pause after the step
+ * has been taken.
+ *
+ * @param DebuggerClient client
+ * @param ThreadClient threadClient
+ * @returns Promise
+ */
+function stepOut(client, threadClient) {
+ dumpn("Stepping out.");
+ return threadClient.stepOut()
+ .then(() => waitForPause(client));
+}
+
+/**
* Get the list of `count` frames currently on stack, starting at the index
* `first` for the specified thread.
*
* @param ThreadClient threadClient
* @param Number first
* @param Number count
* @returns Promise
*/
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_stepping-08.js
@@ -0,0 +1,75 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Check that step out doesn't double stop on a breakpoint. Bug 970469.
+ */
+
+var gDebuggee;
+var gClient;
+var gCallback;
+
+function run_test() {
+ do_test_pending();
+ run_test_with_server(DebuggerServer, function () {
+ run_test_with_server(WorkerDebuggerServer, do_test_finished);
+ });
+}
+
+function run_test_with_server(server, callback) {
+ gCallback = callback;
+ initTestDebuggerServer(server);
+ gDebuggee = addTestGlobal("test-stepping", server);
+ gClient = new DebuggerClient(server.connectPipe());
+ gClient.connect(testStepOutWithBreakpoint);
+}
+
+async function testStepOutWithBreakpoint() {
+ const [attachResponse,, threadClient] = await attachTestTabAndResume(gClient,
+ "test-stepping");
+ ok(!attachResponse.error, "Should not get an error attaching");
+
+ dumpn("Evaluating test code and waiting for first debugger statement");
+ const dbgStmt = await executeOnNextTickAndWaitForPause(evaluateTestCode, gClient);
+ equal(dbgStmt.frame.where.line, 3, "Should be at debugger statement on line 3");
+
+ dumpn("Setting breakpoint in innerFunction");
+ const source = threadClient.source(dbgStmt.frame.where.source);
+ await source.setBreakpoint({ line: 7 });
+
+ dumpn("Step in to innerFunction");
+ const step1 = await stepIn(gClient, threadClient);
+ equal(step1.frame.where.line, 7);
+
+ dumpn("Step out of innerFunction");
+ const step2 = await stepOut(gClient, threadClient);
+ // The bug was that we'd stop again at the breakpoint on line 7.
+ equal(step2.frame.where.line, 10);
+
+ finishClient(gClient, gCallback);
+}
+
+function evaluateTestCode() {
+ /* eslint-disable */
+ Cu.evalInSandbox(
+ ` // 1
+ function outerFunction() { // 2
+ debugger; innerFunction(); // 3
+ } // 4
+ // 5
+ function innerFunction() { // 6
+ let x = 0; // 7
+ let y = 72; // 8
+ return x+y; // 9
+ } // 10
+ outerFunction(); // 11
+ `, // 12
+ gDebuggee,
+ "1.8",
+ "test_stepping-08-test-code.js",
+ 1
+ );
+ /* eslint-enable */
+}
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -180,16 +180,17 @@ reason = only ran on B2G
[test_interrupt.js]
[test_stepping-01.js]
[test_stepping-02.js]
[test_stepping-03.js]
[test_stepping-04.js]
[test_stepping-05.js]
[test_stepping-06.js]
[test_stepping-07.js]
+[test_stepping-08.js]
[test_framebindings-01.js]
[test_framebindings-02.js]
[test_framebindings-03.js]
[test_framebindings-04.js]
[test_framebindings-05.js]
[test_framebindings-06.js]
[test_framebindings-07.js]
[test_pause_exceptions-01.js]