Bug 762979 - Implement conditional breakpoint gutter style. r=bgrins, jlongster draft
authorTim Nguyen <ntim.bugs@gmail.com>
Fri, 22 Jan 2016 20:37:25 +0100
changeset 324415 7d4a56b5918f47f517a798f5b5300e6b9b34f6b6
parent 324414 7417d16632b8d5e7e19d30b540b267e453e0997c
child 324416 d8b7e087abe6950a634a86f46aa8b4cb5a7eaae3
push id9906
push userntim.bugs@gmail.com
push dateFri, 22 Jan 2016 19:38:01 +0000
reviewersbgrins, jlongster
bugs762979
milestone46.0a1
Bug 762979 - Implement conditional breakpoint gutter style. r=bgrins, jlongster
devtools/client/debugger/content/reducers/breakpoints.js
devtools/client/debugger/debugger-view.js
devtools/client/sourceeditor/codemirror/mozilla.css
devtools/client/sourceeditor/debugger.js
--- a/devtools/client/debugger/content/reducers/breakpoints.js
+++ b/devtools/client/debugger/content/reducers/breakpoints.js
@@ -98,31 +98,37 @@ function update(state = initialState, ac
     break;
   }
 
   case constants.SET_BREAKPOINT_CONDITION: {
     const id = makeLocationId(action.breakpoint.location);
     const bp = state.breakpoints[id];
 
     if (action.status === 'start') {
-      return mergeIn(state, ['breakpoints', id], {
+      state = mergeIn(state, ['breakpoints', id], {
         loading: true,
         condition: action.condition
       });
     }
     else if (action.status === 'done') {
-      return mergeIn(state, ['breakpoints', id], {
+      state = mergeIn(state, ['breakpoints', id], {
         loading: false,
         // Setting a condition creates a new breakpoint client as of
         // now, so we need to update the actor
         actor: action.value.actor
       });
     }
     else if (action.status === 'error') {
+      emitChange("breakpoint-removed", bp);
       return deleteIn(state, ['breakpoints', id]);
     }
+
+    emitChange("breakpoint-condition-updated", bp);
+
+    return state;
+
     break;
   }}
 
   return state;
 }
 
 module.exports = update;
--- a/devtools/client/debugger/debugger-view.js
+++ b/devtools/client/debugger/debugger-view.js
@@ -94,16 +94,17 @@ var DebuggerView = {
       "source-text-loaded": this.renderSourceText,
       "source-selected": this.renderSourceText,
       "blackboxed": this.renderBlackBoxed,
       "prettyprinted": this.renderPrettyPrinted,
       "breakpoint-added": this.addEditorBreakpoint,
       "breakpoint-enabled": this.addEditorBreakpoint,
       "breakpoint-disabled": this.removeEditorBreakpoint,
       "breakpoint-removed": this.removeEditorBreakpoint,
+      "breakpoint-condition-updated": this.renderEditorBreakpointCondition,
       "breakpoint-moved": ({ breakpoint, prevLocation }) => {
         const selectedSource = queries.getSelectedSource(getState());
         const { location } = breakpoint;
 
         if (selectedSource &&
            selectedSource.actor === location.actor) {
           this.editor.moveBreakpoint(prevLocation.line - 1,
                                      location.line - 1);
@@ -348,35 +349,48 @@ var DebuggerView = {
       }
       else {
         this.removeEditorBreakpoint(bp);
       }
     }
   },
 
   addEditorBreakpoint: function(breakpoint) {
-    const { location } = breakpoint;
+    const { location, condition } = breakpoint;
     const source = queries.getSelectedSource(this.controller.getState());
 
     if (source &&
        source.actor === location.actor &&
        !breakpoint.disabled) {
-      this.editor.addBreakpoint(location.line - 1);
+      this.editor.addBreakpoint(location.line - 1, condition);
     }
   },
 
   removeEditorBreakpoint: function (breakpoint) {
     const { location } = breakpoint;
     const source = queries.getSelectedSource(this.controller.getState());
 
     if (source && source.actor === location.actor) {
       this.editor.removeBreakpoint(location.line - 1);
     }
   },
 
+  renderEditorBreakpointCondition: function (breakpoint) {
+    const { location, condition } = breakpoint;
+    const source = queries.getSelectedSource(this.controller.getState());
+
+    if (source && source.actor === location.actor) {
+      if (condition) {
+        this.editor.setBreakpointCondition(location.line - 1);
+      } else {
+        this.editor.removeBreakpointCondition(location.line - 1);
+      }
+    }
+  },
+
   /**
    * Display the source editor.
    */
   showEditor: function() {
     this._editorDeck.selectedIndex = 0;
   },
 
   /**
--- a/devtools/client/sourceeditor/codemirror/mozilla.css
+++ b/devtools/client/sourceeditor/codemirror/mozilla.css
@@ -1,22 +1,24 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 :root {
   --breakpoint-background: url("chrome://devtools/skin/images/breakpoint.svg#light");
   --breakpoint-hover-background: url("chrome://devtools/skin/images/breakpoint.svg#light-hover");
   --breakpoint-active-background: url("chrome://devtools/skin/images/breakpoint.svg#light-active");
+  --breakpoint-conditional-background: url("chrome://devtools/skin/images/breakpoint.svg#light-conditional");
 }
 
 .theme-dark:root {
   --breakpoint-background: url("chrome://devtools/skin/images/breakpoint.svg#dark");
   --breakpoint-hover-background: url("chrome://devtools/skin/images/breakpoint.svg#dark-hover");
   --breakpoint-active-background: url("chrome://devtools/skin/images/breakpoint.svg#dark-active");
+  --breakpoint-conditional-background: url("chrome://devtools/skin/images/breakpoint.svg#dark-conditional");
 }
 
 .errors {
   width: 16px;
 }
 
 .hit-counts {
   width: 6px;
@@ -59,16 +61,24 @@
   padding-inline-end: 9px;
   color: var(--theme-body-background);
 }
 
 .breakpoint .CodeMirror-linenumber {
   background-image: var(--breakpoint-background) !important;
 }
 
+.breakpoint.conditional .CodeMirror-linenumber {
+  background-image: var(--breakpoint-conditional-background) !important;
+}
+
+.breakpoint.conditional .CodeMirror-line {
+  background-color: rgba(217,126,0,.15);
+}
+
 .debug-line .CodeMirror-linenumber {
   background-image: var(--breakpoint-active-background) !important;
 }
 
 .debug-line .CodeMirror-line {
   background-color: rgba(44,187,15,.1);
 }
 
--- a/devtools/client/sourceeditor/debugger.js
+++ b/devtools/client/sourceeditor/debugger.js
@@ -146,16 +146,19 @@ function addBreakpoint(ctx, line, cond) 
     meta.breakpoints[line] = { condition: cond };
 
     // TODO(jwl): why is `info` null when breaking on page reload?
     info.handle.on("delete", function onDelete() {
       info.handle.off("delete", onDelete);
       meta.breakpoints[info.line] = null;
     });
 
+    if (cond) {
+      setBreakpointCondition(ctx, line);
+    }
     ed.emit("breakpointAdded", line);
     deferred.resolve();
   }
 
   if (hasBreakpoint(ctx, line)) {
     return null;
   }
 
@@ -180,29 +183,50 @@ function removeBreakpoint(ctx, line) {
   }
 
   let { ed, cm } = ctx;
   let meta = dbginfo.get(ed);
   let info = cm.lineInfo(line);
 
   meta.breakpoints[info.line] = null;
   ed.removeLineClass(info.line, "breakpoint");
+  ed.removeLineClass(info.line, "conditional");
   ed.emit("breakpointRemoved", line);
 }
 
 function moveBreakpoint(ctx, fromLine, toLine) {
   let { ed, cm } = ctx;
 
   let fromTop = cm.cursorCoords({ line: fromLine }).top;
   let toTop = cm.cursorCoords({ line: toLine }).top;
 
   ed.removeBreakpoint(fromLine);
   ed.addBreakpoint(toLine);
 }
 
+function setBreakpointCondition(ctx, line) {
+  let { ed, cm } = ctx;
+  let info = cm.lineInfo(line);
+
+  // The line does not exist in the editor. This is harmless, the
+  // architecture calling this assumes the editor will handle this
+  // gracefully, and make sure breakpoints exist when they need to.
+  if (!info) {
+    return;
+  }
+
+  ed.addLineClass(line, "conditional");
+}
+
+function removeBreakpointCondition(ctx, line) {
+  let { ed, cm } = ctx;
+
+  ed.removeLineClass(line, "conditional");
+}
+
 /**
  * Returns a list of all breakpoints in the current Editor.
  */
 function getBreakpoints(ctx) {
   let { ed } = ctx;
   let meta = dbginfo.get(ed);
 
   return Object.keys(meta.breakpoints).reduce((acc, line) => {
@@ -273,12 +297,13 @@ function findNext(ctx, query) {
  */
 function findPrev(ctx, query) {
   doSearch(ctx, true, query);
 }
 
 // Export functions
 
 [
-  initialize, hasBreakpoint, addBreakpoint, removeBreakpoint,
-  moveBreakpoint, getBreakpoints, setDebugLocation, getDebugLocation,
-  clearDebugLocation, find, findNext, findPrev
+  initialize, hasBreakpoint, addBreakpoint, removeBreakpoint, moveBreakpoint,
+  setBreakpointCondition, removeBreakpointCondition, getBreakpoints,
+  setDebugLocation, getDebugLocation, clearDebugLocation, find, findNext,
+  findPrev
 ].forEach(func => module.exports[func.name] = func);