Bug 1258807 - update conditions on disabled conditional breakpoints properly r=me

devtools/client/debugger/content/actions/breakpoints.js

@@ -116,8 +116,7 @@ function _removeOrDisableBreakpoint(location, isDisabled) {
       return dispatch(Object.assign({}, action, {
         [PROMISE]: bpClient.remove()
       }));
-    }
-    else {
+    } else {
       return dispatch(Object.assign({}, action, { status: "done" }));
     }
   }
@@ -154,21 +153,30 @@ function setBreakpointCondition(location, condition) {
     }
 
     const bpClient = getBreakpointClient(bp.actor);
-
-    return dispatch({
+    const action = {
       type: constants.SET_BREAKPOINT_CONDITION,
       breakpoint: bp,
-      condition: condition,
-      [PROMISE]: Task.spawn(function*() {
-        const newClient = yield bpClient.setCondition(gThreadClient, condition);
+      condition: condition
+    };
 
-        // Remove the old instance and save the new one
-        setBreakpointClient(bpClient.actor, null);
-        setBreakpointClient(newClient.actor, newClient);
+    // If it's not disabled, we need to update the condition on the
+    // server. Otherwise, just dispatch a non-remote action that
+    // updates the condition locally.
+    if(!bp.disabled) {
+      return dispatch(Object.assign({}, action, {
+        [PROMISE]: Task.spawn(function*() {
+          const newClient = yield bpClient.setCondition(gThreadClient, condition);
 
-        return { actor: newClient.actor };
-      })
-    });
+          // Remove the old instance and save the new one
+          setBreakpointClient(bpClient.actor, null);
+          setBreakpointClient(newClient.actor, newClient);
+
+          return { actor: newClient.actor };
+        })
+      }));
+    } else {
+      return dispatch(action);
+    }
   };
 }
 

devtools/client/debugger/content/reducers/breakpoints.js

@@ -117,7 +117,14 @@ function update(state = initialState, action, emitChange) {
     const bp = state.breakpoints[id];
     emitChange("breakpoint-condition-updated", bp);
 
-    if (action.status === 'start') {
+    if (!action.status) {
+      // No status means that it wasn't a remote request. Just update
+      // the condition locally.
+      return mergeIn(state, ['breakpoints', id], {
+        condition: action.condition
+      });
+    }
+    else if (action.status === 'start') {
       return mergeIn(state, ['breakpoints', id], {
         loading: true,
         condition: action.condition
@@ -126,6 +133,7 @@ function update(state = initialState, action, emitChange) {
     else if (action.status === 'done') {
       return mergeIn(state, ['breakpoints', id], {
         loading: false,
+        condition: action.condition,
         // Setting a condition creates a new breakpoint client as of
         // now, so we need to update the actor
         actor: action.value.actor

devtools/client/debugger/debugger-view.js

@@ -374,14 +374,15 @@ var DebuggerView = {
 
     if (source && source.actor === location.actor) {
       this.editor.removeBreakpoint(location.line - 1);
+      this.editor.removeBreakpointCondition(location.line - 1);
     }
   },
 
   renderEditorBreakpointCondition: function (breakpoint) {
-    const { location, condition } = breakpoint;
+    const { location, condition, disabled } = breakpoint;
     const source = queries.getSelectedSource(this.controller.getState());
 
-    if (source && source.actor === location.actor) {
+    if (source && source.actor === location.actor && !disabled) {
       if (condition) {
         this.editor.setBreakpointCondition(location.line - 1);
       } else {

devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-03.js

@@ -25,6 +25,13 @@ function test() {
     var client = gPanel.target.client;
     client.mainRoot.traits.conditionalBreakpoints = false;
 
+    function waitForConditionUpdate() {
+      // This will close the popup and send another request to update
+      // the condition
+      gSources._hideConditionalPopup();
+      return waitForDispatch(gPanel, constants.SET_BREAKPOINT_CONDITION);
+    }
+
     Task.spawn(function*() {
       yield waitForSourceAndCaretAndScopes(gPanel, ".html", 17);
       const location = { actor: gSources.selectedValue, line: 18 };
@@ -36,7 +43,7 @@ function test() {
       const bp = queries.getBreakpoint(getState(), location);
       is(bp.condition, "hello", "The conditional expression is correct.");
 
-      const finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
+      let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
       EventUtils.sendMouseEvent({ type: "click" },
                                 gDebugger.document.querySelector(".dbg-breakpoint"),
                                 gDebugger);
@@ -45,6 +52,18 @@ function test() {
       const textbox = gDebugger.document.getElementById("conditional-breakpoint-panel-textbox");
       is(textbox.value, "hello", "The expression is correct (2).")
 
+      yield waitForConditionUpdate();
+      yield actions.disableBreakpoint(location);
+      yield actions.setBreakpointCondition(location, "foo");
+      yield actions.addBreakpoint(location);
+
+      finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
+      EventUtils.sendMouseEvent({ type: "click" },
+                                gDebugger.document.querySelector(".dbg-breakpoint"),
+                                gDebugger);
+      yield finished;
+      is(textbox.value, "foo", "The expression is correct (3).")
+
       // Reset traits back to default value
       client.mainRoot.traits.conditionalBreakpoints = true;
       resumeDebuggerThenCloseAndFinish(gPanel);

devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-03.js

@@ -21,6 +21,13 @@ function test() {
     const actions = bindActionCreators(gPanel);
     const getState = gDebugger.DebuggerController.getState;
 
+    function waitForConditionUpdate() {
+      // This will close the popup and send another request to update
+      // the condition
+      gSources._hideConditionalPopup();
+      return waitForDispatch(gPanel, constants.SET_BREAKPOINT_CONDITION);
+    }
+
     Task.spawn(function*() {
       yield waitForSourceAndCaretAndScopes(gPanel, ".html", 17);
       const location = { actor: gSources.selectedValue, line: 18 };
@@ -32,7 +39,7 @@ function test() {
       const bp = queries.getBreakpoint(getState(), location);
       is(bp.condition, "hello", "The conditional expression is correct.");
 
-      const finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
+      let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
       EventUtils.sendMouseEvent({ type: "click" },
                                 gDebugger.document.querySelector(".dbg-breakpoint"),
                                 gDebugger);
@@ -41,6 +48,18 @@ function test() {
       const textbox = gDebugger.document.getElementById("conditional-breakpoint-panel-textbox");
       is(textbox.value, "hello", "The expression is correct (2).")
 
+      yield waitForConditionUpdate();
+      yield actions.disableBreakpoint(location);
+      yield actions.setBreakpointCondition(location, "foo");
+      yield actions.addBreakpoint(location);
+
+      finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING);
+      EventUtils.sendMouseEvent({ type: "click" },
+                                gDebugger.document.querySelector(".dbg-breakpoint"),
+                                gDebugger);
+      yield finished;
+      is(textbox.value, "foo", "The expression is correct (3).")
+
       yield resumeDebuggerThenCloseAndFinish(gPanel);
     });