From d17a9df0278be34c206701d8dfc1fb62af3637b3 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 23 Jan 2024 12:43:40 +0100 Subject: [PATCH] revert: refactor: adopt `core/UserContext` on `BidiBrowserContext` (#11721) --- packages/puppeteer-core/src/bidi/Browser.ts | 50 ++++++++++++------- .../puppeteer-core/src/bidi/BrowserContext.ts | 27 +++------- packages/puppeteer-core/src/bidi/Target.ts | 29 +---------- .../puppeteer-core/src/bidi/core/Browser.ts | 26 ++-------- .../src/bidi/core/UserContext.ts | 24 +++------ 5 files changed, 50 insertions(+), 106 deletions(-) diff --git a/packages/puppeteer-core/src/bidi/Browser.ts b/packages/puppeteer-core/src/bidi/Browser.ts index 64f46e8b3f3..9b97d32a5b5 100644 --- a/packages/puppeteer-core/src/bidi/Browser.ts +++ b/packages/puppeteer-core/src/bidi/Browser.ts @@ -27,7 +27,6 @@ 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 type {UserContext} from './core/UserContext.js'; import { BiDiBrowserTarget, BiDiBrowsingContextTarget, @@ -96,8 +95,9 @@ export class BidiBrowser extends Browser { #closeCallback?: BrowserCloseCallback; #browserCore: BrowserCore; #defaultViewport: Viewport | null; + #defaultContext: BidiBrowserContext; #targets = new Map(); - #browserContexts = new WeakMap(); + #contexts: BidiBrowserContext[] = []; #browserTarget: BiDiBrowserTarget; #connectionEventHandlers = new Map< @@ -111,14 +111,18 @@ export class BidiBrowser extends Browser { ['browsingContext.navigationStarted', this.#onContextNavigation.bind(this)], ]); - private constructor(browserCore: BrowserCore, opts: BidiBrowserOptions) { + constructor(browserCore: BrowserCore, opts: BidiBrowserOptions) { super(); this.#process = opts.process; this.#closeCallback = opts.closeCallback; this.#browserCore = browserCore; this.#defaultViewport = opts.defaultViewport; - this.#browserTarget = new BiDiBrowserTarget(this); - this.#createBrowserContext(this.#browserCore.defaultUserContext); + this.#defaultContext = new BidiBrowserContext(this, { + defaultViewport: this.#defaultViewport, + isDefault: true, + }); + this.#browserTarget = new BiDiBrowserTarget(this.#defaultContext); + this.#contexts.push(this.#defaultContext); } #initialize() { @@ -146,14 +150,6 @@ export class BidiBrowser extends Browser { throw new UnsupportedOperation(); } - #createBrowserContext(userContext: UserContext) { - const browserContext = new BidiBrowserContext(this, userContext, { - defaultViewport: this.#defaultViewport, - }); - this.#browserContexts.set(userContext, browserContext); - return browserContext; - } - #onContextDomLoaded(event: Bidi.BrowsingContext.Info) { const target = this.#targets.get(event.context); if (target) { @@ -259,8 +255,13 @@ export class BidiBrowser extends Browser { override async createIncognitoBrowserContext( _options?: BrowserContextOptions ): Promise { - const userContext = await this.#browserCore.createUserContext(); - return this.#createBrowserContext(userContext); + // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289. + const context = new BidiBrowserContext(this, { + defaultViewport: this.#defaultViewport, + isDefault: false, + }); + this.#contexts.push(context); + return context; } override async version(): Promise { @@ -268,17 +269,28 @@ export class BidiBrowser extends Browser { } override browserContexts(): BidiBrowserContext[] { - return [...this.#browserCore.userContexts].map(context => { - return this.#browserContexts.get(context)!; + // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289. + return this.#contexts; + } + + async _closeContext(browserContext: BidiBrowserContext): Promise { + this.#contexts = this.#contexts.filter(c => { + return c !== browserContext; }); + for (const target of browserContext.targets()) { + const page = await target?.page(); + await page?.close().catch(error => { + debugError(error); + }); + } } override defaultBrowserContext(): BidiBrowserContext { - return this.#browserContexts.get(this.#browserCore.defaultUserContext)!; + return this.#defaultContext; } override newPage(): Promise { - return this.defaultBrowserContext().newPage(); + return this.#defaultContext.newPage(); } override targets(): Target[] { diff --git a/packages/puppeteer-core/src/bidi/BrowserContext.ts b/packages/puppeteer-core/src/bidi/BrowserContext.ts index 7619603e034..4f0ad5e4080 100644 --- a/packages/puppeteer-core/src/bidi/BrowserContext.ts +++ b/packages/puppeteer-core/src/bidi/BrowserContext.ts @@ -11,12 +11,10 @@ import {BrowserContext} from '../api/BrowserContext.js'; import type {Page} from '../api/Page.js'; import type {Target} from '../api/Target.js'; import {UnsupportedOperation} from '../common/Errors.js'; -import {debugError} from '../common/util.js'; import type {Viewport} from '../common/Viewport.js'; import type {BidiBrowser} from './Browser.js'; import type {BidiConnection} from './Connection.js'; -import {UserContext} from './core/UserContext.js'; import type {BidiPage} from './Page.js'; /** @@ -24,6 +22,7 @@ import type {BidiPage} from './Page.js'; */ export interface BidiBrowserContextOptions { defaultViewport: Viewport | null; + isDefault: boolean; } /** @@ -33,18 +32,14 @@ export class BidiBrowserContext extends BrowserContext { #browser: BidiBrowser; #connection: BidiConnection; #defaultViewport: Viewport | null; - #userContext: UserContext; + #isDefault = false; - constructor( - browser: BidiBrowser, - userContext: UserContext, - options: BidiBrowserContextOptions - ) { + constructor(browser: BidiBrowser, options: BidiBrowserContextOptions) { super(); this.#browser = browser; - this.#userContext = userContext; this.#connection = this.#browser.connection; this.#defaultViewport = options.defaultViewport; + this.#isDefault = options.isDefault; } override targets(): Target[] { @@ -95,19 +90,11 @@ export class BidiBrowserContext extends BrowserContext { } override async close(): Promise { - if (!this.isIncognito()) { + if (this.#isDefault) { throw new Error('Default context cannot be closed!'); } - // TODO: Remove once we have adopted the new browsing contexts. - for (const target of this.targets()) { - const page = await target?.page(); - await page?.close().catch(error => { - debugError(error); - }); - } - - await this.#userContext.remove(); + await this.#browser._closeContext(this); } override browser(): BidiBrowser { @@ -126,7 +113,7 @@ export class BidiBrowserContext extends BrowserContext { } override isIncognito(): boolean { - return this.#userContext.id !== UserContext.DEFAULT; + return !this.#isDefault; } override overridePermissions(): never { diff --git a/packages/puppeteer-core/src/bidi/Target.ts b/packages/puppeteer-core/src/bidi/Target.ts index fb01c346386..5823e1012cb 100644 --- a/packages/puppeteer-core/src/bidi/Target.ts +++ b/packages/puppeteer-core/src/bidi/Target.ts @@ -53,14 +53,7 @@ export abstract class BidiTarget extends Target { /** * @internal */ -export class BiDiBrowserTarget extends Target { - #browser: BidiBrowser; - - constructor(browser: BidiBrowser) { - super(); - this.#browser = browser; - } - +export class BiDiBrowserTarget extends BidiTarget { override url(): string { return ''; } @@ -68,26 +61,6 @@ export class BiDiBrowserTarget extends Target { override type(): TargetType { return TargetType.BROWSER; } - - override asPage(): Promise { - throw new UnsupportedOperation(); - } - - override browser(): BidiBrowser { - return this.#browser; - } - - override browserContext(): BidiBrowserContext { - return this.#browser.defaultBrowserContext(); - } - - override opener(): never { - throw new UnsupportedOperation(); - } - - override createCDPSession(): Promise { - throw new UnsupportedOperation(); - } } /** diff --git a/packages/puppeteer-core/src/bidi/core/Browser.ts b/packages/puppeteer-core/src/bidi/core/Browser.ts index cbaaecf1932..fff01b3504f 100644 --- a/packages/puppeteer-core/src/bidi/core/Browser.ts +++ b/packages/puppeteer-core/src/bidi/core/Browser.ts @@ -52,7 +52,7 @@ export class Browser extends EventEmitter<{ // keep-sorted start #reason: string | undefined; - readonly #userContexts = new Map(); + readonly #userContexts = new Map(); readonly session: Session; // keep-sorted end @@ -63,10 +63,7 @@ export class Browser extends EventEmitter<{ this.session = session; // keep-sorted end - this.#userContexts.set( - UserContext.DEFAULT, - UserContext.create(this, UserContext.DEFAULT) - ); + this.#userContexts.set('', UserContext.create(this, '')); } async #initialize() { @@ -130,7 +127,7 @@ export class Browser extends EventEmitter<{ get defaultUserContext(): UserContext { // SAFETY: A UserContext is always created for the default context. - return this.#userContexts.get(UserContext.DEFAULT)!; + return this.#userContexts.get('')!; } get userContexts(): Iterable { @@ -190,21 +187,4 @@ export class Browser extends EventEmitter<{ script, }); } - - async createUserContext(): Promise { - // TODO: implement incognito context https://github.com/w3c/webdriver-bidi/issues/289. - // TODO: Call `createUserContext` once available. - // Generating a monotonically increasing context id. - const context = `${++id}`; - - const userContext = UserContext.create(this, context); - userContext.once('destroyed', () => { - this.#userContexts.delete(context); - }); - - this.#userContexts.set(userContext.id, userContext); - return userContext; - } } - -let id = 0; diff --git a/packages/puppeteer-core/src/bidi/core/UserContext.ts b/packages/puppeteer-core/src/bidi/core/UserContext.ts index a1dd8ed58a1..57da3b00cf6 100644 --- a/packages/puppeteer-core/src/bidi/core/UserContext.ts +++ b/packages/puppeteer-core/src/bidi/core/UserContext.ts @@ -33,16 +33,7 @@ export class UserContext extends EventEmitter<{ /** The new browsing context. */ browsingContext: BrowsingContext; }; - /** - * Emitted when the user context is destroyed. - */ - destroyed: { - /** The user context that was destroyed. */ - userContext: UserContext; - }; }> { - static DEFAULT = 'default'; - static create(browser: Browser, id: string): UserContext { const context = new UserContext(browser, id); context.#initialize(); @@ -52,6 +43,8 @@ export class UserContext extends EventEmitter<{ // keep-sorted start // Note these are only top-level contexts. readonly #browsingContexts = new Map(); + // @ts-expect-error -- TODO: This will be used once the WebDriver BiDi + // protocol supports it. readonly #id: string; readonly browser: Browser; // keep-sorted end @@ -98,9 +91,6 @@ export class UserContext extends EventEmitter<{ get browsingContexts(): Iterable { return this.#browsingContexts.values(); } - get id(): string { - return this.#id; - } // keep-sorted end async createBrowsingContext( @@ -125,9 +115,11 @@ export class UserContext extends EventEmitter<{ return browsingContext; } - async remove(): Promise { - // TODO: Call `removeUserContext` once available. - this.emit('destroyed', {userContext: this}); - this.removeAllListeners(); + async close(): Promise { + const promises = []; + for (const browsingContext of this.#browsingContexts.values()) { + promises.push(browsingContext.close()); + } + await Promise.all(promises); } }