From 599c0694eab153b953c00581c96cf19b30e65c03 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 29 Aug 2023 10:52:47 +0200 Subject: [PATCH] chore: emit target configuration via CDP session (#10794) --- packages/browsers/test/src/versions.ts | 2 +- .../src/common/ChromeTargetManager.ts | 54 +++---------------- .../puppeteer-core/src/common/Connection.ts | 5 ++ .../src/common/FirefoxTargetManager.ts | 43 +-------------- packages/puppeteer-core/src/common/Page.ts | 30 ++++------- .../src/common/TargetManager.ts | 16 ------ 6 files changed, 24 insertions(+), 126 deletions(-) diff --git a/packages/browsers/test/src/versions.ts b/packages/browsers/test/src/versions.ts index 3167a244..867af04e 100644 --- a/packages/browsers/test/src/versions.ts +++ b/packages/browsers/test/src/versions.ts @@ -16,6 +16,6 @@ export const testChromeBuildId = '113.0.5672.0'; export const testChromiumBuildId = '1083080'; -export const testFirefoxBuildId = '118.0a1'; +export const testFirefoxBuildId = '119.0a1'; export const testChromeDriverBuildId = '115.0.5763.0'; export const testChromeHeadlessShellBuildId = '118.0.5950.0'; diff --git a/packages/puppeteer-core/src/common/ChromeTargetManager.ts b/packages/puppeteer-core/src/common/ChromeTargetManager.ts index 32bb4515..109f3d2d 100644 --- a/packages/puppeteer-core/src/common/ChromeTargetManager.ts +++ b/packages/puppeteer-core/src/common/ChromeTargetManager.ts @@ -25,7 +25,6 @@ import {CDPSession, CDPSessionEmittedEvents, Connection} from './Connection.js'; import {EventEmitter} from './EventEmitter.js'; import {InitializationStatus, CDPTarget} from './Target.js'; import { - TargetInterceptor, TargetFactory, TargetManager, TargetManagerEmittedEvents, @@ -80,11 +79,6 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { #targetFilterCallback: TargetFilterCallback | undefined; #targetFactory: TargetFactory; - #targetInterceptors = new WeakMap< - CDPSession | Connection, - TargetInterceptor[] - >(); - #attachedToTargetListenersBySession = new WeakMap< CDPSession | Connection, (event: Protocol.Target.AttachedToTargetEvent) => Promise @@ -197,28 +191,6 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { return result; } - addTargetInterceptor( - session: CDPSession | Connection, - interceptor: TargetInterceptor - ): void { - const interceptors = this.#targetInterceptors.get(session) || []; - interceptors.push(interceptor); - this.#targetInterceptors.set(session, interceptors); - } - - removeTargetInterceptor( - client: CDPSession | Connection, - interceptor: TargetInterceptor - ): void { - const interceptors = this.#targetInterceptors.get(client) || []; - this.#targetInterceptors.set( - client, - interceptors.filter(currentInterceptor => { - return currentInterceptor !== interceptor; - }) - ); - } - #setupAttachmentListeners(session: CDPSession | Connection): void { const listener = (event: Protocol.Target.AttachedToTargetEvent) => { return this.#onAttachedToTarget(session, event); @@ -257,7 +229,6 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { #onSessionDetached = (session: CDPSession) => { this.#removeAttachmentListeners(session); - this.#targetInterceptors.delete(session); }; #onTargetCreated = async (event: Protocol.Target.TargetCreatedEvent) => { @@ -392,11 +363,11 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { return; } - const existingTarget = this.#attachedTargetsByTargetId.has( + const isExistingTarget = this.#attachedTargetsByTargetId.has( targetInfo.targetId ); - const target = existingTarget + const target = isExistingTarget ? this.#attachedTargetsByTargetId.get(targetInfo.targetId)! : this.#targetFactory( targetInfo, @@ -411,13 +382,13 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { return; } - if (!existingTarget) { + if (!isExistingTarget) { target._initialize(); } this.#setupAttachmentListeners(session); - if (existingTarget) { + if (isExistingTarget) { this.#attachedTargetsBySessionId.set( session.id(), this.#attachedTargetsByTargetId.get(targetInfo.targetId)! @@ -427,23 +398,10 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { this.#attachedTargetsBySessionId.set(session.id(), target); } - for (const interceptor of this.#targetInterceptors.get(parentSession) || - []) { - if (!(parentSession instanceof Connection)) { - // Sanity check: if parent session is not a connection, it should be - // present in #attachedTargetsBySessionId. - assert(this.#attachedTargetsBySessionId.has(parentSession.id())); - } - interceptor( - target, - parentSession instanceof Connection - ? null - : this.#attachedTargetsBySessionId.get(parentSession.id())! - ); - } + parentSession.emit(CDPSessionEmittedEvents.Ready, session); this.#targetsIdsForInit.delete(target._targetId); - if (!existingTarget && isTargetExposed(target)) { + if (!isExistingTarget && isTargetExposed(target)) { this.emit(TargetManagerEmittedEvents.TargetAvailable, target); } this.#finishInitializationIfReady(); diff --git a/packages/puppeteer-core/src/common/Connection.ts b/packages/puppeteer-core/src/common/Connection.ts index 1e21a5b3..65eaa65f 100644 --- a/packages/puppeteer-core/src/common/Connection.ts +++ b/packages/puppeteer-core/src/common/Connection.ts @@ -431,6 +431,11 @@ export interface CDPSessionOnMessageObject { export const CDPSessionEmittedEvents = { Disconnected: Symbol('CDPSession.Disconnected'), Swapped: Symbol('CDPSession.Swapped'), + /** + * Emitted when the session is ready to be configured during the auto-attach + * process. Right after the event is handled, the session will be resumed. + */ + Ready: Symbol('CDPSession.Ready'), } as const; /** diff --git a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts index 58275425..9513e077 100644 --- a/packages/puppeteer-core/src/common/FirefoxTargetManager.ts +++ b/packages/puppeteer-core/src/common/FirefoxTargetManager.ts @@ -20,12 +20,11 @@ import {TargetFilterCallback} from '../api/Browser.js'; import {assert} from '../util/assert.js'; import {Deferred} from '../util/Deferred.js'; -import {CDPSession, Connection} from './Connection.js'; +import {CDPSession, CDPSessionEmittedEvents, Connection} from './Connection.js'; import {EventEmitter} from './EventEmitter.js'; import {CDPTarget} from './Target.js'; import { TargetFactory, - TargetInterceptor, TargetManagerEmittedEvents, TargetManager, } from './TargetManager.js'; @@ -79,11 +78,6 @@ export class FirefoxTargetManager #targetFilterCallback: TargetFilterCallback | undefined; #targetFactory: TargetFactory; - #targetInterceptors = new WeakMap< - CDPSession | Connection, - TargetInterceptor[] - >(); - #attachedToTargetListenersBySession = new WeakMap< CDPSession | Connection, (event: Protocol.Target.AttachedToTargetEvent) => Promise @@ -108,28 +102,6 @@ export class FirefoxTargetManager this.setupAttachmentListeners(this.#connection); } - addTargetInterceptor( - client: CDPSession | Connection, - interceptor: TargetInterceptor - ): void { - const interceptors = this.#targetInterceptors.get(client) || []; - interceptors.push(interceptor); - this.#targetInterceptors.set(client, interceptors); - } - - removeTargetInterceptor( - client: CDPSession | Connection, - interceptor: TargetInterceptor - ): void { - const interceptors = this.#targetInterceptors.get(client) || []; - this.#targetInterceptors.set( - client, - interceptors.filter(currentInterceptor => { - return currentInterceptor !== interceptor; - }) - ); - } - setupAttachmentListeners(session: CDPSession | Connection): void { const listener = (event: Protocol.Target.AttachedToTargetEvent) => { return this.#onAttachedToTarget(session, event); @@ -141,7 +113,6 @@ export class FirefoxTargetManager #onSessionDetached = (session: CDPSession) => { this.removeSessionListeners(session); - this.#targetInterceptors.delete(session); this.#availableTargetsBySessionId.delete(session.id()); }; @@ -236,17 +207,7 @@ export class FirefoxTargetManager this.#availableTargetsByTargetId.get(targetInfo.targetId)! ); - for (const hook of this.#targetInterceptors.get(parentSession) || []) { - if (!(parentSession instanceof Connection)) { - assert(this.#availableTargetsBySessionId.has(parentSession.id())); - } - await hook( - target, - parentSession instanceof Connection - ? null - : this.#availableTargetsBySessionId.get(parentSession.id())! - ); - } + parentSession.emit(CDPSessionEmittedEvents.Ready, session); }; #finishInitializationIfReady(targetId: string): void { diff --git a/packages/puppeteer-core/src/common/Page.ts b/packages/puppeteer-core/src/common/Page.ts index 235645e8..f2131e6b 100644 --- a/packages/puppeteer-core/src/common/Page.ts +++ b/packages/puppeteer-core/src/common/Page.ts @@ -332,9 +332,7 @@ export class CDPPage extends Page { } #setupEventListeners() { - this.#target - ._targetManager() - .addTargetInterceptor(this.#client, this.#onAttachedToTarget); + this.#client.on(CDPSessionEmittedEvents.Ready, this.#onAttachedToTarget); this.#target ._targetManager() @@ -355,9 +353,10 @@ export class CDPPage extends Page { this.#target._isClosedDeferred .valueOrThrow() .then(() => { - this.#target - ._targetManager() - .removeTargetInterceptor(this.#client, this.#onAttachedToTarget); + this.#client.off( + CDPSessionEmittedEvents.Ready, + this.#onAttachedToTarget + ); this.#target ._targetManager() @@ -382,28 +381,19 @@ export class CDPPage extends Page { this.emit(PageEmittedEvents.WorkerDestroyed, worker); }; - #onAttachedToTarget = (createdTarget: CDPTarget) => { - this.#frameManager.onAttachedToTarget(createdTarget); - if (createdTarget._getTargetInfo().type === 'worker') { - const session = createdTarget._session(); - assert(session); + #onAttachedToTarget = (session: CDPSessionImpl) => { + this.#frameManager.onAttachedToTarget(session._target()); + if (session._target()._getTargetInfo().type === 'worker') { const worker = new WebWorker( session, - createdTarget.url(), + session._target().url(), this.#addConsoleMessage.bind(this), this.#handleException.bind(this) ); this.#workers.set(session.id(), worker); this.emit(PageEmittedEvents.WorkerCreated, worker); } - if (createdTarget._session()) { - this.#target - ._targetManager() - .addTargetInterceptor( - createdTarget._session()!, - this.#onAttachedToTarget - ); - } + session.on(CDPSessionEmittedEvents.Ready, this.#onAttachedToTarget); }; async #initialize(): Promise { diff --git a/packages/puppeteer-core/src/common/TargetManager.ts b/packages/puppeteer-core/src/common/TargetManager.ts index 73708b4f..3fea2692 100644 --- a/packages/puppeteer-core/src/common/TargetManager.ts +++ b/packages/puppeteer-core/src/common/TargetManager.ts @@ -29,14 +29,6 @@ export type TargetFactory = ( parentSession?: CDPSession ) => CDPTarget; -/** - * @internal - */ -export type TargetInterceptor = ( - createdTarget: CDPTarget, - parentTarget: CDPTarget | null -) => void; - /** * TargetManager encapsulates all interactions with CDP targets and is * responsible for coordinating the configuration of targets with the rest of @@ -52,14 +44,6 @@ export interface TargetManager extends EventEmitter { getAvailableTargets(): Map; initialize(): Promise; dispose(): void; - addTargetInterceptor( - session: CDPSession, - interceptor: TargetInterceptor - ): void; - removeTargetInterceptor( - session: CDPSession, - interceptor: TargetInterceptor - ): void; } /**