Bug 1852891 - [bidi] Wait for responseCompleted before resolving browsingContext.navigate r=webdriver-reviewers,whimboo

Introduce internal events for existing network events and monitor them from browsingContext.navigate to ensure a responseCompleted event
was received before resolving the command.

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

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

@@ -1206,6 +1206,63 @@ class BrowsingContextModule extends Module {
       );
     }
 
+    // If WaitCondition is Complete, we should try to wait for the corresponding
+    // responseCompleted event to be received.
+    let onNavigationRequestCompleted;
+
+    // However, a navigation will not necessarily have network events.
+    // For instance: same document navigation, or when using file or data
+    // protocols (for which we don't have network events yet).
+    // Therefore we will not unconditionally wait for a navigation request and
+    // this flag should only be set when a responseCompleted event should be
+    // expected.
+    let shouldWaitForNavigationRequest = false;
+
+    // Cleaning up the listeners will be done at the end of this method.
+    let unsubscribeNavigationListeners;
+
+    if (wait === WaitCondition.Complete) {
+      let resolveOnNetworkEvent;
+      onNavigationRequestCompleted = new Promise(
+        r => (resolveOnNetworkEvent = r)
+      );
+      const onBeforeRequestSent = (name, data) => {
+        if (data.navigation) {
+          shouldWaitForNavigationRequest = true;
+        }
+      };
+      const onResponseCompleted = (name, data) => {
+        if (data.navigation) {
+          resolveOnNetworkEvent();
+        }
+      };
+
+      await this.messageHandler.eventsDispatcher.on(
+        "network._beforeRequestSent",
+        contextDescriptor,
+        onBeforeRequestSent
+      );
+      await this.messageHandler.eventsDispatcher.on(
+        "network._responseCompleted",
+        contextDescriptor,
+        onResponseCompleted
+      );
+
+      unsubscribeNavigationListeners = async () => {
+        await this.messageHandler.eventsDispatcher.off(
+          "network._beforeRequestSent",
+          contextDescriptor,
+          onBeforeRequestSent
+        );
+
+        await this.messageHandler.eventsDispatcher.off(
+          "network._responseCompleted",
+          contextDescriptor,
+          onResponseCompleted
+        );
+      };
+    }
+
     const navigated = listener.start();
     navigated.finally(async () => {
       if (listener.isStarted) {
@@ -1218,6 +1275,14 @@ class BrowsingContextModule extends Module {
           contextDescriptor,
           onDocumentInteractive
         );
+      } else if (
+        wait === WaitCondition.Complete &&
+        shouldWaitForNavigationRequest
+      ) {
+        // We still need to make sure the navigation request has been received
+        // before performing the cleanup.
+        await onNavigationRequestCompleted;
+        await unsubscribeNavigationListeners();
       }
     });
 
@@ -1228,6 +1293,10 @@ class BrowsingContextModule extends Module {
     await startNavigationFn();
     await navigated;
 
+    if (shouldWaitForNavigationRequest) {
+      await onNavigationRequestCompleted;
+    }
+
     let url;
     if (wait === WaitCondition.None) {
       // If wait condition is None, the navigation resolved before the current

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

@@ -591,6 +591,7 @@ class NetworkModule extends Module {
       return;
     }
 
+    const internalEventName = "network._beforeRequestSent";
     const protocolEventName = "network.beforeRequestSent";
 
     // Process the navigation to create potentially missing navigation ids
@@ -602,6 +603,19 @@ class NetworkModule extends Module {
       requestData.url
     );
 
+    // Always emit internal events, they are used to support the browsingContext
+    // navigate command.
+    // Bug 1861922: Replace internal events with a Network listener helper
+    // directly using the NetworkObserver.
+    this.emitEvent(
+      internalEventName,
+      {
+        navigation,
+        url: requestData.url,
+      },
+      this.#getContextInfo(browsingContext)
+    );
+
     const isListening = this.messageHandler.eventsDispatcher.hasListener(
       protocolEventName,
       { contextId }
@@ -676,6 +690,11 @@ class NetworkModule extends Module {
         ? "network.responseStarted"
         : "network.responseCompleted";
 
+    const internalEventName =
+      name === "response-started"
+        ? "network._responseStarted"
+        : "network._responseCompleted";
+
     // Process the navigation to create potentially missing navigation ids
     // before the early return below.
     const navigation = this.#getNavigationId(
@@ -685,6 +704,19 @@ class NetworkModule extends Module {
       requestData.url
     );
 
+    // Always emit internal events, they are used to support the browsingContext
+    // navigate command.
+    // Bug 1861922: Replace internal events with a Network listener helper
+    // directly using the NetworkObserver.
+    this.emitEvent(
+      internalEventName,
+      {
+        navigation,
+        url: requestData.url,
+      },
+      this.#getContextInfo(browsingContext)
+    );
+
     const isListening = this.messageHandler.eventsDispatcher.hasListener(
       protocolEventName,
       { contextId }