[DevTools] make breakpoints better

third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js

Index: third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
diff --git a/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js b/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
index 13de635127018497c6d13123c4da6abc652c2562..5c4a06a5fe445513a81da3bb7c0361c3cfb8512b 100644
--- a/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
+++ b/third_party/WebKit/Source/devtools/front_end/sources/JavaScriptSourceFrame.js
@@ -253,7 +253,6 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
       this._scriptsPanel.appendUILocationItems(contextMenu, uiLocation);
       var breakpoints = this._breakpointManager.findBreakpoints(this.uiSourceCode(), lineNumber);
       if (!breakpoints.length) {
-        // This row doesn't have a breakpoint: We want to show Add Breakpoint and Add and Edit Breakpoint.
         contextMenu.appendItem(
             Common.UIString('Add breakpoint'), this._createNewBreakpoint.bind(this, lineNumber, '', true));
         contextMenu.appendItem(
@@ -261,16 +260,24 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
         contextMenu.appendItem(
             Common.UIString('Never pause here'), this._createNewBreakpoint.bind(this, lineNumber, 'false', true));
       } else {
-        var breakpoint = breakpoints[0];
-
-        // This row has a breakpoint, we want to show edit and remove breakpoint, and either disable or enable.
-        contextMenu.appendItem(Common.UIString('Remove breakpoint'), breakpoint.remove.bind(breakpoint));
-        contextMenu.appendItem(
-            Common.UIString('Edit breakpoint…'), this._editBreakpointCondition.bind(this, lineNumber, breakpoint));
-        if (breakpoint.enabled())
-          contextMenu.appendItem(Common.UIString('Disable breakpoint'), breakpoint.setEnabled.bind(breakpoint, false));
-        else
-          contextMenu.appendItem(Common.UIString('Enable breakpoint'), breakpoint.setEnabled.bind(breakpoint, true));
+        var hasOneBreakpoint = breakpoints.length === 1;
+        var removeTitle = hasOneBreakpoint ? 'Remove breakpoint' : 'Remove all breakpoints in line';
+        contextMenu.appendItem(Common.UIString(removeTitle), () => breakpoints.map(breakpoint => breakpoint.remove()));
+        if (hasOneBreakpoint) {
+          contextMenu.appendItem(
+              Common.UIString('Edit breakpoint\u2026'),
+              this._editBreakpointCondition.bind(this, lineNumber, breakpoints[0]));
+        }
+        if (breakpoints.some(breakpoint => breakpoint.enabled())) {
+          var disableTitle = hasOneBreakpoint ? 'Disable breakpoint' : 'Disable all breakpoints in line';
+          contextMenu.appendItem(
+              Common.UIString(disableTitle), () => breakpoints.map(breakpoint => breakpoint.setEnabled(false)));

[dgozman, 2016/11/15 23:35:13]

Always wrap a literal in Common.UIString() instead of doing it later.

+        }
+        if (breakpoints.some(breakpoint => !breakpoint.enabled())) {
+          var enableTitle = hasOneBreakpoint ? 'Enable breakpoint' : 'Enable all breakpoints in line';
+          contextMenu.appendItem(
+              Common.UIString(enableTitle), () => breakpoints.map(breakpoint => breakpoint.setEnabled(true)));
+        }
       }
       resolve();
     }
@@ -356,13 +363,9 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
     if (this._muted)
       return;
     for (var lineNumber = 0; lineNumber < this._textEditor.linesCount; ++lineNumber) {
-      var breakpointDecoration = this._textEditor.getAttribute(lineNumber, 'breakpoint');
-      if (!breakpointDecoration)
+      if (!this.textEditor.hasLineClass(lineNumber, 'cm-breakpoint'))
         continue;
-      this._removeBreakpointDecoration(lineNumber);
-      this._addBreakpointDecoration(
-          lineNumber, breakpointDecoration.columnNumber, breakpointDecoration.condition, breakpointDecoration.enabled,
-          true);
+      this._updateBreakpointDecoration(lineNumber);
     }
     this._muted = true;
   }
