Repository navigation
fix(desktop): capture covered preview surfaces without hanging - #33719
andrew-metaplanet wants to merge 2 commits into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — Preview viewport screenshots use Electron surface capture to avoid hanging when the preview is covered. Retries are bounded, copies are serialized, and specialized screenshot requests retain their existing routing. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Problem
Agent snapshots and a recording's first frame can hang when an Electron preview guest is covered by the app. The existing painting lease moves the guest into the window, but Chromium's
Page.captureScreenshotcan still wait indefinitely for a presented frame. A stuck first frame then holds the server capture queue and later snapshots hit the broker timeout.Reproduced on macOS with Electron 44.10.1: with the lease held and the guest covered, 2 of 5 baseline CDP captures never answered within the fixture deadline. The same guest captured successfully when uncovered.
Change
Use Electron's
webContents.capturePage()for plain viewport PNG/JPEG requests in the existing guest command relay. Retry rejected or empty surface copies at 50 ms intervals, with a five-second deadline. Serialize native copies per webContents across request timeouts; canceled waiters cannot start extra work, and a later live request captures a fresh frame after the earlier copy settles.Clipped, beyond-viewport, WebP,
fromSurface: false, child-session and unrecognized-option requests retain their existing CDP path. The existing painting/focus leases and idle suspension remain in place.Scope and approval
This is a focused reliability fix for the existing screenshot operation, submitted under the obvious-bug exception in CONTRIBUTING.md. Three files change: the guest relay, its regression tests, and the existing host test fake needed by that relay. No setting, permission, protocol, always-on rendering or unrelated cleanup is introduced.
Related work: #21534 also addresses background captures with a renderer preparation handshake and CDP nudges; #27092 includes native capture as part of a macOS screen-lock solution that keeps guests inside the window between captures. This change keeps the current on-demand painting lifecycle and addresses the covered, unlocked-window repro. It does not claim screen-lock support. The server queue/deadline work in #24523 is complementary.
Verification
Tested tree:
d21df352534b68fe3a60ff02810e867a92089678(identical to the locally tested implementation tree).vp run check:changed: format, lint, knip, affected typechecks and tests passed.sendPageCommandand a synthetic local page: all 5 covered-window captures passed in 20–23 ms. A guest that never paints failed cleanly in 5003 ms; the same guest then recovered for 3 captures. JPEG first-frame capture returned a valid image. Baseline covered captures included two unanswered requests.preview_statusreportingvisible:false: snapshot 124 ms, recording start 3807 ms, snapshot during recording 67 ms, recording stop 51 ms with a valid MP4, then tab close. Both returned PNGs were visually inspected.These timings are local observations, not guarantees. Linux, Windows, screen lock, remote viewing on a second device and packaged release updates were not verified. The integrated run used the local desktop build, not a released installer. Already-issued native capture promises cannot be canceled; the map prevents overlapping copies while the request deadline frees its caller.
Synthetic fixture evidence (no accounts, external sites or private data):
hidden-preview-recording.mp4
Sanitized before/after and integrated timing receipt
Update at
28f73fc9: merged upstream main4c924ff4. The only conflict was in the DesktopBrowserHost test double; both upstream loading/destruction state and the native capture mock are retained. Capture implementation and GuestInput regression tests are unchanged.vp run check:changed origin/mainpasses (56 focused tests); the full desktop preview directory passes 271/271, including upstream first-page attachment cases.Independent code review: GPT-6.1 Sol, APPROVE, no blocker or should-fix findings.
Implemented with Claude Opus and GPT-6.1 Sol through T3 Code, using Claude Code and Codex harnesses.