Bug 1336198 - part10: fix race condition in boxmodel tests;r=gl
☠☠ backed out by 77b0ddb1fd71 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 22 Feb 2017 20:48:23 +0100
changeset 373680 fd85a5aafa9fdc2fbe012408e996f573308fb697
parent 373679 e1624f6a808758de2ea3bb7bcaf426b155663ec3
child 373681 ea11b190dac4446444d68f0fdf0ae44a38aaa0bc
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1336198
milestone54.0a1
Bug 1336198 - part10: 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
@@ -61,20 +61,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 +115,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
   };
 });