From f342a129e8f3760c8ce9e0f3ccec104a6fa889f2 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Fri, 12 May 2023 11:51:28 +0200 Subject: [PATCH] refactor: introduce an internal PageTarget subclass (#10167) --- docs/api/puppeteer.target.md | 20 +- docs/api/puppeteer.target.page.md | 2 +- packages/puppeteer-core/src/common/Browser.ts | 25 ++- packages/puppeteer-core/src/common/Target.ts | 189 +++++++++++------- 4 files changed, 152 insertions(+), 84 deletions(-) diff --git a/docs/api/puppeteer.target.md b/docs/api/puppeteer.target.md index dfb7f2a2149..a20eef9f07a 100644 --- a/docs/api/puppeteer.target.md +++ b/docs/api/puppeteer.target.md @@ -18,13 +18,13 @@ The constructor for this class is marked as internal. Third-party code should no ## Methods -| Method | Modifiers | Description | -| ------------------------------------------------------------ | --------- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| [browser()](./puppeteer.target.browser.md) | | Get the browser the target belongs to. | -| [browserContext()](./puppeteer.target.browsercontext.md) | | Get the browser context the target belongs to. | -| [createCDPSession()](./puppeteer.target.createcdpsession.md) | | Creates a Chrome Devtools Protocol session attached to the target. | -| [opener()](./puppeteer.target.opener.md) | | Get the target that opened this target. Top-level targets return null. | -| [page()](./puppeteer.target.page.md) | | If the target is not of type "page" or "background_page", returns null. | -| [type()](./puppeteer.target.type.md) | | Identifies what kind of target this is. | -| [url()](./puppeteer.target.url.md) | | | -| [worker()](./puppeteer.target.worker.md) | | If the target is not of type "service_worker" or "shared_worker", returns null. | +| Method | Modifiers | Description | +| ------------------------------------------------------------ | --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [browser()](./puppeteer.target.browser.md) | | Get the browser the target belongs to. | +| [browserContext()](./puppeteer.target.browsercontext.md) | | Get the browser context the target belongs to. | +| [createCDPSession()](./puppeteer.target.createcdpsession.md) | | Creates a Chrome Devtools Protocol session attached to the target. | +| [opener()](./puppeteer.target.opener.md) | | Get the target that opened this target. Top-level targets return null. | +| [page()](./puppeteer.target.page.md) | | If the target is not of type "page", "webview" or "background_page", returns null. | +| [type()](./puppeteer.target.type.md) | | Identifies what kind of target this is. | +| [url()](./puppeteer.target.url.md) | | | +| [worker()](./puppeteer.target.worker.md) | | If the target is not of type "service_worker" or "shared_worker", returns null. | diff --git a/docs/api/puppeteer.target.page.md b/docs/api/puppeteer.target.page.md index 4a2fd2a7433..b4d6d53b9d6 100644 --- a/docs/api/puppeteer.target.page.md +++ b/docs/api/puppeteer.target.page.md @@ -4,7 +4,7 @@ sidebar_label: Target.page # Target.page() method -If the target is not of type `"page"` or `"background_page"`, returns `null`. +If the target is not of type `"page"`, `"webview"` or `"background_page"`, returns `null`. #### Signature: diff --git a/packages/puppeteer-core/src/common/Browser.ts b/packages/puppeteer-core/src/common/Browser.ts index 6a2b46f2e20..412bb2ea459 100644 --- a/packages/puppeteer-core/src/common/Browser.ts +++ b/packages/puppeteer-core/src/common/Browser.ts @@ -39,7 +39,7 @@ import {ChromeTargetManager} from './ChromeTargetManager.js'; import {CDPSession, Connection, ConnectionEmittedEvents} from './Connection.js'; import {FirefoxTargetManager} from './FirefoxTargetManager.js'; import {Viewport} from './PuppeteerViewport.js'; -import {Target} from './Target.js'; +import {PageTarget, Target} from './Target.js'; import {TargetManager, TargetManagerEmittedEvents} from './TargetManager.js'; import {TaskQueue} from './TaskQueue.js'; import {waitWithTimeout} from './util.js'; @@ -318,6 +318,23 @@ export class CDPBrowser extends BrowserBase { throw new Error('Missing browser context'); } + if (this.#isPageTargetCallback(targetInfo)) { + return new PageTarget( + targetInfo, + session, + context, + this.#targetManager, + (isAutoAttachEmulated: boolean) => { + return this.#connection._createSession( + targetInfo, + isAutoAttachEmulated + ); + }, + this.#ignoreHTTPSErrors, + this.#defaultViewport ?? null, + this.#screenshotTaskQueue + ); + } return new Target( targetInfo, session, @@ -328,11 +345,7 @@ export class CDPBrowser extends BrowserBase { targetInfo, isAutoAttachEmulated ); - }, - this.#ignoreHTTPSErrors, - this.#defaultViewport ?? null, - this.#screenshotTaskQueue, - this.#isPageTargetCallback + } ); }; diff --git a/packages/puppeteer-core/src/common/Target.ts b/packages/puppeteer-core/src/common/Target.ts index fd9b5f9f27d..103cea572f3 100644 --- a/packages/puppeteer-core/src/common/Target.ts +++ b/packages/puppeteer-core/src/common/Target.ts @@ -16,7 +16,7 @@ import {Protocol} from 'devtools-protocol'; -import type {Browser, IsPageTargetCallback} from '../api/Browser.js'; +import type {Browser} from '../api/Browser.js'; import type {BrowserContext} from '../api/BrowserContext.js'; import {Page, PageEmittedEvents} from '../api/Page.js'; @@ -25,6 +25,7 @@ import {CDPPage} from './Page.js'; import {Viewport} from './PuppeteerViewport.js'; import {TargetManager} from './TargetManager.js'; import {TaskQueue} from './TaskQueue.js'; +import {debugError} from './util.js'; import {WebWorker} from './WebWorker.js'; /** @@ -40,11 +41,7 @@ export class Target { #session?: CDPSession; #targetInfo: Protocol.Target.TargetInfo; #sessionFactory: (isAutoAttachEmulated: boolean) => Promise; - #ignoreHTTPSErrors: boolean; - #defaultViewport?: Viewport; - #pagePromise?: Promise; #workerPromise?: Promise; - #screenshotTaskQueue: TaskQueue; /** * @internal @@ -65,15 +62,11 @@ export class Target { /** * @internal */ - _isInitialized: boolean; + _isInitialized = false; /** * @internal */ _targetId: string; - /** - * @internal - */ - _isPageTargetCallback: IsPageTargetCallback; #targetManager: TargetManager; @@ -85,11 +78,7 @@ export class Target { session: CDPSession | undefined, browserContext: BrowserContext, targetManager: TargetManager, - sessionFactory: (isAutoAttachEmulated: boolean) => Promise, - ignoreHTTPSErrors: boolean, - defaultViewport: Viewport | null, - screenshotTaskQueue: TaskQueue, - isPageTargetCallback: IsPageTargetCallback + sessionFactory: (isAutoAttachEmulated: boolean) => Promise ) { this.#session = session; this.#targetManager = targetManager; @@ -97,37 +86,13 @@ export class Target { this.#browserContext = browserContext; this._targetId = targetInfo.targetId; this.#sessionFactory = sessionFactory; - this.#ignoreHTTPSErrors = ignoreHTTPSErrors; - this.#defaultViewport = defaultViewport ?? undefined; - this.#screenshotTaskQueue = screenshotTaskQueue; - this._isPageTargetCallback = isPageTargetCallback; this._initializedPromise = new Promise(fulfill => { return (this._initializedCallback = fulfill); - }).then(async success => { - if (!success) { - return false; - } - const opener = this.opener(); - if (!opener || !opener.#pagePromise || this.type() !== 'page') { - return true; - } - const openerPage = await opener.#pagePromise; - if (!openerPage.listenerCount(PageEmittedEvents.Popup)) { - return true; - } - const popupPage = await this.page(); - openerPage.emit(PageEmittedEvents.Popup, popupPage); - return true; }); this._isClosedPromise = new Promise(fulfill => { return (this._closedCallback = fulfill); }); - this._isInitialized = - !this._isPageTargetCallback(this.#targetInfo) || - this.#targetInfo.url !== ''; - if (this._isInitialized) { - this._initializedCallback(true); - } + this._initialize(); } /** @@ -137,6 +102,15 @@ export class Target { return this.#session; } + /** + * @internal + */ + protected _sessionFactory(): ( + isAutoAttachEmulated: boolean + ) => Promise { + return this.#sessionFactory; + } + /** * Creates a Chrome Devtools Protocol session attached to the target. */ @@ -158,28 +132,6 @@ export class Target { return this.#targetInfo; } - /** - * If the target is not of type `"page"` or `"background_page"`, returns `null`. - */ - async page(): Promise { - if (this._isPageTargetCallback(this.#targetInfo) && !this.#pagePromise) { - this.#pagePromise = ( - this.#session - ? Promise.resolve(this.#session) - : this.#sessionFactory(true) - ).then(client => { - return CDPPage._create( - client, - this, - this.#ignoreHTTPSErrors, - this.#defaultViewport ?? null, - this.#screenshotTaskQueue - ); - }); - } - return (await this.#pagePromise) ?? null; - } - /** * If the target is not of type `"service_worker"` or `"shared_worker"`, returns `null`. */ @@ -271,15 +223,118 @@ export class Target { */ _targetInfoChanged(targetInfo: Protocol.Target.TargetInfo): void { this.#targetInfo = targetInfo; + this._checkIfInitialized(); + } - if ( - !this._isInitialized && - (!this._isPageTargetCallback(this.#targetInfo) || - this.#targetInfo.url !== '') - ) { + /** + * @internal + */ + protected _initialize(): void { + // TODO: refactor to deferred promises. + this._isInitialized = true; + if (this._isInitialized) { + this._initializedCallback(true); + } + } + + /** + * @internal + */ + protected _checkIfInitialized(): void { + if (!this._isInitialized) { this._isInitialized = true; this._initializedCallback(true); return; } } + + /** + * If the target is not of type `"page"`, `"webview"` or `"background_page"`, + * returns `null`. + */ + async page(): Promise { + return null; + } +} + +/** + * @internal + */ +export class PageTarget extends Target { + #defaultViewport?: Viewport; + protected pagePromise?: Promise; + #screenshotTaskQueue: TaskQueue; + #ignoreHTTPSErrors: boolean; + + /** + * @internal + */ + constructor( + targetInfo: Protocol.Target.TargetInfo, + session: CDPSession | undefined, + browserContext: BrowserContext, + targetManager: TargetManager, + sessionFactory: (isAutoAttachEmulated: boolean) => Promise, + ignoreHTTPSErrors: boolean, + defaultViewport: Viewport | null, + screenshotTaskQueue: TaskQueue + ) { + super(targetInfo, session, browserContext, targetManager, sessionFactory); + this.#ignoreHTTPSErrors = ignoreHTTPSErrors; + this.#defaultViewport = defaultViewport ?? undefined; + this.#screenshotTaskQueue = screenshotTaskQueue; + } + + protected override _initialize(): void { + this._initializedPromise + .then(async success => { + if (!success) { + return false; + } + const opener = this.opener(); + if (!(opener instanceof PageTarget)) { + return true; + } + if (!opener || !opener.pagePromise || this.type() !== 'page') { + return true; + } + const openerPage = await opener.pagePromise; + if (!openerPage.listenerCount(PageEmittedEvents.Popup)) { + return true; + } + const popupPage = await this.page(); + openerPage.emit(PageEmittedEvents.Popup, popupPage); + return true; + }) + .catch(debugError); + this._checkIfInitialized(); + } + + override async page(): Promise { + if (!this.pagePromise) { + const session = this._session(); + this.pagePromise = ( + session ? Promise.resolve(session) : this._sessionFactory()(true) + ).then(client => { + return CDPPage._create( + client, + this, + this.#ignoreHTTPSErrors, + this.#defaultViewport ?? null, + this.#screenshotTaskQueue + ); + }); + } + return (await this.pagePromise) ?? null; + } + + override _checkIfInitialized(): void { + if (this._isInitialized) { + return; + } + this._isInitialized = this._getTargetInfo().url !== ''; + if (this._isInitialized) { + this._initializedCallback(true); + } + } }