fix(browser): prevent macOS PiP black window - #2105
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughYoBrowser previews now use smaller frames and longer capture intervals. Idle or errored sessions release inactive previews. Session deletion destroys the associated browser and records cleanup failures. The message copy control now uses ChangesYoBrowser lifecycle
Message toolbar control
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SessionRuntime
participant YoBrowserPresenter
participant PreviewHost
SessionRuntime->>YoBrowserPresenter: Release preview for idle or errored session
YoBrowserPresenter->>PreviewHost: Hide and stop capture
YoBrowserPresenter->>PreviewHost: Dispose inactive preview host
YoBrowserPresenter-->>SessionRuntime: Release preview claim or report failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
yyhhyyyyyy
left a comment
There was a problem hiding this comment.
I found two issues that should be addressed before this is merged:
-
The hidden macOS host is not a valid capture target.
With the repository's Electron 41.10.4 on macOS 27.0 arm64, a minimal
BaseWindow/WebContentsViewusing the sameshow: falseconfiguration
loads successfully, butcapturePage(rect, { stayHidden: true })rejects
withCurrent display surface not available for capture.Electron checks
IsSurfaceAvailableForCopy()before incrementing the
capturer count, sostayHiddencannot create a compositor surface for a
window that never had one:
https://github.com/electron/electron/blob/v41.10.4/shell/browser/api/electron_api_web_contents.cc#L3793-L3806In the current loop this means no PiP frames on macOS, plus repeated capture
warnings while the Agent remains active. The unit test cannot catch this
becausecapturePageis mocked. Please preserve a renderable surface, for
example by keeping the off-screen host shown while setting
fullscreenable: false, or validate another approach with a real Electron
smoke test on supported macOS versions. -
A timed-out capture can permanently block preview capture for the next run.
stopPreviewCapture()returns after 500 ms but leaves the old
state.previewCapturepromise installed when the timeout wins.
resumeClaimedPreview()will not schedule another frame while that field is
non-null. I reproduced this with a first capture that never settles:
terminal cleanup returns, run 2 claims and recreates the host, and
setPreviewMode('capturing')succeeds, but no secondcapturePagecall is
ever made.Please detach the stale capture slot when the timeout wins and add a
regression test covering a new run after a non-settling capture. The
existing epoch checks can still fence any late completion from the old run.
Summary
Fix the macOS black fullscreen window caused by the Browser PiP off-screen host, and reduce the host's background resource cost.
Root cause
Browser PiP keeps the existing
WebContentsViewrenderable by attaching it to a separate transparent, off-screenBaseWindow. The host was created withshow: true. On macOS, that independent native window could participate in fullscreen window handling despite being transparent and positioned off-screen, surfacing as a black fullscreen window.Changes
BaseWindowhidden and non-fullscreenable on macOS.idleorerror:250 ms -> 500 ms(4 FPS -> 2 FPS)1000 ms -> 2000 ms(1 FPS -> 0.5 FPS)480x300to400x250(about 31% fewer output pixels).1280x800so page layout, CDP coordinates, and automation behavior do not change.The smaller output frame reduces resize, JPEG encoding, IPC, and renderer drawing work. It does not reduce the source viewport captured by Electron.
Lifecycle boundary
The hidden host still exists while an Agent is actively using a background browser because PiP needs a renderable surface. It is now destroyed at the terminal session state. Expanding the Browser side panel continues to detach the view from the hidden host through the existing attach path.
Behavior
Testing
pnpm exec vitest run --config vitest.config.ts test/main/desktop/browser/YoBrowserPresenter.test.ts test/main/session/deletion.test.ts(24 passed)pnpm exec vitest run --config vitest.config.renderer.ts test/renderer/components/AgentBrowserPiP.test.ts(11 passed)pnpm run format:checkpnpm run i18npnpm run lintpnpm run typecheckThe application demo was not run.
Summary by CodeRabbit
Performance Improvements
Bug Fixes
UI Improvements