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 289585 90b4e3e5d0f4d8c58678807286c4e7dd6593a8a9
parent 289584 da1a66f1c4b74c4bd778e91b9589d73763dddfe3
child 289586 91736cfdc9b0128cf40a779163d3fa244dcac933
push id30107
push usercbook@mozilla.com
push dateTue, 22 Mar 2016 10:00:23 +0000
treeherdermozilla-central@3587b25bae30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersejpbruel
bugs1253902
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");
   });
 }