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
--- 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.