Skip to content

Commit

Permalink
chore: introduce manual promise race (#21358)
Browse files Browse the repository at this point in the history
Fixes #21345
  • Loading branch information
pavelfeldman authored Mar 6, 2023
1 parent 9e47c45 commit 86ca7e3
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 43 deletions.
2 changes: 1 addition & 1 deletion packages/playwright-core/src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
21 changes: 6 additions & 15 deletions packages/playwright-core/src/client/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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();
}
}

Expand All @@ -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> {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down
17 changes: 6 additions & 11 deletions packages/playwright-core/src/client/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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[] = [];

Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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> {
Expand Down
10 changes: 4 additions & 6 deletions packages/playwright-core/src/client/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
6 changes: 4 additions & 2 deletions packages/playwright-core/src/client/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
13 changes: 5 additions & 8 deletions packages/playwright-core/src/server/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -62,22 +62,19 @@ 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');
this._delegate = delegate;
}

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> {
Expand Down
34 changes: 34 additions & 0 deletions packages/playwright-core/src/utils/manualPromise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

0 comments on commit 86ca7e3

Please sign in to comment.