Skip to content

Commit 1568220

Browse files
committed
fix: various WebSocketRoute issues
- Do not allow two clients to route simultaneously. - Cleanup after dispatcher disconnects. - Ensure that routing works in context reuse scenarios. - Test context reuse both in the launched browser with a single connection, and in a connected browser with different connections.
1 parent 3fb78b6 commit 1568220

File tree

8 files changed

+437
-321
lines changed

8 files changed

+437
-321
lines changed

packages/playwright-core/src/client/network.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,8 @@ export class WebSocketRoute extends ChannelOwner<channels.WebSocketRouteChannel>
564564
if (this._connected)
565565
return;
566566
// Ensure that websocket is "open" and can send messages without an actual server connection.
567-
await this._channel.ensureOpened();
567+
// If this happens after the page has been closed, ignore the error.
568+
await this._channel.ensureOpened().catch(() => {});
568569
}
569570
}
570571

packages/playwright-core/src/server/browserContext.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,8 @@ export abstract class BrowserContext extends SdkObject {
297297
return this.doSetHTTPCredentials(httpCredentials);
298298
}
299299

300-
hasBinding(name: string) {
301-
return this._pageBindings.has(name);
300+
getBindingClient(name: string): unknown | undefined {
301+
return this._pageBindings.get(name)?.forClient;
302302
}
303303

304304
async exposePlaywrightBindingIfNeeded() {
@@ -317,7 +317,7 @@ export abstract class BrowserContext extends SdkObject {
317317
return this._playwrightBindingExposed;
318318
}
319319

320-
async exposeBinding(progress: Progress, name: string, needsHandle: boolean, playwrightBinding: frames.FunctionWithSource): Promise<PageBinding> {
320+
async exposeBinding(progress: Progress, name: string, needsHandle: boolean, playwrightBinding: frames.FunctionWithSource, forClient?: unknown): Promise<PageBinding> {
321321
if (this._pageBindings.has(name))
322322
throw new Error(`Function "${name}" has been already registered`);
323323
for (const page of this.pages()) {
@@ -326,6 +326,7 @@ export abstract class BrowserContext extends SdkObject {
326326
}
327327
await progress.race(this.exposePlaywrightBindingIfNeeded());
328328
const binding = new PageBinding(name, playwrightBinding, needsHandle);
329+
binding.forClient = forClient;
329330
this._pageBindings.set(name, binding);
330331
progress.cleanupWhenAborted(() => this._pageBindings.delete(name));
331332
await progress.race(this.doAddInitScript(binding.initScript));
@@ -432,8 +433,8 @@ export abstract class BrowserContext extends SdkObject {
432433
this._options.httpCredentials = { username, password: password || '' };
433434
}
434435

435-
async addInitScript(progress: Progress | undefined, source: string, name?: string) {
436-
const initScript = new InitScript(source, name);
436+
async addInitScript(progress: Progress | undefined, source: string) {
437+
const initScript = new InitScript(source);
437438
this.initScripts.push(initScript);
438439
progress?.cleanupWhenAborted(() => this.removeInitScripts([initScript]));
439440
const promise = this.doAddInitScript(initScript);

packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
5757
private _clockPaused = false;
5858
private _requestInterceptor: RouteHandler;
5959
private _interceptionUrlMatchers: (string | RegExp)[] = [];
60+
private _routeWebSocketInitScript: InitScript | undefined;
6061

6162
static from(parentScope: DispatcherScope, context: BrowserContext): BrowserContextDispatcher {
6263
const result = parentScope.connection.existingDispatcher<BrowserContextDispatcher>(context);
@@ -317,8 +318,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
317318

318319
async setWebSocketInterceptionPatterns(params: channels.PageSetWebSocketInterceptionPatternsParams, progress: Progress): Promise<void> {
319320
this._webSocketInterceptionPatterns = params.patterns;
320-
if (params.patterns.length)
321-
await WebSocketRouteDispatcher.installIfNeeded(progress, this.connection, this._context);
321+
if (params.patterns.length && !this._routeWebSocketInitScript)
322+
this._routeWebSocketInitScript = await WebSocketRouteDispatcher.install(progress, this.connection, this._context);
322323
}
323324

324325
async storageState(params: channels.BrowserContextStorageStateParams, progress: Progress): Promise<channels.BrowserContextStorageStateResult> {
@@ -417,6 +418,9 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
417418
this._bindings = [];
418419
this._context.removeInitScripts(this._initScripts).catch(() => {});
419420
this._initScripts = [];
421+
if (this._routeWebSocketInitScript)
422+
WebSocketRouteDispatcher.uninstall(this.connection, this._context, this._routeWebSocketInitScript).catch(() => {});
423+
this._routeWebSocketInitScript = undefined;
420424
if (this._clockPaused)
421425
this._context.clock.resumeNoReply();
422426
this._clockPaused = false;

packages/playwright-core/src/server/dispatchers/pageDispatcher.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
5151
private _initScripts: InitScript[] = [];
5252
private _requestInterceptor: RouteHandler;
5353
private _interceptionUrlMatchers: (string | RegExp)[] = [];
54+
private _routeWebSocketInitScript: InitScript | undefined;
5455
private _locatorHandlers = new Set<number>();
5556
private _jsCoverageActive = false;
5657
private _cssCoverageActive = false;
@@ -207,8 +208,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
207208

208209
async setWebSocketInterceptionPatterns(params: channels.PageSetWebSocketInterceptionPatternsParams, progress: Progress): Promise<void> {
209210
this._webSocketInterceptionPatterns = params.patterns;
210-
if (params.patterns.length)
211-
await WebSocketRouteDispatcher.installIfNeeded(progress, this.connection, this._page);
211+
if (params.patterns.length && !this._routeWebSocketInitScript)
212+
this._routeWebSocketInitScript = await WebSocketRouteDispatcher.install(progress, this.connection, this._page);
212213
}
213214

214215
async expectScreenshot(params: channels.PageExpectScreenshotParams, progress: Progress): Promise<channels.PageExpectScreenshotResult> {
@@ -367,6 +368,9 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
367368
this._bindings = [];
368369
this._page.removeInitScripts(this._initScripts).catch(() => {});
369370
this._initScripts = [];
371+
if (this._routeWebSocketInitScript)
372+
WebSocketRouteDispatcher.uninstall(this.connection, this._page, this._routeWebSocketInitScript).catch(() => {});
373+
this._routeWebSocketInitScript = undefined;
370374
for (const uid of this._locatorHandlers)
371375
this._page.unregisterLocatorHandler(uid);
372376
this._locatorHandlers.clear();

packages/playwright-core/src/server/dispatchers/webSocketRouteDispatcher.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import type { Frame } from '../frames';
2929
import type * as ws from '@injected/webSocketMock';
3030
import type * as channels from '@protocol/channels';
3131
import type { Progress } from '@protocol/progress';
32+
import type { InitScript, PageBinding } from '../page';
3233

3334
export class WebSocketRouteDispatcher extends Dispatcher<SdkObject, channels.WebSocketRouteChannel, PageDispatcher | BrowserContextDispatcher> implements channels.WebSocketRouteChannel {
3435
_type_WebSocketRoute = true;
@@ -58,11 +59,14 @@ export class WebSocketRouteDispatcher extends Dispatcher<SdkObject, channels.Web
5859
(scope as any)._dispatchEvent('webSocketRoute', { webSocketRoute: this });
5960
}
6061

61-
static async installIfNeeded(progress: Progress, connection: DispatcherConnection, target: Page | BrowserContext) {
62-
const kBindingName = '__pwWebSocketBinding';
62+
static async install(progress: Progress, connection: DispatcherConnection, target: Page | BrowserContext): Promise<InitScript> {
6363
const context = target instanceof Page ? target.browserContext : target;
64-
if (!context.hasBinding(kBindingName)) {
65-
await context.exposeBinding(progress, kBindingName, false, (source, payload: ws.BindingPayload) => {
64+
let data = context.getBindingClient(kBindingName) as BindingData | undefined;
65+
if (data && data.connection !== connection)
66+
throw new Error('Another client is already routing WebSockets');
67+
if (!data) {
68+
data = { counter: 0, connection, binding: null as any };
69+
data.binding = await context.exposeBinding(progress, kBindingName, false, (source, payload: ws.BindingPayload) => {
6670
if (payload.type === 'onCreate') {
6771
const contextDispatcher = connection.existingDispatcher<BrowserContextDispatcher>(context);
6872
const pageDispatcher = contextDispatcher ? PageDispatcher.fromNullable(contextDispatcher, source.page) : undefined;
@@ -89,19 +93,27 @@ export class WebSocketRouteDispatcher extends Dispatcher<SdkObject, channels.Web
8993
dispatcher?._dispatchEvent('closePage', { code: payload.code, reason: payload.reason, wasClean: payload.wasClean });
9094
if (payload.type === 'onCloseServer')
9195
dispatcher?._dispatchEvent('closeServer', { code: payload.code, reason: payload.reason, wasClean: payload.wasClean });
92-
});
96+
}, data);
9397
}
98+
++data.counter;
99+
100+
return await target.addInitScript(progress, `
101+
(() => {
102+
const module = {};
103+
${rawWebSocketMockSource.source}
104+
(module.exports.inject())(globalThis);
105+
})();
106+
`);
107+
}
94108

95-
const kInitScriptName = 'webSocketMockSource';
96-
if (!target.initScripts.find(s => s.name === kInitScriptName)) {
97-
await target.addInitScript(progress, `
98-
(() => {
99-
const module = {};
100-
${rawWebSocketMockSource.source}
101-
(module.exports.inject())(globalThis);
102-
})();
103-
`, kInitScriptName);
104-
}
109+
static async uninstall(connection: DispatcherConnection, target: Page | BrowserContext, initScript: InitScript) {
110+
const context = target instanceof Page ? target.browserContext : target;
111+
const data = context.getBindingClient(kBindingName) as BindingData | undefined;
112+
if (!data || data.connection !== connection)
113+
return;
114+
if (--data.counter <= 0)
115+
await context.removeExposedBindings([data.binding]);
116+
await target.removeInitScripts([initScript]);
105117
}
106118

107119
async connect(params: channels.WebSocketRouteConnectParams, progress: Progress) {
@@ -155,3 +167,6 @@ function matchesPattern(dispatcher: PageDispatcher | BrowserContextDispatcher, b
155167
}
156168
return false;
157169
}
170+
171+
const kBindingName = '__pwWebSocketBinding';
172+
type BindingData = { counter: number, connection: DispatcherConnection, binding: PageBinding };

packages/playwright-core/src/server/page.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,8 @@ export class Page extends SdkObject {
562562
await this.delegate.bringToFront();
563563
}
564564

565-
async addInitScript(progress: Progress, source: string, name?: string) {
566-
const initScript = new InitScript(source, name);
565+
async addInitScript(progress: Progress, source: string) {
566+
const initScript = new InitScript(source);
567567
this.initScripts.push(initScript);
568568
progress.cleanupWhenAborted(() => this.removeInitScripts([initScript]));
569569
await progress.race(this.delegate.addInitScript(initScript));
@@ -889,6 +889,7 @@ export class PageBinding {
889889
readonly initScript: InitScript;
890890
readonly needsHandle: boolean;
891891
readonly cleanupScript: string;
892+
forClient?: unknown;
892893

893894
constructor(name: string, playwrightFunction: frames.FunctionWithSource, needsHandle: boolean) {
894895
this.name = name;
@@ -924,13 +925,11 @@ export class PageBinding {
924925

925926
export class InitScript {
926927
readonly source: string;
927-
readonly name?: string;
928928

929-
constructor(source: string, name?: string) {
929+
constructor(source: string) {
930930
this.source = `(() => {
931931
${source}
932932
})();`;
933-
this.name = name;
934933
}
935934
}
936935

0 commit comments

Comments
 (0)