Bug 1927073 - [remote] Retrieve new browsing context if old one was replaced when forwarding a command to the window global. r=webdriver-reviewers,jdescottes

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

remote/shared/messagehandler/Errors.sys.mjs

@@ -68,6 +68,16 @@ class MessageHandlerError extends RemoteError {
   }
 }
 
+/**
+ * A browsing context is no longer available.
+ */
+class DiscardedBrowsingContextError extends MessageHandlerError {
+  constructor(message) {
+    super(message);
+    this.status = `discarded browsing context`;
+  }
+}
+
 /**
  * A command could not be handled by the message handler network.
  */
@@ -79,12 +89,14 @@ class UnsupportedCommandError extends MessageHandlerError {
 }
 
 const STATUSES = new Map([
+  ["discarded browsing context", DiscardedBrowsingContextError],
   ["message handler error", MessageHandlerError],
   ["unsupported message handler command", UnsupportedCommandError],
 ]);
 
 /** @namespace */
 export const error = {
+  DiscardedBrowsingContextError,
   MessageHandlerError,
   UnsupportedCommandError,
 };

remote/shared/messagehandler/test/browser/browser_handle_command_errors.js

@@ -54,7 +54,7 @@ add_task(async function test_destination_error() {
           id: fakeBrowsingContextId,
         },
       }),
-    err => err.message == `Unable to find a BrowsingContext for id -1`
+    err => err.message == `Unable to find a BrowsingContext for id "-1"`
   );
 
   rootMessageHandler.destroy();

remote/shared/messagehandler/test/browser/browser_handle_command_retry.js

@@ -6,6 +6,9 @@
 const { isInitialDocument } = ChromeUtils.importESModule(
   "chrome://remote/content/shared/messagehandler/transports/BrowsingContextUtils.sys.mjs"
 );
