This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-91.11.0esr-11.5-1 in repository tor-browser.
commit 0bf971823825c36d4ee3c68095c0808c48fa4326 Author: Karl Tomlinson karlt+@karlt.net AuthorDate: Wed Jun 1 19:32:46 2022 +1200
Bug 1745595 - wait for expected geometry after move or resize. r=whimboo, a=RyanVM
The requestAnimationFrame() callback used in IdlePromise() may run sooner than 1/60 second, providing insufficient time for changes to be effected. https://searchfox.org/mozilla-central/rev/e567185fa464270f94430e7cf62d134f4d...
Waiting for the "resize" and "MozUpdateWindowPos" events should provide minimum wait in the common cases that the OS completes the changes requested.
This change should also resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1702255
Differential Revision: https://phabricator.services.mozilla.com/D147729 --- remote/marionette/driver.js | 68 +++++++++++++++++++++++++++++--------- remote/marionette/sync.js | 80 ++++++++++++++++++++++++++++++++------------- 2 files changed, 110 insertions(+), 38 deletions(-)
diff --git a/remote/marionette/driver.js b/remote/marionette/driver.js index 133f4dfdd52db..f51f82f25134f 100644 --- a/remote/marionette/driver.js +++ b/remote/marionette/driver.js @@ -52,6 +52,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { "chrome://remote/content/marionette/actors/MarionetteCommandsParent.jsm", unregisterEventsActor: "chrome://remote/content/marionette/actors/MarionetteEventsParent.jsm", + waitForEvent: "chrome://remote/content/marionette/sync.js", waitForLoadEvent: "chrome://remote/content/marionette/sync.js", waitForObserverTopic: "chrome://remote/content/marionette/sync.js", WebDriverSession: "chrome://remote/content/shared/webdriver/Session.jsm", @@ -1247,7 +1248,19 @@ GeckoDriver.prototype.setWindowRect = async function(cmd) { assert.open(this.getBrowsingContext({ top: true })); await this._handleUserPrompts();
- let { x, y, width, height } = cmd.parameters; + const { x = null, y = null, width = null, height = null } = cmd.parameters; + if (x !== null) { + assert.integer(x); + } + if (y !== null) { + assert.integer(y); + } + if (height !== null) { + assert.positiveInteger(height); + } + if (width !== null) { + assert.positiveInteger(width); + }
const win = this.getCurrentWindow(); switch (WindowState.from(win.windowState)) { @@ -1261,23 +1274,48 @@ GeckoDriver.prototype.setWindowRect = async function(cmd) { break; }
- if (width != null && height != null) { - assert.positiveInteger(height); - assert.positiveInteger(width); - - if (win.outerWidth != width || win.outerHeight != height) { + function geometryMatches() { + if ( + width !== null && + height !== null && + (win.outerWidth !== width || win.outerHeight !== height) + ) { + return false; + } + if (x !== null && y !== null && (win.screenX !== x || win.screenY !== y)) { + return false; + } + logger.trace(`Requested window geometry matches`); + return true; + } + + if (!geometryMatches()) { + // There might be more than one resize or MozUpdateWindowPos event due + // to previous geometry changes, such as from restoreWindow(), so + // wait longer if window geometry does not match. + const options = { checkFn: geometryMatches, timeout: 500 }; + const promises = []; + if (width !== null && height !== null) { + promises.push(waitForEvent(win, "resize", options)); win.resizeTo(width, height); - await new IdlePromise(win); } - } - - if (x != null && y != null) { - assert.integer(x); - assert.integer(y); - - if (win.screenX != x || win.screenY != y) { + if (x !== null && y !== null) { + promises.push( + waitForEvent(win.windowRoot, "MozUpdateWindowPos", options) + ); win.moveTo(x, y); - await new IdlePromise(win); + } + try { + await Promise.race(promises); + } catch (e) { + if (e instanceof error.TimeoutError) { + // The operating system might not honor the move or resize, in which + // case assume that geometry will have been adjusted "as close as + // possible" to that requested. There may be no event received if the + // geometry is already as close as possible. + } else { + throw e; + } } }
diff --git a/remote/marionette/sync.js b/remote/marionette/sync.js index 9d66bbf2b9f56..42ffa19583229 100644 --- a/remote/marionette/sync.js +++ b/remote/marionette/sync.js @@ -465,22 +465,32 @@ this.DebounceCallback = DebounceCallback; * Extra options. * @param {boolean=} options.capture * True to use a capturing listener. - * @param {function(Event)=} options.checkFn + * @param {function(Event)=} [null] options.checkFn * Called with the ``Event`` object as argument, should return ``true`` * if the event is the expected one, or ``false`` if it should be * ignored and listening should continue. If not specified, the first * event with the specified name resolves the returned promise. + * @param {number=} [null] options.timeout + * Timeout duration in milliseconds, if provided. + * If specified, then the returned promise will be rejected with + * TimeoutError, if not already resolved, after this duration has elapsed. + * If not specified, then no timeout is used. * @param {boolean=} options.wantsUntrusted * True to receive synthetic events dispatched by web content. * * @return {Promise.<Event>} * Promise which resolves to the received ``Event`` object, or rejects - * in case of a failure. + * in case of a failure or after options.timeout milliseconds if specified. */ function waitForEvent( subject, eventName, - { capture = false, checkFn = null, wantsUntrusted = false } = {} + { + capture = false, + checkFn = null, + timeout = null, + wantsUntrusted = false, + } = {} ) { if (subject == null || !("addEventListener" in subject)) { throw new TypeError(); @@ -494,33 +504,57 @@ function waitForEvent( if (checkFn != null && typeof checkFn != "function") { throw new TypeError(); } + if (timeout != null && typeof timeout != "number") { + throw new TypeError(); + } + if (timeout < 0) { + throw new RangeError(); + } if (wantsUntrusted != null && typeof wantsUntrusted != "boolean") { throw new TypeError(); }
return new Promise((resolve, reject) => { - subject.addEventListener( - eventName, - function listener(event) { - logger.trace(`Received DOM event ${event.type} for ${event.target}`); + let timer; + + function cleanUp() { + timer?.cancel(); + subject.removeEventListener(eventName, listener, capture); + } + + function listener(event) { + logger.trace(`Received DOM event ${event.type} for ${event.target}`); + try { + if (checkFn && !checkFn(event)) { + return; + } + cleanUp(); + executeSoon(() => resolve(event)); + } catch (ex) { try { - if (checkFn && !checkFn(event)) { - return; - } - subject.removeEventListener(eventName, listener, capture); - executeSoon(() => resolve(event)); - } catch (ex) { - try { - subject.removeEventListener(eventName, listener, capture); - } catch (ex2) { - // Maybe the provided object does not support removeEventListener. - } - executeSoon(() => reject(ex)); + cleanUp(); + } catch (ex2) { + // Maybe the provided object does not support removeEventListener. } - }, - capture, - wantsUntrusted - ); + executeSoon(() => reject(ex)); + } + } + + subject.addEventListener(eventName, listener, capture, wantsUntrusted); + + if (timeout !== null) { + timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); + timer.init( + () => { + cleanUp(); + reject( + new error.TimeoutError(`EventPromise timed out after ${timeout} ms`) + ); + }, + timeout, + TYPE_ONE_SHOT + ); + } }); }