diff --git a/devtools/client/debugger/src/actions/breakpoints/modify.js b/devtools/client/debugger/src/actions/breakpoints/modify.js index cbdb56d9def8..32c92d40b096 100644 --- a/devtools/client/debugger/src/actions/breakpoints/modify.js +++ b/devtools/client/debugger/src/actions/breakpoints/modify.js @@ -23,6 +23,7 @@ import { import { setBreakpointPositions } from "./breakpointPositions"; +import { PROMISE } from "../utils/middleware/promise"; import { recordEvent } from "../../utils/telemetry"; import { comparePosition } from "../../utils/location"; import { getTextAtPosition } from "../../utils/source"; @@ -60,24 +61,21 @@ import type { // breakpoint will be added to the reducer, to restore the above invariant. // See syncBreakpoint.js for more. -function clientSetBreakpoint(breakpoint: Breakpoint) { - return ({ getState, client }: ThunkArgs) => { - const breakpointLocation = makeBreakpointLocation( - getState(), - breakpoint.generatedLocation - ); - return client.setBreakpoint(breakpointLocation, breakpoint.options); - }; +function clientSetBreakpoint(client, state, breakpoint: Breakpoint) { + const breakpointLocation = makeBreakpointLocation( + state, + breakpoint.generatedLocation + ); + return client.setBreakpoint(breakpointLocation, breakpoint.options); } -function clientRemoveBreakpoint(generatedLocation: SourceLocation) { - return ({ getState, client }: ThunkArgs) => { - const breakpointLocation = makeBreakpointLocation( - getState(), - generatedLocation - ); - return client.removeBreakpoint(breakpointLocation); - }; +function clientRemoveBreakpoint( + client, + state, + generatedLocation: SourceLocation +) { + const breakpointLocation = makeBreakpointLocation(state, generatedLocation); + return client.removeBreakpoint(breakpointLocation); } export function enableBreakpoint(cx: Context, initialBreakpoint: Breakpoint) { @@ -87,13 +85,12 @@ export function enableBreakpoint(cx: Context, initialBreakpoint: Breakpoint) { return; } - dispatch({ + return dispatch({ type: "SET_BREAKPOINT", cx, breakpoint: { ...breakpoint, disabled: false }, + [PROMISE]: clientSetBreakpoint(client, getState(), breakpoint), }); - - return dispatch(clientSetBreakpoint(breakpoint)); }; } @@ -161,15 +158,16 @@ export function addBreakpoint( return; } - dispatch({ type: "SET_BREAKPOINT", cx, breakpoint }); - - if (disabled) { + return dispatch({ + type: "SET_BREAKPOINT", + cx, + breakpoint, // If we just clobbered an enabled breakpoint with a disabled one, we need // to remove any installed breakpoint in the server. - return dispatch(clientRemoveBreakpoint(generatedLocation)); - } - - return dispatch(clientSetBreakpoint(breakpoint)); + [PROMISE]: disabled + ? clientRemoveBreakpoint(client, getState(), generatedLocation) + : clientSetBreakpoint(client, getState(), breakpoint), + }); }; } @@ -188,18 +186,19 @@ export function removeBreakpoint(cx: Context, initialBreakpoint: Breakpoint) { return; } - dispatch({ + return dispatch({ type: "REMOVE_BREAKPOINT", cx, location: breakpoint.location, + // If the breakpoint is disabled then it is not installed in the server. + [PROMISE]: breakpoint.disabled + ? Promise.resolve() + : clientRemoveBreakpoint( + client, + getState(), + breakpoint.generatedLocation + ), }); - - // If the breakpoint is disabled then it is not installed in the server. - if (breakpoint.disabled) { - return; - } - - return dispatch(clientRemoveBreakpoint(breakpoint.generatedLocation)); }; } @@ -215,6 +214,12 @@ export function removeBreakpointAtGeneratedLocation( target: SourceLocation ) { return ({ dispatch, getState, client }: ThunkArgs) => { + // remove breakpoint from the server + const onBreakpointRemoved = clientRemoveBreakpoint( + client, + getState(), + target + ); // Remove any breakpoints matching the generated location. const breakpoints = getBreakpointsList(getState()); for (const { location, generatedLocation } of breakpoints) { @@ -226,6 +231,7 @@ export function removeBreakpointAtGeneratedLocation( type: "REMOVE_BREAKPOINT", cx, location, + [PROMISE]: onBreakpointRemoved, }); } } @@ -244,9 +250,7 @@ export function removeBreakpointAtGeneratedLocation( }); } } - - // Remove the breakpoint from the client itself. - return dispatch(clientRemoveBreakpoint(target)); + return onBreakpointRemoved; }; } @@ -263,13 +267,16 @@ export function disableBreakpoint(cx: Context, initialBreakpoint: Breakpoint) { return; } - dispatch({ + return dispatch({ type: "SET_BREAKPOINT", cx, breakpoint: { ...breakpoint, disabled: true }, + [PROMISE]: clientRemoveBreakpoint( + client, + getState(), + breakpoint.generatedLocation + ), }); - - return dispatch(clientRemoveBreakpoint(breakpoint.generatedLocation)); }; } @@ -298,12 +305,11 @@ export function setBreakpointOptions( // Note: setting a breakpoint's options implicitly enables it. breakpoint = { ...breakpoint, disabled: false, options }; - dispatch({ + return dispatch({ type: "SET_BREAKPOINT", cx, breakpoint, + [PROMISE]: clientSetBreakpoint(client, getState(), breakpoint), }); - - return dispatch(clientSetBreakpoint(breakpoint)); }; } diff --git a/devtools/client/debugger/src/actions/types/BreakpointAction.js b/devtools/client/debugger/src/actions/types/BreakpointAction.js index 4bfd68a2215e..8736f1760703 100644 --- a/devtools/client/debugger/src/actions/types/BreakpointAction.js +++ b/devtools/client/debugger/src/actions/types/BreakpointAction.js @@ -41,16 +41,16 @@ export type BreakpointAction = +index: number, +breakpoint: XHRBreakpoint, |}> - | {| + | PromiseAction<{| +type: "SET_BREAKPOINT", +cx: Context, +breakpoint: Breakpoint, - |} - | {| + |}> + | PromiseAction<{| +type: "REMOVE_BREAKPOINT", +cx: Context, +location: SourceLocation, - |} + |}> | {| +type: "REMOVE_PENDING_BREAKPOINT", +cx: Context, diff --git a/devtools/client/debugger/src/reducers/breakpoints.js b/devtools/client/debugger/src/reducers/breakpoints.js index 8a13ccb80a61..6003627f8c3e 100644 --- a/devtools/client/debugger/src/reducers/breakpoints.js +++ b/devtools/client/debugger/src/reducers/breakpoints.js @@ -50,11 +50,17 @@ function update( ): BreakpointsState { switch (action.type) { case "SET_BREAKPOINT": { - return setBreakpoint(state, action); + if (action.status === "start") { + return setBreakpoint(state, action); + } + return state; } case "REMOVE_BREAKPOINT": { - return removeBreakpoint(state, action); + if (action.status === "start") { + return removeBreakpoint(state, action); + } + return state; } case "NAVIGATE": { diff --git a/devtools/client/debugger/src/reducers/pending-breakpoints.js b/devtools/client/debugger/src/reducers/pending-breakpoints.js index 628fad03cf24..849dc5d3ae0b 100644 --- a/devtools/client/debugger/src/reducers/pending-breakpoints.js +++ b/devtools/client/debugger/src/reducers/pending-breakpoints.js @@ -29,6 +29,11 @@ function update(state: PendingBreakpointsState = {}, action: Action) { return setBreakpoint(state, action); case "REMOVE_BREAKPOINT": + if (action.status === "start") { + return removeBreakpoint(state, action); + } + return state; + case "REMOVE_PENDING_BREAKPOINT": return removeBreakpoint(state, action); } diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-navigation.js b/devtools/client/debugger/test/mochitest/browser_dbg-navigation.js index 51713b1dda1a..b8439669b897 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-navigation.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-navigation.js @@ -53,5 +53,6 @@ add_task(async function() { await reload(dbg, "long.js"); await waitForSelectedSource(dbg, "long.js"); + await waitForRequestsToSettle(dbg); ok(getSelectedSource().url.includes("long.js"), "Selected source is long.js"); }); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-old-breakpoint.js b/devtools/client/debugger/test/mochitest/browser_dbg-old-breakpoint.js index 29fd7ee8839c..4254aaa578ac 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-old-breakpoint.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-old-breakpoint.js @@ -38,6 +38,7 @@ add_task(async function() { const toolbox = await openNewTabAndToolbox(EXAMPLE_URL + "doc-scripts.html", "jsdebugger"); const dbg = createDebuggerContext(toolbox); + const onBreakpoint = waitForDispatch(dbg, "SET_BREAKPOINT", 2); // Pending breakpoints are installed asynchronously, keep invoking the entry // function until the debugger pauses. @@ -45,12 +46,11 @@ add_task(async function() { invokeInTab("main"); return isPaused(dbg); }); + await onBreakpoint; ok(true, "paused at unmapped breakpoint"); await waitForState(dbg, state => dbg.selectors.getBreakpointCount(state) == 2); ok(true, "unmapped breakpoints shown in UI"); - await waitForRequestsToSettle(dbg); - await toolbox.destroy(); }); // Test that if we show a breakpoint with an old generated location, it is diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-reload.js b/devtools/client/debugger/test/mochitest/browser_dbg-reload.js index dd2828b04cc9..70b667416cbd 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-reload.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-reload.js @@ -25,6 +25,7 @@ add_task(async function() { await selectSource(dbg, "sjs_code_reload"); await addBreakpoint(dbg, "sjs_code_reload", 2); + await waitForRequestsToSettle(dbg); await reload(dbg, "sjs_code_reload.sjs"); await waitForSelectedSource(dbg, "sjs_code_reload.sjs"); @@ -38,4 +39,5 @@ add_task(async function() { is(breakpointList.length, 1); is(breakpoint.location.line, 6); + await waitForRequestsToSettle(dbg); }); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-worker-scopes.js b/devtools/client/debugger/test/mochitest/browser_dbg-worker-scopes.js index b2e225e4f8b5..aeac756fba43 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-worker-scopes.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-worker-scopes.js @@ -43,7 +43,9 @@ add_task(async function() { await dbg.toolbox._target.waitForRequestsToSettle(); invokeInTab("startWorker"); await waitForPaused(dbg, "scopes-worker.js"); + const onRemoved = waitForDispatch(dbg, "REMOVE_BREAKPOINT"); await removeBreakpoint(dbg, workerSource.id, 11); + await onRemoved; // We should be paused at the first line of simple-worker.js assertPausedAtSourceAndLine(dbg, workerSource.id, 11); diff --git a/devtools/client/debugger/test/mochitest/helpers.js b/devtools/client/debugger/test/mochitest/helpers.js index edc32d2cb024..ee443f93fb7e 100644 --- a/devtools/client/debugger/test/mochitest/helpers.js +++ b/devtools/client/debugger/test/mochitest/helpers.js @@ -799,11 +799,13 @@ async function addBreakpoint(dbg, source, line, column, options) { source = findSource(dbg, source); const sourceId = source.id; const bpCount = dbg.selectors.getBreakpointCount(); + const onBreakpoint = waitForDispatch(dbg, "SET_BREAKPOINT"); await dbg.actions.addBreakpoint( getContext(dbg), { sourceId, line, column }, options ); + await onBreakpoint; is( dbg.selectors.getBreakpointCount(), bpCount + 1, diff --git a/devtools/server/tests/unit/test_logpoint-03.js b/devtools/server/tests/unit/test_logpoint-03.js index 92215bb8c61e..f43b9c829e45 100644 --- a/devtools/server/tests/unit/test_logpoint-03.js +++ b/devtools/server/tests/unit/test_logpoint-03.js @@ -44,13 +44,16 @@ function test_simple_breakpoint() { ); // Set a logpoint which should throw an error message. - gThreadClient.setBreakpoint({ + await gThreadClient.setBreakpoint({ sourceUrl: source.url, line: 3, }, { logValue: "c" }); // Execute the rest of the code. - gThreadClient.resume(); + await gThreadClient.resume(); + Assert.equal(lastMessage.level, "logPointError"); + Assert.equal(lastMessage.arguments[0], "[Logpoint threw]: c is not defined"); + finishClient(gClient); }); /* eslint-disable */ @@ -62,8 +65,4 @@ function test_simple_breakpoint() { "test.js", 1); /* eslint-enable */ - - Assert.equal(lastMessage.level, "logPointError"); - Assert.equal(lastMessage.arguments[0], "[Logpoint threw]: c is not defined"); - finishClient(gClient); }