[breakpoints] map sources in pane (#7027)

firefox-devtools · Nov 5, 2018 · b7ff373 · b7ff373
1 parent b04eea1
commit b7ff373
Show file tree

Hide file tree

Showing 9 changed files with 122 additions and 167 deletions.
diff --git a/src/actions/breakpoints/tests/__snapshots__/breakpoints.spec.js.snap b/src/actions/breakpoints/tests/__snapshots__/breakpoints.spec.js.snap
@@ -36,31 +36,13 @@ Array [
   Object {
     "breakpoints": Array [
       Object {
-        "astLocation": Object {
-          "name": undefined,
-          "offset": Object {
-            "line": 2,
-            "sourceId": "a",
-            "sourceUrl": "http://localhost:8000/examples/a",
-          },
-        },
         "condition": null,
         "disabled": false,
-        "generatedLocation": Object {
-          "line": 2,
-          "sourceId": "a",
-          "sourceUrl": "http://localhost:8000/examples/a",
-        },
-        "hidden": false,
-        "hitCount": undefined,
-        "id": "hi",
-        "loading": false,
-        "location": Object {
+        "selectedLocation": Object {
           "line": 2,
           "sourceId": "a",
           "sourceUrl": "http://localhost:8000/examples/a",
         },
-        "originalText": "return a",
         "text": "return a",
       },
     ],
@@ -121,31 +103,13 @@ Array [
   Object {
     "breakpoints": Array [
       Object {
-        "astLocation": Object {
-          "name": undefined,
-          "offset": Object {
-            "line": 5,
-            "sourceId": "a",
-            "sourceUrl": "http://localhost:8000/examples/a",
-          },
-        },
         "condition": null,
         "disabled": true,
-        "generatedLocation": Object {
-          "line": 5,
-          "sourceId": "a",
-          "sourceUrl": "http://localhost:8000/examples/a",
-        },
-        "hidden": false,
-        "hitCount": undefined,
-        "id": "hi",
-        "loading": false,
-        "location": Object {
+        "selectedLocation": Object {
           "line": 5,
           "sourceId": "a",
           "sourceUrl": "http://localhost:8000/examples/a",
         },
-        "originalText": "",
         "text": "",
       },
     ],

diff --git a/src/actions/breakpoints/tests/breakpoints.spec.js b/src/actions/breakpoints/tests/breakpoints.spec.js
@@ -33,7 +33,9 @@ describe("breakpoints", () => {
     expect(bps.size).toBe(1);
     expect(bp.location).toEqual(loc1);
     expect(getTelemetryEvents("add_breakpoint")).toHaveLength(1);
-    expect(selectors.getBreakpointSources(getState())).toMatchSnapshot();
+
+    const bpSources = selectors.getBreakpointSources(getState());
+    expect(bpSources).toMatchSnapshot();
   });
 
   it("should not show a breakpoint that does not have text", async () => {

diff --git a/src/components/SecondaryPanes/Breakpoints/Breakpoint.js b/src/components/SecondaryPanes/Breakpoints/Breakpoint.js
@@ -6,7 +6,7 @@
 
 import React, { PureComponent } from "react";
 import { connect } from "react-redux";
-
+import { createSelector } from "reselect";
 import classnames from "classnames";
 
 import actions from "../../../actions";
@@ -15,32 +15,31 @@ import showContextMenu from "./BreakpointsContextMenu";
 import { CloseButton } from "../../shared/Button";
 
 import { getLocationWithoutColumn } from "../../../utils/breakpoint";
-
+import { getSelectedLocation } from "../../../utils/source-maps";
 import { features } from "../../../utils/prefs";
 import { getEditor } from "../../../utils/editor";
-import { isGenerated } from "../../../utils/source";
 
 import type { BreakpointsMap } from "../../../reducers/types";
+import type { FormattedBreakpoint } from "../../../selectors/breakpointSources";
+
+import type { Frame, Source, Location } from "../../../types";
 
-import type {
-  Frame,
-  Source,
-  Breakpoint as BreakpointType,
-  MappedLocation
-} from "../../../types";
+type FormattedFrame = {
+  ...Frame,
+  selectedLocation: Location
+};
 
 import {
   getBreakpoints,
-  getSelectedSource,
-  getTopFrame
+  getSelectedFrame,
+  getSelectedSource
 } from "../../../selectors";
 
 type Props = {
-  breakpoint: BreakpointType,
+  breakpoint: FormattedBreakpoint,
   breakpoints: BreakpointsMap,
-  selectedSource: ?Source,
   source: Source,
-  frame: ?Frame,
+  frame: ?FormattedFrame,
   enableBreakpoint: typeof actions.enableBreakpoint,
   removeBreakpoint: typeof actions.removeBreakpoint,
   removeBreakpoints: typeof actions.removeBreakpoints,
@@ -52,12 +51,6 @@ type Props = {
   selectSpecificLocation: typeof actions.selectSpecificLocation
 };
 
-function getMappedLocation(mappedLocation: MappedLocation, selectedSource) {
-  return selectedSource && isGenerated(selectedSource)
-    ? mappedLocation.generatedLocation
-    : mappedLocation.location;
-}
-
 class Breakpoint extends PureComponent<Props> {
   onContextMenu = e => {
     showContextMenu({ ...this.props, contextMenuEvent: e });
@@ -66,54 +59,46 @@ class Breakpoint extends PureComponent<Props> {
   onDoubleClick = () => {
     const { breakpoint, openConditionalPanel } = this.props;
     if (breakpoint.condition) {
-      openConditionalPanel(breakpoint.location.line);
+      openConditionalPanel(breakpoint.selectedLocation.line);
     }
   };
 
   selectBreakpoint = () => {
     const { breakpoint, selectSpecificLocation } = this.props;
-    selectSpecificLocation(breakpoint.location);
+    selectSpecificLocation(breakpoint.selectedLocation);
   };
 
   removeBreakpoint = event => {
     const { breakpoint, removeBreakpoint } = this.props;
 
     event.stopPropagation();
-    removeBreakpoint(breakpoint.location);
+    removeBreakpoint(breakpoint.selectedLocation);
   };
 
   handleBreakpointCheckbox = () => {
     const { breakpoint, enableBreakpoint, disableBreakpoint } = this.props;
-    if (breakpoint.loading) {
-      return;
-    }
-
     if (breakpoint.disabled) {
-      enableBreakpoint(breakpoint.location);
+      enableBreakpoint(breakpoint.selectedLocation);
     } else {
-      disableBreakpoint(breakpoint.location);
+      disableBreakpoint(breakpoint.selectedLocation);
     }
   };
 
   isCurrentlyPausedAtBreakpoint() {
-    const { frame, breakpoint, selectedSource } = this.props;
+    const { frame, breakpoint } = this.props;
     if (!frame) {
       return false;
     }
 
-    const bpId = getLocationWithoutColumn(
-      getMappedLocation(breakpoint, selectedSource)
-    );
-    const frameId = getLocationWithoutColumn(
-      getMappedLocation(frame, selectedSource)
-    );
+    const bpId = getLocationWithoutColumn(breakpoint.selectedLocation);
+    const frameId = getLocationWithoutColumn(frame.selectedLocation);
 
     return bpId == frameId;
   }
 
   getBreakpointLocation() {
-    const { breakpoint, source, selectedSource } = this.props;
-    const { column, line } = getMappedLocation(breakpoint, selectedSource);
+    const { breakpoint, source } = this.props;
+    const { column, line } = breakpoint.selectedLocation;
 
     const isWasm = source && source.isWasm;
     const columnVal = features.columnBreakpoints && column ? `:${column}` : "";
@@ -125,18 +110,8 @@ class Breakpoint extends PureComponent<Props> {
   }
 
   getBreakpointText() {
-    const { selectedSource, breakpoint } = this.props;
-    const { condition } = breakpoint;
-
-    if (condition) {
-      return condition;
-    }
-
-    if (selectedSource && isGenerated(selectedSource)) {
-      return breakpoint.text;
-    }
-
-    return breakpoint.originalText;
+    const { breakpoint } = this.props;
+    return breakpoint.condition || breakpoint.text;
   }
 
   highlightText() {
@@ -192,10 +167,24 @@ class Breakpoint extends PureComponent<Props> {
   }
 }
 
+const getFormattedFrame = createSelector(
+  getSelectedSource,
+  getSelectedFrame,
+  (selectedSource: Source, frame: Frame) => {
+    if (!frame) {
+      return null;
+    }
+
+    return {
+      ...frame,
+      selectedLocation: getSelectedLocation(frame, selectedSource)
+    };
+  }
+);
+
 const mapStateToProps = state => ({
   breakpoints: getBreakpoints(state),
-  frame: getTopFrame(state),
-  selectedSource: getSelectedSource(state)
+  frame: getFormattedFrame(state)
 });
 
 export default connect(

diff --git a/src/components/SecondaryPanes/Breakpoints/BreakpointsContextMenu.js b/src/components/SecondaryPanes/Breakpoints/BreakpointsContextMenu.js
@@ -89,7 +89,7 @@ export default function showContextMenu(props) {
     label: deleteSelfLabel,
     accesskey: deleteSelfKey,
     disabled: false,
-    click: () => removeBreakpoint(breakpoint.location)
+    click: () => removeBreakpoint(breakpoint.selectedLocation)
   };
 
   const deleteAllItem = {
@@ -113,7 +113,7 @@ export default function showContextMenu(props) {
     label: enableSelfLabel,
     accesskey: enableSelfKey,
     disabled: false,
-    click: () => toggleDisabledBreakpoint(breakpoint.location.line)
+    click: () => toggleDisabledBreakpoint(breakpoint.selectedLocation.line)
   };
 
   const enableAllItem = {
@@ -137,7 +137,7 @@ export default function showContextMenu(props) {
     label: disableSelfLabel,
     accesskey: disableSelfKey,
     disabled: false,
-    click: () => toggleDisabledBreakpoint(breakpoint.location.line)
+    click: () => toggleDisabledBreakpoint(breakpoint.selectedLocation.line)
   };
 
   const disableAllItem = {
@@ -160,16 +160,16 @@ export default function showContextMenu(props) {
     label: removeConditionLabel,
     accesskey: removeConditionKey,
     disabled: false,
-    click: () => setBreakpointCondition(breakpoint.location)
+    click: () => setBreakpointCondition(breakpoint.selectedLocation)
   };
 
   const addConditionItem = {
     id: "node-menu-add-condition",
     label: addConditionLabel,
     accesskey: addConditionKey,
     click: () => {
-      selectSpecificLocation(breakpoint.location);
-      openConditionalPanel(breakpoint.location.line);
+      selectSpecificLocation(breakpoint.selectedLocation);
+      openConditionalPanel(breakpoint.selectedLocation.line);
     }
   };
 
@@ -178,8 +178,8 @@ export default function showContextMenu(props) {
     label: editConditionLabel,
     accesskey: editConditionKey,
     click: () => {
-      selectSpecificLocation(breakpoint.location);
-      openConditionalPanel(breakpoint.location.line);
+      selectSpecificLocation(breakpoint.selectedLocation);
+      openConditionalPanel(breakpoint.selectedLocation.line);
     }
   };
 

diff --git a/src/components/SecondaryPanes/Breakpoints/index.js b/src/components/SecondaryPanes/Breakpoints/index.js
@@ -104,7 +104,7 @@ class Breakpoints extends Component<Props> {
             <Breakpoint
               breakpoint={breakpoint}
               source={source}
-              key={makeLocationId(breakpoint.location)}
+              key={makeLocationId(breakpoint.selectedLocation)}
             />
           ))
         ];

diff --git a/src/components/SecondaryPanes/Breakpoints/tests/Breakpoint.spec.js b/src/components/SecondaryPanes/Breakpoints/tests/Breakpoint.spec.js
@@ -6,7 +6,7 @@ import React from "react";
 import { shallow } from "enzyme";
 
 import Breakpoint from "../Breakpoint";
-import { makeSource, makeOriginalSource } from "../../../../utils/test-head";
+import { makeSource } from "../../../../utils/test-head";
 
 describe("Breakpoint", () => {
   it("simple", () => {
@@ -21,39 +21,34 @@ describe("Breakpoint", () => {
     expect(component).toMatchSnapshot();
   });
 
-  it("selected source is original", () => {
-    const { component } = render({
-      source: makeOriginalSource("foo"),
-      selectedSource: makeOriginalSource("foo")
-    });
-    expect(component).toMatchSnapshot();
-  });
-
   it("paused at a generatedLocation", () => {
     const { component } = render({
-      frame: { generatedLocation, location }
+      frame: { selectedLocation: generatedLocation }
     });
     expect(component).toMatchSnapshot();
   });
 
   it("paused at an original location", () => {
     const { component } = render({
-      frame: { location, generatedLocation },
-      selectedSource: makeOriginalSource("foo")
+      frame: { selectedLocation: location },
+      breakpoint: { selectedLocation: location }
     });
+
     expect(component).toMatchSnapshot();
   });
 
   it("paused at a different", () => {
     const { component } = render({
-      frame: { location, generatedLocation: { ...generatedLocation, line: 14 } }
+      frame: { selectedLocation: { ...generatedLocation, line: 14 } }
     });
     expect(component).toMatchSnapshot();
   });
 });
 
 const generatedLocation = { sourceId: "foo", line: 53, column: 73 };
 const location = { sourceId: "foo/original", line: 5, column: 7 };
+const selectedLocation = generatedLocation;
+
 function render(overrides = {}) {
   const props = generateDefaults(overrides);
   const component = shallow(<Breakpoint.WrappedComponent {...props} />);
@@ -65,16 +60,15 @@ function render(overrides = {}) {
 
 function makeBreakpoint(overrides = {}) {
   return {
-    generatedLocation,
-    location,
+    selectedLocation,
     disabled: false,
     ...overrides
   };
 }
 
-function generateDefaults(overrides) {
+function generateDefaults(overrides = { ...overrides, breakpoint: {} }) {
   const source = makeSource("foo");
-  const breakpoint = makeBreakpoint();
+  const breakpoint = makeBreakpoint(overrides.breakpoint);
   const selectedSource = makeSource("foo");
   return {
     source,