[tbb-commits] [tor-browser] 35/37: Bug 1745595 - wait for expected geometry after move or resize. r=whimboo, a=RyanVM

gitolite role git at cupani.torproject.org
Wed Jun 22 18:27:44 UTC 2022


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+ at 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/e567185fa464270f94430e7cf62d134f4df9a69f/layout/base/nsRefreshDriver.cpp#1730-1731
    
    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
+      );
+    }
   });
 }
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tbb-commits mailing list