@@ -403,28 +406,25 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
 
   _restoreBreakpointsAfterEditing() {
     delete this._muted;
-    var breakpoints = {};
+    /** @type {!Map<number, !{enabled: boolean, condition: string, lineNumber: number}>} */
+    var linesWithBreakpoints = new Map();
     // Save and remove muted breakpoint decorations.
     for (var lineNumber = 0; lineNumber < this._textEditor.linesCount; ++lineNumber) {
-      var breakpointDecoration = this._textEditor.getAttribute(lineNumber, 'breakpoint');
-      if (breakpointDecoration) {
-        breakpoints[lineNumber] = breakpointDecoration;
-        this._removeBreakpointDecoration(lineNumber);
+      if (this.textEditor.hasLineClass(lineNumber, 'cm-breakpoint')) {
+        var breakpoints = this._breakpointManager.findBreakpoints(this.uiSourceCode(), lineNumber);
+        breakpoints = breakpoints.filter(breakpoint => breakpoint.enabled());
+        var condition = breakpoints.length ? breakpoints[0].condition() : '';
+        linesWithBreakpoints.set(lineNumber,
+          {enabled: !!breakpoints.length, condition: condition, lineNumber: lineNumber});
       }
     }
 
     // Remove all breakpoints.
     this._removeAllBreakpoints();
 
-    // Restore all breakpoints from saved decorations.
-    for (var lineNumberString in breakpoints) {
-      var lineNumber = parseInt(lineNumberString, 10);
-      if (isNaN(lineNumber))
-        continue;
-      var breakpointDecoration = breakpoints[lineNumberString];
-      this._setBreakpoint(
-          lineNumber, breakpointDecoration.columnNumber, breakpointDecoration.condition, breakpointDecoration.enabled);
-    }
+    // Restore first in line breakpoints from saved decorations.
+    for (var breakpoint of linesWithBreakpoints.valuesArray())
+      this._createNewBreakpoint(breakpoint.lineNumber, breakpoint.condition, breakpoint.enabled);
   }
 
   _removeAllBreakpoints() {
@@ -559,23 +559,113 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
 
   /**
    * @param {number} lineNumber
-   * @param {number} columnNumber
-   * @param {string} condition
-   * @param {boolean} enabled
-   * @param {boolean} mutedWhileEditing
    */
-  _addBreakpointDecoration(lineNumber, columnNumber, condition, enabled, mutedWhileEditing) {
-    var breakpoint = {condition: condition, enabled: enabled, columnNumber: columnNumber};
+  _updateBreakpointDecoration(lineNumber) {

[lushnikov, 2016/11/15 22:42:33]

_updateBreakpointDecorations

+    if (!this._scheduledBreakpointDecorationUpdates)
+      this._scheduledBreakpointDecorationUpdates = /** @type {!Set<number>} */(new Set());
+
+    if (this._scheduledBreakpointDecorationUpdates.has(lineNumber))
+      return;
+    if (!this._scheduledBreakpointDecorationUpdates.size)
+      setImmediate(() => this.textEditor.operation(update.bind(this)));
+    this._scheduledBreakpointDecorationUpdates.add(lineNumber);
+
+    /**
+     * @this {!Sources.JavaScriptSourceFrame}
+     */
+    function update() {
+      for (var lineNumber of this._scheduledBreakpointDecorationUpdates) {
+        if (lineNumber >= this.textEditor.linesCount)
+          continue;
+        var lineRange = new Common.TextRange(lineNumber, 0, lineNumber, this.textEditor.line(lineNumber).length);
+        var bookmarks = this.textEditor.bookmarks(lineRange, Sources.JavaScriptSourceFrame._inlineBreakpointSymbol);
+        for (var bookmark of bookmarks)
+          bookmark.clear();
+
+        var breakpointLocations = this._breakpointManager.breakpontLocationsForLineNumber(this.uiSourceCode(), lineNumber);
+        var breakpoints = breakpointLocations.map(location => location.breakpoint);
+        var hasBreakpoints = !!breakpoints.length;
+        this.textEditor.toggleLineClass(lineNumber, 'cm-breakpoint', hasBreakpoints);
+        var hasEnabled = breakpoints.some(breakpoint => breakpoint.enabled()) && !this._muted;
+        var hasConditional = breakpoints.some(breakpoint => !!breakpoint.condition());
+        var hasEnabledConditional = breakpoints.some(breakpoint => !!breakpoint.condition() && breakpoint.enabled());
+        this.textEditor.toggleLineClass(lineNumber, 'cm-breakpoint-disabled', !hasEnabled && hasBreakpoints);
+        this.textEditor.toggleLineClass(lineNumber, 'cm-breakpoint-conditional', hasBreakpoints && (hasEnabledConditional || (!hasEnabled && hasConditional)));
+
+        if (hasBreakpoints)
+          this.addBreakpointForTest(lineNumber, hasEnabled);
+        else
+          this.removeBreakpointForTest(lineNumber);
+
+        if (hasBreakpoints && Runtime.experiments.isEnabled('inlineBreakpoints')) {
+          this._breakpointManager.possibleBreakpoints(this.uiSourceCode(), new Common.TextRange(lineNumber, 0, lineNumber + 1, 0))
+            .then(addMissingBreakpoints.bind(this, lineNumber));

[lushnikov, 2016/11/17 04:31:50]

what if during this asynchronous code the breakpoint gets removed? (fast
double-click in gutter)

+        }
+
+        if (breakpoints.length <= 1 || this._muted || !Runtime.experiments.isEnabled('inlineBreakpoints'))
+          continue;
+
+        for (let i = 0; i < breakpointLocations.length; ++i) {
+          var breakpoint = breakpointLocations[i].breakpoint;
+          var inlineBreakpoint = createElementWithClass('div', 'cm-inline-breakpoint');
+          inlineBreakpoint.classList.toggle('cm-inline-conditional', !!breakpoints[i].condition());
+          inlineBreakpoint.classList.toggle('cm-inline-disabled', !breakpoints[i].enabled());
+          inlineBreakpoint.addEventListener('click', () => breakpoints[i].setEnabled(!breakpoints[i].enabled()), true);
+          inlineBreakpoint.addEventListener('contextmenu', this._populateInlineBreakpointContextMenu.bind(this, breakpoints[i]));
+          this.textEditor.addBookmark(lineNumber, breakpointLocations[i].uiLocation.columnNumber, inlineBreakpoint, Sources.JavaScriptSourceFrame._inlineBreakpointSymbol);
+        }
+      }
+      this._scheduledBreakpointDecorationUpdates.clear();
+    }
+
+    /**
+     * @this {!Sources.JavaScriptSourceFrame}
+     * @param {number} lineNumber
+     * @param {!Array<!Workspace.UILocation>} locations
+     */
+    function addMissingBreakpoints(lineNumber, locations) {

[lushnikov, 2016/11/17 04:31:50]

addInlineBreakpoints

+      var breakpoints = this._breakpointManager.breakpontLocationsForLineNumber(this.uiSourceCode(), lineNumber);
+      if (!breakpoints.length)
+        return;
+      var columns = new Set(breakpoints.map(breakpoint => breakpoint.uiLocation.columnNumber));
+      locations = locations.filter(location => !columns.has(location.columnNumber));
+      for (var location of locations)
+        this._setBreakpoint(location.lineNumber, location.columnNumber, '', false);
+    }
+  }
 
-    this.textEditor.setAttribute(lineNumber, 'breakpoint', breakpoint);
+  /**
+   * @param {number} lineNumber
+   * @param {boolean} disabled
+   */
+  addBreakpointForTest(lineNumber, disabled) {
+  }
 
-    var disabled = !enabled || mutedWhileEditing;
-    this.textEditor.addBreakpoint(lineNumber, disabled, !!condition);
+  /**
+   * @param {number} lineNumber
+   */
+  removeBreakpointForTest(lineNumber) {
   }
 
-  _removeBreakpointDecoration(lineNumber) {
-    this.textEditor.removeAttribute(lineNumber, 'breakpoint');
-    this.textEditor.removeBreakpoint(lineNumber);
+  /**
+   * @param {!Bindings.BreakpointManager.Breakpoint} breakpoint
+   * @param {!Event} event
+   */
+  _populateInlineBreakpointContextMenu(breakpoint, event) {
+    var contextMenu = new UI.ContextMenu(event);
+    if (!breakpoint.enabled()) {
+      contextMenu.appendItem(
+        Common.UIString('Add conditional breakpoint…'), this._editBreakpointCondition.bind(this, breakpoint.lineNumber(), breakpoint));

[lushnikov, 2016/11/17 04:31:50]

nit: let's use \u... for the ellipsis

+      contextMenu.appendItem(
+        Common.UIString('Never pause here'),
+        () => { breakpoint.setCondition('false'); breakpoint.setEnabled(true); });

[lushnikov, 2016/11/17 04:31:50]

It would be easier for me to read if the function is extracted

+    } else {
+      contextMenu.appendItem(
+          Common.UIString('Edit breakpoint…'),

[lushnikov, 2016/11/17 04:31:50]

let's use \u... for the ellipsis

+          this._editBreakpointCondition.bind(this, breakpoint.lineNumber(), breakpoint));
+    }
+    contextMenu.show();
+    event.consume();
   }
 
   _onKeyDown(event) {
@@ -605,10 +695,12 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
       if (!committed)
         return;
 
-      if (breakpoint)
+      if (breakpoint) {
         breakpoint.setCondition(newText);
-      else
+        breakpoint.setEnabled(true);
+      } else {
         this._createNewBreakpoint(lineNumber, newText, true);
+      }
     }
 
     var config = new UI.InplaceEditor.Config(finishEditing.bind(this, true), finishEditing.bind(this, false));
@@ -848,12 +940,8 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
       return;
     if (this._shouldIgnoreExternalBreakpointEvents())
       return;
-
-    var breakpoint = /** @type {!Bindings.BreakpointManager.Breakpoint} */ (event.data.breakpoint);
-    if (this.loaded) {
-      this._addBreakpointDecoration(
-          uiLocation.lineNumber, uiLocation.columnNumber, breakpoint.condition(), breakpoint.enabled(), false);
-    }
+    if (this.loaded)
+      this._updateBreakpointDecoration(uiLocation.lineNumber);
   }
 
   _breakpointRemoved(event) {
@@ -862,10 +950,8 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
       return;
     if (this._shouldIgnoreExternalBreakpointEvents())
       return;
-
-    var remainingBreakpoints = this._breakpointManager.findBreakpoints(this.uiSourceCode(), uiLocation.lineNumber);
-    if (!remainingBreakpoints.length && this.loaded)
-      this._removeBreakpointDecoration(uiLocation.lineNumber);
+    if (this.loaded)
+      this._updateBreakpointDecoration(uiLocation.lineNumber);
   }
 
   /**
@@ -1010,13 +1096,15 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
    */
   _toggleBreakpoint(lineNumber, onlyDisable) {
     var breakpoints = this._breakpointManager.findBreakpoints(this.uiSourceCode(), lineNumber);
-    if (breakpoints.length) {
+    if (!breakpoints.length) {
+      this._createNewBreakpoint(lineNumber, '', true);
+      return;
+    }
+    for (var breakpoint of breakpoints) {
       if (onlyDisable)
-        breakpoints[0].setEnabled(!breakpoints[0].enabled());
+        breakpoint.setEnabled(false);
       else
-        breakpoints[0].remove();
-    } else {
-      this._createNewBreakpoint(lineNumber, '', true);
+        breakpoint.remove();
     }
   }
 
@@ -1065,10 +1153,13 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
      * @param {?Array<!Workspace.UILocation>} locations
      */
     function setBreakpoint(locations) {
-      if (!locations || !locations.length)
+      if (!locations || !locations.length) {
         this._setBreakpoint(lineNumber, 0, condition, enabled);
-      else
-        this._setBreakpoint(locations[0].lineNumber, locations[0].columnNumber, condition, enabled);
+      } else {
+        var maximumBreakpointsAmount = Runtime.experiments.isEnabled('inlineBreakpoints') ? locations.length : 1;

[lushnikov, 2016/11/17 04:31:50]

let's make the experiment check simpler:

if (Runtime.experiments.isEnabled('inlineBreakpoints')) {
   for (...)
} else {
   ...
}

This way it's easier to drop/release experiment.

+        for (var i = 0; i < locations.length && i < maximumBreakpointsAmount; ++i)
+          this._setBreakpoint(locations[i].lineNumber, locations[i].columnNumber, i === 0 ? condition : '', i === 0 && enabled);
+      }
       Host.userMetrics.actionTaken(Host.UserMetrics.Action.ScriptsBreakpointSet);
     }
   }
@@ -1127,3 +1218,5 @@ Sources.JavaScriptSourceFrame = class extends Sources.UISourceCodeFrame {
     super.dispose();
   }
 };
+
+Sources.JavaScriptSourceFrame._inlineBreakpointSymbol = Symbol('inline-breakpoint');