From 5a5e4d46a3775bb11c47aa40ed90700aa56c187d Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 16 May 2023 17:18:22 +0200 Subject: [PATCH] refactor: change target promises to be deferred (#10191) --- packages/puppeteer-core/src/common/Browser.ts | 14 +++-- .../src/common/ChromeTargetManager.ts | 5 +- packages/puppeteer-core/src/common/Target.ts | 53 +++++++------------ .../src/util/DeferredPromise.ts | 6 +++ 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/packages/puppeteer-core/src/common/Browser.ts b/packages/puppeteer-core/src/common/Browser.ts index 0687335718b..9b9224b9090 100644 --- a/packages/puppeteer-core/src/common/Browser.ts +++ b/packages/puppeteer-core/src/common/Browser.ts @@ -39,7 +39,13 @@ import {ChromeTargetManager} from './ChromeTargetManager.js'; import {CDPSession, Connection, ConnectionEmittedEvents} from './Connection.js'; import {FirefoxTargetManager} from './FirefoxTargetManager.js'; import {Viewport} from './PuppeteerViewport.js'; -import {OtherTarget, PageTarget, Target, WorkerTarget} from './Target.js'; +import { + InitializationStatus, + OtherTarget, + PageTarget, + Target, + WorkerTarget, +} from './Target.js'; import {TargetManager, TargetManagerEmittedEvents} from './TargetManager.js'; import {TaskQueue} from './TaskQueue.js'; import {waitWithTimeout} from './util.js'; @@ -364,8 +370,8 @@ export class CDPBrowser extends BrowserBase { }; #onDetachedFromTarget = async (target: Target): Promise => { - target._initializedCallback(false); - target._closedCallback(); + target._initializedPromise.resolve(InitializationStatus.ABORTED); + target._isClosedPromise.resolve(); if (await target._initializedPromise) { this.emit(BrowserEmittedEvents.TargetDestroyed, target); target @@ -447,7 +453,7 @@ export class CDPBrowser extends BrowserBase { return Array.from( this.#targetManager.getAvailableTargets().values() ).filter(target => { - return target._isInitialized; + return target._initializedPromise.resolved(); }); } diff --git a/packages/puppeteer-core/src/common/ChromeTargetManager.ts b/packages/puppeteer-core/src/common/ChromeTargetManager.ts index 153611b5e1c..6598727901d 100644 --- a/packages/puppeteer-core/src/common/ChromeTargetManager.ts +++ b/packages/puppeteer-core/src/common/ChromeTargetManager.ts @@ -22,7 +22,7 @@ import {createDeferredPromise} from '../util/DeferredPromise.js'; import {CDPSession, Connection} from './Connection.js'; import {EventEmitter} from './EventEmitter.js'; -import {Target} from './Target.js'; +import {InitializationStatus, Target} from './Target.js'; import { TargetInterceptor, TargetFactory, @@ -267,7 +267,8 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager { return; } const previousURL = target.url(); - const wasInitialized = target._isInitialized; + const wasInitialized = + target._initializedPromise.value() === InitializationStatus.SUCCESS; target._targetInfoChanged(event.targetInfo); diff --git a/packages/puppeteer-core/src/common/Target.ts b/packages/puppeteer-core/src/common/Target.ts index c997adf0a7b..1c8c7e1eed7 100644 --- a/packages/puppeteer-core/src/common/Target.ts +++ b/packages/puppeteer-core/src/common/Target.ts @@ -19,6 +19,7 @@ import {Protocol} from 'devtools-protocol'; import type {Browser} from '../api/Browser.js'; import type {BrowserContext} from '../api/BrowserContext.js'; import {Page, PageEmittedEvents} from '../api/Page.js'; +import {createDeferredPromise} from '../util/DeferredPromise.js'; import {CDPSession} from './Connection.js'; import {CDPPage} from './Page.js'; @@ -28,6 +29,14 @@ import {TaskQueue} from './TaskQueue.js'; import {debugError} from './util.js'; import {WebWorker} from './WebWorker.js'; +/** + * @internal + */ +export enum InitializationStatus { + SUCCESS = 'success', + ABORTED = 'aborted', +} + /** * Target represents a * {@link https://chromedevtools.github.io/devtools-protocol/tot/Target/ | CDP target}. @@ -40,35 +49,22 @@ export class Target { #browserContext: BrowserContext; #session?: CDPSession; #targetInfo: Protocol.Target.TargetInfo; + #targetManager: TargetManager; #sessionFactory: (isAutoAttachEmulated: boolean) => Promise; /** * @internal */ - _initializedPromise: Promise; + _initializedPromise = createDeferredPromise(); /** * @internal */ - _initializedCallback!: (x: boolean) => void; - /** - * @internal - */ - _isClosedPromise: Promise; - /** - * @internal - */ - _closedCallback!: () => void; - /** - * @internal - */ - _isInitialized = false; + _isClosedPromise = createDeferredPromise(); /** * @internal */ _targetId: string; - #targetManager: TargetManager; - /** * @internal */ @@ -85,12 +81,6 @@ export class Target { this.#browserContext = browserContext; this._targetId = targetInfo.targetId; this.#sessionFactory = sessionFactory; - this._initializedPromise = new Promise(fulfill => { - return (this._initializedCallback = fulfill); - }); - this._isClosedPromise = new Promise(fulfill => { - return (this._closedCallback = fulfill); - }); this._initialize(); } @@ -208,21 +198,15 @@ export class Target { * @internal */ protected _initialize(): void { - // TODO: refactor to deferred promises. - this._isInitialized = true; - if (this._isInitialized) { - this._initializedCallback(true); - } + this._initializedPromise.resolve(InitializationStatus.SUCCESS); } /** * @internal */ protected _checkIfInitialized(): void { - if (!this._isInitialized) { - this._isInitialized = true; - this._initializedCallback(true); - return; + if (!this._initializedPromise.resolved()) { + this._initializedPromise.resolve(InitializationStatus.SUCCESS); } } @@ -309,12 +293,11 @@ export class PageTarget extends Target { } override _checkIfInitialized(): void { - if (this._isInitialized) { + if (this._initializedPromise.resolved()) { return; } - this._isInitialized = this._getTargetInfo().url !== ''; - if (this._isInitialized) { - this._initializedCallback(true); + if (this._getTargetInfo().url !== '') { + this._initializedPromise.resolve(InitializationStatus.SUCCESS); } } } diff --git a/packages/puppeteer-core/src/util/DeferredPromise.ts b/packages/puppeteer-core/src/util/DeferredPromise.ts index 8c7945e3591..f240e362449 100644 --- a/packages/puppeteer-core/src/util/DeferredPromise.ts +++ b/packages/puppeteer-core/src/util/DeferredPromise.ts @@ -8,6 +8,7 @@ export interface DeferredPromise extends Promise { resolved: () => boolean; resolve: (value: T) => void; reject: (reason?: unknown) => void; + value: () => T | undefined; } /** @@ -32,6 +33,7 @@ export function createDeferredPromise( ): DeferredPromise { let isResolved = false; let isRejected = false; + let _value: T | undefined; let resolver: (value: T) => void; let rejector: (reason?: unknown) => void; const taskPromise = new Promise((resolve, reject) => { @@ -57,6 +59,7 @@ export function createDeferredPromise( clearTimeout(timeoutId); } isResolved = true; + _value = value; resolver(value); }, reject: (err?: unknown) => { @@ -64,5 +67,8 @@ export function createDeferredPromise( isRejected = true; rejector(err); }, + value: () => { + return _value; + }, }); }