Skip to content

Commit 5e0d92e

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 790e6a4 commit 5e0d92e

File tree

8 files changed

+431
-314
lines changed

8 files changed

+431
-314
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
@@ -308,8 +308,8 @@ export abstract class BrowserContext extends SdkObject {
308308
return this.doSetHTTPCredentials(httpCredentials);
309309
}
310310

311-
hasBinding(name: string) {
312-
return this._pageBindings.has(name);
311+
getBindingClient(name: string): unknown | undefined {
312+
return this._pageBindings.get(name)?.forClient;
313313
}
314314

315315
async exposePlaywrightBindingIfNeeded() {
@@ -328,7 +328,7 @@ export abstract class BrowserContext extends SdkObject {
328328
return this._playwrightBindingExposed;
329329
}
330330

331-
async exposeBinding(progress: Progress, name: string, needsHandle: boolean, playwrightBinding: frames.FunctionWithSource): Promise<PageBinding> {
331+
async exposeBinding(progress: Progress, name: string, needsHandle: boolean, playwrightBinding: frames.FunctionWithSource, forClient?: unknown): Promise<PageBinding> {
332332
if (this._pageBindings.has(name))
333333
throw new Error(`Function "${name}" has been already registered`);
334334
for (const page of this.pages()) {
@@ -337,6 +337,7 @@ export abstract class BrowserContext extends SdkObject {
337337
}
338338
await progress.race(this.exposePlaywrightBindingIfNeeded());
339339
const binding = new PageBinding(name, playwrightBinding, needsHandle);
340+
binding.forClient = forClient;
340341
this._pageBindings.set(name, binding);
341342
progress.cleanupWhenAborted(() => this._pageBindings.delete(name));
342343
await progress.race(this.doAddInitScript(binding.initScript));
@@ -443,8 +444,8 @@ export abstract class BrowserContext extends SdkObject {
443444
this._options.httpCredentials = { username, password: password || '' };
444445
}
445446

446-
async addInitScript(progress: Progress | undefined, source: string, name?: string) {
447-
const initScript = new InitScript(source, name);
447+
async addInitScript(progress: Progress | undefined, source: string) {
448+
const initScript = new InitScript(source);
448449
this.initScripts.push(initScript);
449450
progress?.cleanupWhenAborted(() => this.removeInitScripts([initScript]));
450451
const promise = this.doAddInitScript(initScript);

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
5858
private _clockPaused = false;
5959
private _requestInterceptor: RouteHandler;
6060
private _interceptionUrlMatchers: (string | RegExp)[] = [];
61+
private _routeWebSocketInitScripts: InitScript[] = [];
6162

6263
static from(parentScope: DispatcherScope, context: BrowserContext): BrowserContextDispatcher {
6364
const result = parentScope.connection.existingDispatcher<BrowserContextDispatcher>(context);
@@ -319,7 +320,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
319320
async setWebSocketInterceptionPatterns(params: channels.PageSetWebSocketInterceptionPatternsParams, progress: Progress): Promise<void> {
320321
this._webSocketInterceptionPatterns = params.patterns;
321322
if (params.patterns.length)
322-
await WebSocketRouteDispatcher.installIfNeeded(progress, this.connection, this._context);
323+
this._routeWebSocketInitScripts.push(await WebSocketRouteDispatcher.install(progress, this.connection, this._context));
323324
}
324325

325326
async storageState(params: channels.BrowserContextStorageStateParams, progress: Progress): Promise<channels.BrowserContextStorageStateResult> {
@@ -418,6 +419,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
418419
this._bindings = [];
419420
this._context.removeInitScripts(this._initScritps).catch(() => {});
420421
this._initScritps = [];
422+
this._routeWebSocketInitScripts.map(script => WebSocketRouteDispatcher.uninstall(this.connection, this._context, script).catch(() => {}));
423+
this._routeWebSocketInitScripts = [];
421424
if (this._clockPaused)
422425
this._context.clock.resumeNoReply();
423426
this._clockPaused = false;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
4949
_webSocketInterceptionPatterns: channels.PageSetWebSocketInterceptionPatternsParams['patterns'] = [];
5050
private _bindings: PageBinding[] = [];
5151
private _initScripts: InitScript[] = [];
52+
private _routeWebSocketInitScripts: InitScript[] = [];
5253
private _requestInterceptor: RouteHandler;
5354
private _interceptionUrlMatchers: (string | RegExp)[] = [];
5455
private _locatorHandlers = new Set<number>();
@@ -208,7 +209,7 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
208209
async setWebSocketInterceptionPatterns(params: channels.PageSetWebSocketInterceptionPatternsParams, progress: Progress): Promise<void> {
209210
this._webSocketInterceptionPatterns = params.patterns;
210211
if (params.patterns.length)
211-
await WebSocketRouteDispatcher.installIfNeeded(progress, this.connection, this._page);
212+
this._routeWebSocketInitScripts.push(await WebSocketRouteDispatcher.install(progress, this.connection, this._page));
212213
}
213214

214215
async expectScreenshot(params: channels.PageExpectScreenshotParams, progress: Progress): Promise<channels.PageExpectScreenshotResult> {
@@ -367,6 +368,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
367368
this._bindings = [];
368369
this._page.removeInitScripts(this._initScripts).catch(() => {});
369370
this._initScripts = [];
371+
this._routeWebSocketInitScripts.map(script => WebSocketRouteDispatcher.uninstall(this.connection, this._page, script).catch(() => {}));
372+
this._routeWebSocketInitScripts = [];
370373
for (const uid of this._locatorHandlers)
371374
this._page.unregisterLocatorHandler(uid);
372375
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)