Skip to content

Finish browser state cleanup after native page closure - #336075

Merged
Dmitriy Vasyura (dmitrivMS) merged 2 commits into
mainfrom
agents/browser-native-close-cleanup-320443
Sep 15, 2026
Merged

Dmitriy Vasyura (dmitrivMS) merged 2 commits into
mainfrom
agents/browser-native-close-cleanup-320443

Conversation

@dmitrivMS

@dmitrivMS Dmitriy Vasyura (dmitrivMS) commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Avoid an unhandled main-process error when Electron has already cleared a browser view's WebContents during native closure. Guard the native close call so registered cleanup still completes.

Strengthen the existing popup lifecycle smoke test to catch the reported disposal exception after restart.

Fixes #320443

Guard against Electron clearing a WebContentsView's WebContents before the destroyed callback, so registered cleanup still completes.

Extend the existing native popup lifecycle smoke test to reject the reported disposal exception after restart.

Fixes #320443.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 13, 2026 22:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused guard addresses the reported lifecycle race and is covered by an appropriate native smoke test.

Review tier: Balanced (auto)
Findings: None

Note

Copilot is running an experiment and ran this review at Balanced.

What changed in this PR

Prevents an unhandled main-process error when Electron clears a browser view’s webContents during native closure.

Changes:

  • Guards native webContents cleanup during disposal.
  • Extends the popup lifecycle smoke test to detect the regression.
File Description
src/​vs/​platform/​browserView/​electron-main/​browserView.ts Safely handles missing native web contents during disposal.
test/​smoke/​src/​areas/​browserView/​browserView.test.ts Verifies disposal does not log the reported exception.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dmitrivMS Dmitriy Vasyura (dmitrivMS) added browser-integration Web browsing features integrated into VS Code (e.g. integrated browser) error-telemetry VS Code - Issues generated by telemetry vscode-build VS Code build process issues engineering VS Code - Build / issue tracking / etc. labels Sep 13, 2026
@dmitrivMS Dmitriy Vasyura (dmitrivMS) added this to the 1.139.0 milestone Sep 13, 2026
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) marked this pull request as ready for review September 13, 2026 22:57
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

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

Kyle Cutler (@kycutler)

Matched files:

  • src/vs/platform/browserView/electron-main/browserView.ts

Joaquín Ruales (@jruales)

Matched files:

  • src/vs/platform/browserView/electron-main/browserView.ts

@dmitrivMS Dmitriy Vasyura (dmitrivMS) changed the title browser: finish cleanup after native page closure Finish browser state cleanup after native page closure Sep 14, 2026
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) merged commit 9854016 into main Sep 15, 2026
53 of 54 checks passed
@dmitrivMS
Dmitriy Vasyura (dmitrivMS) deleted the agents/browser-native-close-cleanup-320443 branch September 15, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

browser-integration Web browsing features integrated into VS Code (e.g. integrated browser) engineering VS Code - Build / issue tracking / etc. error-telemetry VS Code - Issues generated by telemetry vscode-build VS Code build process issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Error] unhandlederror-Cannot read properties of undefined (reading 'isDestroyed')

3 participants