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 344653 fd85a5aafa9fdc2fbe012408e996f573308fb697
parent 344652 e1624f6a808758de2ea3bb7bcaf426b155663ec3
child 344654 ea11b190dac4446444d68f0fdf0ae44a38aaa0bc
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 - 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
   };
 });