Bug 1364319 - Make setWindowRect deterministic; r?automatedtester draft
authorAndreas Tolfsen <ato@sny.no>
Wed, 21 Jun 2017 17:13:51 +0100
changeset 598366 80ad930289d89fa4fa0c13486ea9cc5656b25a9f
parent 598307 92eb911c35da48907d326604c4c92cf55e551895
child 634450 88b29011092fc25ea7a45ab21d1f233b374caef4
push id65176
push userbmo:ato@sny.no
push dateWed, 21 Jun 2017 16:30:07 +0000
reviewersautomatedtester
bugs1364319
milestone56.0a1
Bug 1364319 - Make setWindowRect deterministic; r?automatedtester The Marionette setWindowRect command is meant to provide a blocking API for resizing and moving the window. This currently has an intermittent rate of ~0.254 with the WPT conformance test. The main issue in providing a blocking API is that the DOM resize event fires at a high rate and needs to be throttled. We are able to throttle this successfully with requestAnimationFrame, and before that, a hard-coded 60 fps setTimeout delay. To the naked eye, this appears to synchronise window resizing before returning the resposne to the client. However, occasionally the response contains the wrong window size. window.outerWidth and window.outerHeight do not appear to be deterministic as DOM properties are not synchronously populated. Calls to ChromeWindow.outerWidth and ChromeWindow.outerHeight sometimes also returns inconsistent values. Tests, document in the bug, have shown that somtimes, the returned window size from the setWindowRect command is correct, and that the size from the subsequent getWindowRect command is not. By using a combination of Services.tm.mainThread.idleDispatch and a blocking promise on not returning a response until the window size has changed, we are able to reduce the intermittent rate significantly (by over an order of magnitude). As one would expect, delaying computation allows DOM property values to populate, and as such does not address the underlying problem or make it inconceivable that the race described above could re-appear, but it does seem to be more reliable than the current approach. MozReview-Commit-ID: Lf2yeeXH082
testing/marionette/browser.js
testing/marionette/driver.js
--- a/testing/marionette/browser.js
+++ b/testing/marionette/browser.js
@@ -156,16 +156,31 @@ browser.Context = class {
       return this.contentBrowser.currentURI.spec;
 
     } else {
       throw new NoSuchWindowError("Current window does not have a content browser");
     }
   }
 
   /**
+   * Gets the position and dimensions of the top-level browsing context.
+   *
+   * @return {Map.<string, number>}
+   *     Object with |x|, |y|, |width|, and |height| properties.
+   */
+  get rect() {
+    return {
+      x: this.window.screenX,
+      y: this.window.screenY,
+      width: this.window.outerWidth,
+      height: this.window.outerHeight,
+    };
+  }
+
+  /**
    * Retrieves the current tabmodal UI object.  According to the browser
    * associated with the currently selected tab.
    */
   getTabModalUI() {
     let br = this.contentBrowser;
     if (!br.hasAttribute("tabmodalPromptShowing")) {
       return null;
     }
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -1257,22 +1257,17 @@ GeckoDriver.prototype.getChromeWindowHan
  *     Top-level browsing context has been discarded.
  * @throws {UnexpectedAlertOpenError}
  *     A modal dialog is open, blocking this operation.
  */
 GeckoDriver.prototype.getWindowRect = function (cmd, resp) {
   const win = assert.window(this.getCurrentWindow());
   assert.noUserPrompt(this.dialog);
 
-  return {
-    x: win.screenX,
-    y: win.screenY,
-    width: win.outerWidth,
-    height: win.outerHeight,
-  };
+  return this.curBrowser.rect;
 };
 
 /**
  * Set the window position and size of the browser on the operating
  * system window manager.
  *
  * The supplied |width| and |height| values refer to the window outerWidth
  * and outerHeight values, which include browser chrome and OS-level
@@ -1295,65 +1290,98 @@ GeckoDriver.prototype.getWindowRect = fu
  *
  * @throws {UnsupportedOperationError}
  *     Not applicable to application.
  * @throws {NoSuchWindowError}
  *     Top-level browsing context has been discarded.
  * @throws {UnexpectedAlertOpenError}
  *     A modal dialog is open, blocking this operation.
  */
-GeckoDriver.prototype.setWindowRect = function* (cmd, resp) {
+GeckoDriver.prototype.setWindowRect = async function (cmd, resp) {
   assert.firefox()
   const win = assert.window(this.getCurrentWindow());
   assert.noUserPrompt(this.dialog);
 
   let {x, y, width, height} = cmd.parameters;
-  let optimisedResize = resolve => () => win.requestAnimationFrame(resolve);
-
-  if (win.windowState == win.STATE_FULLSCREEN) {
-    yield new Promise(resolve => {
+  let origRect = this.curBrowser.rect;
+
+  // Throttle resize event by forcing the event queue to flush and delay
+  // until the main thread is idle.
+  function optimisedResize (resolve) {
+    return () => Services.tm.mainThread.idleDispatch(() => {
+      win.requestAnimationFrame(resolve);
+    });
+  }
+
+  // Exit fullscreen and wait for window to resize.
+  async function exitFullscreen () {
+    return new Promise(resolve => {
       win.addEventListener("sizemodechange", optimisedResize(resolve), {once: true});
       win.fullScreen = false;
     });
   }
 
+  // Synchronous resize to |width| and |height| dimensions.
+  async function resizeWindow (width, height) {
+    return new Promise(resolve => {
+      win.addEventListener("resize", optimisedResize(resolve), {once: true});
+      win.resizeTo(width, height);
+    });
+  }
+
+  // Wait until window size has changed.  We can't wait for the
+  // user-requested window size as this may not be achievable on the
+  // current system.
+  const windowResizeChange = async () => {
+    return wait.until((resolve, reject) => {
+      let curRect = this.curBrowser.rect;
+      if (curRect.width != origRect.width &&
+          curRect.height != origRect.height) {
+        resolve();
+      } else {
+        reject();
+      }
+    });
+  };
+
+  // Wait for the window position to change.
+  async function windowPosition (x, y) {
+    return wait.until((resolve, reject) => {
+      if ((x == win.screenX && y == win.screenY) ||
+          (win.screenX != origRect.x || win.screenY != origRect.y)) {
+        resolve();
+      } else {
+        reject();
+      }
+    });
+  }
+
+  if (win.windowState == win.STATE_FULLSCREEN) {
+    await exitFullscreen();
+  }
+
   if (height != null && width != null) {
     assert.positiveInteger(height);
     assert.positiveInteger(width);
 
     if (win.outerWidth != width || win.outerHeight != height) {
-      yield new Promise(resolve => {
-        win.addEventListener("resize", optimisedResize(resolve), {once: true});
-        win.resizeTo(width, height);
-      });
+      await resizeWindow(width, height);
+      await windowResizeChange();
     }
   }
 
   if (x != null && y != null) {
     assert.integer(x);
     assert.integer(y);
-    const orig = {screenX: win.screenX, screenY: win.screenY};
 
     win.moveTo(x, y);
-    yield wait.until((resolve, reject) => {
-      if ((x == win.screenX && y == win.screenY) ||
-        (win.screenX != orig.screenX || win.screenY != orig.screenY)) {
-        resolve();
-      } else {
-        reject();
-      }
-    });
+    await windowPosition(x, y);
   }
 
-  resp.body = {
-    "x": win.screenX,
-    "y": win.screenY,
-    "width": win.outerWidth,
-    "height": win.outerHeight,
-  };
+  return this.curBrowser.rect;
 };
 
 /**
  * Switch current top-level browsing context by name or server-assigned ID.
  * Searches for windows by name, then ID.  Content windows take precedence.
  *
  * @param {string} name
  *     Target name or ID of the window to switch to.