author | Jason Laster <jason.laster.11@gmail.com> |
Wed, 06 Feb 2019 14:35:21 -0800 | |
changeset 457718 | dbe151dc94dfcc2ff2ea1a98f4a1ef6ad4cd8acd |
parent 457717 | 6886438cfe00c633c14480173e1747e59319cb1d |
child 457719 | 74fbb77838c94a4de0a3ed5602bce8dbe298142d |
push id | 35518 |
push user | opoprus@mozilla.com |
push date | Fri, 08 Feb 2019 09:55:14 +0000 |
treeherder | mozilla-central@3a3e393396f4 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | dwalsh |
bugs | 1525757 |
milestone | 67.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
|
--- a/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js +++ b/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js @@ -15,20 +15,23 @@ import { makeBreakpointId, makeSourceActorLocation } from "../../utils/breakpoint"; import { PROMISE } from "../utils/middleware/promise"; import { getSource, getSourceActors, getSymbols, + getFirstVisibleBreakpointPosition } from "../../selectors"; import { getGeneratedLocation } from "../../utils/source-maps"; import { getTextAtPosition } from "../../utils/source"; import { recordEvent } from "../../utils/telemetry"; +import { features } from "../../utils/prefs"; +import { setBreakpointPositions } from "./breakpointPositions"; import type { BreakpointOptions, Breakpoint, SourceLocation } from "../../types"; import type { ThunkArgs } from "../types"; @@ -139,32 +142,37 @@ export function enableBreakpoint(breakpo return dispatch({ type: "ENABLE_BREAKPOINT", breakpoint: enabledBreakpoint, [PROMISE]: addBreakpointPromise(getState, client, sourceMaps, breakpoint) }); }; } -/** - * Add a new breakpoint - * - * @memberof actions/breakpoints - * @static - * @param {BreakpointOptions} options Any options for the new breakpoint. - */ - export function addBreakpoint( location: SourceLocation, options: BreakpointOptions = {} ) { - return ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => { + return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => { recordEvent("add_breakpoint"); + let breakpointPosition = location; - const breakpoint = createBreakpoint(location, options); + if (features.columnBreakpoints && location.column === undefined) { + await dispatch(setBreakpointPositions(location)); + breakpointPosition = getFirstVisibleBreakpointPosition( + getState(), + location + ); + } + + if (!breakpointPosition) { + return; + } + + const breakpoint = createBreakpoint(breakpointPosition, options); return dispatch({ type: "ADD_BREAKPOINT", breakpoint, [PROMISE]: addBreakpointPromise(getState, client, sourceMaps, breakpoint) }); }; }
new file mode 100644 --- /dev/null +++ b/devtools/client/debugger/new/src/actions/breakpoints/breakpointPositions.js @@ -0,0 +1,32 @@ +/* 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 <http://mozilla.org/MPL/2.0/>. */ + +import { + getSourceActors, + getBreakpointPositionsForLine +} from "../../selectors"; + +import { makeSourceActorLocation } from "../../utils/breakpoint"; + +export function setBreakpointPositions(location) { + return async ({ getState, dispatch, client }: ThunkArgs) => { + if ( + getBreakpointPositionsForLine( + getState(), + location.sourceId, + location.line + ) + ) { + return; + } + + const sourceActors = getSourceActors(getState(), location.sourceId); + const sourceActor = sourceActors[0]; + + const sourceActorLocation = makeSourceActorLocation(sourceActor, location); + const positions = await client.getBreakpointPositions(sourceActorLocation); + + return dispatch({ type: "ADD_BREAKPOINT_POSITIONS", positions, location }); + }; +}
--- a/devtools/client/debugger/new/src/actions/breakpoints/moz.build +++ b/devtools/client/debugger/new/src/actions/breakpoints/moz.build @@ -4,12 +4,13 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. DIRS += [ ] DebuggerModules( 'addBreakpoint.js', + 'breakpointPositions.js', 'index.js', 'remapLocations.js', 'syncBreakpoint.js', )
--- a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js +++ b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js @@ -1,26 +1,30 @@ /* 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 <http://mozilla.org/MPL/2.0/>. */ // @flow + +import { setBreakpointPositions } from "./breakpointPositions"; import { locationMoved, createBreakpoint, assertBreakpoint, assertPendingBreakpoint, findScopeByName, makeSourceActorLocation } from "../../utils/breakpoint"; import { getGeneratedLocation } from "../../utils/source-maps"; import { getTextAtPosition } from "../../utils/source"; import { originalToGeneratedId, isOriginalId } from "devtools-source-map"; import { getSource, getSourceActors } from "../../selectors"; +import { features } from "../../utils/prefs"; + import type { ThunkArgs, Action } from "../types"; import type { SourceLocation, ASTLocation, PendingBreakpoint, SourceId, Breakpoint @@ -70,16 +74,17 @@ function createSyncData( } // we have three forms of syncing: disabled syncing, existing server syncing // and adding a new breakpoint export async function syncBreakpointPromise( getState: Function, client: Object, sourceMaps: Object, + dispatch: Function, sourceId: SourceId, pendingBreakpoint: PendingBreakpoint ): Promise<BreakpointSyncData | null> { assertPendingBreakpoint(pendingBreakpoint); const source = getSource(getState(), sourceId); const generatedSourceId = isOriginalId(sourceId) @@ -116,20 +121,29 @@ export async function syncBreakpointProm }; const isSameLocation = !locationMoved( generatedLocation, scopedGeneratedLocation ); const sourceActors = getSourceActors(getState(), sourceId); + let possiblePosition = true; + if (features.columnBreakpoints && generatedLocation.column != undefined) { + const { positions } = await dispatch( + setBreakpointPositions(generatedLocation) + ); + if (!positions.includes(generatedLocation.column)) { + possiblePosition = false; + } + } /** ******* CASE 1: No server change ***********/ // early return if breakpoint is disabled or we are in the sameLocation - if (pendingBreakpoint.disabled || isSameLocation) { + if (possiblePosition && (pendingBreakpoint.disabled || isSameLocation)) { // Make sure the breakpoint is installed on all source actors. if (!pendingBreakpoint.disabled) { for (const sourceActor of sourceActors) { const sourceActorLocation = makeSourceActorLocation( sourceActor, generatedLocation ); if (!client.getBreakpointByLocation(sourceActorLocation)) { @@ -161,24 +175,24 @@ export async function syncBreakpointProm sourceActor, generatedLocation ); if (client.getBreakpointByLocation(sourceActorLocation)) { await client.removeBreakpoint(sourceActorLocation); } } + if (!possiblePosition || !scopedGeneratedLocation.line) { + return { previousLocation, breakpoint: null }; + } + /** ******* Case 2: Add New Breakpoint ***********/ // If we are not disabled, set the breakpoint on the server and get // that info so we can set it on our breakpoints. - if (!scopedGeneratedLocation.line) { - return { previousLocation, breakpoint: null }; - } - const newGeneratedLocation = { ...scopedGeneratedLocation }; for (const sourceActor of sourceActors) { const sourceActorLocation = makeSourceActorLocation( sourceActor, scopedGeneratedLocation ); const { actualLocation } = await client.setBreakpoint( sourceActorLocation, @@ -221,16 +235,17 @@ export function syncBreakpoint( sourceId: SourceId, pendingBreakpoint: PendingBreakpoint ) { return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { const response = await syncBreakpointPromise( getState, client, sourceMaps, + dispatch, sourceId, pendingBreakpoint ); if (!response) { return; }
--- a/devtools/client/debugger/new/src/actions/breakpoints/tests/syncing.spec.js +++ b/devtools/client/debugger/new/src/actions/breakpoints/tests/syncing.spec.js @@ -130,16 +130,17 @@ describe("loading the debugger", () => { await dispatch(actions.newSource(reloadedSource)); expect(selectors.getBreakpointCount(getState())).toEqual(0); // manually sync const update = await syncBreakpointPromise( getState, threadClient, sourceMaps, + dispatch, reloadedSource.source.id, pendingBreakpoint() ); expect(threadClient.removeBreakpoint.mock.calls).toHaveLength(0); expect(update).toMatchSnapshot(); }); @@ -160,16 +161,17 @@ describe("loading the debugger", () => { await dispatch(actions.newSource(reloadedSource)); expect(selectors.getBreakpointCount(getState())).toEqual(0); // manually sync const update = await syncBreakpointPromise( getState, threadClient, sourceMaps, + dispatch, reloadedSource.source.id, pendingBreakpoint() ); expect(threadClient.removeBreakpoint.mock.calls).toHaveLength(0); expect(update && update.breakpoint && update.breakpoint.location.line).toBe( location.line + generated ); @@ -204,16 +206,17 @@ describe("reloading debuggee", () => { await dispatch(actions.newSource(generatedSource)); await dispatch(actions.addBreakpoint(loc1)); // manually sync const update = await syncBreakpointPromise( getState, threadClient, sourceMaps, + dispatch, reloadedSource.source.id, pendingBreakpoint({ location: loc1 }) ); expect(threadClient.removeBreakpoint.mock.calls).toHaveLength(0); expect(update).toMatchSnapshot(); }); it("updates a corresponding breakpoint for a changed source", async () => { @@ -247,16 +250,17 @@ describe("reloading debuggee", () => { const generatedSource = makeSource("gen.js"); await dispatch(actions.newSource(generatedSource)); // manually sync const update = await syncBreakpointPromise( getState, threadClient, sourceMaps, + dispatch, reloadedSource.source.id, pendingBreakpoint() ); expect(threadClient.removeBreakpoint.mock.calls).toHaveLength(1); expect(findScopeByName).toHaveBeenCalled(); expect(update).toMatchSnapshot(); });
--- a/devtools/client/debugger/new/src/actions/types/BreakpointAction.js +++ b/devtools/client/debugger/new/src/actions/types/BreakpointAction.js @@ -1,15 +1,20 @@ /* 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 <http://mozilla.org/MPL/2.0/>. */ // @flow -import type { Breakpoint, SourceLocation, XHRBreakpoint } from "../../types"; +import type { + Breakpoint, + SourceLocation, + XHRBreakpoint, + BreakpointLinePositions +} from "../../types"; import type { PromiseAction } from "../utils/middleware/promise"; type AddBreakpointResult = { previousLocation: SourceLocation, breakpoint: Breakpoint }; @@ -86,9 +91,14 @@ export type BreakpointAction = |} | {| +type: "ENABLE_ALL_BREAKPOINTS", +breakpoints: Breakpoint[] |} | {| +type: "REMAP_BREAKPOINTS", +breakpoints: Breakpoint[] + |} + | {| + type: "ADD_BREAKPOINT_POSITIONS", + positions: BreakpointLinePositions, + location: SourceLocation |};
--- a/devtools/client/debugger/new/src/client/firefox/commands.js +++ b/devtools/client/debugger/new/src/client/firefox/commands.js @@ -451,16 +451,33 @@ async function fetchWorkers(): Promise<W const { workers } = await tabTarget.listWorkers(); return workers; } function getMainThread() { return threadClient.actor; } +async function getBreakpointPositions( + location: SourceActorLocation +): Promise<Array<Number>> { + const { + sourceActor: { thread, actor }, + line + } = location; + const sourceThreadClient = lookupThreadClient(thread); + const sourceClient = sourceThreadClient.source({ actor }); + const { positions } = await sourceClient.getBreakpointPositionsCompressed({ + start: { line }, + end: { line } + }); + + return positions ? positions[line] : []; +} + const clientCommands = { autocomplete, blackBox, createObjectClient, releaseActor, interrupt, eventListeners, pauseGrip, @@ -471,16 +488,17 @@ const clientCommands = { rewind, reverseStepIn, reverseStepOut, reverseStepOver, breakOnNext, sourceContents, getSourceForActor, getBreakpointByLocation, + getBreakpointPositions, setBreakpoint, setXHRBreakpoint, removeXHRBreakpoint, removeBreakpoint, setBreakpointOptions, evaluate, evaluateInFrame, evaluateExpressions,
--- a/devtools/client/debugger/new/src/client/firefox/types.js +++ b/devtools/client/debugger/new/src/client/firefox/types.js @@ -328,16 +328,20 @@ export type SourceClient = { _activeThread: ThreadClient, actor: string, setBreakpoint: ({ line: number, column: ?number, condition: ?string, noSliding: boolean }) => Promise<BreakpointResponse>, + getBreakpointPositionsCompressed: (range: { + start: { line: number }, + end: { line: number } + }) => Promise<any>, prettyPrint: number => Promise<*>, disablePrettyPrint: () => Promise<*>, blackBox: (range?: Range) => Promise<*>, unblackBox: (range?: Range) => Promise<*> }; /** * ObjectClient
--- a/devtools/client/debugger/new/src/reducers/ast.js +++ b/devtools/client/debugger/new/src/reducers/ast.js @@ -227,40 +227,16 @@ export function getPausePoint( for (const point of pausePoints) { const { location: pointLocation } = point; if (pointLocation.line == line && pointLocation.column == column) { return point; } } } -export function getFirstPausePointLocation( - state: OuterState, - location: SourceLocation -): SourceLocation { - const { sourceId } = location; - const pausePoints = getPausePoints(state, location.sourceId); - if (!pausePoints) { - return location; - } - - const pausesAtLine = pausePoints.filter( - point => point.location.line == location.line - ); - - if (pausesAtLine) { - const values: PausePoint[] = (Object.values(pausesAtLine): any); - const firstPausePoint = values.find(pausePoint => pausePoint.types.break); - if (firstPausePoint) { - return { ...firstPausePoint.location, sourceId }; - } - } - return location; -} - export function hasPausePoints(state: OuterState, sourceId: string): boolean { const pausePoints = getPausePoints(state, sourceId); return !!pausePoints; } export function getOutOfScopeLocations(state: OuterState) { return state.ast.get("outOfScopeLocations"); }
--- a/devtools/client/debugger/new/src/reducers/breakpoints.js +++ b/devtools/client/debugger/new/src/reducers/breakpoints.js @@ -13,34 +13,40 @@ import { isGeneratedId } from "devtools- import { isEqual } from "lodash"; import { makeBreakpointId } from "../utils/breakpoint"; import type { XHRBreakpoint, Breakpoint, BreakpointId, - SourceLocation + SourceLocation, + BreakpointPositions, + BreakpointLinePositions, + BreakpointSourcePositions } from "../types"; import type { Action, DonePromiseAction } from "../actions/types"; export type BreakpointsMap = { [BreakpointId]: Breakpoint }; export type XHRBreakpointsList = $ReadOnlyArray<XHRBreakpoint>; export type BreakpointsState = { breakpoints: BreakpointsMap, - xhrBreakpoints: XHRBreakpointsList + breakpointPositions: BreakpointPositions, + xhrBreakpoints: XHRBreakpointsList, + breakpointsDisabled: boolean }; export function initialBreakpointsState( xhrBreakpoints?: XHRBreakpointsList = [] ): BreakpointsState { return { breakpoints: {}, xhrBreakpoints: xhrBreakpoints, + breakpointPositions: {}, breakpointsDisabled: false }; } function update( state: BreakpointsState = initialBreakpointsState(), action: Action ): BreakpointsState { @@ -99,16 +105,31 @@ function update( case "ENABLE_XHR_BREAKPOINT": { return updateXHRBreakpoint(state, action); } case "DISABLE_XHR_BREAKPOINT": { return updateXHRBreakpoint(state, action); } + + case "ADD_BREAKPOINT_POSITIONS": { + const { + location: { sourceId, line }, + positions + } = action; + const sourcePositions = state.breakpointPositions[sourceId] || {}; + return { + ...state, + breakpointPositions: { + ...state.breakpointPositions, + [sourceId]: { ...sourcePositions, [line]: positions } + } + }; + } } return state; } function addXHRBreakpoint(state, action) { const { xhrBreakpoints } = state; const { breakpoint } = action; @@ -326,9 +347,32 @@ export function getBreakpointForLocation }); } export function getHiddenBreakpoint(state: OuterState): ?Breakpoint { const breakpoints = getBreakpointsList(state); return breakpoints.find(bp => bp.options.hidden); } +export function getBreakpointPositions( + state: OuterState +): ?BreakpointPositions { + return state.breakpoints.breakpointPositions; +} + +export function getBreakpointPositionsForSource( + state: OuterState, + sourceId: string +): ?BreakpointSourcePositions { + const positions = getBreakpointPositions(state); + return positions && positions[sourceId]; +} + +export function getBreakpointPositionsForLine( + state: OuterState, + sourceId: string, + line: number +): ?BreakpointLinePositions { + const positions = getBreakpointPositionsForSource(state, sourceId); + return positions ? positions[line] : null; +} + export default update;
--- a/devtools/client/debugger/new/src/selectors/index.js +++ b/devtools/client/debugger/new/src/selectors/index.js @@ -25,17 +25,18 @@ export { } from "../reducers/quick-open"; export { getBreakpointAtLocation, getBreakpointsAtLine } from "./breakpointAtLocation"; export { getVisibleBreakpoints, - getFirstVisibleBreakpoints + getFirstVisibleBreakpoints, + getFirstVisibleBreakpointPosition } from "./visibleBreakpoints"; export { inComponent } from "./inComponent"; export { isSelectedFrameVisible } from "./isSelectedFrameVisible"; export { getCallStackFrames } from "./getCallStackFrames"; export { getVisibleSelectedFrame } from "./visibleSelectedFrame"; export { getBreakpointSources } from "./breakpointSources"; export { getXHRBreakpoints, shouldPauseOnAnyXHR } from "./breakpoints"; export { visibleColumnBreakpoints } from "./visibleColumnBreakpoints";
--- a/devtools/client/debugger/new/src/selectors/visibleBreakpoints.js +++ b/devtools/client/debugger/new/src/selectors/visibleBreakpoints.js @@ -2,48 +2,90 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */ // @flow import { createSelector } from "reselect"; import { uniqBy } from "lodash"; -import { getBreakpointsList } from "../reducers/breakpoints"; +import { + getBreakpointsList, + getBreakpointPositionsForLine, + getBreakpointPositions +} from "../reducers/breakpoints"; +import { getPausePoints } from "../reducers/ast"; import { getSelectedSource } from "../reducers/sources"; import { sortBreakpoints } from "../utils/breakpoint"; import { getSelectedLocation } from "../utils/source-maps"; -import type { Breakpoint, Source } from "../types"; -import type { Selector } from "../reducers/types"; +import type { + Breakpoint, + Source, + SourceLocation, + BreakpointPositions +} from "../types"; +import type { Selector, State } from "../reducers/types"; function isVisible(breakpoint: Breakpoint, selectedSource: Source) { const location = getSelectedLocation(breakpoint, selectedSource); return location.sourceId === selectedSource.id; } /* * Finds the breakpoints, which appear in the selected source. */ export const getVisibleBreakpoints: Selector<?(Breakpoint[])> = createSelector( getSelectedSource, getBreakpointsList, - (selectedSource: ?Source, breakpoints: Breakpoint[]) => { + getBreakpointPositions, + ( + selectedSource: ?Source, + breakpoints: Breakpoint[], + positions: ?BreakpointPositions + ) => { if (selectedSource == null) { return null; } // FIXME: Even though selectedSource is checked above, it fails type // checking for isVisible const source: Source = selectedSource; return breakpoints.filter(bp => isVisible(bp, source)); } ); +export function getFirstVisibleBreakpointPosition( + state: State, + location: SourceLocation +): ?SourceLocation { + const { sourceId, line } = location; + const pausePoints = getPausePoints(state, location.sourceId); + const positions = getBreakpointPositionsForLine(state, sourceId, line); + + if (!pausePoints || !positions) { + return null; + } + + const pausesAtLine = pausePoints.filter( + p => p.location.line == line && positions.includes(p.location.column) + ); + + if (pausesAtLine.length > 0) { + const firstPausePoint = pausesAtLine.find( + pausePoint => pausePoint.types.break + ); + if (firstPausePoint) { + return { ...firstPausePoint.location, sourceId }; + } + } + return location; +} + /* * Finds the first breakpoint per line, which appear in the selected source. */ export const getFirstVisibleBreakpoints: Selector< Breakpoint[] > = createSelector(getVisibleBreakpoints, breakpoints => { if (!breakpoints) { return [];
--- a/devtools/client/debugger/new/src/types.js +++ b/devtools/client/debugger/new/src/types.js @@ -436,8 +436,13 @@ export type WorkerList = Array<Worker>; export type Cancellable = { cancel: () => void }; export type EventListenerBreakpoints = string[]; export type SourceDocuments = { [string]: Object }; + +export type BreakpointPosition = { line: number, string: number }; +export type BreakpointLinePositions = Array<BreakpointPosition>; +export type BreakpointSourcePositions = { [number]: BreakpointLinePositions }; +export type BreakpointPositions = { [string]: BreakpointSourcePositions };
--- a/devtools/client/debugger/new/test/mochitest/helpers.js +++ b/devtools/client/debugger/new/test/mochitest/helpers.js @@ -762,20 +762,23 @@ function findBreakpoint(dbg, url, line) selectors: { getBreakpoint }, getState } = dbg; const source = findSource(dbg, url); let column; if ( Services.prefs.getBoolPref("devtools.debugger.features.column-breakpoints") ) { - column = dbg.selectors.getFirstPausePointLocation(dbg.store.getState(), { - sourceId: source.id, - line - }).column; + ({ column } = dbg.selectors.getFirstVisibleBreakpointPosition( + dbg.store.getState(), + { + sourceId: source.id, + line + } + )); } return getBreakpoint(getState(), { sourceId: source.id, line, column }); } async function loadAndAddBreakpoint(dbg, filename, line, column) { const { selectors: { getBreakpoint, getBreakpointCount, getBreakpointsMap }, getState