Bug 1318583: Part 2 - Avoid races due to remote browser resizing round-trips. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 29 Nov 2016 16:09:32 -0800
changeset 445577 d104fbc6aef4a0b15c2bd155e0d649656407e633
parent 445576 a8ba0d1b8d2c512a78f81846139d42cd3b66bd30
child 445578 e7fe082c4349cd750506b25730433d7e21653561
push id37567
push usermaglione.k@gmail.com
push dateWed, 30 Nov 2016 01:09:16 +0000
reviewersaswan
bugs1318583
milestone53.0a1
Bug 1318583: Part 2 - Avoid races due to remote browser resizing round-trips. r?aswan MozReview-Commit-ID: KPFKNhlLsCP
browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
browser/components/extensions/test/browser/browser_ext_pageAction_popup_resize.js
browser/components/extensions/test/browser/head.js
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup_resize.js
@@ -175,17 +175,17 @@ function* testPopupSize(standardsMode, b
       ok(panelTop >= browserWin.screen.availTop, `Top of popup should be on-screen. (${panelTop} >= ${browserWin.screen.availTop})`);
     }
   };
 
   yield awaitBrowserLoaded(browser);
 
   // Wait long enough to make sure the initial resize debouncing timer has
   // expired.
-  yield new Promise(resolve => setTimeout(resolve, 100));
+  yield delay(100);
 
   let dims = yield promiseContentDimensions(browser);
 
   is(dims.isStandards, standardsMode, "Document has the expected compat mode");
 
   // If the browser's preferred height is smaller than the initial height of the
   // panel, then it will still take up the full available vertical space. Even
   // so, we need to check that we've gotten the preferred height calculation
