From cdccd0a891caeb0488a4bc0bbcf7c1b1e598bafa Mon Sep 17 00:00:00 2001 From: Jason Laster Date: Wed, 7 Jun 2017 18:06:50 -0400 Subject: [PATCH] Revert breakpoint clearing (#3113) * Revert breakpoint clearing * fix breakpoint regression --- src/actions/breakpoints.js | 60 +++++++++++++------ .../__snapshots__/pending-breakpoints.js.snap | 1 + src/actions/tests/helpers/breakpoints.js | 2 + src/client/firefox/commands.js | 30 ++++++++++ src/reducers/breakpoints.js | 4 -- src/utils/breakpoint/index.js | 4 +- 6 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/actions/breakpoints.js b/src/actions/breakpoints.js index f525de5cf8..32be03777e 100644 --- a/src/actions/breakpoints.js +++ b/src/actions/breakpoints.js @@ -45,6 +45,27 @@ async function _getGeneratedLocation(source, sourceMaps, location) { return await sourceMaps.getGeneratedLocation(location, source.toJS()); } +async function _formatClientBreakpoint(clientBreakpoint, sourceMaps, location) { + const clientOriginalLocation = await sourceMaps.getOriginalLocation( + clientBreakpoint.actualLocation + ); + + // make sure that we are re-adding the same type of breakpoint. Column + // or line + const actualLocation = equalizeLocationColumn( + clientOriginalLocation, + location + ); + + // the generatedLocation might have slid, so now we can adjust it + const generatedLocation = clientBreakpoint.actualLocation; + + const { id, hitCount } = clientBreakpoint; + return { id, actualLocation, hitCount, generatedLocation }; +} + +// we have three forms of syncing: disabled syncing, existing server syncing +// and adding a new breakpoint async function syncClientBreakpoint( sourceId: string, client, @@ -62,16 +83,31 @@ async function syncClientBreakpoint( sourceId: generatedSourceId }; - // early return if breakpoint is disabled with overrides to update - // the id as expected, without talking to server + /** ******* CASE 1: Disabled ***********/ + // early return if breakpoint is disabled, send overrides to update + // the id as expected if (pendingBreakpoint.disabled) { return { id: generatedSourceId, actualLocation: { ...pendingBreakpoint.location, id: sourceId }, - oldGeneratedLocation + generatedLocation: oldGeneratedLocation }; } + /** ******* CASE 2: Merge Server Breakpoint ***********/ + // early return if breakpoint exists on the server, send overrides + // to update the id as expected + const existingClient = client.getBreakpointByLocation(oldGeneratedLocation); + + if (existingClient) { + return _formatClientBreakpoint( + existingClient, + sourceMaps, + pendingBreakpoint.location + ); + } + + /** ******* CASE 3: 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. const clientBreakpoint = await client.setBreakpoint( @@ -80,23 +116,11 @@ async function syncClientBreakpoint( sourceMaps.isOriginalId(sourceId) ); - // Original location is the location in the src file - const clientOriginalLocation = await sourceMaps.getOriginalLocation( - clientBreakpoint.actualLocation - ); - - // make sure that we are re-adding the same type of breakpoint. Column - // or line - const actualLocation = equalizeLocationColumn( - clientOriginalLocation, + return _formatClientBreakpoint( + clientBreakpoint, + sourceMaps, pendingBreakpoint.location ); - - // the generatedLocation might have slid, so now we can adjust it - const generatedLocation = clientBreakpoint.actualLocation; - - const { id, hitCount } = clientBreakpoint; - return { id, actualLocation, hitCount, generatedLocation }; } async function addClientBreakpoint(state, client, sourceMaps, breakpoint) { diff --git a/src/actions/tests/__snapshots__/pending-breakpoints.js.snap b/src/actions/tests/__snapshots__/pending-breakpoints.js.snap index 45c7f699cd..4ae28aed59 100644 --- a/src/actions/tests/__snapshots__/pending-breakpoints.js.snap +++ b/src/actions/tests/__snapshots__/pending-breakpoints.js.snap @@ -13,6 +13,7 @@ Object { "id": "bar.js:7:", "loading": false, "location": Object { + "column": undefined, "line": 7, "sourceId": "bar.js", "sourceUrl": "http://localhost:8000/examples/bar.js", diff --git a/src/actions/tests/helpers/breakpoints.js b/src/actions/tests/helpers/breakpoints.js index d0ed387b34..fad5f775b4 100644 --- a/src/actions/tests/helpers/breakpoints.js +++ b/src/actions/tests/helpers/breakpoints.js @@ -21,6 +21,7 @@ export function mockPendingBreakpoint(overrides = {}) { function generateCorrectingThreadClient(offset = 0) { return { + getBreakpointByLocation: jest.fn(), setBreakpoint: (location, condition) => { const actualLocation = Object.assign({}, location, { line: location.line + offset @@ -60,6 +61,7 @@ export function generateBreakpoint(filename) { } export const simpleMockThreadClient = { + getBreakpointByLocation: jest.fn(), setBreakpoint: (location, _condition) => Promise.resolve({ id: "hi", actualLocation: location }), diff --git a/src/client/firefox/commands.js b/src/client/firefox/commands.js index ca78368d91..e046fee187 100644 --- a/src/client/firefox/commands.js +++ b/src/client/firefox/commands.js @@ -1,4 +1,5 @@ // @flow +import _ from "lodash"; import type { BreakpointId, @@ -75,6 +76,34 @@ function sourceContents(sourceId: SourceId): Source { return sourceClient.source(); } +function getBreakpointByLocation(location: Location) { + const values = _.values(bpClients); + const bpClient = values.find(value => { + const { actor, line, column, condition } = value.location; + return ( + location.line === line && + location.sourceId === actor && + location.column === column && + location.condition === condition + ); + }); + + if (bpClient) { + const { actor, url, line, column, condition } = bpClient.location; + return { + id: bpClient.actor, + actualLocation: { + line, + column, + condition, + sourceId: actor, + sourceUrl: url + } + }; + } + return null; +} + function setBreakpoint( location: Location, condition: boolean, @@ -261,6 +290,7 @@ const clientCommands = { stepOver, breakOnNext, sourceContents, + getBreakpointByLocation, setBreakpoint, removeBreakpoint, setBreakpointCondition, diff --git a/src/reducers/breakpoints.js b/src/reducers/breakpoints.js index 062314404d..8ab2f7962c 100644 --- a/src/reducers/breakpoints.js +++ b/src/reducers/breakpoints.js @@ -90,10 +90,6 @@ function update( setPendingBreakpoints(newState); return newState; } - - case "NAVIGATE": { - return initialState(); - } } return state; diff --git a/src/utils/breakpoint/index.js b/src/utils/breakpoint/index.js index 6576b43bea..a4315c067c 100644 --- a/src/utils/breakpoint/index.js +++ b/src/utils/breakpoint/index.js @@ -34,8 +34,8 @@ export function allBreakpointsDisabled(state) { } // syncing -export function equalizeLocationColumn(location, hasColumn) { - if (hasColumn) { +export function equalizeLocationColumn(location, referenceLocation) { + if (referenceLocation.column) { return location; } return { ...location, column: undefined };