Bug 1518613 - [release 118] [Breakpoints] Fix timing issue with showi…

…ng breakpoints (#7365). r=dwalsh
dothq · Jan 9, 2019 · 4e91dc3 · 4e91dc3
1 parent f20f95b
commit 4e91dc3
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 35 deletions.
diff --git a/devtools/client/debugger/new/src/components/Editor/Breakpoint.js b/devtools/client/debugger/new/src/components/Editor/Breakpoint.js
@@ -52,12 +52,9 @@ class Breakpoint extends PureComponent<Props> {
 
     const sourceId = selectedSource.id;
     const line = toEditorLine(sourceId, breakpoint.location.line);
+    const doc = getDocument(sourceId);
 
-    editor.codeMirror.setGutterMarker(
-      line,
-      "breakpoints",
-      makeMarker(breakpoint.disabled)
-    );
+    doc.setGutterMarker(line, "breakpoints", makeMarker(breakpoint.disabled));
 
     editor.codeMirror.addLineClass(line, "line", "new-breakpoint");
     if (breakpoint.condition) {
@@ -80,31 +77,21 @@ class Breakpoint extends PureComponent<Props> {
   }
 
   componentWillUnmount() {
-    const { editor, breakpoint, selectedSource } = this.props;
-
-    if (!selectedSource) {
-      return;
-    }
-
-    if (breakpoint.loading) {
+    const { breakpoint, selectedSource } = this.props;
+    if (!selectedSource || breakpoint.loading) {
       return;
     }
 
     const sourceId = selectedSource.id;
     const doc = getDocument(sourceId);
+
     if (!doc) {
       return;
     }
 
     const line = toEditorLine(sourceId, breakpoint.location.line);
 
-    // NOTE: when we upgrade codemirror we can use `doc.setGutterMarker`
-    if (doc.setGutterMarker) {
-      doc.setGutterMarker(line, "breakpoints", null);
-    } else {
-      editor.codeMirror.setGutterMarker(line, "breakpoints", null);
-    }
-
+    doc.setGutterMarker(line, "breakpoints", null);
     doc.removeLineClass(line, "line", "new-breakpoint");
     doc.removeLineClass(line, "line", "has-condition");
   }

diff --git a/devtools/client/debugger/new/src/components/Editor/Breakpoints.js b/devtools/client/debugger/new/src/components/Editor/Breakpoints.js
@@ -8,7 +8,6 @@ import Breakpoint from "./Breakpoint";
 
 import { getSelectedSource, getFirstVisibleBreakpoints } from "../../selectors";
 import { makeLocationId } from "../../utils/breakpoint";
-import { isLoaded } from "../../utils/source";
 import { connect } from "../../utils/connect";
 
 import type { Breakpoint as BreakpointType, Source } from "../../types";
@@ -20,18 +19,10 @@ type Props = {
 };
 
 class Breakpoints extends Component<Props> {
-  shouldComponentUpdate(nextProps: Props) {
-    if (nextProps.selectedSource && !isLoaded(nextProps.selectedSource)) {
-      return false;
-    }
-
-    return true;
-  }
-
   render() {
     const { breakpoints, selectedSource, editor } = this.props;
 
-    if (!selectedSource || !breakpoints || selectedSource.isBlackBoxed) {
+    if (!breakpoints || selectedSource.isBlackBoxed) {
       return null;
     }
 

diff --git a/devtools/client/debugger/new/src/components/Editor/HighlightLine.js b/devtools/client/debugger/new/src/components/Editor/HighlightLine.js
@@ -54,6 +54,14 @@ export class HighlightLine extends Component<Props> {
     return this.shouldSetHighlightLine(selectedLocation, selectedSource);
   }
 
+  componentDidUpdate(prevProps: Props) {
+    this.completeHighlightLine(prevProps);
+  }
+
+  componentDidMount() {
+    this.completeHighlightLine(null);
+  }
+
   shouldSetHighlightLine(
     selectedLocation: SourceLocation,
     selectedSource: Source
@@ -72,7 +80,7 @@ export class HighlightLine extends Component<Props> {
     return true;
   }
 
-  componentDidUpdate(prevProps: Props) {
+  completeHighlightLine(prevProps: Props | null) {
     const {
       pauseCommand,
       selectedLocation,
@@ -84,10 +92,12 @@ export class HighlightLine extends Component<Props> {
     }
 
     startOperation();
-    this.clearHighlightLine(
-      prevProps.selectedLocation,
-      prevProps.selectedSource
-    );
+    if (prevProps) {
+      this.clearHighlightLine(
+        prevProps.selectedLocation,
+        prevProps.selectedSource
+      );
+    }
     this.setHighlightLine(selectedLocation, selectedFrame, selectedSource);
     endOperation();
   }
@@ -101,6 +111,7 @@ export class HighlightLine extends Component<Props> {
     if (!this.shouldSetHighlightLine(selectedLocation, selectedSource)) {
       return;
     }
+
     this.isStepping = false;
     const editorLine = toEditorLine(sourceId, line);
     this.previousEditorLine = editorLine;

diff --git a/devtools/client/debugger/new/src/components/Editor/index.js b/devtools/client/debugger/new/src/components/Editor/index.js
@@ -576,7 +576,7 @@ class Editor extends PureComponent<Props, State> {
     const { horizontal, selectedSource, conditionalPanelLocation } = this.props;
     const { editor } = this.state;
 
-    if (!editor || !selectedSource) {
+    if (!selectedSource || !editor || !getDocument(selectedSource.id)) {
       return null;
     }