Bootstrap Playwright service

[kycutler]

No description provided.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@bpasero

Matched files:

• src/vs/code/electron-main/app.ts
• src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts

@deepak1556

Matched files:

• src/vs/code/electron-main/app.ts
• src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts

[Copilot]

Pull request overview

This pull request bootstraps a Playwright service that enables automation of VS Code's integrated browser views through Playwright's CDP (Chrome DevTools Protocol) connection capabilities. The implementation creates a multi-layer architecture spanning the main process, shared process, and workbench renderer process.

Changes:

• Adds IPlaywrightService running in the shared process that connects to browser views via Playwright's CDP client
• Implements a CDP proxy server (BrowserViewCDPProxyServer) in the main process that provides WebSocket endpoints with token-based security
• Exposes getDebugWebSocketEndpoint() API through the browser view service layers to enable CDP connections

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/vs/workbench/workbench.desktop.main.ts	Registers the playwright workbench service bootstrap file
src/vs/workbench/services/browserView/electron-browser/playwrightWorkbenchService.ts	Bootstraps the playwright service registration for the renderer process
src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts	Adds getDebugWebSocketEndpoint() method to workbench service
src/vs/workbench/contrib/browserView/common/browserView.ts	Adds getDebugWebSocketEndpoint() to IBrowserViewWorkbenchService interface
src/vs/platform/browserView/node/playwrightService.ts	Implements PlaywrightService in shared process that connects via CDP
src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts	Implements WebSocket CDP proxy server with token-based security
src/vs/platform/browserView/electron-main/browserViewMainService.ts	Instantiates CDP proxy server and exposes endpoint method
src/vs/platform/browserView/common/playwrightService.ts	Defines IPlaywrightService interface
src/vs/platform/browserView/common/browserView.ts	Adds getDebugWebSocketEndpoint() to IBrowserViewService interface
src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts	Registers PlaywrightService in shared process and exposes IPC channel
src/vs/code/electron-main/app.ts	Registers browser view channel with shared process client
package.json	Adds playwright-core ^1.58.2 dependency
package-lock.json	Locks playwright-core dependency versions

Comments suppressed due to low confidence (3)

src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts:114

• The disposables (DisposableStore and upgraded ISocket) are registered to the parent BrowserViewCDPProxyServer using this._register() for each WebSocket connection. Since handleWebSocketUpgrade can be called multiple times (once per connection), this will cause all connection disposables to accumulate in the parent's disposal lifecycle. This can lead to memory leaks as these disposables will only be cleaned up when the entire server is disposed.

The DisposableStore already disposes itself when the socket closes (line 172), so it should not be registered to the parent. Similarly, the upgraded socket already disposes itself when closed, and is also disposed by the DisposableStore, so it should not be registered to the parent either.

Remove the two this._register() calls on lines 113-114.

		const proxy = new CDPBrowserProxy(this.targetService);
		const disposables = this.wireWebSocket(upgraded, proxy);
		this._register(disposables);
		this._register(upgraded);

src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts:149

• The connection.sendMessage() promises on line 130 are not tracked or cancelled when the socket closes. If the socket closes while a message is being processed, the promise handlers (then/catch on lines 131-149) may still attempt to write to the closed socket using upgraded.write(), which could throw errors or behave unexpectedly.

Consider checking if the socket is disposed before writing in the promise handlers, or implement proper cancellation of in-flight requests when the socket closes.

				connection.sendMessage(method, params, sessionId)
					.then((result: unknown) => {
						const response = { id, result, sessionId };
						const responseStr = JSON.stringify(response);
						this.logService.debug(`[BrowserViewDebugProxy] -> ${responseStr}`);
						upgraded.write(VSBuffer.fromString(responseStr));
					})
					.catch((error: Error) => {
						const response = {
							id,
							error: {
								code: error instanceof CDPError ? error.code : CDPErrorCode.ServerError,
								message: error.message || 'Unknown error'
							},
							sessionId
						};
						const responseStr = JSON.stringify(response);
						this.logService.debug(`[BrowserViewDebugProxy] -> ${responseStr}`);
						upgraded.write(VSBuffer.fromString(responseStr));
					});

src/vs/platform/browserView/node/playwrightService.ts:66

• If an error occurs during the initialization (lines 47-62), the error is silently swallowed by the finally block on line 64. The _initPromise is set to undefined, but the error is not propagated to the caller. This means calling initialize() will complete without throwing an error even if the connection failed, leaving the service in an inconsistent state where _browser is undefined but no error was reported.

The error should be allowed to propagate to the caller so they can handle the connection failure appropriately.

		this._initPromise = (async () => {
			try {
				this.logService.debug('[PlaywrightService] Connecting to browser via CDP');

				const playwright = await import('playwright-core');
				const endpoint = await this.browserViewService.getDebugWebSocketEndpoint();
				const browser = await playwright.chromium.connectOverCDP(endpoint);

				this.logService.debug('[PlaywrightService] Connected to browser');

				browser.on('disconnected', () => {
					this.logService.debug('[PlaywrightService] Browser disconnected');
					if (this._browser === browser) {
						this._browser = undefined;
					}
				});

				this._browser = browser;
			} finally {
				this._initPromise = undefined;
			}
		})();

[jruales]

Mermaid diagram from Copilot for posterity:

[bpasero]

@kycutler just glancing over the electron-main and shared process changes, good to see that the import is lazy. This helps reducing cost on startup when the module is actually not needed. Let me know if you want a more thorough review, but it seems legit so far, at least from the plumbing how its wired in via services.