Bug 1534786 - Show pending breakpoints as soon as their generated source appears, r=jlast.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 14 Mar 2019 04:42:47 -1000
changeset 521969 25f9343a6480
parent 521968 56553ce76324
child 521970 49d94c83bb22
child 522000 355c9ff9b895
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1534786
milestone67.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
Bug 1534786 - Show pending breakpoints as soon as their generated source appears, r=jlast.
devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js
devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
devtools/client/debugger/new/src/actions/sources/newSources.js
devtools/client/debugger/new/src/reducers/pending-breakpoints.js
devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
--- a/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/addBreakpoint.js
@@ -11,17 +11,17 @@ import {
   getASTLocation,
   makeBreakpointId,
   makeBreakpointLocation,
   findPosition
 } from "../../utils/breakpoint";
 import { PROMISE } from "../utils/middleware/promise";
 import {
   getSymbols,
-  getFirstVisibleBreakpointPosition,
+  getFirstBreakpointPosition,
   getBreakpointPositionsForSource,
   getSourceFromId
 } from "../../selectors";
 
 import { getTextAtPosition } from "../../utils/source";
 import { recordEvent } from "../../utils/telemetry";
 
 import type {
@@ -100,17 +100,17 @@ export function addBreakpoint(
   options: BreakpointOptions = {}
 ) {
   return async ({ dispatch, getState, sourceMaps, client }: ThunkArgs) => {
     recordEvent("add_breakpoint");
     let position;
     const { sourceId, column } = location;
 
     if (column === undefined) {
-      position = getFirstVisibleBreakpointPosition(getState(), location);
+      position = getFirstBreakpointPosition(getState(), location);
     } else {
       const positions = getBreakpointPositionsForSource(getState(), sourceId);
       position = findPosition(positions, location);
     }
 
     if (!position) {
       return;
     }
--- a/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
+++ b/devtools/client/debugger/new/src/actions/breakpoints/syncBreakpoint.js
@@ -13,17 +13,18 @@ import {
   findPosition,
   makeBreakpointLocation
 } from "../../utils/breakpoint";
 
 import { getTextAtPosition } from "../../utils/source";
 import { comparePosition } from "../../utils/location";
 
 import { originalToGeneratedId, isOriginalId } from "devtools-source-map";
-import { getSource } from "../../selectors";
+import { getSource, getBreakpointsList } from "../../selectors";
+import { removeBreakpoint } from ".";
 
 import type { ThunkArgs, Action } from "../types";
 
 import type {
   SourceLocation,
   ASTLocation,
   PendingBreakpoint,
   SourceId,
@@ -82,24 +83,37 @@ function createSyncData(
     { 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<?BreakpointSyncData> {
-  const { getState, client } = thunkArgs;
+  const { getState, client, dispatch } = thunkArgs;
   assertPendingBreakpoint(pendingBreakpoint);
 
   const source = getSource(getState(), sourceId);
 
   const generatedSourceId = isOriginalId(sourceId)
     ? originalToGeneratedId(sourceId)
     : sourceId;
 
@@ -147,18 +161,21 @@ export async function syncBreakpointProm
       newLocation,
       newGeneratedLocation,
       previousLocation,
       text,
       originalText
     );
   }
 
-  // clear server breakpoints if they exist and we have moved
-  await client.removeBreakpoint(generatedLocation);
+  // Clear any breakpoint for the generated location.
+  const bp = findExistingBreakpoint(getState(), generatedLocation);
+  if (bp) {
+    await dispatch(removeBreakpoint(bp));
+  }
 
   if (!newGeneratedLocation) {
     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.
--- a/devtools/client/debugger/new/src/actions/sources/newSources.js
+++ b/devtools/client/debugger/new/src/actions/sources/newSources.js
@@ -8,17 +8,17 @@
  * Redux actions for the sources state
  * @module actions/sources
  */
 
 import { generatedToOriginalId } from "devtools-source-map";
 import { flatten } from "lodash";
 
 import { toggleBlackBox } from "./blackbox";
-import { syncBreakpoint } from "../breakpoints";
+import { syncBreakpoint, addBreakpoint } from "../breakpoints";
 import { loadSourceText } from "./loadSourceText";
 import { togglePrettyPrint } from "./prettyPrint";
 import { selectLocation } from "../sources";
 import { getRawSourceURL, isPrettyURL, isOriginal } from "../../utils/source";
 import {
   getBlackBoxList,
   getSource,
   getPendingSelectedLocation,
@@ -181,18 +181,29 @@ function checkPendingBreakpoints(sourceI
 
     if (pendingBreakpoints.length === 0) {
       return;
     }
 
     // load the source text if there is a pending breakpoint for it
     await dispatch(loadSourceText(source));
 
+    // Matching pending breakpoints could have either the same generated or the
+    // same original source. We expect the generated source to appear first and
+    // will add a breakpoint at that location initially. If the original source
+    // appears later then we use syncBreakpoint to see if the generated location
+    // changed and we need to remove the breakpoint we added earlier.
     await Promise.all(
-      pendingBreakpoints.map(bp => dispatch(syncBreakpoint(sourceId, bp)))
+      pendingBreakpoints.map(bp => {
+        if (source.url == bp.location.sourceUrl) {
+          return dispatch(syncBreakpoint(sourceId, bp));
+        }
+        const { line, column } = bp.generatedLocation;
+        return dispatch(addBreakpoint({ sourceId, line, column }, bp.options));
+      })
     );
   };
 }
 
 function restoreBlackBoxedSources(sources: Source[]) {
   return async ({ dispatch }: ThunkArgs) => {
     const tabs = getBlackBoxList();
     if (tabs.length == 0) {
--- a/devtools/client/debugger/new/src/reducers/pending-breakpoints.js
+++ b/devtools/client/debugger/new/src/reducers/pending-breakpoints.js
@@ -156,14 +156,17 @@ export function getPendingBreakpointsFor
   source: Source
 ): PendingBreakpoint[] {
   const sources = getSourcesByURL(state, source.url);
   if (sources.length > 1 && isGenerated(source)) {
     // Don't return pending breakpoints for duplicated generated sources
     return [];
   }
 
-  return getPendingBreakpointList(state).filter(
-    pendingBreakpoint => pendingBreakpoint.location.sourceUrl === source.url
-  );
+  return getPendingBreakpointList(state).filter(pendingBreakpoint => {
+    return (
+      pendingBreakpoint.location.sourceUrl === source.url ||
+      pendingBreakpoint.generatedLocation.sourceUrl == source.url
+    );
+  });
 }
 
 export default update;
--- a/devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
+++ b/devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
@@ -168,24 +168,8 @@ export function getFirstBreakpointPositi
   if (!source || !positions) {
     return;
   }
 
   return positions.find(
     position => getSelectedLocation(position, source).line == line
   );
 }
-
-export function getFirstVisibleBreakpointPosition(
-  state: State,
-  { line }: SourceLocation
-) {
-  const positions = getVisibleBreakpointPositions(state);
-  const selectedSource = getSelectedSource(state);
-
-  if (!selectedSource || !positions) {
-    return;
-  }
-
-  return positions.find(
-    position => getSelectedLocation(position, selectedSource).line == line
-  );
-}