From dd31f3bd432bcfe003771a0b3a2253ef9630531a Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Sun, 29 Aug 2021 11:21:06 -0700 Subject: [PATCH] chore: introduce manual promise helper (#8533) --- src/server/artifact.ts | 9 ++---- src/server/firefox/ffPage.ts | 10 +++--- src/server/frames.ts | 10 +++--- src/server/network.ts | 36 +++++++-------------- src/server/page.ts | 23 ++++--------- src/server/webkit/wkInterceptableRequest.ts | 11 +++---- src/server/webkit/wkPage.ts | 18 +++++------ src/test/workerRunner.ts | 6 ++-- src/utils/utils.ts | 32 ++++++++++++++++++ 9 files changed, 76 insertions(+), 79 deletions(-) diff --git a/src/server/artifact.ts b/src/server/artifact.ts index b13da3e13b..231e5745e2 100644 --- a/src/server/artifact.ts +++ b/src/server/artifact.ts @@ -15,7 +15,7 @@ */ import fs from 'fs'; -import { assert } from '../utils/utils'; +import { assert, ManualPromise } from '../utils/utils'; import { SdkObject } from './instrumentation'; type SaveCallback = (localPath: string, error?: string) => Promise; @@ -25,8 +25,7 @@ export class Artifact extends SdkObject { private _localPath: string; private _unaccessibleErrorMessage: string | undefined; private _cancelCallback: CancelCallback | undefined; - private _finishedCallback: () => void; - private _finishedPromise: Promise; + private _finishedPromise = new ManualPromise(); private _saveCallbacks: SaveCallback[] = []; private _finished: boolean = false; private _deleted = false; @@ -37,8 +36,6 @@ export class Artifact extends SdkObject { this._localPath = localPath; this._unaccessibleErrorMessage = unaccessibleErrorMessage; this._cancelCallback = cancelCallback; - this._finishedCallback = () => {}; - this._finishedPromise = new Promise(f => this._finishedCallback = f); } finishedPromise() { @@ -122,6 +119,6 @@ export class Artifact extends SdkObject { } this._saveCallbacks = []; - this._finishedCallback(); + this._finishedPromise.resolve(); } } diff --git a/src/server/firefox/ffPage.ts b/src/server/firefox/ffPage.ts index f4d53a7aa4..49e64fb31e 100644 --- a/src/server/firefox/ffPage.ts +++ b/src/server/firefox/ffPage.ts @@ -19,7 +19,7 @@ import * as dialog from '../dialog'; import * as dom from '../dom'; import * as frames from '../frames'; import { eventsHelper, RegisteredListener } from '../../utils/eventsHelper'; -import { assert } from '../../utils/utils'; +import { assert, ManualPromise } from '../../utils/utils'; import { Page, PageBinding, PageDelegate, Worker } from '../page'; import * as types from '../types'; import { getAccessibilityTree } from './ffAccessibility'; @@ -44,8 +44,7 @@ export class FFPage implements PageDelegate { readonly _page: Page; readonly _networkManager: FFNetworkManager; readonly _browserContext: FFBrowserContext; - private _pagePromise: Promise; - private _pageCallback: (pageOrError: Page | Error) => void = () => {}; + private _pagePromise = new ManualPromise(); _initializedPage: Page | null = null; private _initializationFailed = false; readonly _opener: FFPage | null; @@ -95,7 +94,6 @@ export class FFPage implements PageDelegate { eventsHelper.addEventListener(this._session, 'Page.screencastFrame', this._onScreencastFrame.bind(this)), ]; - this._pagePromise = new Promise(f => this._pageCallback = f); session.once(FFSessionEvents.Disconnected, () => { this._markAsError(new Error('Page closed')); this._page._didDisconnect(); @@ -108,7 +106,7 @@ export class FFPage implements PageDelegate { // so that anyone who awaits pageOrError got a ready and reported page. this._initializedPage = this._page; this._page.reportAsNew(); - this._pageCallback(this._page); + this._pagePromise.resolve(this._page); }); // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy. // Therefore, we can end up with an initialized page without utility world, although very unlikely. @@ -124,7 +122,7 @@ export class FFPage implements PageDelegate { if (!this._initializedPage) { await this._page.initOpener(this._opener); this._page.reportAsNew(error); - this._pageCallback(error); + this._pagePromise.resolve(error); } } diff --git a/src/server/frames.ts b/src/server/frames.ts index 7ce7001c7e..b5b07696e1 100644 --- a/src/server/frames.ts +++ b/src/server/frames.ts @@ -26,7 +26,7 @@ import { Page } from './page'; import * as types from './types'; import { BrowserContext } from './browserContext'; import { Progress, ProgressController } from './progress'; -import { assert, constructURLBasedOnBaseURL, makeWaitForNextTask } from '../utils/utils'; +import { assert, constructURLBasedOnBaseURL, makeWaitForNextTask, ManualPromise } from '../utils/utils'; import { debugLogger } from '../utils/debugLogger'; import { CallMetadata, internalCallMetadata, SdkObject } from './instrumentation'; import { ElementStateWithoutStable } from './injected/injectedScript'; @@ -1364,16 +1364,14 @@ class RerunnableTask { class SignalBarrier { private _progress: Progress | null; private _protectCount = 0; - private _promise: Promise; - private _promiseCallback = () => {}; + private _promise = new ManualPromise(); constructor(progress: Progress | null) { this._progress = progress; - this._promise = new Promise(f => this._promiseCallback = f); this.retain(); } - waitFor(): Promise { + waitFor(): PromiseLike { this.release(); return this._promise; } @@ -1405,7 +1403,7 @@ class SignalBarrier { release() { --this._protectCount; if (!this._protectCount) - this._promiseCallback(); + this._promise.resolve(); } } diff --git a/src/server/network.ts b/src/server/network.ts index e01807fbc5..fb26bf52b7 100644 --- a/src/server/network.ts +++ b/src/server/network.ts @@ -16,7 +16,7 @@ import * as frames from './frames'; import * as types from './types'; -import { assert } from '../utils/utils'; +import { assert, ManualPromise } from '../utils/utils'; import { SdkObject } from './instrumentation'; export function filterCookies(cookies: types.NetworkCookie[], urls: string[]): types.NetworkCookie[] { @@ -97,8 +97,7 @@ export class Request extends SdkObject { private _headers: types.HeadersArray; private _headersMap = new Map(); private _frame: frames.Frame; - private _waitForResponsePromise: Promise; - private _waitForResponsePromiseCallback: (value: Response | null) => void = () => {}; + private _waitForResponsePromise = new ManualPromise(); _responseEndTiming = -1; _sizes: RequestSizes = { responseBodySize: 0, transferSize: 0 }; @@ -118,13 +117,12 @@ export class Request extends SdkObject { this._headers = headers; for (const { name, value } of this._headers) this._headersMap.set(name.toLowerCase(), value); - this._waitForResponsePromise = new Promise(f => this._waitForResponsePromiseCallback = f); this._isFavicon = url.endsWith('/favicon.ico'); } _setFailureText(failureText: string) { this._failureText = failureText; - this._waitForResponsePromiseCallback(null); + this._waitForResponsePromise.resolve(null); } url(): string { @@ -151,7 +149,7 @@ export class Request extends SdkObject { return this._headersMap.get(name); } - response(): Promise { + response(): PromiseLike { return this._waitForResponsePromise; } @@ -161,7 +159,7 @@ export class Request extends SdkObject { _setResponse(response: Response) { this._response = response; - this._waitForResponsePromiseCallback(response); + this._waitForResponsePromise.resolve(response); } _finalRequest(): Request { @@ -321,8 +319,7 @@ export type SecurityDetails = { export class Response extends SdkObject { private _request: Request; private _contentPromise: Promise | null = null; - _finishedPromise: Promise<{ error?: string }>; - private _finishedPromiseCallback: (arg: { error?: string }) => void = () => {}; + _finishedPromise = new ManualPromise<{ error?: string }>(); private _status: number; private _statusText: string; private _url: string; @@ -330,10 +327,8 @@ export class Response extends SdkObject { private _headersMap = new Map(); private _getResponseBodyCallback: GetResponseBodyCallback; private _timing: ResourceTiming; - private _serverAddrPromise: Promise; - private _serverAddrPromiseCallback: (arg?: RemoteAddr) => void = () => {}; - private _securityDetailsPromise: Promise; - private _securityDetailsPromiseCallback: (arg?: SecurityDetails) => void = () => {}; + private _serverAddrPromise = new ManualPromise(); + private _securityDetailsPromise = new ManualPromise(); private _httpVersion: string | undefined; constructor(request: Request, status: number, statusText: string, headers: types.HeadersArray, timing: ResourceTiming, getResponseBodyCallback: GetResponseBodyCallback, httpVersion?: string) { @@ -347,30 +342,21 @@ export class Response extends SdkObject { for (const { name, value } of this._headers) this._headersMap.set(name.toLowerCase(), value); this._getResponseBodyCallback = getResponseBodyCallback; - this._serverAddrPromise = new Promise(f => { - this._serverAddrPromiseCallback = f; - }); - this._securityDetailsPromise = new Promise(f => { - this._securityDetailsPromiseCallback = f; - }); - this._finishedPromise = new Promise(f => { - this._finishedPromiseCallback = f; - }); this._request._setResponse(this); this._httpVersion = httpVersion; } _serverAddrFinished(addr?: RemoteAddr) { - this._serverAddrPromiseCallback(addr); + this._serverAddrPromise.resolve(addr); } _securityDetailsFinished(securityDetails?: SecurityDetails) { - this._securityDetailsPromiseCallback(securityDetails); + this._securityDetailsPromise.resolve(securityDetails); } _requestFinished(responseEndTiming: number, error?: string) { this._request._responseEndTiming = Math.max(responseEndTiming, this._timing.responseStart); - this._finishedPromiseCallback({ error }); + this._finishedPromise.resolve({ error }); } _setHttpVersion(httpVersion: string) { diff --git a/src/server/page.ts b/src/server/page.ts index 0698a2f89e..bc68850687 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -28,7 +28,7 @@ import { ConsoleMessage } from './console'; import * as accessibility from './accessibility'; import { FileChooser } from './fileChooser'; import { Progress, ProgressController } from './progress'; -import { assert, isError } from '../utils/utils'; +import { assert, isError, ManualPromise } from '../utils/utils'; import { debugLogger } from '../utils/debugLogger'; import { SelectorInfo, Selectors } from './selectors'; import { CallMetadata, SdkObject } from './instrumentation'; @@ -115,14 +115,11 @@ export class Page extends SdkObject { }; private _closedState: 'open' | 'closing' | 'closed' = 'open'; - private _closedCallback: () => void; - private _closedPromise: Promise; + private _closedPromise = new ManualPromise(); private _disconnected = false; private _initialized = false; - private _disconnectedCallback: (e: Error) => void; - readonly _disconnectedPromise: Promise; - private _crashedCallback: (e: Error) => void; - readonly _crashedPromise: Promise; + readonly _disconnectedPromise = new ManualPromise(); + readonly _crashedPromise = new ManualPromise(); readonly _browserContext: BrowserContext; readonly keyboard: input.Keyboard; readonly mouse: input.Mouse; @@ -150,12 +147,6 @@ export class Page extends SdkObject { super(browserContext, 'page'); this.attribution.page = this; this._delegate = delegate; - this._closedCallback = () => {}; - this._closedPromise = new Promise(f => this._closedCallback = f); - this._disconnectedCallback = () => {}; - this._disconnectedPromise = new Promise(f => this._disconnectedCallback = f); - this._crashedCallback = () => {}; - this._crashedPromise = new Promise(f => this._crashedCallback = f); this._browserContext = browserContext; this._state = { emulatedSize: browserContext._options.viewport ? { viewport: browserContext._options.viewport, screen: browserContext._options.screen || browserContext._options.viewport } : null, @@ -218,20 +209,20 @@ export class Page extends SdkObject { assert(this._closedState !== 'closed', 'Page closed twice'); this._closedState = 'closed'; this.emit(Page.Events.Close); - this._closedCallback(); + this._closedPromise.resolve(); } _didCrash() { this._frameManager.dispose(); this.emit(Page.Events.Crash); - this._crashedCallback(new Error('Page crashed')); + this._crashedPromise.resolve(new Error('Page crashed')); } _didDisconnect() { this._frameManager.dispose(); assert(!this._disconnected, 'Page disconnected twice'); this._disconnected = true; - this._disconnectedCallback(new Error('Page closed')); + this._disconnectedPromise.resolve(new Error('Page closed')); } async _onFileChooserOpened(handle: dom.ElementHandle) { diff --git a/src/server/webkit/wkInterceptableRequest.ts b/src/server/webkit/wkInterceptableRequest.ts index c6f7e389fb..863d6b3996 100644 --- a/src/server/webkit/wkInterceptableRequest.ts +++ b/src/server/webkit/wkInterceptableRequest.ts @@ -20,7 +20,7 @@ import * as network from '../network'; import * as types from '../types'; import { Protocol } from './protocol'; import { WKSession } from './wkConnection'; -import { assert, headersObjectToArray, headersArrayToObject } from '../../utils/utils'; +import { assert, headersObjectToArray, headersArrayToObject, ManualPromise } from '../../utils/utils'; import { InterceptedResponse } from '../network'; import { WKPage } from './wkPage'; @@ -95,17 +95,14 @@ export class WKInterceptableRequest { export class WKRouteImpl implements network.RouteDelegate { private readonly _session: WKSession; private readonly _requestId: string; - _requestInterceptedCallback: () => void = () => {}; - private readonly _requestInterceptedPromise: Promise; - _responseInterceptedCallback: ((payload: { response?: Protocol.Network.Response, error?: Protocol.Network.loadingFailedPayload }) => void) | undefined; - private _responseInterceptedPromise: Promise<{ response?: Protocol.Network.Response, error?: Protocol.Network.loadingFailedPayload }> | undefined; + readonly _requestInterceptedPromise = new ManualPromise(); + _responseInterceptedPromise: ManualPromise<{ response?: Protocol.Network.Response, error?: Protocol.Network.loadingFailedPayload }> | undefined; private readonly _page: WKPage; constructor(session: WKSession, page: WKPage, requestId: string) { this._session = session; this._page = page; this._requestId = requestId; - this._requestInterceptedPromise = new Promise(f => this._requestInterceptedCallback = f); } async responseBody(): Promise { @@ -151,7 +148,7 @@ export class WKRouteImpl implements network.RouteDelegate { async continue(request: network.Request, overrides: types.NormalizedContinueOverrides): Promise { if (overrides.interceptResponse) { await this._page._ensureResponseInterceptionEnabled(); - this._responseInterceptedPromise = new Promise(f => this._responseInterceptedCallback = f); + this._responseInterceptedPromise = new ManualPromise(); } await this._requestInterceptedPromise; // In certain cases, protocol will return error if the request was already canceled diff --git a/src/server/webkit/wkPage.ts b/src/server/webkit/wkPage.ts index 3c49952023..229cded4c6 100644 --- a/src/server/webkit/wkPage.ts +++ b/src/server/webkit/wkPage.ts @@ -19,7 +19,7 @@ import * as jpeg from 'jpeg-js'; import path from 'path'; import * as png from 'pngjs'; import { splitErrorMessage } from '../../utils/stackTrace'; -import { assert, createGuid, debugAssert, headersArrayToObject, headersObjectToArray, hostPlatform } from '../../utils/utils'; +import { assert, createGuid, debugAssert, headersArrayToObject, headersObjectToArray, hostPlatform, ManualPromise } from '../../utils/utils'; import * as accessibility from '../accessibility'; import * as dialog from '../dialog'; import * as dom from '../dom'; @@ -52,8 +52,7 @@ export class WKPage implements PageDelegate { _session: WKSession; private _provisionalPage: WKProvisionalPage | null = null; readonly _page: Page; - private readonly _pagePromise: Promise; - private _pagePromiseCallback: (page: Page | Error) => void = () => {}; + private readonly _pagePromise = new ManualPromise(); private readonly _pageProxySession: WKSession; readonly _opener: WKPage | null; private readonly _requestIdToRequest = new Map(); @@ -96,7 +95,6 @@ export class WKPage implements PageDelegate { eventsHelper.addEventListener(this._pageProxySession, 'Target.didCommitProvisionalTarget', this._onDidCommitProvisionalTarget.bind(this)), eventsHelper.addEventListener(this._pageProxySession, 'Screencast.screencastFrame', this._onScreencastFrame.bind(this)), ]; - this._pagePromise = new Promise(f => this._pagePromiseCallback = f); this._firstNonInitialNavigationCommittedPromise = new Promise((f, r) => { this._firstNonInitialNavigationCommittedFulfill = f; this._firstNonInitialNavigationCommittedReject = r; @@ -335,7 +333,7 @@ export class WKPage implements PageDelegate { // so that anyone who awaits pageOrError got a ready and reported page. this._initializedPage = pageOrError instanceof Page ? pageOrError : null; this._page.reportAsNew(pageOrError instanceof Page ? undefined : pageOrError); - this._pagePromiseCallback(pageOrError); + this._pagePromise.resolve(pageOrError); } else { assert(targetInfo.isProvisional); assert(!this._provisionalPage); @@ -997,18 +995,18 @@ export class WKPage implements PageDelegate { // Just continue. session.sendMayFail('Network.interceptWithRequest', { requestId: request._requestId }); } else { - request._route._requestInterceptedCallback(); + request._route._requestInterceptedPromise.resolve(); } } _onResponseIntercepted(session: WKSession, event: Protocol.Network.responseInterceptedPayload) { const request = this._requestIdToRequest.get(event.requestId); const route = request?._routeForRedirectChain(); - if (!route?._responseInterceptedCallback) { + if (!route?._responseInterceptedPromise) { session.sendMayFail('Network.interceptContinue', { requestId: event.requestId, stage: 'response' }); return; } - route._responseInterceptedCallback({ response: event.response }); + route._responseInterceptedPromise.resolve({ response: event.response }); } _onResponseReceived(event: Protocol.Network.responseReceivedPayload) { @@ -1068,8 +1066,8 @@ export class WKPage implements PageDelegate { if (!request) return; const route = request._routeForRedirectChain(); - if (route?._responseInterceptedCallback) - route._responseInterceptedCallback({ error: event }); + if (route?._responseInterceptedPromise) + route._responseInterceptedPromise.resolve({ error: event }); const response = request.request._existingResponse(); if (response) { response._serverAddrFinished(); diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index e89bc03735..421dba9654 100644 --- a/src/test/workerRunner.ts +++ b/src/test/workerRunner.ts @@ -122,8 +122,8 @@ export class WorkerRunner extends EventEmitter { } async run(runPayload: RunPayload) { - let runFinishedCalback = () => {}; - this._runFinished = new Promise(f => runFinishedCalback = f); + let runFinishedCallback = () => {}; + this._runFinished = new Promise(f => runFinishedCallback = f); try { this._entries = new Map(runPayload.entries.map(e => [ e.testId, e ])); await this._loadIfNeeded(); @@ -146,7 +146,7 @@ export class WorkerRunner extends EventEmitter { this.unhandledError(e); } finally { this._reportDone(); - runFinishedCalback(); + runFinishedCallback(); } } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 302ca4f454..d2359817e3 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -392,3 +392,35 @@ export function wrapInASCIIBox(text: string, padding = 0): string { '╚' + '═'.repeat(maxLength + padding * 2) + '╝', ].join('\n'); } + +export class ManualPromise extends Promise { + private _resolve!: (t: T) => void; + private _reject!: (e: Error) => void; + + constructor() { + let resolve: (t: T) => void; + let reject: (e: Error) => void; + super((f, r) => { + resolve = f; + reject = r; + }); + this._resolve = resolve!; + this._reject = reject!; + } + + resolve(t: T) { + this._resolve(t); + } + + reject(e: Error) { + this._reject(e); + } + + static override get [Symbol.species]() { + return Promise; + } + + override get [Symbol.toStringTag]() { + return 'ManualPromise'; + } +}