Bug 1336198 - Part 10: Fix race condition in boxmodel tests. r=gl
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 22 Feb 2017 20:48:23 +0100
changeset 344675 87332bfc1a315266d7f985fcbc84a72669d9fc40
parent 344674 9dc27009b8612c767612d157441609f2bbb0afd2
child 344676 5fc0af6d26ce88ebb456d55e80a7f24f66fc6d87
push id31414
push usercbook@mozilla.com
push dateFri, 24 Feb 2017 10:47:41 +0000
treeherdermozilla-central@be661bae6cb9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1336198
milestone54.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 1336198 - Part 10: Fix race condition in boxmodel tests. r=gl MozReview-Commit-ID: IDBgbnlMX2c
devtools/client/inspector/boxmodel/box-model.js
devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-navigation.js
devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-reload.js
devtools/client/inspector/boxmodel/test/browser_boxmodel_update-in-iframes.js
devtools/client/inspector/boxmodel/test/head.js
devtools/client/inspector/test/shared-head.js
--- a/devtools/client/inspector/boxmodel/box-model.js
+++ b/devtools/client/inspector/boxmodel/box-model.js
@@ -121,18 +121,26 @@ BoxModel.prototype = {
     }
 
     this.reflowFront.off("reflows", this.updateBoxModel);
     this.reflowFront.stop();
   },
 
   /**
    * Updates the box model panel by dispatching the new layout data.
+   *
+   * @param  {String} reason
+   *         Optional string describing the reason why the boxmodel is updated.
    */
