chore: emulate frameStartedNavigating event (#2879)

Implementation of [Alternative 3. Rely on “Network.requestWillBeSent” CDP event](http://goto.google.com/webdriver:detect-navigation-started#bookmark=id.64balpqrmadv). Required for #2856. Emit `frameStartedNavigating` event on CdpTarget before `Network.requestWillBeSent`.
GoogleChromeLabs · Dec 11, 2024 · 43ad1d0 · 43ad1d0
1 parent 34f4caf
commit 43ad1d0
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 80 deletions.
diff --git a/src/bidiMapper/modules/cdp/CdpTarget.ts b/src/bidiMapper/modules/cdp/CdpTarget.ts
@@ -21,6 +21,7 @@ import type {CdpClient} from '../../../cdp/CdpClient.js';
 import {BiDiModule} from '../../../protocol/chromium-bidi.js';
 import type {ChromiumBidi, Session} from '../../../protocol/protocol.js';
 import {Deferred} from '../../../utils/Deferred.js';
+import {EventEmitter} from '../../../utils/EventEmitter.js';
 import type {LoggerFn} from '../../../utils/log.js';
 import {LogType} from '../../../utils/log.js';
 import type {Result} from '../../../utils/result.js';
@@ -33,12 +34,15 @@ import type {PreloadScriptStorage} from '../script/PreloadScriptStorage.js';
 import type {RealmStorage} from '../script/RealmStorage.js';
 import type {EventManager} from '../session/EventManager.js';
 
+import {type TargetEventMap, TargetEvents} from './TargetEvents.js';
+
 interface FetchStages {
   request: boolean;
   response: boolean;
   auth: boolean;
 }
-export class CdpTarget {
+
+export class CdpTarget extends EventEmitter<TargetEventMap> {
   readonly #id: Protocol.Target.TargetID;
   readonly #cdpClient: CdpClient;
   readonly #browserCdpClient: CdpClient;
@@ -57,7 +61,6 @@ export class CdpTarget {
 
   #deviceAccessEnabled = false;
   #cacheDisableState = false;
-  #networkDomainEnabled = false;
   #fetchDomainStages: FetchStages = {
     request: false,
     response: false,
@@ -118,6 +121,7 @@ export class CdpTarget {
     unhandledPromptBehavior?: Session.UserPromptHandler,
     logger?: LoggerFn,
   ) {
+    super();
     this.#id = targetId;
     this.#cdpClient = cdpClient;
     this.#browserCdpClient = browserCdpClient;
@@ -192,7 +196,11 @@ export class CdpTarget {
             // prerendered pages. Generic catch, as the error can vary between CdpClient
             // implementations: Tab vs Puppeteer.
           }),
-        this.toggleNetworkIfNeeded(),
+        // Enabling CDP Network domain is required for navigation detection:
+        // https://github.com/GoogleChromeLabs/chromium-bidi/issues/2856.
+        this.#cdpClient
+          .sendCommand('Network.enable')
+          .then(() => this.toggleNetworkIfNeeded()),
         this.#cdpClient.sendCommand('Target.setAutoAttach', {
           autoAttach: true,
           waitForDebuggerOnStart: true,
@@ -265,11 +273,9 @@ export class CdpTarget {
     const stages = this.#networkStorage.getInterceptionStages(this.topLevelId);
 
     if (
-      // Only toggle interception when Network is enabled
-      !this.#networkDomainEnabled ||
-      (this.#fetchDomainStages.request === stages.request &&
-        this.#fetchDomainStages.response === stages.response &&
-        this.#fetchDomainStages.auth === stages.auth)
+      this.#fetchDomainStages.request === stages.request &&
+      this.#fetchDomainStages.response === stages.response &&
+      this.#fetchDomainStages.auth === stages.auth
     ) {
       return;
     }
@@ -317,25 +323,18 @@ export class CdpTarget {
   }
 
   /**
-   * Toggles both Network and Fetch domains.
+   * Toggles CDP "Fetch" domain and enable/disable network cache.
    */
   async toggleNetworkIfNeeded(): Promise<void> {
-    const enabled = this.isSubscribedTo(BiDiModule.Network);
-    if (enabled === this.#networkDomainEnabled) {
-      return;
-    }
-
-    this.#networkDomainEnabled = enabled;
+    // Although the Network domain remains active, Fetch domain activation and caching
+    // settings should be managed dynamically.
     try {
       await Promise.all([
-        this.#cdpClient
-          .sendCommand(enabled ? 'Network.enable' : 'Network.disable')
-          .then(async () => await this.toggleSetCacheDisabled()),
+        this.toggleSetCacheDisabled(),
         this.toggleFetchIfNeeded(),
       ]);
     } catch (err) {
       this.#logger?.(LogType.debugError, err);
-      this.#networkDomainEnabled = !enabled;
       if (!this.#isExpectedError(err)) {
         throw err;
       }
@@ -347,10 +346,7 @@ export class CdpTarget {
       this.#networkStorage.defaultCacheBehavior === 'bypass';
     const cacheDisabled = disable ?? defaultCacheDisabled;
 
-    if (
-      !this.#networkDomainEnabled ||
-      this.#cacheDisableState === cacheDisabled
-    ) {
+    if (this.#cacheDisableState === cacheDisabled) {
       return;
     }
     this.#cacheDisableState = cacheDisabled;
@@ -401,6 +397,15 @@ export class CdpTarget {
   }
 
   #setEventListeners() {
+    this.#cdpClient.on('Network.requestWillBeSent', (eventParams) => {
+      if (eventParams.loaderId === eventParams.requestId) {
+        this.emit(TargetEvents.FrameStartedNavigating, {
+          loaderId: eventParams.loaderId,
+          url: eventParams.request.url,
+          frameId: eventParams.frameId,
+        });
+      }
+    });
     this.#cdpClient.on('*', (event, params) => {
       // We may encounter uses for EventEmitter other than CDP events,
       // which we want to skip.
@@ -436,17 +441,6 @@ export class CdpTarget {
     });
   }
 
-  async #toggleNetwork(enable: boolean): Promise<void> {
-    this.#networkDomainEnabled = enable;
-    try {
-      await this.#cdpClient.sendCommand(
-        enable ? 'Network.enable' : 'Network.disable',
-      );
-    } catch {
-      this.#networkDomainEnabled = !enable;
-    }
-  }
-
   async #enableFetch(stages: FetchStages) {
     const patterns: Protocol.Fetch.EnableRequest['patterns'] = [];
 
@@ -463,11 +457,7 @@ export class CdpTarget {
         requestStage: 'Response',
       });
     }
-    if (
-      // Only enable interception when Network is enabled
-      this.#networkDomainEnabled &&
-      patterns.length
-    ) {
+    if (patterns.length) {
       const oldStages = this.#fetchDomainStages;
       this.#fetchDomainStages = stages;
       try {
@@ -503,29 +493,19 @@ export class CdpTarget {
       this.#fetchDomainStages.request !== stages.request ||
       this.#fetchDomainStages.response !== stages.response ||
       this.#fetchDomainStages.auth !== stages.auth;
-    const networkEnable = this.isSubscribedTo(BiDiModule.Network);
-    const networkChanged = this.#networkDomainEnabled !== networkEnable;
 
     this.#logger?.(
       LogType.debugInfo,
       'Toggle Network',
       `Fetch (${fetchEnable}) ${fetchChanged}`,
-      `Network (${networkEnable}) ${networkChanged}`,
     );
 
-    if (networkEnable && networkChanged) {
-      await this.#toggleNetwork(true);
-    }
     if (fetchEnable && fetchChanged) {
       await this.#enableFetch(stages);
     }
     if (!fetchEnable && fetchChanged) {
       await this.#disableFetch();
     }
-
-    if (!networkEnable && networkChanged && !fetchEnable && !fetchChanged) {
-      await this.#toggleNetwork(false);
-    }
   }
 
   /**

diff --git a/src/bidiMapper/modules/cdp/TargetEvents.ts b/src/bidiMapper/modules/cdp/TargetEvents.ts
@@ -0,0 +1,36 @@
+/*
+ *  Copyright 2024 Google LLC.
+ *  Copyright (c) Microsoft Corporation.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+/**
+ * `FrameStartedNavigating` event addressing lack of such an event in CDP. It is emitted
+ * on CdpTarget before each `Network.requestWillBeSent` event. Note that there can be
+ * several `Network.requestWillBeSent` events for a single navigation e.g. on redirection,
+ * so the `FrameStartedNavigating` can be duplicated as well.
+ * http://go/webdriver:detect-navigation-started#bookmark=id.64balpqrmadv
+ */
+export const enum TargetEvents {
+  FrameStartedNavigating = 'frameStartedNavigating',
+}
+
+export type TargetEventMap = {
+  [TargetEvents.FrameStartedNavigating]: {
+    loaderId: string;
+    url: string;
+    frameId: string | undefined;
+  };
+};
diff --git a/src/bidiMapper/modules/context/BrowsingContextImpl.ts b/src/bidiMapper/modules/context/BrowsingContextImpl.ts
@@ -36,6 +36,7 @@ import {type LoggerFn, LogType} from '../../../utils/log.js';
 import {getTimestamp} from '../../../utils/time.js';
 import {inchesFromCm} from '../../../utils/unitConversions.js';
 import type {CdpTarget} from '../cdp/CdpTarget.js';
+import {TargetEvents} from '../cdp/TargetEvents';
 import type {Realm} from '../script/Realm.js';
 import type {RealmStorage} from '../script/RealmStorage.js';
 import {WindowRealm} from '../script/WindowRealm.js';
@@ -390,6 +391,14 @@ export class BrowsingContextImpl {
       this.#deleteAllChildren();
     });
 
+    this.#cdpTarget.on(TargetEvents.FrameStartedNavigating, (params) => {
+      this.#logger?.(
+        LogType.debugInfo,
+        `Received ${TargetEvents.FrameStartedNavigating} event`,
+        params,
+      );
+    });
+
     this.#cdpTarget.cdpClient.on('Page.navigatedWithinDocument', (params) => {
       if (this.id !== params.frameId) {
         return;

diff --git a/src/cdp/CdpClient.ts b/src/cdp/CdpClient.ts
@@ -26,7 +26,7 @@ export type CdpEvents = {
   [Property in keyof ProtocolMapping.Events]: ProtocolMapping.Events[Property][0];
 };
 
-/** A error that will be thrown if/when the connection is closed. */
+/** An error that will be thrown if/when the connection is closed. */
 export class CloseError extends Error {}
 
 export interface CdpClient extends EventEmitter<CdpEvents> {

diff --git a/tests/network/test_network.py b/tests/network/test_network.py
@@ -421,36 +421,6 @@ async def test_network_before_request_sent_event_with_data_url_emitted(
     })
 
 
-@pytest.mark.asyncio
-async def test_network_specific_context_subscription_does_not_enable_cdp_network_globally(
-        websocket, context_id, create_context, url_base):
-    await subscribe(websocket, ["network.beforeRequestSent"], [context_id])
-
-    new_context_id = await create_context()
-
-    await subscribe(websocket, ["cdp.Network.requestWillBeSent"])
-
-    command_id = await send_JSON_command(
-        websocket, {
-            "method": "browsingContext.navigate",
-            "params": {
-                "url": url_base,
-                "wait": "complete",
-                "context": new_context_id
-            }
-        })
-    resp = await read_JSON_message(websocket)
-    while "id" not in resp:
-        # Assert CDP events are not from Network.
-        assert resp["method"].startswith("cdp")
-        assert not resp["params"]["event"].startswith("Network"), \
-            "There should be no `Network` cdp events, but was " \
-            f"`{ resp['params']['event'] }` "
-        resp = await read_JSON_message(websocket)
-
-    assert resp == AnyExtending({"type": "success", "id": command_id})
-
-
 @pytest.mark.asyncio
 async def test_network_sends_only_included_cookies(websocket, context_id,
                                                    url_base):