From bb45951e2f402dcaeaae4c228f92faa06c5a902c Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:38:34 +0100 Subject: [PATCH] refactor: adopt `core/Browser` on `BidiBrowser` (#11702) --- packages/puppeteer-core/src/bidi/Browser.ts | 129 +++++++++--------- .../puppeteer-core/src/bidi/core/Browser.ts | 27 ++-- .../puppeteer-core/src/bidi/core/Realm.ts | 12 ++ .../puppeteer-core/src/bidi/core/Session.ts | 73 +++++----- 4 files changed, 135 insertions(+), 106 deletions(-) diff --git a/packages/puppeteer-core/src/bidi/Browser.ts b/packages/puppeteer-core/src/bidi/Browser.ts index 7a148f022e4..9b97d32a5b5 100644 --- a/packages/puppeteer-core/src/bidi/Browser.ts +++ b/packages/puppeteer-core/src/bidi/Browser.ts @@ -25,6 +25,8 @@ import type {Viewport} from '../common/Viewport.js'; import {BidiBrowserContext} from './BrowserContext.js'; import {BrowsingContext, BrowsingContextEvent} from './BrowsingContext.js'; import type {BidiConnection} from './Connection.js'; +import type {Browser as BrowserCore} from './core/Browser.js'; +import {Session} from './core/Session.js'; import { BiDiBrowserTarget, BiDiBrowsingContextTarget, @@ -70,47 +72,28 @@ export class BidiBrowser extends Browser { ]; static async create(opts: BidiBrowserOptions): Promise { - let browserName = ''; - let browserVersion = ''; + const session = await Session.from(opts.connection, { + alwaysMatch: { + acceptInsecureCerts: opts.ignoreHTTPSErrors, + webSocketUrl: true, + }, + }); - // TODO: await until the connection is established. - try { - const {result} = await opts.connection.send('session.new', { - capabilities: { - alwaysMatch: { - acceptInsecureCerts: opts.ignoreHTTPSErrors, - }, - }, - }); - browserName = result.capabilities.browserName ?? ''; - browserVersion = result.capabilities.browserVersion ?? ''; - } catch (err) { - // Chrome does not support session.new. - debugError(err); - } - - await opts.connection.send('session.subscribe', { - events: browserName.toLocaleLowerCase().includes('firefox') + await session.subscribe( + session.capabilities.browserName.toLocaleLowerCase().includes('firefox') ? BidiBrowser.subscribeModules - : [...BidiBrowser.subscribeModules, ...BidiBrowser.subscribeCdpEvents], - }); - - const browser = new BidiBrowser({ - ...opts, - browserName, - browserVersion, - }); + : [...BidiBrowser.subscribeModules, ...BidiBrowser.subscribeCdpEvents] + ); + const browser = new BidiBrowser(session.browser, opts); + browser.#initialize(); await browser.#getTree(); - return browser; } - #browserName = ''; - #browserVersion = ''; #process?: ChildProcess; #closeCallback?: BrowserCloseCallback; - #connection: BidiConnection; + #browserCore: BrowserCore; #defaultViewport: Viewport | null; #defaultContext: BidiBrowserContext; #targets = new Map(); @@ -128,36 +111,41 @@ export class BidiBrowser extends Browser { ['browsingContext.navigationStarted', this.#onContextNavigation.bind(this)], ]); - constructor( - opts: BidiBrowserOptions & { - browserName: string; - browserVersion: string; - } - ) { + constructor(browserCore: BrowserCore, opts: BidiBrowserOptions) { super(); this.#process = opts.process; this.#closeCallback = opts.closeCallback; - this.#connection = opts.connection; + this.#browserCore = browserCore; this.#defaultViewport = opts.defaultViewport; - this.#browserName = opts.browserName; - this.#browserVersion = opts.browserVersion; - - this.#process?.once('close', () => { - this.#connection.dispose(); - this.emit(BrowserEvent.Disconnected, undefined); - }); this.#defaultContext = new BidiBrowserContext(this, { defaultViewport: this.#defaultViewport, isDefault: true, }); this.#browserTarget = new BiDiBrowserTarget(this.#defaultContext); this.#contexts.push(this.#defaultContext); + } + + #initialize() { + this.#browserCore.once('disconnect', () => { + this.emit(BrowserEvent.Disconnected, undefined); + }); + this.#process?.once('close', () => { + this.#browserCore.dispose('Browser process closed.', true); + this.connection.dispose(); + }); for (const [eventName, handler] of this.#connectionEventHandlers) { - this.#connection.on(eventName, handler); + this.connection.on(eventName, handler); } } + get #browserName() { + return this.#browserCore.session.capabilities.browserName; + } + get #browserVersion() { + return this.#browserCore.session.capabilities.browserVersion; + } + override userAgent(): never { throw new UnsupportedOperation(); } @@ -179,11 +167,11 @@ export class BidiBrowser extends Browser { #onContextCreated(event: Bidi.BrowsingContext.ContextCreated['params']) { const context = new BrowsingContext( - this.#connection, + this.connection, event, this.#browserName ); - this.#connection.registerBrowsingContexts(context); + this.connection.registerBrowsingContexts(context); // TODO: once more browsing context types are supported, this should be // updated to support those. Currently, all top-level contexts are treated // as pages. @@ -200,13 +188,13 @@ export class BidiBrowser extends Browser { target.browserContext().emit(BrowserContextEvent.TargetCreated, target); if (context.parent) { - const topLevel = this.#connection.getTopLevelContext(context.parent); + const topLevel = this.connection.getTopLevelContext(context.parent); topLevel.emit(BrowsingContextEvent.Created, context); } } async #getTree(): Promise { - const {result} = await this.#connection.send('browsingContext.getTree', {}); + const {result} = await this.connection.send('browsingContext.getTree', {}); for (const context of result.contexts) { this.#onContextCreated(context); } @@ -215,8 +203,8 @@ export class BidiBrowser extends Browser { async #onContextDestroyed( event: Bidi.BrowsingContext.ContextDestroyed['params'] ) { - const context = this.#connection.getBrowsingContext(event.context); - const topLevelContext = this.#connection.getTopLevelContext(event.context); + const context = this.connection.getBrowsingContext(event.context); + const topLevelContext = this.connection.getTopLevelContext(event.context); topLevelContext.emit(BrowsingContextEvent.Destroyed, context); const target = this.#targets.get(event.context); const page = await target?.page(); @@ -229,29 +217,35 @@ export class BidiBrowser extends Browser { } get connection(): BidiConnection { - return this.#connection; + // SAFETY: We only have one implementation. + return this.#browserCore.session.connection as BidiConnection; } override wsEndpoint(): string { - return this.#connection.url; + return this.connection.url; } override async close(): Promise { for (const [eventName, handler] of this.#connectionEventHandlers) { - this.#connection.off(eventName, handler); + this.connection.off(eventName, handler); } - if (this.#connection.closed) { + if (this.connection.closed) { return; } - // `browser.close` can close connection before the response is received. - await this.#connection.send('browser.close', {}).catch(debugError); - await this.#closeCallback?.call(null); - this.#connection.dispose(); + try { + await this.#browserCore.close(); + await this.#closeCallback?.call(null); + } catch (error) { + // Fail silently. + debugError(error); + } finally { + this.connection.dispose(); + } } override get connected(): boolean { - return !this.#connection.closed; + return !this.#browserCore.disposed; } override process(): ChildProcess | null { @@ -317,11 +311,12 @@ export class BidiBrowser extends Browser { override async disconnect(): Promise { try { - // Fail silently if the session cannot be ended. - await this.#connection.send('session.end', {}); - } catch (e) { - debugError(e); + await this.#browserCore.session.end(); + } catch (error) { + // Fail silently. + debugError(error); + } finally { + this.connection.dispose(); } - this.#connection.dispose(); } } diff --git a/packages/puppeteer-core/src/bidi/core/Browser.ts b/packages/puppeteer-core/src/bidi/core/Browser.ts index 259406940ad..fff01b3504f 100644 --- a/packages/puppeteer-core/src/bidi/core/Browser.ts +++ b/packages/puppeteer-core/src/bidi/core/Browser.ts @@ -34,7 +34,7 @@ export class Browser extends EventEmitter<{ reason: string; }; /** Emitted after the browser disconnects. */ - disconnected: { + disconnect: { /** The reason for disconnecting the browser. */ reason: string; }; @@ -81,9 +81,7 @@ export class Browser extends EventEmitter<{ // Parent listeners // // /////////////////// this.session.once('ended', ({reason}) => { - this.#reason = reason; - this.emit('disconnected', {reason}); - this.removeAllListeners(); + this.dispose(reason); }); // ////////////////////////////// @@ -136,15 +134,28 @@ export class Browser extends EventEmitter<{ return this.#userContexts.values(); } + dispose(reason?: string, close?: boolean): void { + if (this.disposed) { + return; + } + this.#reason = reason ?? `Browser was disposed.`; + if (close) { + this.emit('closed', {reason: this.#reason}); + } + this.emit('disconnect', {reason: this.#reason}); + this.removeAllListeners(); + } + @throwIfDisposed((browser: Browser) => { // SAFETY: By definition of `disposed`, `#reason` is defined. return browser.#reason!; }) async close(): Promise { - await this.#session.send('browser.close', {}); - this.#reason = `Browser has already closed.`; - this.emit('closed', {reason: this.#reason}); - this.removeAllListeners(); + try { + await this.#session.send('browser.close', {}); + } finally { + this.dispose(`Browser was closed.`, true); + } } @throwIfDisposed((browser: Browser) => { diff --git a/packages/puppeteer-core/src/bidi/core/Realm.ts b/packages/puppeteer-core/src/bidi/core/Realm.ts index 7aff1eb8ebf..145be4f3ca9 100644 --- a/packages/puppeteer-core/src/bidi/core/Realm.ts +++ b/packages/puppeteer-core/src/bidi/core/Realm.ts @@ -96,6 +96,9 @@ export abstract class Realm extends EventEmitter<{ } } +/** + * @internal + */ export class WindowRealm extends Realm { static from(context: BrowsingContext, sandbox?: string): WindowRealm { const realm = new WindowRealm(context, sandbox); @@ -173,11 +176,17 @@ export class WindowRealm extends Realm { } } +/** + * @internal + */ export type DedicatedWorkerOwnerRealm = | DedicatedWorkerRealm | SharedWorkerRealm | WindowRealm; +/** + * @internal + */ export class DedicatedWorkerRealm extends Realm { static from( owner: DedicatedWorkerOwnerRealm, @@ -228,6 +237,9 @@ export class DedicatedWorkerRealm extends Realm { } } +/** + * @internal + */ export class SharedWorkerRealm extends Realm { static from( owners: [WindowRealm, ...WindowRealm[]], diff --git a/packages/puppeteer-core/src/bidi/core/Session.ts b/packages/puppeteer-core/src/bidi/core/Session.ts index 93c87afd322..6f68fb69acc 100644 --- a/packages/puppeteer-core/src/bidi/core/Session.ts +++ b/packages/puppeteer-core/src/bidi/core/Session.ts @@ -11,11 +11,10 @@ import {debugError} from '../../common/util.js'; import {throwIfDisposed} from '../../util/decorators.js'; import {Browser} from './Browser.js'; -import type {Connection} from './Connection.js'; -import type {Commands} from './Connection.js'; -import type {BidiEvents} from './Connection.js'; +import type {BidiEvents, Commands, Connection} from './Connection.js'; -const MAX_RETRIES = 5; +// TODO: Once Chrome supports session.status properly, uncomment this block. +// const MAX_RETRIES = 5; /** * @internal @@ -29,20 +28,24 @@ export class Session capabilities: Bidi.Session.CapabilitiesRequest ): Promise { // Wait until the session is ready. - let status = {message: '', ready: false}; - for (let i = 0; i < MAX_RETRIES; ++i) { - status = (await connection.send('session.status', {})).result; - if (status.ready) { - break; - } - // Backoff a little bit each time. - await new Promise(resolve => { - return setTimeout(resolve, (1 << i) * 100); - }); - } - if (!status.ready) { - throw new Error(status.message); - } + // + // TODO: Once Chrome supports session.status properly, uncomment this block + // and remove `getBiDiConnection` in BrowserConnector. + + // let status = {message: '', ready: false}; + // for (let i = 0; i < MAX_RETRIES; ++i) { + // status = (await connection.send('session.status', {})).result; + // if (status.ready) { + // break; + // } + // // Backoff a little bit each time. + // await new Promise(resolve => { + // return setTimeout(resolve, (1 << i) * 100); + // }); + // } + // if (!status.ready) { + // throw new Error(status.message); + // } let result; try { @@ -58,7 +61,7 @@ export class Session sessionId: '', capabilities: { acceptInsecureCerts: false, - browserName: 'chrome', + browserName: '', browserVersion: '', platformName: '', setWindowRect: false, @@ -72,7 +75,7 @@ export class Session return session; } - readonly #connection: Connection; + readonly connection: Connection; readonly #info: Bidi.Session.NewResult; readonly browser!: Browser; @@ -81,7 +84,7 @@ export class Session private constructor(connection: Connection, info: Bidi.Session.NewResult) { super(); - this.#connection = connection; + this.connection = connection; this.#info = info; } @@ -89,7 +92,7 @@ export class Session // /////////////////////// // Connection listeners // // /////////////////////// - this.#connection.pipeTo(this); + this.connection.pipeTo(this); // ////////////////////////////// // Asynchronous initialization // @@ -101,9 +104,7 @@ export class Session // Child listeners // // ////////////////// this.browser.once('closed', ({reason}) => { - this.#reason = reason; - this.emit('ended', {reason}); - this.removeAllListeners(); + this.dispose(reason); }); } @@ -119,8 +120,17 @@ export class Session return this.#info.capabilities; } + dispose(reason?: string): void { + if (this.disposed) { + return; + } + this.#reason = reason ?? 'Session was disposed.'; + this.emit('ended', {reason: this.#reason}); + this.removeAllListeners(); + } + pipeTo(emitter: EventEmitter): void { - this.#connection.pipeTo(emitter); + this.connection.pipeTo(emitter); } /** @@ -138,7 +148,7 @@ export class Session method: T, params: Commands[T]['params'] ): Promise<{result: Commands[T]['returnType']}> { - return await this.#connection.send(method, params); + return await this.connection.send(method, params); } @throwIfDisposed((session: Session) => { @@ -156,9 +166,10 @@ export class Session return session.#reason!; }) async end(): Promise { - await this.send('session.end', {}); - this.#reason = `Session (${this.id}) has already ended.`; - this.emit('ended', {reason: this.#reason}); - this.removeAllListeners(); + try { + await this.send('session.end', {}); + } finally { + this.dispose(`Session (${this.id}) has already ended.`); + } } }