fix: fail previous navigation on the next one (#2569)

Addressing #2568

src/bidiMapper/modules/context/BrowsingContextImpl.ts

@@ -95,6 +95,9 @@ export class BrowsingContextImpl {
   // `None`, the command result should have `navigation` value, but mapper does not have
   // it yet. This value will be set to `navigationId` after next .
   #pendingNavigationId: string | undefined;
+  // Set if there is a pending navigation initiated by `BrowsingContext.navigate` command.
+  // The promise is resolved when the navigation is finished or rejected when canceled.
+  #pendingCommandNavigation: Deferred<void> | undefined;
 
   #originalOpener?: string;
 
@@ -212,6 +215,9 @@ export class BrowsingContextImpl {
   }
 
   dispose(emitContextDestroyed: boolean) {
+    this.#pendingCommandNavigation?.reject(
+      new UnknownErrorException('navigation canceled by context disposal')
+    );
     this.#deleteAllChildren();
 
     this.#realmStorage.deleteRealms({
@@ -468,6 +474,12 @@ export class BrowsingContextImpl {
       if (this.id !== params.frameId) {
         return;
       }
+      // If there is a pending navigation, reject it.
+      this.#pendingCommandNavigation?.reject(
+        new UnknownErrorException(
+          `navigation canceled, as new navigation is requested by ${params.reason}`
+        )
+      );
       this.#pendingNavigationUrl = params.url;
     });
 
@@ -815,16 +827,19 @@ export class BrowsingContextImpl {
       throw new InvalidArgumentException(`Invalid URL: ${url}`);
     }
 
+    this.#pendingCommandNavigation?.reject(
+      new UnknownErrorException('navigation canceled by concurrent navigation')
+    );
     await this.targetUnblockedOrThrow();
 
     // Set the pending navigation URL to provide it in `browsingContext.navigationStarted`
     // event.
     // TODO: detect navigation start not from CDP. Check if
     //  `Page.frameRequestedNavigation` can be used for this purpose.
     this.#pendingNavigationUrl = url;
-
     const navigationId = uuidv4();
     this.#pendingNavigationId = navigationId;
+    this.#pendingCommandNavigation = new Deferred<void>();
 
     // Navigate and wait for the result. If the navigation fails, the error event is
     // emitted and the promise is rejected.
@@ -863,6 +878,9 @@ export class BrowsingContextImpl {
 
     if (wait === BrowsingContext.ReadinessState.None) {
       // Do not wait for the result of the navigation promise.
+      this.#pendingCommandNavigation.resolve();
+      this.#pendingCommandNavigation = undefined;
+
       return {
         navigation: navigationId,
         url,
@@ -871,32 +889,44 @@ export class BrowsingContextImpl {
 
     const cdpNavigateResult = await cdpNavigatePromise;
 
-    switch (wait) {
-      case BrowsingContext.ReadinessState.Interactive:
-        // No `loaderId` means same-document navigation.
-        if (cdpNavigateResult.loaderId === undefined) {
-          await this.#navigation.withinDocument;
-        } else {
-          await this.#lifecycle.DOMContentLoaded;
-        }
-        break;
-      case BrowsingContext.ReadinessState.Complete:
-        // No `loaderId` means same-document navigation.
-        if (cdpNavigateResult.loaderId === undefined) {
-          await this.#navigation.withinDocument;
-        } else {
-          await this.#lifecycle.load;
-        }
-        break;
-    }
+    // Wait for either the navigation is finished or canceled by another navigation.
+    await Promise.race([
+      // No `loaderId` means same-document navigation.
+      this.#waitNavigation(wait, cdpNavigateResult.loaderId === undefined),
+      // Throw an error if the navigation is canceled.
+      this.#pendingCommandNavigation,
+    ]);
 
+    this.#pendingCommandNavigation.resolve();
+    this.#pendingCommandNavigation = undefined;
     return {
       navigation: navigationId,
       // Url can change due to redirect get the latest one.
       url: this.#url,
     };
   }
 
+  async #waitNavigation(
+    wait: BrowsingContext.ReadinessState,
+    withinDocument: boolean
+  ) {
+    if (withinDocument) {
+      await this.#navigation.withinDocument;
+      return;
+    }
+    switch (wait) {
+      case BrowsingContext.ReadinessState.None:
+        return;
+      case BrowsingContext.ReadinessState.Interactive:
+        await this.#lifecycle.DOMContentLoaded;
+        return;
+      case BrowsingContext.ReadinessState.Complete:
+        await this.#lifecycle.load;
+        return;
+    }
+  }
+
+  // TODO: support concurrent navigations analogous to `navigate`.
   async reload(
     ignoreCache: boolean,
     wait: BrowsingContext.ReadinessState

tests/browsing_context/test_navigate.py

@@ -516,7 +516,7 @@ async def test_browsingContext_navigationStarted_browsingContextClosedBeforeNavi
         'id': navigate_command_id,
         'type': 'error',
         'error': 'unknown error',
-        'message': 'navigation canceled',
+        'message': 'navigation canceled by context disposal',
     })
 
     assert close_command_result == AnyExtending({