fix(browser): keep webview guests alive across worktree switches#6408
Conversation
Electron <webview> guests are destroyed when their DOM parent is removed. When switching worktrees, BrowserPane chrome unmounts and previously tore down the webview, forcing a reload on return. Introduce persistent page viewports inside overlay slots so webviews stay in a stable parent without keeping the full React chrome mounted. On worktree switch, park hidden viewports instead of destroying guests; explicit close paths still call destroyPersistentWebview. Fixes stablyai#6385 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds persistent browser page viewport management for overlay slots. The renderer now registers slot viewport elements, creates and parks page viewport shells, applies viewport layout and chrome inset sizing, and coordinates BrowserPane webview setup, drag/drop handling, and overlay rendering inside the viewport container. Persistent webview destruction now removes the associated viewport. New tests cover viewport creation, layout, chrome inset syncing, and parking. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/src/components/browser-pane/BrowserPane.tsx (1)
5718-5737: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse semantic style tokens for the grab toast colors.
This new UI hard-codes
white,gray,red, andbluevalues. Please map these to existing semantic tokens/classes so the toast follows theme and dark-mode styling. As per coding guidelines, “Never invent new color values … when a documented one in STYLEGUIDE.md already covers the role.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a9c7230e-b086-464a-8a60-2179b9803632
📒 Files selected for processing (5)
src/renderer/src/components/browser-pane/BrowserPane.tsxsrc/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsxsrc/renderer/src/components/browser-pane/browser-page-viewport.test.tssrc/renderer/src/components/browser-pane/browser-page-viewport.tssrc/renderer/src/components/browser-pane/webview-registry.ts
|
thx for the PR! now fixed in the 1.4.101 version |
Electron
<webview>guests are destroyed when their DOM parent is removed. When switching worktrees,BrowserPanechrome unmounts and previously tore down the webview, forcing a reload on return.Introduce persistent page viewports inside overlay slots so webviews stay in a stable parent without keeping the full React chrome mounted. On worktree switch, park hidden viewports instead of destroying guests; explicit close paths still call
destroyPersistentWebview.Fixes #6385
Summary
<webview>guest lives in that stable DOM parent and is registered in the existingwebviewRegistry. WhenBrowserPanechrome unmounts on worktree switch, the viewport is parked (display: none) instead of destroyed. React chrome (toolbar, address bar, find bar, overlays) continues to mount/unmount normally viashouldMountPane.BrowserPaneReact subtree mounted for inactive worktrees. That caused performance issues (React reconciliation, event listeners, layout work for every hidden browser). Only the lightweight viewport shell + webview guest persist; the heavy chrome unmounts.display: none,inert,pointer-events: none).Screenshots
Behavior change only, same browser UI as before. Verified on macOS that pages render correctly after the fix (Google homepage and loaded sites display in the browser pane; no blank/white regression).
No visual change to layout or chrome.
Testing
pnpm lintpnpm typecheckpnpm test— browser-pane suite: 112 tests passed (src/renderer/src/components/browser-pane)pnpm build— not run end-to-end in this sessionManual verification (macOS, local dev):
getWebContentsId()before and after switch (no guest recreation); URL and title preserved.display: flex, non-zero container rect).New tests:
browser-page-viewport.test.tscovers viewport creation under slot root, chrome inset sync, layout show/hide, and park behavior.AI Review Report
Reviewed the full diff with focus on Electron webview lifecycle, overlay-slot anchoring, and worktree switch paths.
Risks checked:
destroyPersistentWebviewonly runs on explicit close.applyBrowserPageViewportLayoutis called in the webview lifecycle effect beforewebview.srcis set, and again in auseLayoutEffectkeyed onslotViewportReady.moveFocusToRendererBeforeWebviewDetachruns before parking to avoid macOS reactivating the previous app when a focused webview is hidden.pointer-events-auto.Cross-platform (macOS, Linux, Windows):
ResizeObserver,inert,display,pointer-events) available in Electron’s Chromium on all platforms.<webview>OOPIF behavior is the same across Orca’s desktop targets; the persistent-parent pattern applies uniformly.RemoteBrowserPagePane) is untouched.Not flagged / verified:
BrowserPaneOverlayLayerslot viewport ref registration is keyed by workspace tab id, matching existing overlay architecture.webview-registry.tsteardown now also removes orphaned viewport shells on explicit close.Security Audit
registerGuest/unregisterGuestpaths unchanged.inert,pointer-events: none, anddisplay: noneso hidden guests cannot receive input.No security follow-up identified.