chore: introduce manual promise race (#21358)

Fixes https://github.com/microsoft/playwright/issues/21345
This commit is contained in:
Pavel Feldman 2023-03-06 08:50:03 -08:00 committed by GitHub
parent 9e47c450c7
commit 86ca7e3949
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 60 additions and 43 deletions

View File

@ -149,7 +149,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
if (page)
page.emit(Events.Page.RequestFinished, request);
if (response)
response._finishedPromise.resolve();
response._finishedPromise.resolve(null);
}
async _onRoute(route: network.Route) {

View File

@ -23,7 +23,7 @@ import type { Headers, RemoteAddr, SecurityDetails, WaitForEventOptions } from '
import fs from 'fs';
import { mime } from '../utilsBundle';
import { assert, isString, headersObjectToArray, isRegExp } from '../utils';
import { ManualPromise } from '../utils/manualPromise';
import { ManualPromise, ScopedRace } from '../utils/manualPromise';
import { Events } from './events';
import type { Page } from './page';
import { Waiter } from './waiter';
@ -33,7 +33,6 @@ import { urlMatches } from '../utils/network';
import { MultiMap } from '../utils/multimap';
import { APIResponse } from './fetch';
import type { Serializable } from '../../types/structs';
import { kBrowserOrContextClosedError } from '../common/errors';
export type NetworkCookie = {
name: string,
@ -271,8 +270,8 @@ export class Request extends ChannelOwner<channels.RequestChannel> implements ap
return this._fallbackOverrides;
}
_targetClosedPromise(): Promise<void> {
return this.serviceWorker()?._closedPromise || this.frame()._page?._closedOrCrashedPromise || new Promise(() => {});
_targetClosedRace(): ScopedRace {
return this.serviceWorker()?._closedRace || this.frame()._page?._closedOrCrashedRace || new ScopedRace();
}
}
@ -295,10 +294,7 @@ export class Route extends ChannelOwner<channels.RouteChannel> implements api.Ro
// When page closes or crashes, we catch any potential rejects from this Route.
// Note that page could be missing when routing popup's initial request that
// does not have a Page initialized just yet.
return Promise.race([
promise,
this.request()._targetClosedPromise(),
]);
return this.request()._targetClosedRace().safeRace(promise);
}
_startHandling(): Promise<boolean> {
@ -452,7 +448,7 @@ export class Response extends ChannelOwner<channels.ResponseChannel> implements
private _provisionalHeaders: RawHeaders;
private _actualHeadersPromise: Promise<RawHeaders> | undefined;
private _request: Request;
readonly _finishedPromise = new ManualPromise<void>();
readonly _finishedPromise = new ManualPromise<null>();
static from(response: channels.ResponseChannel): Response {
return (response as any)._object;
@ -523,12 +519,7 @@ export class Response extends ChannelOwner<channels.ResponseChannel> implements
}
async finished(): Promise<null> {
return Promise.race([
this._finishedPromise.then(() => null),
this.request()._targetClosedPromise().then(() => {
throw new Error(kBrowserOrContextClosedError);
}),
]);
return this.request()._targetClosedRace().race(this._finishedPromise);
}
async body(): Promise<Buffer> {

View File

@ -19,12 +19,12 @@ import fs from 'fs';
import path from 'path';
import type * as structs from '../../types/structs';
import type * as api from '../../types/types';
import { isSafeCloseError } from '../common/errors';
import { isSafeCloseError, kBrowserOrContextClosedError } from '../common/errors';
import { urlMatches } from '../utils/network';
import { TimeoutSettings } from '../common/timeoutSettings';
import type * as channels from '@protocol/channels';
import { parseError, serializeError } from '../protocol/serializers';
import { assert, headersObjectToArray, isObject, isRegExp, isString } from '../utils';
import { assert, headersObjectToArray, isObject, isRegExp, isString, ScopedRace } from '../utils';
import { mkdirIfNeeded } from '../utils/fileUtils';
import { Accessibility } from './accessibility';
import { Artifact } from './artifact';
@ -80,7 +80,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
private _frames = new Set<Frame>();
_workers = new Set<Worker>();
private _closed = false;
_closedOrCrashedPromise: Promise<void>;
readonly _closedOrCrashedRace = new ScopedRace();
private _viewportSize: Size | null;
private _routes: RouteHandler[] = [];
@ -153,10 +153,8 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this.coverage = new Coverage(this._channel);
this._closedOrCrashedPromise = Promise.race([
new Promise<void>(f => this.once(Events.Page.Close, f)),
new Promise<void>(f => this.once(Events.Page.Crash, f)),
]);
this.once(Events.Page.Close, () => this._closedOrCrashedRace.scopeClosed(new Error(kBrowserOrContextClosedError)));
this.once(Events.Page.Crash, () => this._closedOrCrashedRace.scopeClosed(new Error(kBrowserOrContextClosedError)));
this._setEventToSubscriptionMapping(new Map<string, channels.PageUpdateSubscriptionParams['event']>([
[Events.Page.Request, 'request'],
@ -693,10 +691,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
async pause() {
if (require('inspector').url())
return;
await Promise.race([
this.context()._channel.pause(),
this._closedOrCrashedPromise
]);
await this._closedOrCrashedRace.safeRace(this.context()._channel.pause());
}
async pdf(options: PDFOptions = {}): Promise<Buffer> {

View File

@ -18,22 +18,20 @@ import type { Page } from './page';
import type * as api from '../../types/types';
import type { Artifact } from './artifact';
import type { Connection } from './connection';
import { ManualPromise } from '../utils';
export class Video implements api.Video {
private _artifact: Promise<Artifact | null> | null = null;
private _artifactCallback = (artifact: Artifact) => {};
private _artifactReadyPromise = new ManualPromise<Artifact>();
private _isRemote = false;
constructor(page: Page, connection: Connection) {
this._isRemote = connection.isRemote();
this._artifact = Promise.race([
new Promise<Artifact>(f => this._artifactCallback = f),
page._closedOrCrashedPromise.then(() => null),
]);
this._artifact = page._closedOrCrashedRace.safeRace(this._artifactReadyPromise);
}
_artifactReady(artifact: Artifact) {
this._artifactCallback(artifact);
this._artifactReadyPromise.resolve(artifact);
}
async path(): Promise<string> {

View File

@ -22,11 +22,13 @@ import type { Page } from './page';
import type { BrowserContext } from './browserContext';
import type * as api from '../../types/types';
import type * as structs from '../../types/structs';
import { ScopedRace } from '../utils';
import { kBrowserOrContextClosedError } from '../common/errors';
export class Worker extends ChannelOwner<channels.WorkerChannel> implements api.Worker {
_page: Page | undefined; // Set for web workers.
_context: BrowserContext | undefined; // Set for service workers.
_closedPromise: Promise<void>;
readonly _closedRace = new ScopedRace();
static from(worker: channels.WorkerChannel): Worker {
return (worker as any)._object;
@ -41,7 +43,7 @@ export class Worker extends ChannelOwner<channels.WorkerChannel> implements api.
this._context._serviceWorkers.delete(this);
this.emit(Events.Worker.Close, this);
});
this._closedPromise = new Promise(f => this.once(Events.Worker.Close, f));
this.once(Events.Worker.Close, () => this._closedRace.scopeClosed(new Error(kBrowserOrContextClosedError)));
}
url(): string {

View File

@ -19,7 +19,7 @@ import * as utilityScriptSource from '../generated/utilityScriptSource';
import { serializeAsCallArgument } from './isomorphic/utilityScriptSerializers';
import { type UtilityScript } from './injected/utilityScript';
import { SdkObject } from './instrumentation';
import { ManualPromise } from '../utils/manualPromise';
import { ScopedRace } from '../utils/manualPromise';
export type ObjectId = string;
export type RemoteObject = {
@ -62,7 +62,7 @@ export interface ExecutionContextDelegate {
export class ExecutionContext extends SdkObject {
private _delegate: ExecutionContextDelegate;
private _utilityScriptPromise: Promise<JSHandle> | undefined;
private _destroyedPromise = new ManualPromise<Error>();
private _contextDestroyedRace = new ScopedRace();
constructor(parent: SdkObject, delegate: ExecutionContextDelegate) {
super(parent, 'execution-context');
@ -70,14 +70,11 @@ export class ExecutionContext extends SdkObject {
}
contextDestroyed(error: Error) {
this._destroyedPromise.resolve(error);
this._contextDestroyedRace.scopeClosed(error);
}
_raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
return Promise.race([
this._destroyedPromise.then(e => { throw e; }),
promise,
]);
async _raceAgainstContextDestroyed<T>(promise: Promise<T>): Promise<T> {
return this._contextDestroyedRace.race(promise);
}
rawEvaluateJSON(expression: string): Promise<any> {

View File

@ -53,3 +53,37 @@ export class ManualPromise<T = void> extends Promise<T> {
return 'ManualPromise';
}
}
export class ScopedRace {
private _terminateError: Error | undefined;
private _terminatePromises = new Set<ManualPromise<Error>>();
scopeClosed(error: Error) {
this._terminateError = error;
for (const p of this._terminatePromises)
p.resolve(error);
}
async race<T>(promise: Promise<T>): Promise<T> {
return this._race([promise], false) as Promise<T>;
}
async safeRace<T>(promise: Promise<T>, defaultValue?: T): Promise<T> {
return this._race([promise], true, defaultValue);
}
private async _race(promises: Promise<any>[], safe: boolean, defaultValue?: any): Promise<any> {
const terminatePromise = new ManualPromise<Error>();
if (this._terminateError)
terminatePromise.resolve(this._terminateError);
this._terminatePromises.add(terminatePromise);
try {
return await Promise.race([
terminatePromise.then(e => safe ? defaultValue : Promise.reject(e)),
...promises
]);
} finally {
this._terminatePromises.delete(terminatePromise);
}
}
}