Revert breakpoint clearing (#3113)

* Revert breakpoint clearing

* fix breakpoint regression

Author: jasonLaster

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) {

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",

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 }),
 

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,

src/reducers/breakpoints.js

@@ -90,10 +90,6 @@ function update(
       setPendingBreakpoints(newState);
       return newState;
     }
-
-    case "NAVIGATE": {
-      return initialState();
-    }
   }
 
   return state;

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 };