+const { RootMessageHandler } = ChromeUtils.importESModule(
+  "chrome://remote/content/shared/messagehandler/RootMessageHandler.sys.mjs"
+);
 
 // We are forcing the actors to shutdown while queries are unresolved.
 const { PromiseTestUtils } = ChromeUtils.importESModule(
@@ -116,8 +119,81 @@ add_task(async function test_forced_no_retry() {
 
     await Assert.rejects(
       onBlockedOneTime,
-      e => e.name == "AbortError",
-      "Caught the expected abort error when reloading"
+      e => e.name == "DiscardedBrowsingContextError",
+      "Caught the expected error when reloading"
+    );
+  } finally {
+    await cleanup(rootMessageHandler, tab);
+  }
+});
+
+// Test that without retry behavior, a pending command rejects when the
+// underlying browsing context is discarded.
+add_task(async function test_forced_no_retry_cross_group() {
+  const tab = BrowserTestUtils.addTab(
+    gBrowser,
+    "https://example.com/document-builder.sjs?html=COM" +
+      // Attach an unload listener to prevent the page from going into bfcache,
+      // so that pending queries will be rejected with an AbortError.
+      "<script type='text/javascript'>window.onunload = function() {};</script>"
+  );
+  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  const browsingContext = tab.linkedBrowser.browsingContext;
+
+  const rootMessageHandler = createRootMessageHandler("session-id-no-retry");
+
+  try {
+    const onBlockedOneTime = rootMessageHandler.handleCommand({
+      moduleName: "retry",
+      commandName: "blockedOneTime",
+      destination: {
+        type: WindowGlobalMessageHandler.type,
+        id: browsingContext.id,
+      },
+      retryOnAbort: false,
+    });
+
+    // This command will return when the old browsing context was discarded.
+    const onDiscarded = rootMessageHandler.handleCommand({
+      moduleName: "retry",
+      commandName: "waitForDiscardedBrowsingContext",
+      destination: {
+        type: RootMessageHandler.type,
+      },
+      params: {
+        browsingContext,
+        retryOnAbort: false,
+      },
+    });
+
+    ok(
+      !(await hasPromiseResolved(onBlockedOneTime)),
+      "blockedOneTime should not have resolved yet"
+    );
+    // Bug 1927144: Causes a "A promise chain failed to handle a rejection" error.
+    // ok(
+    //   !(await hasPromiseResolved(onDiscarded)),
+    //   "waitForDiscardedBrowsingContext should not have resolved yet"
+    // );
+
+    info(
+      "Navigate to example.net with COOP headers to destroy browsing context"
+    );
+    await loadURL(
+      tab.linkedBrowser,
+      "https://example.net/document-builder.sjs?headers=Cross-Origin-Opener-Policy:same-origin&html=NET"
+    );
+
+    await Assert.rejects(
+      onBlockedOneTime,
+      e => e.name == "DiscardedBrowsingContextError",
+      "Caught the expected error when navigating"
+    );
+
+    await Assert.rejects(
+      onDiscarded,
+      e => e.name == "DiscardedBrowsingContextError",
+      "Caught the expected error when navigating"
     );
   } finally {
     await cleanup(rootMessageHandler, tab);
@@ -218,8 +294,8 @@ add_task(async function test_forced_retry() {
     );
     await Assert.rejects(
       onBlockedElevenTimes,
-      e => e.name == "AbortError",
-      "Caught the expected abort error when reloading"
+      e => e.name == "DiscardedBrowsingContextError",
+      "Caught the expected error when reloading"
     );
   } finally {
     await cleanup(rootMessageHandler, tab);
@@ -262,12 +338,28 @@ add_task(async function test_retry_cross_group() {
       retryOnAbort: true,
     });
 
+    // This command will return when the old browsing context was discarded.
+    const onDiscarded = rootMessageHandler.handleCommand({
+      moduleName: "retry",
+      commandName: "waitForDiscardedBrowsingContext",
+      destination: {
+        type: RootMessageHandler.type,
+      },
+      params: {
+        browsingContext,
+        retryOnAbort: true,
+      },
+    });
+
     info("Reload one time");
     await BrowserTestUtils.reloadTab(tab);
 
     info("blockedOnNetDomain should not have resolved yet");
     ok(!(await hasPromiseResolved(onBlockedOnNetDomain)));
 
+    info("waitForDiscardedBrowsingContext should not have resolved yet");
+    ok(!(await hasPromiseResolved(onDiscarded)));
+
     info(
       "Navigate to example.net with COOP headers to destroy browsing context"
     );
@@ -279,6 +371,9 @@ add_task(async function test_retry_cross_group() {
     info("blockedOnNetDomain should resolve now");
     let { foo } = await onBlockedOnNetDomain;
     is(foo, "bar", "The parameter was sent when the command was retried");
+
+    info("waitForDiscardedBrowsingContext should resolve now");
+    await onDiscarded;
   } finally {
     await cleanup(rootMessageHandler, tab);
   }

remote/shared/messagehandler/test/browser/browser_handle_command_retry_fallback.js

@@ -86,7 +86,7 @@ add_task(async function test_default_fallback_retry_initial_document_only() {
 
     await Assert.rejects(
       onBlockedOneTime,
-      e => e.name == "AbortError",
+      e => e.name == "DiscardedBrowsingContextError",
       "Caught the expected abort error when reloading"
     );
   } finally {

remote/shared/messagehandler/test/browser/resources/modules/ModuleRegistry.sys.mjs

@@ -16,6 +16,7 @@ ChromeUtils.defineESModuleGetters(modules.root, {
   command: `${BASE_FOLDER}/root/command.sys.mjs`,
   event: `${BASE_FOLDER}/root/event.sys.mjs`,
   invalid: `${BASE_FOLDER}/root/invalid.sys.mjs`,
+  retry: `${BASE_FOLDER}/root/retry.sys.mjs`,
   rootOnly: `${BASE_FOLDER}/root/rootOnly.sys.mjs`,
   windowglobaltoroot: `${BASE_FOLDER}/root/windowglobaltoroot.sys.mjs`,
 });

remote/shared/messagehandler/test/browser/resources/modules/root/retry.sys.mjs

@@ -0,0 +1,49 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import { Module } from "chrome://remote/content/shared/messagehandler/Module.sys.mjs";
+
+const lazy = {};
+
+ChromeUtils.defineESModuleGetters(lazy, {
+  WindowGlobalMessageHandler:
+    "chrome://remote/content/shared/messagehandler/WindowGlobalMessageHandler.sys.mjs",
+});
+
+// The test is supposed to trigger the command and then destroy the
+// JSWindowActor pair by any mean (eg a navigation) in order to trigger an
+// AbortError and a retry.
+class RetryModule extends Module {
+  destroy() {}
+
+  /**
+   * Commands
+   */
+
+  async waitForDiscardedBrowsingContext(params = {}) {
+    const { browsingContext, retryOnAbort } = params;
+
+    // Wait for the browsing context to be discarded (replaced or destroyed)
+    // before calling the internal command.
+    await new Promise(resolve => {
+      const observe = (_subject, _topic, _data) => {
+        Services.obs.removeObserver(observe, "browsing-context-discarded");
+        resolve();
+      };
+      Services.obs.addObserver(observe, "browsing-context-discarded");
+    });
+
+    return this.messageHandler.forwardCommand({
+      moduleName: "retry",
+      commandName: "_internalForward",
+      destination: {
+        type: lazy.WindowGlobalMessageHandler.type,
+        id: browsingContext.id,
+      },
+      retryOnAbort,
+    });
+  }
+}
+
+export const retry = RetryModule;

remote/shared/messagehandler/test/browser/resources/modules/windowglobal/retry.sys.mjs

@@ -20,6 +20,8 @@ class RetryModule extends Module {
    * Commands
    */
 
+  async _internalForward() {}
+
   // Resolves only if called while on the example.net domain.
   async blockedOnNetDomain(params) {
     // Note: we do not store a call counter here, because this is used for a

remote/shared/messagehandler/transports/RootTransport.sys.mjs

@@ -7,6 +7,7 @@ const lazy = {};
 ChromeUtils.defineESModuleGetters(lazy, {
   ContextDescriptorType:
     "chrome://remote/content/shared/messagehandler/MessageHandler.sys.mjs",
+  error: "chrome://remote/content/shared/messagehandler/Errors.sys.mjs",
   isBrowsingContextCompatible:
     "chrome://remote/content/shared/messagehandler/transports/BrowsingContextUtils.sys.mjs",
   isInitialDocument:
@@ -66,8 +67,8 @@ export class RootTransport {
     if (command.destination.id) {
       const browsingContext = BrowsingContext.get(command.destination.id);
       if (!browsingContext) {
-        throw new Error(
-          "Unable to find a BrowsingContext for id " + command.destination.id
+        throw new lazy.error.DiscardedBrowsingContextError(
+          `Unable to find a BrowsingContext for id "${command.destination.id}"`
         );
       }
       return this._sendCommandToBrowsingContext(command, browsingContext);
@@ -109,11 +110,6 @@ export class RootTransport {
   async _sendCommandToBrowsingContext(command, browsingContext) {
     const name = `${command.moduleName}.${command.commandName}`;
 
-    // The browsing context might be destroyed by a navigation. Keep a reference
-    // to the webProgress, which will persist, and always use it to retrieve the
-    // currently valid browsing context.
-    const webProgress = browsingContext.webProgress;
-
     let retryOnAbort = true;
     if (command.retryOnAbort !== undefined) {
       // The caller should always be able to force a value.
@@ -123,6 +119,27 @@ export class RootTransport {
       retryOnAbort = lazy.isInitialDocument(browsingContext);
     }
 
+    // If a top-level browsing context was replaced and retrying is allowed,
+    // retrieve the new one for the current browser.
+    if (
+      browsingContext.isReplaced &&
+      browsingContext.top === browsingContext &&
+      retryOnAbort
+    ) {
+      browsingContext = BrowsingContext.getCurrentTopByBrowserId(
+        browsingContext.browserId
+      );
+    }
+
+    // Keep a reference to the webProgress, which will persist, and always use
+    // it to retrieve the currently valid browsing context.
+    const webProgress = browsingContext.webProgress;
+    if (!webProgress) {
+      throw new lazy.error.DiscardedBrowsingContextError(
+        `BrowsingContext with id "${browsingContext.id}" does no longer exist`
+      );
+    }
+
     let attempts = 0;
     while (true) {
       try {
@@ -133,18 +150,23 @@ export class RootTransport {
           .getActor("MessageHandlerFrame")
           .sendCommand(command, this._messageHandler.sessionId);
       } catch (e) {
-        if (!retryOnAbort || e.name != "AbortError") {
-          // Only retry if the command supports retryOnAbort and when the
-          // JSWindowActor pair gets destroyed.
+        // Re-throw the error in case it is not an AbortError.
+        if (e.name != "AbortError") {
           throw e;
         }
 
+        // Only retry if the command supports retryOnAbort and when the
+        // JSWindowActor pair gets destroyed.
+        if (!retryOnAbort) {
+          throw new lazy.error.DiscardedBrowsingContextError(e.message);
+        }
+
         if (++attempts > MAX_RETRY_ATTEMPTS) {
           lazy.logger.trace(
             `RootTransport reached the limit of retry attempts (${MAX_RETRY_ATTEMPTS})` +
               ` for command ${name} and browsing context ${webProgress.browsingContext.id}.`
           );
-          throw e;
+          throw new lazy.error.DiscardedBrowsingContextError(e.message);
         }
 
         lazy.logger.trace(