Browser: make Playwright per workspace#299055
Merged
Conversation
📬 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
Fixes an issue where Playwright-driven integrated browser pages could open in the wrong VS Code workspace window by making the Playwright/browser-view plumbing window-aware end-to-end.
Changes:
- Send the renderer’s
vscodeWindowIdto the shared process and create a per-window Playwright service instance. - Thread
windowIdthrough browser view group creation and CDP target creation so editor-opening IPC targets the intended window. - Replace the shared-process Playwright service channel wiring with a custom
PlaywrightChannelthat scopes instances per IPC connection.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/services/browserView/electron-browser/playwrightWorkbenchService.ts | Initializes the shared-process Playwright channel with the current window id. |
| src/vs/platform/browserView/node/playwrightService.ts | Binds Playwright operations to a specific window via windowId when creating a browser view group. |
| src/vs/platform/browserView/node/playwrightChannel.ts | New per-connection IPC channel that creates/disposes a per-window PlaywrightService. |
| src/vs/platform/browserView/node/browserViewGroupRemoteService.ts | Updates remote group creation to require windowId and forward it to main process. |
| src/vs/platform/browserView/electron-main/browserViewMainService.ts | Opens browser view editors in a specific window (by id) instead of always the focused window. |
| src/vs/platform/browserView/electron-main/browserViewGroupMainService.ts | Requires windowId when creating a new browser view group. |
| src/vs/platform/browserView/electron-main/browserViewGroup.ts | Stores a default windowId for the group and forwards it to target creation. |
| src/vs/platform/browserView/common/playwrightService.ts | Updates the Playwright service interface (brand marker removed in this PR). |
| src/vs/platform/browserView/common/cdp/types.ts | Extends CDP createTarget to optionally accept a windowId. |
| src/vs/platform/browserView/common/browserViewGroup.ts | Updates group service contract to require windowId for group creation. |
| src/vs/code/electron-utility/sharedProcess/sharedProcessMain.ts | Registers the new PlaywrightChannel instead of proxying a singleton Playwright service. |
Comments suppressed due to low confidence (1)
src/vs/platform/browserView/common/playwrightService.ts:22
IPlaywrightServiceis declared viacreateDecorator, but the interface no longer includes the standardreadonly _serviceBrand: undefined;marker used throughout the codebase to make service types nominal. Consider restoring the_serviceBrandto keep type-safety consistent with other services (e.g.IBrowserViewGroupMainServiceinsrc/vs/platform/browserView/electron-main/browserViewGroupMainService.ts).
export const IPlaywrightService = createDecorator<IPlaywrightService>('playwrightService');
/**
* A service for using Playwright to connect to and automate the integrated browser.
*
* Pages must be explicitly tracked via {@link startTrackingPage} (or implicitly via
* {@link openPage}) before they can be interacted with.
*/
export interface IPlaywrightService {
/**
* Fires when the set of tracked pages changes.
* The event value is the full list of currently tracked view IDs.
*/
readonly onDidChangeTrackedPages: Event<readonly string[]>;
src/vs/workbench/services/browserView/electron-browser/playwrightWorkbenchService.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/browserView/electron-browser/playwrightWorkbenchService.ts
Outdated
Show resolved
Hide resolved
src/vs/platform/browserView/node/browserViewGroupRemoteService.ts
Outdated
Show resolved
Hide resolved
jruales
approved these changes
Mar 4, 2026
| return source as Event<T>; | ||
| } | ||
|
|
||
| call<T>(ctx: string, command: string, arg?: unknown): Promise<T> { |
Contributor
There was a problem hiding this comment.
Nit: since an __initialize call is kind of like a constructor, maybe put call method definition before listen method definition in this file
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #298763