@@ -290,15 +290,15 @@ add_task(function* testBrowserActionMenu
   // it is off-screen, so we need to try more than once.
   for (let i = 0; i < 20; i++) {
     win.moveTo(left, top);
 
     if (win.screenX == left && win.screenY == top) {
       break;
     }
 
-    yield new Promise(resolve => setTimeout(resolve, 100));
+    yield delay(100);
   }
 
   yield testPopupSize(true, win, "bottom");
 
   yield BrowserTestUtils.closeWindow(win);
 });
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_popup_resize.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_popup_resize.js
@@ -1,16 +1,12 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-let delay = ms => new Promise(resolve => {
-  setTimeout(resolve, ms);
-});
-
 add_task(function* testPageActionPopupResize() {
   let browser;
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "page_action": {
         "default_popup": "popup.html",
         "browser_style": true,
@@ -34,27 +30,18 @@ add_task(function* testPageActionPopupRe
 
   yield extension.startup();
   yield extension.awaitMessage("action-shown");
 
   clickPageAction(extension, window);
 
   browser = yield awaitExtensionPanel(extension);
 
-  function* waitForSize(size) {
+  function* checkSize(expected) {
     let dims = yield promiseContentDimensions(browser);
-    for (let i = 0; i < 100 && dims.window.innerWidth != size; i++) {
-      yield delay(50);
-      dims = yield promiseContentDimensions(browser);
-    }
-    return dims;
-  }
-
-  function* checkSize(expected) {
-    let dims = yield waitForSize(expected);
     let {body, root} = dims;
 
     is(dims.window.innerHeight, expected, `Panel window should be ${expected}px tall`);
     is(body.clientHeight, body.scrollHeight,
       "Panel body should be tall enough to fit its contents");
     is(root.clientHeight, root.scrollHeight,
       "Panel root should be tall enough to fit its contents");
 
@@ -78,19 +65,17 @@ add_task(function* testPageActionPopupRe
     300,
   ];
 
   for (let size of sizes) {
     yield alterContent(browser, setSize, size);
     yield checkSize(size);
   }
 
-  yield alterContent(browser, setSize, 1400);
-
-  let dims = yield waitForSize(800);
+  let dims = yield alterContent(browser, setSize, 1400);
   let {body, root} = dims;
 
   is(dims.window.innerWidth, 800, "Panel window width");
   ok(body.clientWidth <= 800, `Panel body width ${body.clientWidth} is less than 800`);
   is(body.scrollWidth, 1400, "Panel body scroll width");
 
   is(dims.window.innerHeight, 600, "Panel window height");
   ok(root.clientHeight <= 600, `Panel root height (${root.clientHeight}px) is less than 600px`);
@@ -142,23 +127,16 @@ add_task(function* testPageActionPopupRe
   /* eslint-disable mozilla/no-cpows-in-tests */
   function setSize(size) {
     content.document.body.style.fontSize = `${size}px`;
   }
   /* eslint-enable mozilla/no-cpows-in-tests */
 
   let dims = yield alterContent(browser, setSize, 18);
 
-  if (AppConstants.platform == "win") {
-    while (dims.window.innerWidth < 800) {
-      yield delay(50);
-      dims = yield promiseContentDimensions(browser);
-    }
-  }
-
   is(dims.window.innerWidth, 800, "Panel window should be 800px wide");
   is(dims.body.clientWidth, 800, "Panel body should be 800px wide");
   is(dims.body.clientWidth, dims.body.scrollWidth,
      "Panel body should be wide enough to fit its contents");
 
   ok(dims.window.innerHeight > 36,
      `Panel window height (${dims.window.innerHeight}px) should be taller than two lines of text.`);
 
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -92,17 +92,17 @@ function promisePopupHidden(popup) {
     let onPopupHidden = event => {
       popup.removeEventListener("popuphidden", onPopupHidden);
       resolve();
     };
     popup.addEventListener("popuphidden", onPopupHidden);
   });
 }
 
-function promiseContentDimensions(browser) {
+function promisePossiblyInaccurateContentDimensions(browser) {
   return ContentTask.spawn(browser, null, function* () {
     function copyProps(obj, props) {
       let res = {};
       for (let prop of props) {
         res[prop] = obj[prop];
       }
       return res;
     }
@@ -116,28 +116,49 @@ function promiseContentDimensions(browse
       root: copyProps(content.document.documentElement,
                       ["clientWidth", "clientHeight", "scrollWidth", "scrollHeight"]),
 
       isStandards: content.document.compatMode !== "BackCompat",
     };
   });
 }
 
-function* awaitPopupResize(browser) {
-  return BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized",
-                                       event => event.detail === "delayed");
+function delay(ms = 0) {
+  return new Promise(resolve => setTimeout(resolve, ms));
 }
 
+var promiseContentDimensions = Task.async(function* (browser) {
+  // For remote browsers, each resize operation requires an asynchronous
+  // round-trip to resize the content window. Since there's a certain amount of
+  // unpredictability in the timing, mainly due to the unpredictability of
+  // reflows, we need to wait until the content window dimensions match the
+  // <browser> dimensions before returning data.
+
+  let dims = yield promisePossiblyInaccurateContentDimensions(browser);
+  while (browser.clientWidth !== dims.window.innerWidth ||
+         browser.clientHeight !== dims.window.innerHeight) {
+    yield delay(50);
+    dims = yield promisePossiblyInaccurateContentDimensions(browser);
+  }
+
+  return dims;
+});
+
+var awaitPopupResize = Task.async(function* awaitPopupResize(browser) {
+  yield BrowserTestUtils.waitForEvent(browser, "WebExtPopupResized",
+                                      event => event.detail === "delayed");
+
+  return promiseContentDimensions(browser);
+});
+
 function alterContent(browser, task, arg = null) {
   return Promise.all([
     ContentTask.spawn(browser, arg, task),
     awaitPopupResize(browser),
-  ]).then(() => {
-    return promiseContentDimensions(browser);
-  });
+  ]).then(([, dims]) => dims);
 }
 
 function getPanelForNode(node) {
   while (node.localName != "panel") {
     node = node.parentNode;
   }
   return node;
 }