diff --git a/src/bidiMapper/modules/cdp/CdpTarget.ts b/src/bidiMapper/modules/cdp/CdpTarget.ts index 8b8ca47b2..a44815753 100644 --- 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 { 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 { - 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 { - 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 new file mode 100644 index 000000000..1f547994d --- /dev/null +++ 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 index 4b3f89755..34629255b 100644 --- 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 index 22edd7dbd..a46681a49 100644 --- 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 { diff --git a/tests/network/test_network.py b/tests/network/test_network.py index c942187be..defeb5683 100644 --- 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):