Bug 1582697 - Don't add the task middleware by default in create-store. r=davidwalsh.

The disableTask options is renamed to enableTaskMiddleware,
which defaults to false.

This caused failure in the dom mutation breakpoint test, because
we weren't waiting until the call to the server to remove the
breakpoints were done.
It wasn't an issue before because this was handle by the task
middleware, which catch rejections.
We fix this by only dispatching the action when the breakpoints are
indeed removed. We also tweak to waitForAllElements helper to be
able to wait for a specific count of element (it used to resolve
when there was at least the specified count, which is not ideal
when trying to assert removal of an element).

Differential Revision: https://phabricator.services.mozilla.com/D47584

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/test/mochitest/browser_dbg-dom-mutation-breakpoints.js

@@ -2,10 +2,8 @@
  * 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/>. */
 
-
 // Tests adding, disble/enable, and removal of dom mutation breakpoints
 
-
 /* import-globals-from ../../../inspector/test/shared-head.js */
 
 // Import helpers for the inspector
@@ -14,7 +12,8 @@ Services.scriptloader.loadSubScript(
   this
 );
 
-const DMB_TEST_URL = "http://example.com/browser/devtools/client/debugger/test/mochitest/examples/doc-dom-mutation.html";
+const DMB_TEST_URL =
+  "http://example.com/browser/devtools/client/debugger/test/mochitest/examples/doc-dom-mutation.html";
 
 add_task(async function() {
   // Enable features
@@ -26,16 +25,20 @@ add_task(async function() {
 
   const { inspector, toolbox } = await openInspectorForURL(DMB_TEST_URL);
 
-  info("Sellecting the body node");
+  info("Selecting the body node");
   await selectNode("body", inspector);
 
   info("Adding DOM mutation breakpoints to body");
   const allMenuItems = openContextMenuAndGetAllItems(inspector);
 
-  const attributeMenuItem = allMenuItems.find(item => item.id === "node-menu-mutation-breakpoint-attribute");
+  const attributeMenuItem = allMenuItems.find(
+    item => item.id === "node-menu-mutation-breakpoint-attribute"
+  );
   attributeMenuItem.click();
 
-  const subtreeMenuItem = allMenuItems.find(item => item.id === "node-menu-mutation-breakpoint-subtree");
+  const subtreeMenuItem = allMenuItems.find(
+    item => item.id === "node-menu-mutation-breakpoint-subtree"
+  );
   subtreeMenuItem.click();
 
   info("Switches over to the debugger pane");
@@ -70,7 +73,7 @@ add_task(async function() {
   await waitForPaused(dbg);
   await resume(dbg);
 
-  info("Removing a breakpoint works")
+  info("Removing breakpoints works");
   dbg.win.document.querySelector(".dom-mutation-list .close-btn").click();
-  await waitForAllElements(dbg, "domMutationItem", 1);
+  await waitForAllElements(dbg, "domMutationItem", 1, true);
 });

devtools/client/debugger/test/mochitest/helpers.js

@@ -64,8 +64,8 @@ async function waitFor(
     interval,
     maxTries
   );
-   return condition();
- }
+  return condition();
+}
 
 // Wait until an action of `type` is dispatched. This is different
 // then `waitForDispatch` because it doesn't wait for async actions
@@ -202,8 +202,27 @@ async function waitForElement(dbg, name, ...args) {
   return findElement(dbg, name, ...args);
 }
 
-async function waitForAllElements(dbg, name, count = 1) {
-  await waitUntil(() => findAllElements(dbg, name).length >= count);
+/**
+ * Wait for a count of given elements to be rendered on screen.
+ *
+ * @param {DebuggerPanel} dbg
+ * @param {String} name
+ * @param {Integer} count: Number of elements to match. Defaults to 1.
+ * @param {Boolean} countStrictlyEqual: When set to true, will wait until the exact number
+ *                  of elements is displayed on screen. When undefined or false, will wait
+ *                  until there's at least `${count}` elements on screen (e.g. if count
+ *                  is 1, it will resolve if there are 2 elements rendered).
+ */
+async function waitForAllElements(
+  dbg,
+  name,
+  count = 1,
+  countStrictlyEqual = false
+) {
+  await waitUntil(() => {
+    const elsCount = findAllElements(dbg, name).length;
+    return countStrictlyEqual ? elsCount === count : elsCount >= count;
+  });
   return findAllElements(dbg, name);
 }
 
@@ -419,7 +438,10 @@ function assertPausedAtSourceAndLine(dbg, expectedSourceId, expectedLine) {
   ok(frames.length >= 1, "Got at least one frame");
   const { sourceId, line } = frames[0].location;
   ok(sourceId == expectedSourceId, "Frame has correct source");
-  ok(line == expectedLine, `Frame paused at ${line}, but expected ${expectedLine}`);
+  ok(
+    line == expectedLine,
+    `Frame paused at ${line}, but expected ${expectedLine}`
+  );
 }
 
 // Get any workers associated with the debugger.
@@ -866,7 +888,10 @@ function findBreakpoint(dbg, url, line) {
 // helper for finding column breakpoints.
 function findColumnBreakpoint(dbg, url, line, column) {
   const source = findSource(dbg, url);
-  const lineBreakpoints = dbg.selectors.getBreakpointsForSource(source.id, line);
+  const lineBreakpoints = dbg.selectors.getBreakpointsForSource(
+    source.id,
+    line
+  );
   return lineBreakpoints.find(bp => {
     return bp.generatedLocation.column === column;
   });
@@ -1046,12 +1071,11 @@ function invokeInTab(fnc, ...args) {
   });
 }
 
-
 function clickElementInTab(selector) {
   info(`click element ${selector} in tab`);
 
   return ContentTask.spawn(gBrowser.selectedBrowser, { selector }, function*({
-    selector
+    selector,
   }) {
     content.wrappedJSObject.document.querySelector(selector).click();
   });
@@ -1827,7 +1851,7 @@ async function checkEvaluateInTopFrame(dbg, text, expected) {
   ok(rval == expected, `Eval returned ${expected}`);
 }
 
-async function findConsoleMessage({toolbox}, query) {
+async function findConsoleMessage({ toolbox }, query) {
   const [message] = await findConsoleMessages(toolbox, query);
   const value = message.querySelector(".message-body").innerText;
   const link = message.querySelector(".frame-link-source-inner").innerText;
@@ -1843,7 +1867,7 @@ async function findConsoleMessages(toolbox, query) {
   );
 }
 
-async function hasConsoleMessage({toolbox}, msg) {
+async function hasConsoleMessage({ toolbox }, msg) {
   return waitFor(async () => {
     const messages = await findConsoleMessages(toolbox, msg);
     return messages.length > 0;

devtools/client/framework/actions/dom-mutation-breakpoints.js

@@ -53,16 +53,16 @@ function deleteDOMMutationBreakpoint(nodeFront, mutationType) {
   assert(typeof mutationType === "string");
 
   return async function(dispatch) {
+    const walker = nodeFront.parent();
+    await walker.setMutationBreakpoints(nodeFront, {
+      [mutationType]: false,
+    });
+
     dispatch({
       type: "REMOVE_DOM_MUTATION_BREAKPOINT",
       nodeFront,
       mutationType,
     });
-
-    const walker = nodeFront.parent();
-    await walker.setMutationBreakpoints(nodeFront, {
-      [mutationType]: false,
-    });
   };
 }
 

devtools/client/inspector/store.js

@@ -9,7 +9,6 @@ const reducers = require("devtools/client/inspector/reducers");
 
 module.exports = services =>
   createStore(reducers, {
-    disableTask: true,
     // Enable log middleware in tests
     shouldLog: true,
     thunkOptions: {

devtools/client/memory/store.js

@@ -9,6 +9,7 @@ const reducers = require("./reducers");
 
 module.exports = () =>
   createStore(reducers, {
+    enableTaskMiddleware: true,
     // Uncomment this for logging in tests.
     // shouldLog: true,
   });

devtools/client/shared/redux/create-store.js

@@ -34,7 +34,7 @@ loader.lazyRequireGetter(
  * various ways, such as logging and recording.
  *
  * @param {object} opts:
- *        - disableTask: if true, don't include the task middleware
+ *        - enableTaskMiddleware: if true, include the task middleware
  *        - log: log all dispatched actions to console
  *        - history: an array to store every action in. Should only be used in tests.
  *        - middleware: array of middleware to be included in the redux store
@@ -43,7 +43,7 @@ loader.lazyRequireGetter(
  */
 const createStoreWithMiddleware = (opts = {}) => {
   const middleware = [];
-  if (!opts.disableTask) {
+  if (opts.enableTaskMiddleware) {
     middleware.push(task);
   }
   middleware.push(
@@ -79,7 +79,7 @@ module.exports = (
     shouldLog = false,
     initialState = undefined,
     thunkOptions,
-    disableTask = false,
+    enableTaskMiddleware = false,
   } = {}
 ) => {
   const reducer =
@@ -94,7 +94,7 @@ module.exports = (
   }
 
   const store = createStoreWithMiddleware({
-    disableTask,
+    enableTaskMiddleware,
     log: flags.testing && shouldLog,
     history: historyEntries,
     thunkOptions,