Conversation
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
@deepak1556Matched files:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces managed CDP “context groups” for BrowserView, moving from a single debug WebSocket endpoint to per-group CDP endpoints so different consumers can see different subsets of BrowserViews.
Changes:
- Removes the global
getDebugWebSocketEndpoint()surface fromIBrowserViewService/ workbench service. - Adds main/remote services and types for
IBrowserViewGroup, including IPC channel wiring. - Updates the CDP proxy server to register multiple
ICDPBrowserTargets and serve/devtools/browser/{targetId}endpoints with target-bound single-use tokens; updates Playwright to connect via a group.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/browserView/electron-browser/browserViewWorkbenchService.ts | Removes workbench-level CDP endpoint passthrough. |
| src/vs/workbench/contrib/browserView/common/browserView.ts | Removes getDebugWebSocketEndpoint() from workbench service contract. |
| src/vs/platform/browserView/node/playwrightService.ts | Switches Playwright connection flow to group-based CDP endpoint; adds page↔view correlation logic. |
| src/vs/platform/browserView/electron-main/browserViewMainService.ts | Removes dependency and API for global CDP endpoint. |
| src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts | New main-process group manager implementing the IPC-facing service. |
| src/vs/platform/browserView/electron-main/browserViewGroup.ts | New group implementation exposing a per-group CDP “browser” target and managing included views/contexts. |
| src/vs/platform/browserView/electron-main/browserViewCDPProxyServer.ts | Refactors proxy to support multiple targets and per-target endpoints/tokens. |
| src/vs/platform/browserView/common/browserViewGroupRemoteService.ts | New remote service/proxy to manage groups via IPC. |
| src/vs/platform/browserView/common/browserViewGroup.ts | New shared group/service interfaces + IPC channel name. |
| src/vs/platform/browserView/common/browserView.ts | Removes getDebugWebSocketEndpoint() from IBrowserViewService. |
| src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts | Registers BrowserViewGroupRemoteService in shared process DI. |
| src/vs/code/electron-main/app.ts | Registers main group service + IPC channel wiring for the new group channel. |
Comments suppressed due to low confidence (2)
src/vs/platform/browserView/electron-main/browserViewGroup.ts:200
dispose()callsthis.cdpProxyServer.removeTarget(this)but does not await or handle the returned promise. IfremoveTargetrejects (e.g.getTargetInfo()throws), this can surface as an unhandled rejection during shutdown. Consider makingremoveTargetsynchronous (passtargetIddirectly), or call it asvoid …catch(...)here.
override dispose(): void {
this._onDidDestroy.fire();
this.cdpProxyServer.removeTarget(this);
super.dispose();
src/vs/platform/browserView/node/playwrightService.ts:181
onViewRemovedonly deletes the resolved mapping. If a view is removed before it is matched (still in_viewIdQueue) or while a caller is waiting in_waiters, the waiter can hang until timeout and the FIFO matching can drift. Consider removing theviewIdfrom_viewIdQueue(if present) and failing any matching waiter immediately when the group reports removal.
private onViewRemoved(viewId: string): void {
this._viewIdToPage.delete(viewId);
}
jruales
previously approved these changes
Feb 18, 2026
Contributor
|
Some diagrams for posterity: Big Picture: Before vs. Aftergraph TD
subgraph BEFORE["Before: One global CDP endpoint"]
BVS[BrowserViewMainService\nglobal CDP endpoint]
PW_OLD[PlaywrightService]
ALL[All BrowserViews\nvisible to everyone]
PW_OLD -->|connects to global endpoint| BVS
BVS --- ALL
end
subgraph AFTER["After: Isolated groups"]
GROUP1[BrowserViewGroup A\nPlaywright's views only]
GROUP2[BrowserViewGroup B\nsome other consumer]
PW_NEW[PlaywrightService\nnow with PlaywrightPageMap]
CONSUMER2[Other CDP Consumer]
PW_NEW -->|own CDP endpoint| GROUP1
CONSUMER2 -->|own CDP endpoint| GROUP2
GROUP1 -.->|can only see| V1[View 1, View 2]
GROUP2 -.->|can only see| V2[View 3]
end
PR File Map - 12 Changed Filesgraph TD
subgraph MainProcess["Main Process (Electron)"]
APP["app.ts<br/>Registers 2 new services + IPC channels"]
BVGMS["browserViewGroupMainService.ts ✨NEW<br/>Manages group lifecycle (create/destroy)"]
BVG["browserViewGroup.ts ✨NEW<br/>One isolated group — implements ICDPBrowserTarget"]
CDPPROXY["browserViewCDPProxyServer.ts<br/>Now handles MULTIPLE targets (was 1 global)"]
BVMS["browserViewMainService.ts<br/>Lost getDebugWebSocketEndpoint() — no longer its job"]
end
subgraph SharedProcess["Shared Process (Node)"]
SPMAIN["sharedProcessMain.ts<br/>Registers BrowserViewGroupRemoteService"]
BVGRS["browserViewGroupRemoteService.ts ✨NEW<br/>IPC proxy — wraps the main process service"]
PW["playwrightService.ts<br/>Now creates a group + uses PlaywrightPageMap"]
end
subgraph Common["Common (shared types)"]
BVGC["browserViewGroup.ts ✨NEW<br/>Interfaces: IBrowserViewGroup, IBrowserViewGroupService"]
BVC["browserView.ts<br/>Removed getDebugWebSocketEndpoint()"]
end
subgraph WorkbenchLayer["Workbench (Renderer)"]
WBC["browserView.ts (workbench)<br/>Removed getDebugWebSocketEndpoint()"]
WBCS["browserViewWorkbenchService.ts<br/>Removed getDebugWebSocketEndpoint()"]
end
BVGC -.->|interfaces| BVGMS
BVGC -.->|interfaces| BVGRS
BVGMS --> BVG
BVG --> CDPPROXY
BVGRS -->|IPC| BVGMS
PW --> BVGRS
APP --> BVGMS
SPMAIN --> BVGRS
End-to-End: createGroup() to live CDP connectionsequenceDiagram
participant PW as PlaywrightService<br/>(shared process)
participant RS as BrowserViewGroupRemoteService<br/>(shared process)
participant IPC as IPC Channel<br/>(ProxyChannel)
participant MS as BrowserViewGroupMainService<br/>(main process)
participant BVG as BrowserViewGroup<br/>(main process)
participant CDP as BrowserViewCDPProxyServer<br/>(main process)
PW->>RS: createGroup()
RS->>IPC: createGroup() [IPC call]
IPC->>MS: createGroup()
MS->>BVG: new BrowserViewGroup(uuid)
MS-->>IPC: return groupId
IPC-->>RS: return groupId
RS-->>PW: return RemoteBrowserViewGroup(groupId)
PW->>RS: group.getDebugWebSocketEndpoint()
RS->>IPC: getDebugWebSocketEndpoint(groupId)
IPC->>MS: getDebugWebSocketEndpoint(groupId)
MS->>BVG: getDebugWebSocketEndpoint()
BVG->>CDP: getWebSocketEndpointForTarget(this)
CDP->>CDP: register target + issue token
CDP-->>BVG: ws://localhost:{port}/devtools/browser/{groupId}?token=X
BVG-->>MS: URL
MS-->>IPC: URL
IPC-->>RS: URL
RS-->>PW: URL
PW->>PW: playwright.chromium.connectOverCDP(URL)
PW->>CDP: WebSocket upgrade request
CDP->>CDP: verify token matches groupId
CDP->>BVG: attach() → CDPBrowserProxy
CDP-->>PW: WebSocket connection established
CDP Proxy Server: Before vs. Aftergraph TD
subgraph BEFORE["Before"]
EP1["/devtools/browser?token=X<br/>→ hardcoded single target"]
end
subgraph AFTER["After"]
EP2["/devtools/browser/groupA?token=X<br/>→ BrowserViewGroup A"]
EP3["/devtools/browser/groupB?token=Y<br/>→ BrowserViewGroup B"]
REG["Target Registry<br/>Map of targetId → ICDPBrowserTarget"]
TM["TokenManager<string><br/>token → targetId (single-use, 30s TTL)"]
EP2 -->|lookup| REG
EP3 -->|lookup| REG
EP2 -->|verify| TM
EP3 -->|verify| TM
end
Group System - Three Layersgraph LR
subgraph SharedProcess["Shared Process"]
RS["BrowserViewGroupRemoteService<br/>(IBrowserViewGroupRemoteService)<br/>─────────────────────<br/>createGroup() → IBrowserViewGroup<br/><br/>Wraps IPC into a friendly object"]
RG["RemoteBrowserViewGroup<br/>(IBrowserViewGroup)<br/>─────────────────────<br/>addView / removeView<br/>getDebugWebSocketEndpoint<br/>onDidAddView / onDidRemoveView"]
end
subgraph MainProcess["Main Process"]
MS["BrowserViewGroupMainService<br/>(IBrowserViewGroupService)<br/>─────────────────────<br/>Map of id → BrowserViewGroup<br/>createGroup / destroyGroup<br/>addViewToGroup / removeViewFromGroup"]
BVG["BrowserViewGroup<br/>─────────────────────<br/>Map of views<br/>Implements ICDPBrowserTarget<br/>Has its own CDP endpoint"]
end
RS -->|"IPC (ProxyChannel)"| MS
RS -->|creates| RG
RG -->|"IPC calls forwarded to"| MS
MS -->|manages| BVG
|
bhavyaus
approved these changes
Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solves two issues: