Bug 1258807 - update conditions on disabled conditional breakpoints properly r=me
authorJames Long <longster@gmail.com>
Mon, 28 Mar 2016 10:51:10 -0400
changeset 290705 24b5fb250c49007ee5a239b57e9cecd790cd1ebf
parent 290532 a0f366bd78a48278208304775a1166396a3daad4
child 290706 e5c8ba7cabaad7382c9b7f6b54fa10b268e1e1cc
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1258807
milestone48.0a1
Bug 1258807 - update conditions on disabled conditional breakpoints properly r=me
devtools/client/debugger/content/actions/breakpoints.js
devtools/client/debugger/content/reducers/breakpoints.js
devtools/client/debugger/debugger-view.js
devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-03.js
devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-03.js
--- a/devtools/client/debugger/content/actions/breakpoints.js
+++ b/devtools/client/debugger/content/actions/breakpoints.js
@@ -111,18 +111,17 @@ function _removeOrDisableBreakpoint(loca
     // If the breakpoint is already disabled, we don't need to remove
     // it from the server. We just need to dispatch an action
     // simulating a successful server request to remove it, and it
     // will be removed completely from the state.
     if(!bp.disabled) {
       return dispatch(Object.assign({}, action, {
         [PROMISE]: bpClient.remove()
       }));
-    }
-    else {
+    } else {
       return dispatch(Object.assign({}, action, { status: "done" }));
     }
   }
 }
 
 function removeAllBreakpoints() {
   return (dispatch, getState) => {
     const breakpoints = getBreakpoints(getState());
@@ -149,31 +148,40 @@ function setBreakpointCondition(location
     }
     if (bp.loading){
       // TODO(jwl): when this function is called, make sure the action
       // creator waits for the breakpoint to exist
       throw new Error("breakpoint must be saved");
     }
 
     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
+    };
+
+    // 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);
 
-        // Remove the old instance and save the new one
-        setBreakpointClient(bpClient.actor, null);
-        setBreakpointClient(newClient.actor, newClient);
+          // Remove the old instance and save the new one
+          setBreakpointClient(bpClient.actor, null);
+          setBreakpointClient(newClient.actor, newClient);
 
-        return { actor: newClient.actor };
-      })
-    });
+          return { actor: newClient.actor };
+        })
+      }));
+    } else {
+      return dispatch(action);
+    }
   };
 }
 
 module.exports = {
   enableBreakpoint,
   addBreakpoint,
   disableBreakpoint,
   removeBreakpoint,
--- a/devtools/client/debugger/content/reducers/breakpoints.js
+++ b/devtools/client/debugger/content/reducers/breakpoints.js
@@ -112,25 +112,33 @@ function update(state = initialState, ac
     break;
   }
 
   case constants.SET_BREAKPOINT_CONDITION: {
     const id = makeLocationId(action.breakpoint.location);
     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
       });
     }
     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
       });
     }
     else if (action.status === 'error') {
       emitChange("breakpoint-removed", bp);
       return deleteIn(state, ['breakpoints', id]);
--- a/devtools/client/debugger/debugger-view.js
+++ b/devtools/client/debugger/debugger-view.js
@@ -369,24 +369,25 @@ var DebuggerView = {
   },
 
   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);
+      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 {
         this.editor.removeBreakpointCondition(location.line - 1);
       }
     }
   },
 
--- a/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-03.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-03.js
@@ -20,36 +20,55 @@ function test() {
     const actions = bindActionCreators(gPanel);
     const getState = gDebugger.DebuggerController.getState;
 
     // This test forces conditional breakpoints to be evaluated on the
     // client-side
     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 };
 
       yield actions.addBreakpoint(location, "hello");
       yield actions.disableBreakpoint(location);
       yield actions.addBreakpoint(location);
 
       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);
       yield finished;
 
       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);
     });
 
     callInTab(gTab, "ermahgerd");
   });
 }
--- a/devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-03.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-03.js
@@ -16,34 +16,53 @@ function test() {
     const gPanel = aPanel;
     const gDebugger = gPanel.panelWin;
     const gSources = gDebugger.DebuggerView.Sources;
     const queries = gDebugger.require('./content/queries');
     const constants = gDebugger.require('./content/constants');
     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 };
 
       yield actions.addBreakpoint(location, "hello");
       yield actions.disableBreakpoint(location);
       yield actions.addBreakpoint(location);
 
       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);
       yield finished;
 
       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);
     });
 
     callInTab(gTab, "ermahgerd");
   });
 }