Bug 1253902 - fix adding a conditional breakpoint from context menu r=ejpbruel
authorJames Long <longster@gmail.com>
Mon, 21 Mar 2016 13:03:12 -0400
changeset 313359 90b4e3e5d0f4d8c58678807286c4e7dd6593a8a9
parent 313358 da1a66f1c4b74c4bd778e91b9589d73763dddfe3
child 313360 91736cfdc9b0128cf40a779163d3fa244dcac933
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersejpbruel
bugs1253902
milestone48.0a1
Bug 1253902 - fix adding a conditional breakpoint from context menu r=ejpbruel
devtools/client/debugger/content/reducers/breakpoints.js
devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-02.js
devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-04.js
devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-02.js
devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-04.js
--- a/devtools/client/debugger/content/reducers/breakpoints.js
+++ b/devtools/client/debugger/content/reducers/breakpoints.js
@@ -7,29 +7,43 @@ const constants = require('../constants'
 const Immutable = require('devtools/client/shared/vendor/seamless-immutable');
 const { mergeIn, setIn, deleteIn } = require('../utils');
 const { makeLocationId } = require('../queries');
 
 const initialState = Immutable({
   breakpoints: {}
 });
 
+// Return the first argument that is a string, or null if nothing is a
+// string.
+function firstString(...args) {
+  for (var arg of args) {
+    if (typeof arg === "string") {
+      return arg;
+    }
+  }
+  return null;
+}
+
 function update(state = initialState, action, emitChange) {
   switch(action.type) {
   case constants.ADD_BREAKPOINT: {
     const id = makeLocationId(action.breakpoint.location);
 
     if (action.status === 'start') {
       const existingBp = state.breakpoints[id];
       const bp = existingBp || Immutable(action.breakpoint);
 
       state = setIn(state, ['breakpoints', id], bp.merge({
         disabled: false,
         loading: true,
-        condition: action.condition || bp.condition || undefined
+        // We want to do an OR here, but we can't because we need
+        // empty strings to be truthy, i.e. an empty string is a valid
+        // condition.
+        condition: firstString(action.condition, bp.condition)
       }));
 
       emitChange(existingBp ? "breakpoint-enabled" : "breakpoint-added",
                  state.breakpoints[id]);
       return state;
     }
     else if (action.status === 'done') {
       const { actor, text } = action.value;
--- a/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-02.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-02.js
@@ -115,16 +115,23 @@ function test() {
     }
 
     function clickOnBreakpoint(aIndex) {
       EventUtils.sendMouseEvent({ type: "click" },
                                 gDebugger.document.querySelectorAll(".dbg-breakpoint")[aIndex],
                                 gDebugger);
     }
 
+    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);
 
       is(gDebugger.gThreadClient.state, "paused",
          "Should only be getting stack frames while paused.");
       is(queries.getSourceCount(getState()), 1,
          "Found the expected number of sources.");
       is(gEditor.getText().indexOf("ermahgerd"), 253,
@@ -137,18 +144,20 @@ function test() {
 
       yield addBreakpoint1();
       testBreakpoint(18, false, undefined)
 
       yield addBreakpoint2();
       testBreakpoint(19, false, undefined);
       yield modBreakpoint2();
       testBreakpoint(19, true, undefined);
+      yield waitForConditionUpdate();
       yield addBreakpoint3();
-      testBreakpoint(20, false, undefined);
+      testBreakpoint(20, true, "");
+      yield waitForConditionUpdate();
       yield modBreakpoint3();
       testBreakpoint(20, false, "bamboocha");
       yield addBreakpoint4();
       testBreakpoint(21, false, undefined);
       yield delBreakpoint4();
 
       setCaretPosition(18);
       is(gSources._selectedBreakpoint.location.line, 18,
--- a/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-04.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_conditional-breakpoints-04.js
@@ -1,16 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Make sure that conditional breakpoints with blank expressions
- * are stored as plain breakpoints when re-enabling them.
+ * maintain their conditions after enabling them.
  */
 
 const TAB_URL = EXAMPLE_URL + "doc_conditional-breakpoints.html";
 
 function test() {
   initDebugger(TAB_URL).then(([aTab,, aPanel]) => {
     const gTab = aTab;
     const gPanel = aPanel;
@@ -30,17 +30,17 @@ function test() {
       yield waitForSourceAndCaretAndScopes(gPanel, ".html", 17);
       const location = { actor: gSources.selectedValue, line: 18 };
 
       yield actions.addBreakpoint(location, "");
       yield actions.disableBreakpoint(location);
       yield actions.addBreakpoint(location);
 
       const bp = queries.getBreakpoint(getState(), location);
-      is(bp.condition, undefined, "The conditional expression is correct.");
+      is(bp.condition, "", "The conditional expression is correct.");
 
       // 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-02.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-02.js
@@ -110,16 +110,23 @@ function test() {
     }
 
     function clickOnBreakpoint(aIndex) {
       EventUtils.sendMouseEvent({ type: "click" },
                                 gDebugger.document.querySelectorAll(".dbg-breakpoint")[aIndex],
                                 gDebugger);
     }
 
+    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);
 
       is(gDebugger.gThreadClient.state, "paused",
          "Should only be getting stack frames while paused.");
       is(queries.getSourceCount(getState()), 1,
          "Found the expected number of sources.");
       is(gEditor.getText().indexOf("ermahgerd"), 253,
@@ -132,18 +139,20 @@ function test() {
 
       yield addBreakpoint1();
       testBreakpoint(18, false, undefined)
 
       yield addBreakpoint2();
       testBreakpoint(19, false, undefined);
       yield modBreakpoint2();
       testBreakpoint(19, true, undefined);
+      yield waitForConditionUpdate();
       yield addBreakpoint3();
-      testBreakpoint(20, false, undefined);
+      testBreakpoint(20, true, "");
+      yield waitForConditionUpdate();
       yield modBreakpoint3();
       testBreakpoint(20, false, "bamboocha");
       yield addBreakpoint4();
       testBreakpoint(21, false, undefined);
       yield delBreakpoint4();
 
       setCaretPosition(18);
       is(gSources._selectedBreakpoint.location.line, 18,
--- a/devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-04.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_server-conditional-bp-04.js
@@ -1,16 +1,16 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Make sure that conditional breakpoints with undefined expressions
- * are stored as plain breakpoints when re-enabling them (with
+ * maintain their conditions when re-enabling them (with
  * server-side support)
  */
 
 const TAB_URL = EXAMPLE_URL + "doc_conditional-breakpoints.html";
 
 function test() {
   initDebugger(TAB_URL).then(([aTab,, aPanel]) => {
     const gTab = aTab;
@@ -26,16 +26,16 @@ function test() {
       yield waitForSourceAndCaretAndScopes(gPanel, ".html", 17);
       const location = { actor: gSources.selectedValue, line: 18 };
 
       yield actions.addBreakpoint(location, "");
       yield actions.disableBreakpoint(location);
       yield actions.addBreakpoint(location);
 
       const bp = queries.getBreakpoint(getState(), location);
-      is(bp.condition, undefined, "The conditional expression is correct.");
+      is(bp.condition, "", "The conditional expression is correct.");
 
       resumeDebuggerThenCloseAndFinish(gPanel);
     });
 
     callInTab(gTab, "ermahgerd");
   });
 }