diff --git a/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js b/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js deleted file mode 100644 index ab232f99f8ff..000000000000 --- a/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js +++ /dev/null @@ -1,112 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at . */ - -// @flow -import { setBreakpointPositions } from "./breakpointPositions"; -import { - assertBreakpoint, - createBreakpoint, - getASTLocation, - makeBreakpointId, - makeBreakpointLocation -} from "../../utils/breakpoint"; -import { PROMISE } from "../utils/middleware/promise"; -import { - getSymbols, - getFirstBreakpointPosition, - getBreakpointPositionsForLocation, - getSourceFromId -} from "../../selectors"; - -import { getTextAtPosition } from "../../utils/source"; -import { recordEvent } from "../../utils/telemetry"; - -import type { - BreakpointOptions, - Breakpoint, - SourceLocation -} from "../../types"; -import type { ThunkArgs } from "../types"; - -async function addBreakpointPromise(getState, client, sourceMaps, breakpoint) { - const state = getState(); - const { location, generatedLocation } = breakpoint; - const source = getSourceFromId(state, location.sourceId); - const generatedSource = getSourceFromId(state, generatedLocation.sourceId); - - const breakpointLocation = makeBreakpointLocation( - getState(), - generatedLocation - ); - await client.setBreakpoint(breakpointLocation, breakpoint.options); - - const symbols = getSymbols(getState(), source); - const astLocation = await getASTLocation(source, symbols, location); - - const originalText = getTextAtPosition(source, location); - const text = getTextAtPosition(generatedSource, generatedLocation); - - const newBreakpoint = { - id: makeBreakpointId(generatedLocation), - disabled: false, - options: breakpoint.options, - location, - astLocation, - generatedLocation, - text, - originalText - }; - - assertBreakpoint(newBreakpoint); - - return newBreakpoint; -} - -export function addHiddenBreakpoint(location: SourceLocation) { - return ({ dispatch }: ThunkArgs) => { - return dispatch(addBreakpoint(location, { hidden: true })); - }; -} - -export function enableBreakpoint(breakpoint: Breakpoint) { - return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { - // To instantly reflect in the UI, we optimistically enable the breakpoint - const enabledBreakpoint = { ...breakpoint, disabled: false }; - - return dispatch({ - type: "ENABLE_BREAKPOINT", - breakpoint: enabledBreakpoint, - [PROMISE]: addBreakpointPromise(getState, client, sourceMaps, breakpoint) - }); - }; -} - -export function addBreakpoint( - location: SourceLocation, - options: BreakpointOptions = {} -) { - return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => { - recordEvent("add_breakpoint"); - - const { sourceId, column } = location; - - await dispatch(setBreakpointPositions(sourceId)); - - const position = column - ? getBreakpointPositionsForLocation(getState(), location) - : getFirstBreakpointPosition(getState(), location); - - if (!position) { - return; - } - - const breakpoint = createBreakpoint(position, { options }); - - return dispatch({ - type: "ADD_BREAKPOINT", - breakpoint, - [PROMISE]: addBreakpointPromise(getState, client, sourceMaps, breakpoint) - }); - }; -} diff --git a/devtools/client/debugger/new/src/actions/breakpoints/index.js b/devtools/client/debugger/new/src/actions/breakpoints/index.js index 1278dd731830..506f4901ecb3 100644 --- a/devtools/client/debugger/new/src/actions/breakpoints/index.js +++ b/devtools/client/debugger/new/src/actions/breakpoints/index.js @@ -11,7 +11,6 @@ import { PROMISE } from "../utils/middleware/promise"; import { - getBreakpoint, getBreakpointsList, getXHRBreakpoints, getSelectedSource, @@ -21,86 +20,33 @@ import { isEmptyLineInSource, getBreakpointsAtLine } from "../../selectors"; -import { - assertBreakpoint, - createXHRBreakpoint, - makeBreakpointLocation -} from "../../utils/breakpoint"; +import { createXHRBreakpoint } from "../../utils/breakpoint"; import { addBreakpoint, - addHiddenBreakpoint, - enableBreakpoint -} from "./addBreakpoint"; + removeBreakpoint, + enableBreakpoint, + disableBreakpoint +} from "./modify"; import remapLocations from "./remapLocations"; -import { syncBreakpoint } from "./syncBreakpoint"; import { closeConditionalPanel } from "../ui"; // this will need to be changed so that addCLientBreakpoint is removed -import type { ThunkArgs, Action } from "../types"; +import type { ThunkArgs } from "../types"; import type { Breakpoint, - BreakpointOptions, Source, SourceLocation, XHRBreakpoint } from "../../types"; -import { recordEvent } from "../../utils/telemetry"; - export * from "./breakpointPositions"; +export * from "./modify"; +export * from "./syncBreakpoint"; -async function removeBreakpointsPromise(client, state, breakpoint) { - const breakpointLocation = makeBreakpointLocation( - state, - breakpoint.generatedLocation - ); - await client.removeBreakpoint(breakpointLocation); -} - -/** - * Remove a single breakpoint - * - * @memberof actions/breakpoints - * @static - */ -export function removeBreakpoint(breakpoint: Breakpoint) { - return ({ dispatch, getState, client }: ThunkArgs) => { - recordEvent("remove_breakpoint"); - - // If the breakpoint is already disabled, we don't need to communicate - // with the server. We just need to dispatch an action - // simulating a successful server request - if (breakpoint.disabled) { - return dispatch( - ({ type: "REMOVE_BREAKPOINT", breakpoint, status: "done" }: Action) - ); - } - - return dispatch({ - type: "REMOVE_BREAKPOINT", - breakpoint, - disabled: false, - [PROMISE]: removeBreakpointsPromise(client, getState(), breakpoint) - }); - }; -} - -/** - * Disable a single breakpoint - * - * @memberof actions/breakpoints - * @static - */ -export function disableBreakpoint(breakpoint: Breakpoint) { - return async ({ dispatch, getState, client }: ThunkArgs) => { - await removeBreakpointsPromise(client, getState(), breakpoint); - - const newBreakpoint: Breakpoint = { ...breakpoint, disabled: true }; - - return dispatch( - ({ type: "DISABLE_BREAKPOINT", breakpoint: newBreakpoint }: Action) - ); +export function addHiddenBreakpoint(location: SourceLocation) { + return ({ dispatch }: ThunkArgs) => { + return dispatch(addBreakpoint(location, { hidden: true })); }; } @@ -148,34 +94,13 @@ export function toggleAllBreakpoints(shouldDisableBreakpoints: boolean) { return async ({ dispatch, getState, client }: ThunkArgs) => { const breakpoints = getBreakpointsList(getState()); - const modifiedBreakpoints = []; - for (const breakpoint of breakpoints) { if (shouldDisableBreakpoints) { - await removeBreakpointsPromise(client, getState(), breakpoint); - const newBreakpoint: Breakpoint = { ...breakpoint, disabled: true }; - modifiedBreakpoints.push(newBreakpoint); + dispatch(disableBreakpoint(breakpoint)); } else { - const newBreakpoint: Breakpoint = { ...breakpoint, disabled: false }; - modifiedBreakpoints.push(newBreakpoint); + dispatch(enableBreakpoint(breakpoint)); } } - - if (shouldDisableBreakpoints) { - return dispatch( - ({ - type: "DISABLE_ALL_BREAKPOINTS", - breakpoints: modifiedBreakpoints - }: Action) - ); - } - - return dispatch( - ({ - type: "ENABLE_ALL_BREAKPOINTS", - breakpoints: modifiedBreakpoints - }: Action) - ); }; } @@ -262,56 +187,9 @@ export function remapBreakpoints(sourceId: string) { sourceMaps ); - return dispatch( - ({ - type: "REMAP_BREAKPOINTS", - breakpoints: newBreakpoints - }: Action) - ); - }; -} - -/** - * Update the options of a breakpoint. - * - * @throws {Error} "not implemented" - * @memberof actions/breakpoints - * @static - * @param {SourceLocation} location - * @see DebuggerController.Breakpoints.addBreakpoint - * @param {Object} options - * Any options to set on the breakpoint - */ -export function setBreakpointOptions( - location: SourceLocation, - options: BreakpointOptions = {} -) { - return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { - const bp = getBreakpoint(getState(), location); - if (!bp) { - return dispatch(addBreakpoint(location, options)); + for (const bp of newBreakpoints) { + await dispatch(addBreakpoint(bp.location, bp.options, bp.disabled)); } - - if (bp.disabled) { - await dispatch(enableBreakpoint(bp)); - } - - const breakpointLocation = makeBreakpointLocation( - getState(), - bp.generatedLocation - ); - await client.setBreakpoint(breakpointLocation, options); - - const newBreakpoint = { ...bp, disabled: false, options }; - - assertBreakpoint(newBreakpoint); - - return dispatch( - ({ - type: "SET_BREAKPOINT_OPTIONS", - breakpoint: newBreakpoint - }: Action) - ); }; } @@ -514,5 +392,3 @@ export function removeXHRBreakpoint(index: number) { }); }; } - -export { addBreakpoint, addHiddenBreakpoint, enableBreakpoint, syncBreakpoint }; diff --git a/devtools/client/debugger/new/src/actions/breakpoints/modify.js b/devtools/client/debugger/new/src/actions/breakpoints/modify.js new file mode 100644 index 000000000000..c60064e2bbf2 --- /dev/null +++ b/devtools/client/debugger/new/src/actions/breakpoints/modify.js @@ -0,0 +1,262 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at . */ + +// @flow + +import type { ThunkArgs } from "../types"; +import type { + Breakpoint, + BreakpointOptions, + SourceLocation +} from "../../types"; + +import { + makeBreakpointLocation, + makeBreakpointId, + getASTLocation +} from "../../utils/breakpoint"; + +import { getTextAtPosition } from "../../utils/source"; + +import { + getBreakpoint, + getBreakpointPositionsForLocation, + getFirstBreakpointPosition, + getSourceFromId, + getSymbols +} from "../../selectors"; + +import { loadSourceText } from "../sources/loadSourceText"; +import { setBreakpointPositions } from "./breakpointPositions"; + +import { recordEvent } from "../../utils/telemetry"; + +// This file has the primitive operations used to modify individual breakpoints +// and keep them in sync with the breakpoints installed on server threads. These +// are collected here to make it easier to preserve the following invariant: +// +// Breakpoints are included in reducer state iff they are disabled or requests +// have been dispatched to set them in all server threads. +// +// To maintain this property, updates to the reducer and installed breakpoints +// must happen with no intervening await. Using await allows other operations to +// modify the breakpoint state in the interim and potentially cause breakpoint +// state to go out of sync. +// +// The reducer is optimistically updated when users set or remove a breakpoint, +// but it might take a little while before the breakpoints have been set or +// removed in each thread. Once all outstanding requests sent to a thread have +// been processed, the reducer and server threads will be in sync. +// +// There is another exception to the above invariant when first connecting to +// the server: breakpoints have been installed on all generated locations in the +// pending breakpoints, but no breakpoints have been added to the reducer. When +// a matching source appears, either the server breakpoint will be removed or a +// 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 clientRemoveBreakpoint(breakpoint: Breakpoint) { + return ({ getState, client }: ThunkArgs) => { + const breakpointLocation = makeBreakpointLocation( + getState(), + breakpoint.generatedLocation + ); + return client.removeBreakpoint(breakpointLocation); + }; +} + +export function enableBreakpoint(initialBreakpoint: Breakpoint) { + return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { + const breakpoint = getBreakpoint(getState(), initialBreakpoint.location); + if (!breakpoint || !breakpoint.disabled) { + return; + } + + dispatch({ + type: "SET_BREAKPOINT", + breakpoint: { ...breakpoint, disabled: false } + }); + + return dispatch(clientSetBreakpoint(breakpoint)); + }; +} + +export function addBreakpoint( + initialLocation: SourceLocation, + options: BreakpointOptions = {}, + disabled: boolean = false +) { + return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => { + recordEvent("add_breakpoint"); + + const { sourceId, column } = initialLocation; + + await dispatch(setBreakpointPositions(sourceId)); + + const position = column + ? getBreakpointPositionsForLocation(getState(), initialLocation) + : getFirstBreakpointPosition(getState(), initialLocation); + + if (!position) { + return; + } + + const { location, generatedLocation } = position; + + // Both the original and generated sources must be loaded to get the + // breakpoint's text. + await dispatch( + loadSourceText(getSourceFromId(getState(), location.sourceId)) + ); + await dispatch( + loadSourceText(getSourceFromId(getState(), generatedLocation.sourceId)) + ); + + const source = getSourceFromId(getState(), location.sourceId); + const generatedSource = getSourceFromId( + getState(), + generatedLocation.sourceId + ); + + const symbols = getSymbols(getState(), source); + const astLocation = await getASTLocation(source, symbols, location); + + const originalText = getTextAtPosition(source, location); + const text = getTextAtPosition(generatedSource, generatedLocation); + + const id = makeBreakpointId(location); + const breakpoint = { + id, + disabled, + options, + location, + astLocation, + generatedLocation, + text, + originalText + }; + + // There cannot be multiple breakpoints with the same generated location. + // Because a generated location cannot map to multiple original locations, + // the only breakpoints that can map to this generated location have the + // new breakpoint's |location| or |generatedLocation| as their own + // |location|. We will overwrite any breakpoint at |location| with the + // SET_BREAKPOINT action below, but need to manually remove any breakpoint + // at |generatedLocation|. + const generatedId = makeBreakpointId(breakpoint.generatedLocation); + if (id != generatedId && getBreakpoint(getState(), generatedLocation)) { + dispatch({ + type: "REMOVE_BREAKPOINT", + location: generatedLocation + }); + } + + dispatch({ + type: "SET_BREAKPOINT", + breakpoint + }); + + if (disabled) { + // If we just clobbered an enabled breakpoint with a disabled one, we need + // to remove any installed breakpoint in the server. + return dispatch(clientRemoveBreakpoint(breakpoint)); + } + + return dispatch(clientSetBreakpoint(breakpoint)); + }; +} + +/** + * Remove a single breakpoint + * + * @memberof actions/breakpoints + * @static + */ +export function removeBreakpoint(initialBreakpoint: Breakpoint) { + return ({ dispatch, getState, client }: ThunkArgs) => { + recordEvent("remove_breakpoint"); + + const breakpoint = getBreakpoint(getState(), initialBreakpoint.location); + if (!breakpoint) { + return; + } + + dispatch({ + type: "REMOVE_BREAKPOINT", + location: breakpoint.location + }); + + // If the breakpoint is disabled then it is not installed in the server. + if (breakpoint.disabled) { + return; + } + + return dispatch(clientRemoveBreakpoint(breakpoint)); + }; +} + +/** + * Disable a single breakpoint + * + * @memberof actions/breakpoints + * @static + */ +export function disableBreakpoint(initialBreakpoint: Breakpoint) { + return ({ dispatch, getState, client }: ThunkArgs) => { + const breakpoint = getBreakpoint(getState(), initialBreakpoint.location); + if (!breakpoint || breakpoint.disabled) { + return; + } + + dispatch({ + type: "SET_BREAKPOINT", + breakpoint: { ...breakpoint, disabled: true } + }); + + return dispatch(clientRemoveBreakpoint(breakpoint)); + }; +} + +/** + * Update the options of a breakpoint. + * + * @throws {Error} "not implemented" + * @memberof actions/breakpoints + * @static + * @param {SourceLocation} location + * @see DebuggerController.Breakpoints.addBreakpoint + * @param {Object} options + * Any options to set on the breakpoint + */ +export function setBreakpointOptions( + location: SourceLocation, + options: BreakpointOptions = {} +) { + return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { + let breakpoint = getBreakpoint(getState(), location); + if (!breakpoint) { + return dispatch(addBreakpoint(location, options)); + } + + // Note: setting a breakpoint's options implicitly enables it. + breakpoint = { ...breakpoint, disabled: false, options }; + + dispatch({ + type: "SET_BREAKPOINT", + breakpoint + }); + + return dispatch(clientSetBreakpoint(breakpoint)); + }; +} diff --git a/devtools/client/debugger/new/src/actions/breakpoints/moz.build b/devtools/client/debugger/new/src/actions/breakpoints/moz.build index f21ab1db64e7..b71a40ba2866 100644 --- a/devtools/client/debugger/new/src/actions/breakpoints/moz.build +++ b/devtools/client/debugger/new/src/actions/breakpoints/moz.build @@ -8,9 +8,9 @@ DIRS += [ ] CompiledModules( - 'addBreakpoint.js', 'breakpointPositions.js', 'index.js', + 'modify.js', 'remapLocations.js', 'syncBreakpoint.js', ) diff --git a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js index dfc8ef6cc407..432974710d09 100644 --- a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js +++ b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js @@ -6,36 +6,27 @@ import { setBreakpointPositions } from "./breakpointPositions"; import { - createBreakpoint, - assertBreakpoint, assertPendingBreakpoint, findFunctionByName, findPosition, makeBreakpointLocation } from "../../utils/breakpoint"; -import { getTextAtPosition, isInlineScript } from "../../utils/source"; -import { comparePosition } from "../../utils/location"; +import { comparePosition, createLocation } from "../../utils/location"; import { originalToGeneratedId, isOriginalId } from "devtools-source-map"; -import { getSource, getBreakpointsList } from "../../selectors"; -import { removeBreakpoint } from "."; +import { getSource, getBreakpoint } from "../../selectors"; +import { removeBreakpoint, addBreakpoint } from "."; -import type { ThunkArgs, Action } from "../types"; +import type { ThunkArgs } from "../types"; import type { SourceLocation, ASTLocation, PendingBreakpoint, - SourceId, - Breakpoint + SourceId } from "../../types"; -type BreakpointSyncData = { - previousLocation: SourceLocation, - breakpoint: ?Breakpoint -}; - async function findBreakpointPosition( { getState, dispatch }, location: SourceLocation @@ -66,172 +57,123 @@ async function findNewLocation( }; } -function createSyncData( - pendingBreakpoint: PendingBreakpoint, - location: SourceLocation, - generatedLocation: SourceLocation, - previousLocation: SourceLocation, - text: string, - originalText: string -): BreakpointSyncData { - const overrides = { - ...pendingBreakpoint, - text, - originalText - }; - const breakpoint = createBreakpoint( - { generatedLocation, location }, - overrides - ); - - assertBreakpoint(breakpoint); - return { breakpoint, previousLocation }; -} - -// Look for an existing breakpoint at the specified generated location. -function findExistingBreakpoint(state, generatedLocation) { - const breakpoints = getBreakpointsList(state); - - return breakpoints.find(bp => { - return ( - bp.generatedLocation.sourceUrl == generatedLocation.sourceUrl && - bp.generatedLocation.line == generatedLocation.line && - bp.generatedLocation.column == generatedLocation.column - ); - }); -} - -// we have three forms of syncing: disabled syncing, existing server syncing -// and adding a new breakpoint -export async function syncBreakpointPromise( - thunkArgs: ThunkArgs, - sourceId: SourceId, - pendingBreakpoint: PendingBreakpoint -): Promise { - const { getState, client, dispatch } = thunkArgs; - assertPendingBreakpoint(pendingBreakpoint); - - const source = getSource(getState(), sourceId); - - const generatedSourceId = isOriginalId(sourceId) - ? originalToGeneratedId(sourceId) - : sourceId; - - const generatedSource = getSource(getState(), generatedSourceId); - - if (!source || !generatedSource) { - return; - } - - const { location, generatedLocation, astLocation } = pendingBreakpoint; - const previousLocation = { ...location, sourceId }; - - const newLocation = await findNewLocation( - astLocation, - previousLocation, - source - ); - - const newGeneratedLocation = await findBreakpointPosition( - thunkArgs, - newLocation - ); - - const isSameLocation = comparePosition( - generatedLocation, - newGeneratedLocation - ); - - /** ******* CASE 1: No server change ***********/ - // early return if breakpoint is disabled or we are in the sameLocation - if (newGeneratedLocation && (pendingBreakpoint.disabled || isSameLocation)) { - // Make sure the breakpoint is installed on all source actors. - if (!pendingBreakpoint.disabled) { - await client.setBreakpoint( - makeBreakpointLocation(getState(), newGeneratedLocation), - pendingBreakpoint.options - ); - } - - const originalText = getTextAtPosition(source, previousLocation); - const text = getTextAtPosition(generatedSource, newGeneratedLocation); - - return createSyncData( - pendingBreakpoint, - newLocation, - newGeneratedLocation, - previousLocation, - text, - originalText - ); - } - - // Clear any breakpoint for the generated location. - const bp = findExistingBreakpoint(getState(), generatedLocation); - if (bp) { - await dispatch(removeBreakpoint(bp)); - } - - if (!newGeneratedLocation) { - return { - previousLocation, - // We return the original bp here for HTML scripts because there may - // be multiple