Bug 1832891 - [remote] Set timeout to 5s for waitForInitialNavigation for new contexts r=webdriver-reviewers,whimboo, a=dmeehan

Extending the timeout only when we explicitly create a new window or tab should be safe.
On the other hand, initial windows from the browser or new tabs created by the browser UI can remain on the initial document forever, for instance on Android.
So for those cases we keep the current unloadTimeout value.

Differential Revision: https://phabricator.services.mozilla.com/D179710

remote/marionette/driver.sys.mjs

@@ -2290,7 +2290,10 @@ GeckoDriver.prototype.newWindow = async function (cmd) {
   // Actors need the new window to be loaded to safely execute queries.
   // Wait until the initial page load has been finished.
   await lazy.waitForInitialNavigationCompleted(
-    contentBrowser.browsingContext.webProgress
+    contentBrowser.browsingContext.webProgress,
+    {
+      unloadTimeout: 5000,
+    }
   );
 
   const id = lazy.TabManager.getIdForBrowser(contentBrowser);

remote/shared/Navigate.sys.mjs

@@ -39,6 +39,16 @@ XPCOMUtils.defineLazyGetter(lazy, "UNLOAD_TIMEOUT_MULTIPLIER", () => {
   return 1;
 });
 
+export const DEFAULT_UNLOAD_TIMEOUT = 200;
+
+/**
+ * Returns the multiplier used for the unload timer. Useful for tests which
+ * assert the behavior of this timeout.
+ */
+export function getUnloadTimeoutMultiplier() {
+  return lazy.UNLOAD_TIMEOUT_MULTIPLIER;
+}
+
 // Used to keep weak references of webProgressListeners alive.
 const webProgressListeners = new Set();
 
@@ -52,7 +62,8 @@ const webProgressListeners = new Set();
  *     Flag to indicate that the Promise has to be resolved when the
  *     page load has been started. Otherwise wait until the page has
  *     finished loading. Defaults to `false`.
- *
+ * @param {number=} options.unloadTimeout
+ *     Time to allow before the page gets unloaded. See ProgressListener options.
  * @returns {Promise}
  *     Promise which resolves when the page load is in the expected state.
  *     Values as returned:
@@ -63,13 +74,14 @@ export async function waitForInitialNavigationCompleted(
   webProgress,
   options = {}
 ) {
-  const { resolveWhenStarted = false } = options;
+  const { resolveWhenStarted = false, unloadTimeout } = options;
 
   const browsingContext = webProgress.browsingContext;
 
   // Start the listener right away to avoid race conditions.
   const listener = new ProgressListener(webProgress, {
     resolveWhenStarted,
+    unloadTimeout,
   });
   const navigated = listener.start();
 
@@ -145,7 +157,7 @@ export class ProgressListener {
     const {
       expectNavigation = false,
       resolveWhenStarted = false,
-      unloadTimeout = 200,
+      unloadTimeout = DEFAULT_UNLOAD_TIMEOUT,
       waitForExplicitStart = false,
     } = options;
 

remote/shared/test/xpcshell/test_Navigate.js

@@ -6,8 +6,14 @@ const { setTimeout } = ChromeUtils.importESModule(
   "resource://gre/modules/Timer.sys.mjs"
 );
 
-const { ProgressListener, waitForInitialNavigationCompleted } =
-  ChromeUtils.importESModule("chrome://remote/content/shared/Navigate.sys.mjs");
+const {
+  DEFAULT_UNLOAD_TIMEOUT,
+  getUnloadTimeoutMultiplier,
+  ProgressListener,
+  waitForInitialNavigationCompleted,
+} = ChromeUtils.importESModule(
+  "chrome://remote/content/shared/Navigate.sys.mjs"
+);
 
 const CURRENT_URI = Services.io.newURI("http://foo.bar/");
 const INITIAL_URI = Services.io.newURI("about:blank");
@@ -15,6 +21,11 @@ const TARGET_URI = Services.io.newURI("http://foo.cheese/");
 const TARGET_URI_IS_ERROR_PAGE = Services.io.newURI("doesnotexist://");
 const TARGET_URI_WITH_HASH = Services.io.newURI("http://foo.cheese/#foo");
 
+function wait(time) {
+  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+  return new Promise(resolve => setTimeout(resolve, time));
+}
+
 class MockRequest {
   constructor(uri) {
     this.originalURI = uri;
@@ -296,8 +307,7 @@ add_task(
 
     await webProgress.sendStopState();
 
-    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-    await new Promise(resolve => setTimeout(resolve, 100));
+    await wait(100);
 
     await webProgress.sendStartState({ isInitial: false });
     await webProgress.sendStopState();
@@ -335,8 +345,7 @@ add_task(
       "waitForInitialNavigationCompleted has not resolved yet"
     );
 
-    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-    await new Promise(resolve => setTimeout(resolve, 100));
+    await wait(100);
 
     await webProgress.sendStartState({ isInitial: false });
     await webProgress.sendStopState();
@@ -503,6 +512,80 @@ add_task(async function test_waitForInitialNavigation_crossOrigin() {
   equal(targetURI.spec, TARGET_URI.spec, "Expected target URI has been set");
 });
 
+add_task(async function test_waitForInitialNavigation_unloadTimeout_default() {
+  const browsingContext = new MockTopContext();
+  const webProgress = browsingContext.webProgress;
+
+  // Stop the navigation on an initial page which is not loading anymore.
+  // This situation happens with new tabs on Android, even though they are on
+  // the initial document, they will not start another navigation on their own.
+  await webProgress.sendStartState({ isInitial: true });
+  await webProgress.sendStopState();
+
+  ok(!webProgress.isLoadingDocument, "Document is not loading");
+
+  const navigated = waitForInitialNavigationCompleted(webProgress);
+
+  // Start a timer longer than the timeout which will be used by
+  // waitForInitialNavigationCompleted, and check that navigated resolves first.
+  const waitForMoreThanDefaultTimeout = wait(
+    DEFAULT_UNLOAD_TIMEOUT * 1.5 * getUnloadTimeoutMultiplier()
+  );
+  await Promise.race([navigated, waitForMoreThanDefaultTimeout]);
+
+  ok(
+    await hasPromiseResolved(navigated),
+    "waitForInitialNavigationCompleted has resolved"
+  );
+
+  ok(!webProgress.isLoadingDocument, "Document is not loading");
+  ok(
+    webProgress.browsingContext.currentWindowGlobal.isInitialDocument,
+    "Document is still on the initial document"
+  );
+});
+
+add_task(async function test_waitForInitialNavigation_unloadTimeout_longer() {
+  const browsingContext = new MockTopContext();
+  const webProgress = browsingContext.webProgress;
+
+  // Stop the navigation on an initial page which is not loading anymore.
+  // This situation happens with new tabs on Android, even though they are on
+  // the initial document, they will not start another navigation on their own.
+  await webProgress.sendStartState({ isInitial: true });
+  await webProgress.sendStopState();
+
+  ok(!webProgress.isLoadingDocument, "Document is not loading");
+
+  const navigated = waitForInitialNavigationCompleted(webProgress, {
+    unloadTimeout: DEFAULT_UNLOAD_TIMEOUT * 3,
+  });
+
+  // Start a timer longer than the default timeout of the Navigate module.
+  // However here we used a custom timeout, so we expect that the navigation
+  // will not be done yet by the time this timer is done.
+  const waitForMoreThanDefaultTimeout = wait(
+    DEFAULT_UNLOAD_TIMEOUT * 1.5 * getUnloadTimeoutMultiplier()
+  );
+  await Promise.race([navigated, waitForMoreThanDefaultTimeout]);
+
+  // The promise should not have resolved because we didn't reached the custom
+  // timeout which is 3 times the default one.
+  ok(
+    !(await hasPromiseResolved(navigated)),
+    "waitForInitialNavigationCompleted has not resolved yet"
+  );
+
+  // The navigation should eventually resolve once we reach the custom timeout.
+  await navigated;
+
+  ok(!webProgress.isLoadingDocument, "Document is not loading");
+  ok(
+    webProgress.browsingContext.currentWindowGlobal.isInitialDocument,
+    "Document is still on the initial document"
+  );
+});
+
 add_task(async function test_ProgressListener_expectNavigation() {
   const browsingContext = new MockTopContext();
   const webProgress = browsingContext.webProgress;
@@ -514,8 +597,7 @@ add_task(async function test_ProgressListener_expectNavigation() {
   const navigated = progressListener.start();
 
   // Wait for unloadTimeout to finish in case it started
-  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-  await new Promise(resolve => setTimeout(resolve, 30));
+  await wait(30);
 
   ok(!(await hasPromiseResolved(navigated)), "Listener has not resolved yet");
 
@@ -542,8 +624,7 @@ add_task(
     await webProgress.sendStopState();
 
     // Wait for unloadTimeout to finish in case it started
-    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-    await new Promise(resolve => setTimeout(resolve, 30));
+    await wait(30);
 
     ok(!(await hasPromiseResolved(navigated)), "Listener has not resolved yet");
 

remote/webdriver-bidi/modules/root/browsingContext.sys.mjs

@@ -252,7 +252,10 @@ class BrowsingContextModule extends Module {
     }
 
     await lazy.waitForInitialNavigationCompleted(
-      browser.browsingContext.webProgress
+      browser.browsingContext.webProgress,
+      {
+        unloadTimeout: 5000,
+      }
     );
 
     return {