-  updateBoxModel() {
+  updateBoxModel(reason) {
+    this._updateReasons = this._updateReasons || [];
+    if (reason) {
+      this._updateReasons.push(reason);
+    }
+
     let lastRequest = Task.spawn((function* () {
       if (!(this.isPanelVisible() &&
           this.inspector.selection.isConnected() &&
           this.inspector.selection.isElementNode())) {
         return null;
       }
 
       let node = this.inspector.selection.nodeFront;
@@ -146,19 +154,21 @@ BoxModel.prototype = {
       // model view.
       this.store.dispatch(updateLayout(layout));
 
       // If a subsequent request has been made, wait for that one instead.
       if (this._lastRequest != lastRequest) {
         return this._lastRequest;
       }
 
-      this._lastRequest = null;
+      this.inspector.emit("boxmodel-view-updated", this._updateReasons);
 
-      this.inspector.emit("boxmodel-view-updated");
+      this._lastRequest = null;
+      this._updateReasons = [];
+
       return null;
     }).bind(this)).catch(console.error);
 
     this._lastRequest = lastRequest;
   },
 
   /**
    * Selection 'new-node-front' event handler.
@@ -168,17 +178,17 @@ BoxModel.prototype = {
       return;
     }
 
     if (this.inspector.selection.isConnected() &&
         this.inspector.selection.isElementNode()) {
       this.trackReflows();
     }
 
-    this.updateBoxModel();
+    this.updateBoxModel("new-selection");
   },
 
   /**
    * Hides the box-model highlighter on the currently selected element.
    */
   onHideBoxModelHighlighter() {
     let toolbox = this.inspector.toolbox;
     toolbox.highlighterUtils.unhighlight();
--- a/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-navigation.js
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-navigation.js
@@ -12,24 +12,26 @@ const IFRAME2 = URL_ROOT + "doc_boxmodel
 
 add_task(function* () {
   yield addTab(IFRAME1);
   let {inspector, view, testActor} = yield openBoxModelView();
 
   yield testFirstPage(inspector, view, testActor);
 
   info("Navigate to the second page");
+  let onMarkupLoaded = waitForMarkupLoaded(inspector);
   yield testActor.eval(`content.location.href="${IFRAME2}"`);
-  yield inspector.once("markuploaded");
+  yield onMarkupLoaded;
 
   yield testSecondPage(inspector, view, testActor);
 
   info("Go back to the first page");
+  onMarkupLoaded = waitForMarkupLoaded(inspector);
   yield testActor.eval("content.history.back();");
-  yield inspector.once("markuploaded");
+  yield onMarkupLoaded;
 
   yield testBackToFirstPage(inspector, view, testActor);
 });
 
 function* testFirstPage(inspector, view, testActor) {
   info("Test that the box model view works on the first page");
 
   yield selectNode("p", inspector);
--- a/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-reload.js
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-after-reload.js
@@ -9,18 +9,19 @@
 add_task(function* () {
   yield addTab(URL_ROOT + "doc_boxmodel_iframe1.html");
   let {inspector, view, testActor} = yield openBoxModelView();
 
   info("Test that the box model view works on the first page");
   yield assertBoxModelView(inspector, view, testActor);
 
   info("Reload the page");
+  let onMarkupLoaded = waitForMarkupLoaded(inspector);
   yield testActor.reload();
-  yield inspector.once("markuploaded");
+  yield onMarkupLoaded;
 
   info("Test that the box model view works on the reloaded page");
   yield assertBoxModelView(inspector, view, testActor);
 });
 
 function* assertBoxModelView(inspector, view, testActor) {
   yield selectNode("p", inspector);
 
--- a/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-in-iframes.js
+++ b/devtools/client/inspector/boxmodel/test/browser_boxmodel_update-in-iframes.js
@@ -35,18 +35,19 @@ function* testResizingInIframe(inspector
   is(sizeElt.textContent, "200\u00D7200");
 }
 
 function* testReflowsAfterIframeDeletion(inspector, view, testActor) {
   info("Test reflows are still sent to the box model view after deleting an " +
        "iframe");
 
   info("Deleting the iframe2");
+  let onInspectorUpdated = inspector.once("inspector-updated");
   yield removeIframe2(testActor);
-  yield inspector.once("inspector-updated");
+  yield onInspectorUpdated;
 
   info("Selecting the test node in iframe1");
   yield selectNodeInIframe1("p", inspector);
 
   info("Checking that the box model view shows the right value");
   let sizeElt = view.document.querySelector(".boxmodel-size > span");
   is(sizeElt.textContent, "100\u00D7100");
 
--- a/devtools/client/inspector/boxmodel/test/head.js
+++ b/devtools/client/inspector/boxmodel/test/head.js
@@ -14,37 +14,39 @@ Services.scriptloader.loadSubScript(
 Services.prefs.setIntPref("devtools.toolbox.footer.height", 350);
 registerCleanupFunction(() => {
   Services.prefs.clearUserPref("devtools.toolbox.footer.height");
 });
 
 /**
  * Highlight a node and set the inspector's current selection to the node or
  * the first match of the given css selector.
- * @param {String|NodeFront} selectorOrNodeFront
- *        The selector for the node to be set, or the nodeFront
- * @param {InspectorPanel} inspector
- *        The instance of InspectorPanel currently loaded in the toolbox
- * @return a promise that resolves when the inspector is updated with the new
- * node
+ *
+ * @param  {String|NodeFront} selectorOrNodeFront
+ *         The selector for the node to be set, or the nodeFront.
+ * @param  {InspectorPanel} inspector
+ *         The instance of InspectorPanel currently loaded in the toolbox.
+ * @return {Promise} a promise that resolves when the inspector is updated with the new
+ *         node.
  */
 function* selectAndHighlightNode(selectorOrNodeFront, inspector) {
   info("Highlighting and selecting the node " + selectorOrNodeFront);
 
   let nodeFront = yield getNodeFront(selectorOrNodeFront, inspector);
   let updated = inspector.toolbox.once("highlighter-ready");
   inspector.selection.setNodeFront(nodeFront, "test-highlight");
   yield updated;
 }
 
 /**
  * Open the toolbox, with the inspector tool visible, and the computed view
  * sidebar tab selected to display the box model view.
- * @return a promise that resolves when the inspector is ready and the box model
- * view is visible and ready
+ *
+ * @return {Promise} a promise that resolves when the inspector is ready and the box model
+ *         view is visible and ready.
  */
 function openBoxModelView() {
   return openInspectorSidebarTab("computedview").then(data => {
     // The actual highligher show/hide methods are mocked in box model tests.
     // The highlighter is tested in devtools/inspector/test.
     function mockHighlighter({highlighter}) {
       highlighter.showBoxModel = function () {
         return promise.resolve();
@@ -61,20 +63,47 @@ function openBoxModelView() {
       view: data.inspector.computedview,
       testActor: data.testActor
     };
   });
 }
 
 /**
  * Wait for the boxmodel-view-updated event.
- * @return a promise
+ *
+ * @param  {InspectorPanel} inspector
+ *         The instance of InspectorPanel currently loaded in the toolbox.
+ * @param  {Boolean} waitForSelectionUpdate
+ *         Should the boxmodel-view-updated event come from a new selection.
+ * @return {Promise} a promise
  */
-function waitForUpdate(inspector) {
-  return inspector.once("boxmodel-view-updated");
+function waitForUpdate(inspector, waitForSelectionUpdate) {
+  return new Promise(resolve => {
+    inspector.on("boxmodel-view-updated", function onUpdate(e, reasons) {
+      // Wait for another update event if we are waiting for a selection related event.
+      if (waitForSelectionUpdate && !reasons.includes("new-selection")) {
+        return;
+      }
+
+      inspector.off("boxmodel-view-updated", onUpdate);
+      resolve();
+    });
+  });
+}
+
+/**
+ * Wait for both boxmode-view-updated and markuploaded events.
+ *
+ * @return {Promise} a promise that resolves when both events have been received.
+ */
+function waitForMarkupLoaded(inspector) {
+  return Promise.all([
+    waitForUpdate(inspector),
+    inspector.once("markuploaded"),
+  ]);
 }
 
 function getStyle(testActor, selector, propertyName) {
   return testActor.eval(`
     content.document.querySelector("${selector}")
                     .style.getPropertyValue("${propertyName}");
   `);
 }
@@ -88,11 +117,12 @@ function setStyle(testActor, selector, p
 
 /**
  * The box model doesn't participate in the inspector's update mechanism, so simply
  * calling the default selectNode isn't enough to guarantee that the box model view has
  * finished updating. We also need to wait for the "boxmodel-view-updated" event.
  */
 var _selectNode = selectNode;
 selectNode = function* (node, inspector, reason) {
+  let onUpdate = waitForUpdate(inspector, true);
   yield _selectNode(node, inspector, reason);
-  yield waitForUpdate(inspector);
+  yield onUpdate;
 };
--- a/devtools/client/inspector/test/shared-head.js
+++ b/devtools/client/inspector/test/shared-head.js
@@ -47,17 +47,25 @@ var openInspector = Task.async(function*
  *        The ID of the sidebar tab to be opened
  * @return a promise that resolves when the inspector is ready and the tab is
  * visible and ready
  */
 var openInspectorSidebarTab = Task.async(function* (id) {
   let {toolbox, inspector, testActor} = yield openInspector();
 
   info("Selecting the " + id + " sidebar");
-  inspector.sidebar.select(id);
+
+  if (id === "computedview" || id === "layoutview") {
+    // The layout and computed views should wait until the box-model widget is ready.
+    let onBoxModelViewReady = inspector.once("boxmodel-view-updated");
+    inspector.sidebar.select(id);
+    yield onBoxModelViewReady;
+  } else {
+    inspector.sidebar.select(id);
+  }
 
   return {
     toolbox,
     inspector,
     testActor
   };
 });