From 6ee69bb525dd6142f474aaa4859a376996546141 Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Mon, 16 Feb 2026 17:05:33 -0800 Subject: [PATCH 1/6] Browser: Managed CDP context groups --- src/vs/code/electron-main/app.ts | 8 + .../sharedProcess/sharedProcessMain.ts | 2 + .../browserView/common/browserView.ts | 5 - .../browserView/common/browserViewGroup.ts | 86 +++++++ .../common/browserViewGroupRemoteService.ts | 109 +++++++++ .../browserViewCDPProxyServer.ts | 95 +++++--- .../electron-main/browserViewGroup.ts | 202 ++++++++++++++++ .../browserViewGroupMainService.ts | 88 +++++++ .../electron-main/browserViewMainService.ts | 8 +- .../browserView/node/playwrightService.ts | 217 ++++++++++++++++-- .../contrib/browserView/common/browserView.ts | 5 - .../browserViewWorkbenchService.ts | 4 - 12 files changed, 760 insertions(+), 69 deletions(-) create mode 100644 src/vs/platform/browserView/common/browserViewGroup.ts create mode 100644 src/vs/platform/browserView/common/browserViewGroupRemoteService.ts create mode 100644 src/vs/platform/browserView/electron-main/browserViewGroup.ts create mode 100644 src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts diff --git a/src/vs/code/electron-main/app.ts b/src/vs/code/electron-main/app.ts index fb4dc89183063..9ad21026d2859 100644 --- a/src/vs/code/electron-main/app.ts +++ b/src/vs/code/electron-main/app.ts @@ -37,7 +37,9 @@ import { IEncryptionMainService } from '../../platform/encryption/common/encrypt import { EncryptionMainService } from '../../platform/encryption/electron-main/encryptionMainService.js'; import { NativeBrowserElementsMainService, INativeBrowserElementsMainService } from '../../platform/browserElements/electron-main/nativeBrowserElementsMainService.js'; import { ipcBrowserViewChannelName } from '../../platform/browserView/common/browserView.js'; +import { ipcBrowserViewGroupChannelName } from '../../platform/browserView/common/browserViewGroup.js'; import { BrowserViewMainService, IBrowserViewMainService } from '../../platform/browserView/electron-main/browserViewMainService.js'; +import { BrowserViewGroupMainService, IBrowserViewGroupMainService } from '../../platform/browserView/electron-main/browserViewGroupMainService.js'; import { BrowserViewCDPProxyServer, IBrowserViewCDPProxyServer } from '../../platform/browserView/electron-main/browserViewCDPProxyServer.js'; import { NativeParsedArgs } from '../../platform/environment/common/argv.js'; import { IEnvironmentMainService } from '../../platform/environment/electron-main/environmentMainService.js'; @@ -1043,6 +1045,7 @@ export class CodeApplication extends Disposable { // Browser View services.set(IBrowserViewCDPProxyServer, new SyncDescriptor(BrowserViewCDPProxyServer, undefined, true)); services.set(IBrowserViewMainService, new SyncDescriptor(BrowserViewMainService, undefined, false /* proxied to other processes */)); + services.set(IBrowserViewGroupMainService, new SyncDescriptor(BrowserViewGroupMainService, undefined, false /* proxied to other processes */)); // Keyboard Layout services.set(IKeyboardLayoutMainService, new SyncDescriptor(KeyboardLayoutMainService)); @@ -1206,6 +1209,11 @@ export class CodeApplication extends Disposable { mainProcessElectronServer.registerChannel(ipcBrowserViewChannelName, browserViewChannel); sharedProcessClient.then(client => client.registerChannel(ipcBrowserViewChannelName, browserViewChannel)); + // Browser View Group + const browserViewGroupChannel = ProxyChannel.fromService(accessor.get(IBrowserViewGroupMainService), disposables); + mainProcessElectronServer.registerChannel(ipcBrowserViewGroupChannelName, browserViewGroupChannel); + sharedProcessClient.then(client => client.registerChannel(ipcBrowserViewGroupChannelName, browserViewGroupChannel)); + // Signing const signChannel = ProxyChannel.fromService(accessor.get(ISignService), disposables); mainProcessElectronServer.registerChannel('sign', signChannel); diff --git a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts index 6569ef1fbc808..2fb79b662da93 100644 --- a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts @@ -136,6 +136,7 @@ import { IMeteredConnectionService } from '../../../platform/meteredConnection/c import { MeteredConnectionChannelClient, METERED_CONNECTION_CHANNEL } from '../../../platform/meteredConnection/common/meteredConnectionIpc.js'; import { IPlaywrightService } from '../../../platform/browserView/common/playwrightService.js'; import { PlaywrightService } from '../../../platform/browserView/node/playwrightService.js'; +import { IBrowserViewGroupRemoteService, BrowserViewGroupRemoteService } from '../../../platform/browserView/common/browserViewGroupRemoteService.js'; class SharedProcessMain extends Disposable implements IClientConnectionFilter { @@ -404,6 +405,7 @@ class SharedProcessMain extends Disposable implements IClientConnectionFilter { services.set(ISharedWebContentExtractorService, new SyncDescriptor(SharedWebContentExtractorService)); // Playwright + services.set(IBrowserViewGroupRemoteService, new SyncDescriptor(BrowserViewGroupRemoteService)); services.set(IPlaywrightService, new SyncDescriptor(PlaywrightService)); return new InstantiationService(services); diff --git a/src/vs/platform/browserView/common/browserView.ts b/src/vs/platform/browserView/common/browserView.ts index 5c8c9517dfb4a..f22fd39e70b0c 100644 --- a/src/vs/platform/browserView/common/browserView.ts +++ b/src/vs/platform/browserView/common/browserView.ts @@ -275,9 +275,4 @@ export interface IBrowserViewService { * @param id The browser view identifier */ clearStorage(id: string): Promise; - - /** - * Get a CDP WebSocket endpoint URL. - */ - getDebugWebSocketEndpoint(): Promise; } diff --git a/src/vs/platform/browserView/common/browserViewGroup.ts b/src/vs/platform/browserView/common/browserViewGroup.ts new file mode 100644 index 0000000000000..0f43b98c8b080 --- /dev/null +++ b/src/vs/platform/browserView/common/browserViewGroup.ts @@ -0,0 +1,86 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from '../../../base/common/event.js'; +import { IDisposable } from '../../../base/common/lifecycle.js'; + +export const ipcBrowserViewGroupChannelName = 'browserViewGroup'; + +/** + * Fired when a browser view is added to or removed from a group. + */ +export interface IBrowserViewGroupViewEvent { + /** The ID of the browser view that was added or removed. */ + readonly viewId: string; +} + +/** + * A browser view group - an isolated collection of browser views. + * + * This interface is shared between the main-process entity and remote proxies. + */ +export interface IBrowserViewGroup extends IDisposable { + readonly id: string; + + readonly onDidAddView: Event; + readonly onDidRemoveView: Event; + readonly onDidDestroy: Event; + + addView(viewId: string): Promise; + removeView(viewId: string): Promise; + getDebugWebSocketEndpoint(): Promise; +} + +/** + * Common service for managing browser view groups across processes. + * + * A browser view group is an isolated collection of browser views that can be + * independently exposed to different services or CDP clients. + * + * This interface is consumed via {@link ProxyChannel}. + * The main-process implementation is {@link BrowserViewGroupMainService}. + */ +export interface IBrowserViewGroupService { + + // Dynamic events - one per group instance, keyed by group ID. + onDynamicDidAddView(groupId: string): Event; + onDynamicDidRemoveView(groupId: string): Event; + onDynamicDidDestroy(groupId: string): Event; + + /** + * Create a new browser view group. + * @returns The id of the newly created group. + */ + createGroup(): Promise; + + /** + * Destroy a browser view group. + * Views in the group are **not** destroyed - they are simply detached. + * @param groupId The group identifier. + */ + destroyGroup(groupId: string): Promise; + + /** + * Add a browser view to a group. + * A view can belong to multiple groups simultaneously. + * @param groupId The group identifier. + * @param viewId The browser view identifier. + */ + addViewToGroup(groupId: string, viewId: string): Promise; + + /** + * Remove a browser view from a group. + * @param groupId The group identifier. + * @param viewId The browser view identifier. + */ + removeViewFromGroup(groupId: string, viewId: string): Promise; + + /** + * Get a short-lived CDP WebSocket endpoint URL for a specific group. + * The returned URL contains a single-use token. + * @param groupId The group identifier. + */ + getDebugWebSocketEndpoint(groupId: string): Promise; +} diff --git a/src/vs/platform/browserView/common/browserViewGroupRemoteService.ts b/src/vs/platform/browserView/common/browserViewGroupRemoteService.ts new file mode 100644 index 0000000000000..87cffd8858a1d --- /dev/null +++ b/src/vs/platform/browserView/common/browserViewGroupRemoteService.ts @@ -0,0 +1,109 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from '../../../base/common/event.js'; +import { Disposable } from '../../../base/common/lifecycle.js'; +import { ProxyChannel } from '../../../base/parts/ipc/common/ipc.js'; +import { createDecorator } from '../../instantiation/common/instantiation.js'; +import { IMainProcessService } from '../../ipc/common/mainProcessService.js'; +import { IBrowserViewGroup, IBrowserViewGroupService, IBrowserViewGroupViewEvent, ipcBrowserViewGroupChannelName } from './browserViewGroup.js'; + +export const IBrowserViewGroupRemoteService = createDecorator('browserViewGroupRemoteService'); + +/** + * Remote-process service for managing browser view groups. + * + * Connects to the main-process {@link BrowserViewGroupMainService} via + * IPC and provides {@link IBrowserViewGroup} instances for + * interacting with groups. + * + * Usable from the shared process or the workbench renderer. + */ +export interface IBrowserViewGroupRemoteService { + readonly _serviceBrand: undefined; + + /** + * Create a new browser view group. + */ + createGroup(): Promise; +} + +/** + * Remote proxy for a browser view group living in the main process. + */ +class RemoteBrowserViewGroup extends Disposable implements IBrowserViewGroup { + constructor( + readonly id: string, + private readonly groupService: IBrowserViewGroupService, + ) { + super(); + + this._register(groupService.onDynamicDidDestroy(this.id)(() => { + // Avoid loops + this.dispose(true); + })); + } + + get onDidAddView(): Event { + return this.groupService.onDynamicDidAddView(this.id); + } + + get onDidRemoveView(): Event { + return this.groupService.onDynamicDidRemoveView(this.id); + } + + get onDidDestroy(): Event { + return this.groupService.onDynamicDidDestroy(this.id); + } + + async addView(viewId: string): Promise { + return this.groupService.addViewToGroup(this.id, viewId); + } + + async removeView(viewId: string): Promise { + return this.groupService.removeViewFromGroup(this.id, viewId); + } + + async getDebugWebSocketEndpoint(): Promise { + return this.groupService.getDebugWebSocketEndpoint(this.id); + } + + override dispose(fromService = false): void { + if (!fromService) { + this.groupService.destroyGroup(this.id); + } + super.dispose(); + } +} + +export class BrowserViewGroupRemoteService implements IBrowserViewGroupRemoteService { + declare readonly _serviceBrand: undefined; + + private readonly _groupService: IBrowserViewGroupService; + private readonly _groups = new Map(); + + constructor( + @IMainProcessService mainProcessService: IMainProcessService, + ) { + const channel = mainProcessService.getChannel(ipcBrowserViewGroupChannelName); + this._groupService = ProxyChannel.toService(channel); + } + + async createGroup(): Promise { + const id = await this._groupService.createGroup(); + return this._wrap(id); + } + + private _wrap(id: string): IBrowserViewGroup { + const group = new RemoteBrowserViewGroup(id, this._groupService); + this._groups.set(id, group); + + Event.once(group.onDidDestroy)(() => { + this._groups.delete(id); + }); + + return group; + } +} diff --git a/src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts b/src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts index 9142b497166dd..30ad512c042d0 100644 --- a/src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts +++ b/src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts @@ -22,44 +22,60 @@ export interface IBrowserViewCDPProxyServer { readonly _serviceBrand: undefined; /** - * Returns a debug endpoint with a short-lived, single-use token. + * Returns a debug endpoint with a short-lived, single-use token for a specific browser target. */ - getWebSocketEndpoint(): Promise; + getWebSocketEndpointForTarget(target: ICDPBrowserTarget): Promise; + + /** + * Unregister a previously registered browser target. + */ + removeTarget(target: ICDPBrowserTarget): Promise; } /** * WebSocket server that provides CDP debugging for browser views. + * + * Manages a registry of {@link ICDPBrowserTarget} instances, each reachable + * at its own `/devtools/browser/{id}` WebSocket endpoint. */ export class BrowserViewCDPProxyServer extends Disposable implements IBrowserViewCDPProxyServer { declare readonly _serviceBrand: undefined; private server: http.Server | undefined; private port: number | undefined; - private readonly tokens: TokenManager; + + private readonly tokens = this._register(new TokenManager()); + private readonly targets = new Map(); constructor( - private readonly browserTarget: ICDPBrowserTarget, @ILogService private readonly logService: ILogService ) { super(); - - this.tokens = this._register(new TokenManager()); } /** - * Returns a debug endpoint with a short-lived, single-use token in the - * WebSocket URL. The token is revoked once a WebSocket connection is made - * or after 30 seconds, whichever comes first. + * Register a browser target and return a WebSocket endpoint URL for it. + * The target is reachable at `/devtools/browser/{targetId}`. */ - async getWebSocketEndpoint(): Promise { + async getWebSocketEndpointForTarget(target: ICDPBrowserTarget): Promise { await this.ensureServerStarted(); - const token = await this.tokens.issueToken(); - return this.getWebSocketUrl(token); + const targetInfo = await target.getTargetInfo(); + const targetId = targetInfo.targetId; + + // Register (or re-register) the target + this.targets.set(targetId, target); + + const token = await this.tokens.issueToken(targetId); + return `ws://localhost:${this.port}/devtools/browser/${targetId}?token=${token}`; } - private getWebSocketUrl(token: string): string { - return `ws://localhost:${this.port}/devtools/browser?token=${token}`; + /** + * Unregister a previously registered browser target. + */ + async removeTarget(target: ICDPBrowserTarget): Promise { + const targetInfo = await target.getTargetInfo(); + this.targets.delete(targetInfo.targetId); } private async ensureServerStarted(): Promise { @@ -93,19 +109,30 @@ export class BrowserViewCDPProxyServer extends Disposable implements IBrowserVie private handleWebSocketUpgrade(req: http.IncomingMessage, socket: Socket): void { const [pathname, params] = (req.url || '').split('?'); - const token = new URLSearchParams(params).get('token'); - if (!token || !this.tokens.consumeToken(token)) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + const browserMatch = pathname.match(/^\/devtools\/browser\/([^/?]+)$/); + + this.logService.debug(`[BrowserViewDebugProxy] WebSocket upgrade requested: ${pathname}`); + + if (!browserMatch) { + this.logService.warn(`[BrowserViewDebugProxy] Rejecting WebSocket on unknown path: ${pathname}`); + socket.write('HTTP/1.1 404 Not Found\r\n\r\n'); socket.end(); return; } - const browserMatch = pathname.match(/^\/devtools\/browser(\/.*)?$/); + const targetId = browserMatch[1]; - this.logService.debug(`[BrowserViewDebugProxy] WebSocket upgrade requested: ${pathname}`); + const token = new URLSearchParams(params).get('token'); + const tokenTargetId = token && this.tokens.consumeToken(token); + if (!tokenTargetId || tokenTargetId !== targetId) { + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.end(); + return; + } - if (!browserMatch) { - this.logService.warn(`[BrowserViewDebugProxy] Rejecting WebSocket on unknown path: ${pathname}`); + const target = this.targets.get(targetId); + if (!target) { + this.logService.warn(`[BrowserViewDebugProxy] Browser target not found: ${targetId}`); socket.write('HTTP/1.1 404 Not Found\r\n\r\n'); socket.end(); return; @@ -122,7 +149,7 @@ export class BrowserViewCDPProxyServer extends Disposable implements IBrowserVie return; } - const proxy = new CDPBrowserProxy(this.browserTarget); + const proxy = new CDPBrowserProxy(target); const disposables = this.wireWebSocket(upgraded, proxy); this._register(disposables); this._register(upgraded); @@ -200,31 +227,35 @@ export class BrowserViewCDPProxyServer extends Disposable implements IBrowserVie } } -class TokenManager extends Disposable { - /** Map of currently valid single-use tokens. Each expires after 30 seconds. */ - private readonly tokens = new Map(); +class TokenManager extends Disposable { + /** Map of currently valid single-use tokens to their associated details. */ + private readonly tokens = new Map(); /** - * Creates a short-lived, single-use token. + * Creates a short-lived, single-use token bound to a specific target. * The token is revoked once consumed or after 30 seconds. */ - async issueToken(): Promise { + async issueToken(details: TDetails): Promise { const token = this.makeToken(); - this.tokens.set(token, { expiresAt: Date.now() + 30_000 }); + this.tokens.set(token, { details: Object.freeze(details), expiresAt: Date.now() + 30_000 }); this._register(disposableTimeout(() => this.tokens.delete(token), 30_000)); return token; } - consumeToken(token: string): boolean { + /** + * Consume a token. Returns the details it was issued with, or + * `undefined` if the token is invalid or expired. + */ + consumeToken(token: string): TDetails | undefined { if (!token) { - return false; + return undefined; } const info = this.tokens.get(token); if (!info) { - return false; + return undefined; } this.tokens.delete(token); - return Date.now() <= info.expiresAt; + return Date.now() <= info.expiresAt ? info.details : undefined; } private makeToken(): string { diff --git a/src/vs/platform/browserView/electron-main/browserViewGroup.ts b/src/vs/platform/browserView/electron-main/browserViewGroup.ts new file mode 100644 index 0000000000000..3b2002cee3fae --- /dev/null +++ b/src/vs/platform/browserView/electron-main/browserViewGroup.ts @@ -0,0 +1,202 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable, DisposableStore } from '../../../base/common/lifecycle.js'; +import { Emitter, Event } from '../../../base/common/event.js'; +import { BrowserView } from './browserView.js'; +import { ICDPTarget, CDPBrowserVersion, CDPWindowBounds, CDPTargetInfo, ICDPConnection, ICDPBrowserTarget } from '../common/cdp/types.js'; +import { CDPBrowserProxy } from '../common/cdp/proxy.js'; +import { IBrowserViewGroup, IBrowserViewGroupViewEvent } from '../common/browserViewGroup.js'; +import { IBrowserViewCDPProxyServer } from './browserViewCDPProxyServer.js'; +import { IBrowserViewMainService } from './browserViewMainService.js'; + +/** + * An isolated group of {@link BrowserView} instances exposed as CDP targets. + * + * Each group represents an independent CDP "browser" endpoint + * (`/devtools/browser/{id}`). Different groups can expose different + * subsets of browser views, enabling selective target visibility across + * CDP sessions. + * + * Created via {@link BrowserViewGroupMainService.createGroup}. + */ +export class BrowserViewGroup extends Disposable implements ICDPBrowserTarget, IBrowserViewGroup { + + private readonly views = new Map(); + private readonly viewListeners = this._register(new DisposableStore()); + + /** All context IDs known to this group, including those from views added to it. */ + private readonly knownContextIds = new Set(); + /** Browser context IDs created by this group via {@link createBrowserContext}. */ + private readonly ownedContextIds = new Set(); + + private readonly _onTargetCreated = this._register(new Emitter()); + readonly onTargetCreated: Event = this._onTargetCreated.event; + + private readonly _onTargetDestroyed = this._register(new Emitter()); + readonly onTargetDestroyed: Event = this._onTargetDestroyed.event; + + private readonly _onDidAddView = this._register(new Emitter()); + readonly onDidAddView: Event = this._onDidAddView.event; + + private readonly _onDidRemoveView = this._register(new Emitter()); + readonly onDidRemoveView: Event = this._onDidRemoveView.event; + + private readonly _onDidDestroy = this._register(new Emitter()); + readonly onDidDestroy: Event = this._onDidDestroy.event; + + constructor( + readonly id: string, + @IBrowserViewMainService private readonly browserViewMainService: IBrowserViewMainService, + @IBrowserViewCDPProxyServer private readonly cdpProxyServer: IBrowserViewCDPProxyServer, + ) { + super(); + } + + // #region View management + + /** + * Add a {@link BrowserView} to this group. + * Fires {@link onDidAddView} and {@link onTargetCreated}. + * Automatically removes the view when it closes. + */ + async addView(viewId: string): Promise { + if (this.views.has(viewId)) { + return; + } + const view = this.browserViewMainService.tryGetBrowserView(viewId); + if (!view) { + throw new Error(`Browser view ${viewId} not found`); + } + this.views.set(view.id, view); + this.knownContextIds.add(view.session.id); + this._onDidAddView.fire({ viewId: view.id }); + this._onTargetCreated.fire(view); + + this.viewListeners.add(Event.once(view.onDidClose)(() => { + this.removeView(viewId); + })); + } + + /** + * Remove a {@link BrowserView} from this group. + * Fires {@link onDidRemoveView} and {@link onTargetDestroyed} if the view was tracked. + */ + async removeView(viewId: string): Promise { + const view = this.views.get(viewId); + if (view && this.views.delete(viewId)) { + this._onDidRemoveView.fire({ viewId: view.id }); + this._onTargetDestroyed.fire(view); + } + } + + // #endregion + + // #region ICDPBrowserTarget implementation + + getVersion(): CDPBrowserVersion { + return this.browserViewMainService.getVersion(); + } + + getWindowForTarget(target: ICDPTarget): { windowId: number; bounds: CDPWindowBounds } { + return this.browserViewMainService.getWindowForTarget(target); + } + + async attach(): Promise { + return new CDPBrowserProxy(this); + } + + async getTargetInfo(): Promise { + return { + targetId: this.id, + type: 'browser', + title: this.getVersion().product, + url: '', + attached: true, + canAccessOpener: false + }; + } + + getTargets(): IterableIterator { + return this.views.values(); + } + + async createTarget(url: string, browserContextId?: string): Promise { + if (browserContextId && !this.knownContextIds.has(browserContextId)) { + throw new Error(`Unknown browser context ${browserContextId}`); + } + + const target = await this.browserViewMainService.createTarget(url, browserContextId); + if (target instanceof BrowserView) { + this.addView(target.id); + } + return target; + } + + async activateTarget(target: ICDPTarget): Promise { + return this.browserViewMainService.activateTarget(target); + } + + async closeTarget(target: ICDPTarget): Promise { + if (target instanceof BrowserView) { + await this.removeView(target.id); + } + return this.browserViewMainService.closeTarget(target); + } + + // Browser context management + + /** + * Returns only the browser context IDs that are visible to this group, + * i.e. contexts used by views currently in the group. + */ + getBrowserContexts(): string[] { + return [...this.knownContextIds]; + } + + async createBrowserContext(): Promise { + const contextId = await this.browserViewMainService.createBrowserContext(); + this.knownContextIds.add(contextId); + this.ownedContextIds.add(contextId); + return contextId; + } + + async disposeBrowserContext(browserContextId: string): Promise { + if (!this.ownedContextIds.has(browserContextId)) { + throw new Error('Can only dispose browser contexts created by this group'); + } + + // Close views in this group that belong to the context before disposing + for (const view of this.views.values()) { + if (view.session.id === browserContextId) { + await this.removeView(view.id); + } + } + + this.knownContextIds.delete(browserContextId); + this.ownedContextIds.delete(browserContextId); + return this.browserViewMainService.disposeBrowserContext(browserContextId); + } + + // #endregion + + // #region CDP endpoint + + /** + * Get a WebSocket endpoint URL for connecting to this group's CDP + * session. The URL contains a short-lived, single-use token. + */ + async getDebugWebSocketEndpoint(): Promise { + return this.cdpProxyServer.getWebSocketEndpointForTarget(this); + } + + // #endregion + + override dispose(): void { + this._onDidDestroy.fire(); + this.cdpProxyServer.removeTarget(this); + super.dispose(); + } +} diff --git a/src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts b/src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts new file mode 100644 index 0000000000000..20dd6331c0ea5 --- /dev/null +++ b/src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts @@ -0,0 +1,88 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable, DisposableMap } from '../../../base/common/lifecycle.js'; +import { Event } from '../../../base/common/event.js'; +import { createDecorator, IInstantiationService } from '../../instantiation/common/instantiation.js'; +import { generateUuid } from '../../../base/common/uuid.js'; +import { IBrowserViewGroupService, IBrowserViewGroupViewEvent } from '../common/browserViewGroup.js'; +import { BrowserViewGroup } from './browserViewGroup.js'; + +export const IBrowserViewGroupMainService = createDecorator('browserViewGroupMainService'); + +export interface IBrowserViewGroupMainService extends IBrowserViewGroupService { + readonly _serviceBrand: undefined; +} + +/** + * Main-process service that manages {@link BrowserViewGroup} instances. + * + * Implements {@link IBrowserViewGroupService} so it can be surfaced to + * the workbench/shared process via {@link ProxyChannel}. + */ +export class BrowserViewGroupMainService extends Disposable implements IBrowserViewGroupMainService { + declare readonly _serviceBrand: undefined; + + private readonly groups = this._register(new DisposableMap()); + + constructor( + @IInstantiationService private readonly instantiationService: IInstantiationService + ) { + super(); + } + + async createGroup(): Promise { + const id = generateUuid(); + const group = this.instantiationService.createInstance(BrowserViewGroup, id); + this.groups.set(id, group); + + // Auto-cleanup when the group disposes itself + Event.once(group.onDidDestroy)(() => { + this.groups.deleteAndLeak(id); + }); + + return id; + } + + async destroyGroup(groupId: string): Promise { + this.groups.deleteAndDispose(groupId); + } + + async addViewToGroup(groupId: string, viewId: string): Promise { + return this._getGroup(groupId).addView(viewId); + } + + async removeViewFromGroup(groupId: string, viewId: string): Promise { + return this._getGroup(groupId).removeView(viewId); + } + + async getDebugWebSocketEndpoint(groupId: string): Promise { + return this._getGroup(groupId).getDebugWebSocketEndpoint(); + } + + onDynamicDidAddView(groupId: string): Event { + return this._getGroup(groupId).onDidAddView; + } + + onDynamicDidRemoveView(groupId: string): Event { + return this._getGroup(groupId).onDidRemoveView; + } + + onDynamicDidDestroy(groupId: string): Event { + return this._getGroup(groupId).onDidDestroy; + } + + /** + * Get a group or throw if not found. + */ + private _getGroup(groupId: string): BrowserViewGroup { + const group = this.groups.get(groupId); + if (!group) { + throw new Error(`Browser view group ${groupId} not found`); + } + return group; + } +} + diff --git a/src/vs/platform/browserView/electron-main/browserViewMainService.ts b/src/vs/platform/browserView/electron-main/browserViewMainService.ts index f29b4c18a438c..68e97b58b4f47 100644 --- a/src/vs/platform/browserView/electron-main/browserViewMainService.ts +++ b/src/vs/platform/browserView/electron-main/browserViewMainService.ts @@ -15,7 +15,6 @@ import { generateUuid } from '../../../base/common/uuid.js'; import { BrowserViewUri } from '../common/browserViewUri.js'; import { IWindowsMainService } from '../../windows/electron-main/windows.js'; import { BrowserSession } from './browserSession.js'; -import { IBrowserViewCDPProxyServer } from './browserViewCDPProxyServer.js'; import { IProductService } from '../../product/common/productService.js'; import { CDPBrowserProxy } from '../common/cdp/proxy.js'; @@ -51,8 +50,7 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa @IEnvironmentMainService private readonly environmentMainService: IEnvironmentMainService, @IInstantiationService private readonly instantiationService: IInstantiationService, @IWindowsMainService private readonly windowsMainService: IWindowsMainService, - @IProductService private readonly productService: IProductService, - @IBrowserViewCDPProxyServer private readonly cdpProxyServer: IBrowserViewCDPProxyServer, + @IProductService private readonly productService: IProductService ) { super(); } @@ -363,8 +361,4 @@ export class BrowserViewMainService extends Disposable implements IBrowserViewMa ); await browserSession.electronSession.clearData(); } - - async getDebugWebSocketEndpoint(): Promise { - return this.cdpProxyServer.getWebSocketEndpoint(); - } } diff --git a/src/vs/platform/browserView/node/playwrightService.ts b/src/vs/platform/browserView/node/playwrightService.ts index 2dcd8fbd04e9c..a33fd3c203641 100644 --- a/src/vs/platform/browserView/node/playwrightService.ts +++ b/src/vs/platform/browserView/node/playwrightService.ts @@ -4,14 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import { Disposable } from '../../../base/common/lifecycle.js'; -import { ProxyChannel } from '../../../base/parts/ipc/common/ipc.js'; +import { DeferredPromise } from '../../../base/common/async.js'; import { ILogService } from '../../log/common/log.js'; -import { IBrowserViewService, ipcBrowserViewChannelName } from '../common/browserView.js'; import { IPlaywrightService } from '../common/playwrightService.js'; -import { IMainProcessService } from '../../ipc/common/mainProcessService.js'; +import { IBrowserViewGroupRemoteService } from '../common/browserViewGroupRemoteService.js'; +import { IBrowserViewGroup } from '../common/browserViewGroup.js'; // eslint-disable-next-line local/code-import-patterns -import type { Browser } from 'playwright-core'; +import type { Browser, BrowserContext, Page } from 'playwright-core'; /** * Shared-process implementation of {@link IPlaywrightService}. @@ -19,22 +19,22 @@ import type { Browser } from 'playwright-core'; export class PlaywrightService extends Disposable implements IPlaywrightService { declare readonly _serviceBrand: undefined; - private readonly browserViewService: IBrowserViewService; private _browser: Browser | undefined; + private _pages: PlaywrightPageMap | undefined; private _initPromise: Promise | undefined; constructor( - @IMainProcessService mainProcessService: IMainProcessService, + @IBrowserViewGroupRemoteService private readonly groupRemoteService: IBrowserViewGroupRemoteService, @ILogService private readonly logService: ILogService, ) { super(); - - const channel = mainProcessService.getChannel(ipcBrowserViewChannelName); - this.browserViewService = ProxyChannel.toService(channel); } + /** + * Ensure the Playwright browser connection and page map are initialized. + */ async initialize(): Promise { - if (this._browser?.isConnected()) { + if (this._pages) { return; } @@ -44,19 +44,21 @@ export class PlaywrightService extends Disposable implements IPlaywrightService this._initPromise = (async () => { try { - this.logService.debug('[PlaywrightService] Connecting to browser via CDP'); + this.logService.debug('[PlaywrightService] Creating browser view group'); + const group = this._register(await this.groupRemoteService.createGroup()); + this.logService.debug('[PlaywrightService] Connecting to browser via CDP'); const playwright = await import('playwright-core'); - const endpoint = await this.browserViewService.getDebugWebSocketEndpoint(); + const endpoint = await group.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 = undefined; + this._pages = undefined; + this._initPromise = undefined; }); // This can happen if the service was disposed while we were waiting for the connection. In that case, clean up immediately. @@ -66,8 +68,11 @@ export class PlaywrightService extends Disposable implements IPlaywrightService } this._browser = browser; - } finally { + const pageMap = this._register(new PlaywrightPageMap(group, browser, this.logService)); + this._pages = pageMap; + } catch (e) { this._initPromise = undefined; + throw e; } })(); @@ -83,3 +88,183 @@ export class PlaywrightService extends Disposable implements IPlaywrightService super.dispose(); } } + +/** + * Correlates browser view IDs with Playwright {@link Page} instances. + * + * When a browser view is added to a group, two asynchronous events follow + * through independent channels: + * + * 1. The group fires {@link IBrowserViewGroup.onDidAddView} (via IPC). + * 2. Playwright receives a CDP `Target.targetCreated` event (via WebSocket) + * and fires a `page` event on the matching {@link BrowserContext}. + * + * This class pairs the two event streams by FIFO ordering: the first view-ID + * received is matched with the first page event received. + * + * A periodic scan handles the case where Playwright creates a new + * {@link BrowserContext} for a target whose session was previously unknown. + */ +class PlaywrightPageMap extends Disposable { + + private readonly _viewIdToPage = new Map(); + + /** View IDs received from the group but not yet matched with a page. */ + private readonly _viewIdQueue: string[] = []; + + /** Pages received from Playwright but not yet matched with a view ID. */ + private readonly _pageQueue: Page[] = []; + + /** Callers waiting for a specific view ID to be resolved to a page. */ + private readonly _waiters = new Map>(); + + private readonly _watchedContexts = new WeakSet(); + private _scanTimer: ReturnType | undefined; + + constructor( + private readonly _group: IBrowserViewGroup, + private readonly _browser: Browser, + private readonly logService: ILogService, + ) { + super(); + + this._register(_group.onDidAddView(e => this.onViewAdded(e.viewId))); + this._register(_group.onDidRemoveView(e => this.onViewRemoved(e.viewId))); + this.scanForNewContexts(); + } + + /** + * Get the Playwright {@link Page} for a browser view. + * If the view is not yet in the group, it is added automatically. + */ + async getPage(viewId: string, timeoutMs = 10000): Promise { + const existing = this._viewIdToPage.get(viewId); + if (existing) { + return existing; + } + + const existingWaiter = this._waiters.get(viewId); + if (existingWaiter) { + return existingWaiter.p; + } + + const deferred = new DeferredPromise(); + const timeout = setTimeout(() => deferred.error(new Error(`Timed out after waiting for page`)), timeoutMs); + + deferred.p.finally(() => { + clearTimeout(timeout); + this._waiters.delete(viewId); + if (this._waiters.size === 0) { + this.stopScanning(); + } + }); + + this._waiters.set(viewId, deferred); + this.ensureScanning(); + + // Adding the view fires onDidAddView (pushes to viewIdQueue) and + // eventually a Playwright page event (pushes to pageQueue). + await this._group.addView(viewId); + + return deferred.p; + } + + // --- Event handlers --- + + private onViewAdded(viewId: string): void { + this._viewIdQueue.push(viewId); + this.tryMatch(); + } + + private onViewRemoved(viewId: string): void { + this._viewIdToPage.delete(viewId); + } + + private onPage(page: Page): void { + if (this.isKnownPage(page)) { + return; + } + this._pageQueue.push(page); + this.tryMatch(); + } + + // --- Matching --- + + /** + * Pair up queued view IDs with queued pages in FIFO order and resolve + * any callers waiting for the matched view IDs. + */ + private tryMatch(): void { + while (this._viewIdQueue.length > 0 && this._pageQueue.length > 0) { + const viewId = this._viewIdQueue.shift()!; + const page = this._pageQueue.shift()!; + + this._viewIdToPage.set(viewId, page); + page.once('close', () => this._viewIdToPage.delete(viewId)); + + this.logService.debug(`[PlaywrightPageMap] Matched view ${viewId} → page`); + + const waiter = this._waiters.get(viewId); + if (waiter) { + waiter.complete(page); + this._waiters.delete(viewId); + } + } + } + + // --- Context scanning --- + + /** + * Watch all current {@link BrowserContext BrowserContexts} for new pages. + * Also processes any existing pages in newly discovered contexts. + */ + private scanForNewContexts(): void { + for (const context of this._browser.contexts()) { + if (this._watchedContexts.has(context)) { + continue; + } + this._watchedContexts.add(context); + + context.on('page', (page: Page) => this.onPage(page)); + context.on('close', () => this._watchedContexts.delete(context)); + + for (const page of context.pages()) { + this.onPage(page); + } + } + } + + private ensureScanning(): void { + if (this._scanTimer === undefined) { + this._scanTimer = setInterval(() => this.scanForNewContexts(), 100); + } + } + + private stopScanning(): void { + if (this._scanTimer !== undefined) { + clearInterval(this._scanTimer); + this._scanTimer = undefined; + } + } + + private isKnownPage(page: Page): boolean { + if (this._pageQueue.includes(page)) { + return true; + } + for (const p of this._viewIdToPage.values()) { + if (p === page) { + return true; + } + } + return false; + } + + override dispose(): void { + this.stopScanning(); + for (const waiter of this._waiters.values()) { + waiter.error(new Error('PlaywrightPageMap disposed')); + } + this._waiters.clear(); + super.dispose(); + } +} diff --git a/src/vs/workbench/contrib/browserView/common/browserView.ts b/src/vs/workbench/contrib/browserView/common/browserView.ts index d717ada29b959..732fa1974e481 100644 --- a/src/vs/workbench/contrib/browserView/common/browserView.ts +++ b/src/vs/workbench/contrib/browserView/common/browserView.ts @@ -68,11 +68,6 @@ export interface IBrowserViewWorkbenchService { * Clear all storage data for the current workspace browser session */ clearWorkspaceStorage(): Promise; - - /** - * Get the endpoint for connecting to a browser view's CDP proxy server - */ - getDebugWebSocketEndpoint(): Promise; } diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts index 30199e27cbf40..68d2376c587d1 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts @@ -54,8 +54,4 @@ export class BrowserViewWorkbenchService implements IBrowserViewWorkbenchService const workspaceId = this.workspaceContextService.getWorkspace().id; return this._browserViewService.clearWorkspaceStorage(workspaceId); } - - async getDebugWebSocketEndpoint() { - return this._browserViewService.getDebugWebSocketEndpoint(); - } } From 2dcf7191e3d0a7db3aa931aabc0516c7520a0f6d Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Mon, 16 Feb 2026 17:19:59 -0800 Subject: [PATCH 2/6] layering --- src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts | 2 +- .../{common => node}/browserViewGroupRemoteService.ts | 2 +- src/vs/platform/browserView/node/playwrightService.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/vs/platform/browserView/{common => node}/browserViewGroupRemoteService.ts (97%) diff --git a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts index 2fb79b662da93..e112b958d730e 100644 --- a/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts @@ -136,7 +136,7 @@ import { IMeteredConnectionService } from '../../../platform/meteredConnection/c import { MeteredConnectionChannelClient, METERED_CONNECTION_CHANNEL } from '../../../platform/meteredConnection/common/meteredConnectionIpc.js'; import { IPlaywrightService } from '../../../platform/browserView/common/playwrightService.js'; import { PlaywrightService } from '../../../platform/browserView/node/playwrightService.js'; -import { IBrowserViewGroupRemoteService, BrowserViewGroupRemoteService } from '../../../platform/browserView/common/browserViewGroupRemoteService.js'; +import { IBrowserViewGroupRemoteService, BrowserViewGroupRemoteService } from '../../../platform/browserView/node/browserViewGroupRemoteService.js'; class SharedProcessMain extends Disposable implements IClientConnectionFilter { diff --git a/src/vs/platform/browserView/common/browserViewGroupRemoteService.ts b/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts similarity index 97% rename from src/vs/platform/browserView/common/browserViewGroupRemoteService.ts rename to src/vs/platform/browserView/node/browserViewGroupRemoteService.ts index 87cffd8858a1d..78f548be1fb62 100644 --- a/src/vs/platform/browserView/common/browserViewGroupRemoteService.ts +++ b/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts @@ -8,7 +8,7 @@ import { Disposable } from '../../../base/common/lifecycle.js'; import { ProxyChannel } from '../../../base/parts/ipc/common/ipc.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { IMainProcessService } from '../../ipc/common/mainProcessService.js'; -import { IBrowserViewGroup, IBrowserViewGroupService, IBrowserViewGroupViewEvent, ipcBrowserViewGroupChannelName } from './browserViewGroup.js'; +import { IBrowserViewGroup, IBrowserViewGroupService, IBrowserViewGroupViewEvent, ipcBrowserViewGroupChannelName } from '../common/browserViewGroup.js'; export const IBrowserViewGroupRemoteService = createDecorator('browserViewGroupRemoteService'); diff --git a/src/vs/platform/browserView/node/playwrightService.ts b/src/vs/platform/browserView/node/playwrightService.ts index a33fd3c203641..e928cfe1644cb 100644 --- a/src/vs/platform/browserView/node/playwrightService.ts +++ b/src/vs/platform/browserView/node/playwrightService.ts @@ -7,7 +7,7 @@ import { Disposable } from '../../../base/common/lifecycle.js'; import { DeferredPromise } from '../../../base/common/async.js'; import { ILogService } from '../../log/common/log.js'; import { IPlaywrightService } from '../common/playwrightService.js'; -import { IBrowserViewGroupRemoteService } from '../common/browserViewGroupRemoteService.js'; +import { IBrowserViewGroupRemoteService } from '../node/browserViewGroupRemoteService.js'; import { IBrowserViewGroup } from '../common/browserViewGroup.js'; // eslint-disable-next-line local/code-import-patterns From f5c60f9730637dc5ae4416bd359d15fe38f29fd9 Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Tue, 17 Feb 2026 07:45:40 -0800 Subject: [PATCH 3/6] feedback --- .../electron-main/browserViewGroup.ts | 2 +- .../browserView/node/playwrightService.ts | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/vs/platform/browserView/electron-main/browserViewGroup.ts b/src/vs/platform/browserView/electron-main/browserViewGroup.ts index 3b2002cee3fae..efd4d01abf3fb 100644 --- a/src/vs/platform/browserView/electron-main/browserViewGroup.ts +++ b/src/vs/platform/browserView/electron-main/browserViewGroup.ts @@ -130,7 +130,7 @@ export class BrowserViewGroup extends Disposable implements ICDPBrowserTarget, I const target = await this.browserViewMainService.createTarget(url, browserContextId); if (target instanceof BrowserView) { - this.addView(target.id); + await this.addView(target.id); } return target; } diff --git a/src/vs/platform/browserView/node/playwrightService.ts b/src/vs/platform/browserView/node/playwrightService.ts index e928cfe1644cb..11e667df46150 100644 --- a/src/vs/platform/browserView/node/playwrightService.ts +++ b/src/vs/platform/browserView/node/playwrightService.ts @@ -54,21 +54,27 @@ export class PlaywrightService extends Disposable implements IPlaywrightService this.logService.debug('[PlaywrightService] Connected to browser'); - browser.on('disconnected', () => { - this.logService.debug('[PlaywrightService] Browser disconnected'); - this._browser = undefined; - this._pages = undefined; - this._initPromise = undefined; - }); - // This can happen if the service was disposed while we were waiting for the connection. In that case, clean up immediately. if (this._initPromise === undefined) { browser.close().catch(() => { /* ignore */ }); throw new Error('PlaywrightService was disposed during initialization'); } - this._browser = browser; const pageMap = this._register(new PlaywrightPageMap(group, browser, this.logService)); + + browser.on('disconnected', () => { + this.logService.debug('[PlaywrightService] Browser disconnected'); + if (this._browser === browser) { + group.dispose(); + pageMap.dispose(); + + this._browser = undefined; + this._pages = undefined; + this._initPromise = undefined; + } + }); + + this._browser = browser; this._pages = pageMap; } catch (e) { this._initPromise = undefined; From d0941f994ccf0b21edbcb0d86a9e7d0ca230955e Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Tue, 17 Feb 2026 08:19:25 -0800 Subject: [PATCH 4/6] Fail fast --- src/vs/platform/browserView/node/playwrightService.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/browserView/node/playwrightService.ts b/src/vs/platform/browserView/node/playwrightService.ts index 11e667df46150..49c995fa74bc9 100644 --- a/src/vs/platform/browserView/node/playwrightService.ts +++ b/src/vs/platform/browserView/node/playwrightService.ts @@ -170,7 +170,13 @@ class PlaywrightPageMap extends Disposable { // Adding the view fires onDidAddView (pushes to viewIdQueue) and // eventually a Playwright page event (pushes to pageQueue). - await this._group.addView(viewId); + try { + await this._group.addView(viewId); + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + this.logService.error('[PlaywrightService] Failed to add view:', errorMessage); + deferred.error(new Error(`Failed to get page: ${errorMessage}`)); + } return deferred.p; } From 61f9af4f72ba1db5239e457e17ca5f10d8c25e96 Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Tue, 17 Feb 2026 11:59:17 -0800 Subject: [PATCH 5/6] two-way lookup --- .../browserView/node/playwrightService.ts | 136 ++++++++++-------- 1 file changed, 79 insertions(+), 57 deletions(-) diff --git a/src/vs/platform/browserView/node/playwrightService.ts b/src/vs/platform/browserView/node/playwrightService.ts index 49c995fa74bc9..7590dafe93381 100644 --- a/src/vs/platform/browserView/node/playwrightService.ts +++ b/src/vs/platform/browserView/node/playwrightService.ts @@ -114,15 +114,19 @@ export class PlaywrightService extends Disposable implements IPlaywrightService class PlaywrightPageMap extends Disposable { private readonly _viewIdToPage = new Map(); + private readonly _pageToViewId = new WeakMap(); /** View IDs received from the group but not yet matched with a page. */ - private readonly _viewIdQueue: string[] = []; + private _viewIdQueue: Array<{ + viewId: string; + page: DeferredPromise; + }> = []; /** Pages received from Playwright but not yet matched with a view ID. */ - private readonly _pageQueue: Page[] = []; - - /** Callers waiting for a specific view ID to be resolved to a page. */ - private readonly _waiters = new Map>(); + private _pageQueue: Array<{ + page: Page; + viewId: DeferredPromise; + }> = []; private readonly _watchedContexts = new WeakSet(); private _scanTimer: ReturnType | undefined; @@ -134,38 +138,49 @@ class PlaywrightPageMap extends Disposable { ) { super(); - this._register(_group.onDidAddView(e => this.onViewAdded(e.viewId))); - this._register(_group.onDidRemoveView(e => this.onViewRemoved(e.viewId))); + this._register(_group.onDidAddView(e => this.getPage(e.viewId))); + this._register(_group.onDidRemoveView(e => this.removePage(e.viewId))); this.scanForNewContexts(); } + /** + * Create a new page in the browser and return its associated page and view ID. + */ + async newPage(): Promise<{ viewId: string; page: Page }> { + const page = await this._browser.newPage(); + const viewId = await this.getViewId(page); + + return { viewId, page }; + } + /** * Get the Playwright {@link Page} for a browser view. * If the view is not yet in the group, it is added automatically. */ async getPage(viewId: string, timeoutMs = 10000): Promise { - const existing = this._viewIdToPage.get(viewId); - if (existing) { - return existing; + const resolved = this._viewIdToPage.get(viewId); + if (resolved) { + return resolved; } - - const existingWaiter = this._waiters.get(viewId); - if (existingWaiter) { - return existingWaiter.p; + const queued = this._viewIdQueue.find(item => item.viewId === viewId); + if (queued) { + return queued.page.p; } const deferred = new DeferredPromise(); - const timeout = setTimeout(() => deferred.error(new Error(`Timed out after waiting for page`)), timeoutMs); + const timeout = setTimeout(() => deferred.error(new Error(`Timed out waiting for page`)), timeoutMs); deferred.p.finally(() => { clearTimeout(timeout); - this._waiters.delete(viewId); - if (this._waiters.size === 0) { + this._viewIdQueue = this._viewIdQueue.filter(item => item.viewId !== viewId); + if (this._viewIdQueue.length === 0) { this.stopScanning(); } }); - this._waiters.set(viewId, deferred); + this._viewIdQueue.push({ viewId, page: deferred }); + this.tryMatch(); + this.ensureScanning(); // Adding the view fires onDidAddView (pushes to viewIdQueue) and @@ -181,23 +196,36 @@ class PlaywrightPageMap extends Disposable { return deferred.p; } - // --- Event handlers --- - - private onViewAdded(viewId: string): void { - this._viewIdQueue.push(viewId); - this.tryMatch(); - } - - private onViewRemoved(viewId: string): void { + private removePage(viewId: string): void { + this._viewIdQueue = this._viewIdQueue.filter(item => item.viewId !== viewId); + const page = this._viewIdToPage.get(viewId); + if (page) { + this._pageToViewId.delete(page); + } this._viewIdToPage.delete(viewId); } - private onPage(page: Page): void { - if (this.isKnownPage(page)) { - return; + private getViewId(page: Page, timeoutMs = 10000): Promise { + const resolved = this._pageToViewId.get(page); + if (resolved) { + return Promise.resolve(resolved); + } + const queued = this._pageQueue.find(item => item.page === page); + if (queued) { + return queued.viewId.p; } - this._pageQueue.push(page); + + const deferred = new DeferredPromise(); + const timeout = setTimeout(() => deferred.error(new Error(`Timed out waiting for browser view`)), timeoutMs); + deferred.p.finally(() => { + clearTimeout(timeout); + this._pageQueue = this._pageQueue.filter(item => item.page !== page); + }); + + this._pageQueue.push({ page, viewId: deferred }); this.tryMatch(); + + return deferred.p; } // --- Matching --- @@ -208,19 +236,21 @@ class PlaywrightPageMap extends Disposable { */ private tryMatch(): void { while (this._viewIdQueue.length > 0 && this._pageQueue.length > 0) { - const viewId = this._viewIdQueue.shift()!; - const page = this._pageQueue.shift()!; + const viewIdItem = this._viewIdQueue.shift()!; + const pageItem = this._pageQueue.shift()!; - this._viewIdToPage.set(viewId, page); - page.once('close', () => this._viewIdToPage.delete(viewId)); + this._viewIdToPage.set(viewIdItem.viewId, pageItem.page); + this._pageToViewId.set(pageItem.page, viewIdItem.viewId); + pageItem.page.once('close', () => this._viewIdToPage.delete(viewIdItem.viewId)); - this.logService.debug(`[PlaywrightPageMap] Matched view ${viewId} → page`); + this.logService.debug(`[PlaywrightPageMap] Matched view ${viewIdItem.viewId} → page`); - const waiter = this._waiters.get(viewId); - if (waiter) { - waiter.complete(page); - this._waiters.delete(viewId); - } + viewIdItem.page.complete(pageItem.page); + pageItem.viewId.complete(viewIdItem.viewId); + } + + if (this._viewIdQueue.length === 0) { + this.stopScanning(); } } @@ -237,11 +267,11 @@ class PlaywrightPageMap extends Disposable { } this._watchedContexts.add(context); - context.on('page', (page: Page) => this.onPage(page)); + context.on('page', (page: Page) => this.getViewId(page)); context.on('close', () => this._watchedContexts.delete(context)); for (const page of context.pages()) { - this.onPage(page); + this.getViewId(page); } } } @@ -259,24 +289,16 @@ class PlaywrightPageMap extends Disposable { } } - private isKnownPage(page: Page): boolean { - if (this._pageQueue.includes(page)) { - return true; - } - for (const p of this._viewIdToPage.values()) { - if (p === page) { - return true; - } - } - return false; - } - override dispose(): void { this.stopScanning(); - for (const waiter of this._waiters.values()) { - waiter.error(new Error('PlaywrightPageMap disposed')); + for (const { page } of this._viewIdQueue) { + page.error(new Error('PlaywrightPageMap disposed')); + } + for (const { viewId } of this._pageQueue) { + viewId.error(new Error('PlaywrightPageMap disposed')); } - this._waiters.clear(); + this._viewIdQueue = []; + this._pageQueue = []; super.dispose(); } } From 7aa21a747e382047515bd12432d1a356b6e6bb12 Mon Sep 17 00:00:00 2001 From: Kyle Cutler Date: Tue, 17 Feb 2026 20:34:20 -0800 Subject: [PATCH 6/6] feedback, cleanup --- .../node/browserViewGroupRemoteService.ts | 2 +- .../browserView/node/playwrightService.ts | 141 +++++++++++++----- 2 files changed, 102 insertions(+), 41 deletions(-) diff --git a/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts b/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts index 78f548be1fb62..b4aaffb612d17 100644 --- a/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts +++ b/src/vs/platform/browserView/node/browserViewGroupRemoteService.ts @@ -19,7 +19,7 @@ export const IBrowserViewGroupRemoteService = createDecorator | undefined; constructor( - @IBrowserViewGroupRemoteService private readonly groupRemoteService: IBrowserViewGroupRemoteService, + @IBrowserViewGroupRemoteService private readonly browserViewGroupRemoteService: IBrowserViewGroupRemoteService, @ILogService private readonly logService: ILogService, ) { super(); @@ -45,7 +45,7 @@ export class PlaywrightService extends Disposable implements IPlaywrightService this._initPromise = (async () => { try { this.logService.debug('[PlaywrightService] Creating browser view group'); - const group = this._register(await this.groupRemoteService.createGroup()); + const group = this._register(await this.browserViewGroupRemoteService.createGroup()); this.logService.debug('[PlaywrightService] Connecting to browser via CDP'); const playwright = await import('playwright-core'); @@ -60,13 +60,13 @@ export class PlaywrightService extends Disposable implements IPlaywrightService throw new Error('PlaywrightService was disposed during initialization'); } - const pageMap = this._register(new PlaywrightPageMap(group, browser, this.logService)); + const pageManager = this._register(new PlaywrightPageManager(group, browser, this.logService)); browser.on('disconnected', () => { this.logService.debug('[PlaywrightService] Browser disconnected'); if (this._browser === browser) { group.dispose(); - pageMap.dispose(); + pageManager.dispose(); this._browser = undefined; this._pages = undefined; @@ -75,7 +75,7 @@ export class PlaywrightService extends Disposable implements IPlaywrightService }); this._browser = browser; - this._pages = pageMap; + this._pages = pageManager; } catch (e) { this._initPromise = undefined; throw e; @@ -111,7 +111,7 @@ export class PlaywrightService extends Disposable implements IPlaywrightService * A periodic scan handles the case where Playwright creates a new * {@link BrowserContext} for a target whose session was previously unknown. */ -class PlaywrightPageMap extends Disposable { +class PlaywrightPageManager extends Disposable { private readonly _viewIdToPage = new Map(); private readonly _pageToViewId = new WeakMap(); @@ -138,8 +138,8 @@ class PlaywrightPageMap extends Disposable { ) { super(); - this._register(_group.onDidAddView(e => this.getPage(e.viewId))); - this._register(_group.onDidRemoveView(e => this.removePage(e.viewId))); + this._register(_group.onDidAddView(e => this.onViewAdded(e.viewId))); + this._register(_group.onDidRemoveView(e => this.onViewRemoved(e.viewId))); this.scanForNewContexts(); } @@ -148,16 +148,52 @@ class PlaywrightPageMap extends Disposable { */ async newPage(): Promise<{ viewId: string; page: Page }> { const page = await this._browser.newPage(); - const viewId = await this.getViewId(page); + const viewId = await this.onPageAdded(page); return { viewId, page }; } /** - * Get the Playwright {@link Page} for a browser view. - * If the view is not yet in the group, it is added automatically. + * Explicitly add an existing browser view to the CDP group. */ - async getPage(viewId: string, timeoutMs = 10000): Promise { + async addPage(viewId: string): Promise { + if (this._viewIdToPage.has(viewId)) { + return; + } + if (this._viewIdQueue.some(item => item.viewId === viewId)) { + return; + } + + // ensure the viewId is queued so we can immediately fetch the promise via getPage(). + this.onViewAdded(viewId); + + try { + await this._group.addView(viewId); + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + this.logService.error('[PlaywrightPageMap] Failed to add view:', errorMessage); + this.onViewRemoved(viewId); + } + } + + /** + * Remove a browser view from the CDP group. + */ + async removePage(viewId: string): Promise { + this.onViewRemoved(viewId); + try { + await this._group.removeView(viewId); + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + this.logService.error('[PlaywrightPageMap] Failed to remove view:', errorMessage); + } + } + + /** + * Get the Playwright {@link Page} for a browser view that has already been added. + * Throws if the view has not been added. + */ + async getPage(viewId: string): Promise { const resolved = this._viewIdToPage.get(viewId); if (resolved) { return resolved; @@ -167,6 +203,23 @@ class PlaywrightPageMap extends Disposable { return queued.page.p; } + throw new Error(`Page "${viewId}" has not been added to the Playwright service`); + } + + /** + * Called when the group fires onDidAddView. Creates a deferred entry in + * the view ID queue and attempts to match it with a page. + */ + private onViewAdded(viewId: string, timeoutMs = 10000): Promise { + const resolved = this._viewIdToPage.get(viewId); + if (resolved) { + return Promise.resolve(resolved); + } + const queued = this._viewIdQueue.find(item => item.viewId === viewId); + if (queued) { + return queued.page.p; + } + const deferred = new DeferredPromise(); const timeout = setTimeout(() => deferred.error(new Error(`Timed out waiting for page`)), timeoutMs); @@ -180,23 +233,12 @@ class PlaywrightPageMap extends Disposable { this._viewIdQueue.push({ viewId, page: deferred }); this.tryMatch(); - this.ensureScanning(); - // Adding the view fires onDidAddView (pushes to viewIdQueue) and - // eventually a Playwright page event (pushes to pageQueue). - try { - await this._group.addView(viewId); - } catch (err: unknown) { - const errorMessage = err instanceof Error ? err.message : String(err); - this.logService.error('[PlaywrightService] Failed to add view:', errorMessage); - deferred.error(new Error(`Failed to get page: ${errorMessage}`)); - } - return deferred.p; } - private removePage(viewId: string): void { + private onViewRemoved(viewId: string): void { this._viewIdQueue = this._viewIdQueue.filter(item => item.viewId !== viewId); const page = this._viewIdToPage.get(viewId); if (page) { @@ -205,7 +247,7 @@ class PlaywrightPageMap extends Disposable { this._viewIdToPage.delete(viewId); } - private getViewId(page: Page, timeoutMs = 10000): Promise { + private onPageAdded(page: Page, timeoutMs = 10000): Promise { const resolved = this._pageToViewId.get(page); if (resolved) { return Promise.resolve(resolved); @@ -215,6 +257,9 @@ class PlaywrightPageMap extends Disposable { return queued.viewId.p; } + this.onContextAdded(page.context()); + page.once('close', () => this.onPageRemoved(page)); + const deferred = new DeferredPromise(); const timeout = setTimeout(() => deferred.error(new Error(`Timed out waiting for browser view`)), timeoutMs); deferred.p.finally(() => { @@ -228,6 +273,33 @@ class PlaywrightPageMap extends Disposable { return deferred.p; } + private onPageRemoved(page: Page): void { + this._pageQueue = this._pageQueue.filter(item => item.page !== page); + const viewId = this._pageToViewId.get(page); + if (viewId) { + this._viewIdToPage.delete(viewId); + } + this._pageToViewId.delete(page); + } + + private onContextAdded(context: BrowserContext): void { + if (this._watchedContexts.has(context)) { + return; + } + this._watchedContexts.add(context); + + context.on('page', (page: Page) => this.onPageAdded(page)); + context.on('close', () => this.onContextRemoved(context)); + + for (const page of context.pages()) { + this.onPageAdded(page); + } + } + + private onContextRemoved(context: BrowserContext): void { + this._watchedContexts.delete(context); + } + // --- Matching --- /** @@ -241,12 +313,11 @@ class PlaywrightPageMap extends Disposable { this._viewIdToPage.set(viewIdItem.viewId, pageItem.page); this._pageToViewId.set(pageItem.page, viewIdItem.viewId); - pageItem.page.once('close', () => this._viewIdToPage.delete(viewIdItem.viewId)); - - this.logService.debug(`[PlaywrightPageMap] Matched view ${viewIdItem.viewId} → page`); viewIdItem.page.complete(pageItem.page); pageItem.viewId.complete(viewIdItem.viewId); + + this.logService.debug(`[PlaywrightPageMap] Matched view ${viewIdItem.viewId} → page`); } if (this._viewIdQueue.length === 0) { @@ -262,17 +333,7 @@ class PlaywrightPageMap extends Disposable { */ private scanForNewContexts(): void { for (const context of this._browser.contexts()) { - if (this._watchedContexts.has(context)) { - continue; - } - this._watchedContexts.add(context); - - context.on('page', (page: Page) => this.getViewId(page)); - context.on('close', () => this._watchedContexts.delete(context)); - - for (const page of context.pages()) { - this.getViewId(page); - } + this.onContextAdded(context); } }