Repository navigation
perf(browser): throttle idle guests between active operations - #34776
Conversation
Keep the renderer paint hold, host unthrottle, and host re-hide readback on capture paths only, and never let the one-way paint-hint send fail the capture.
3d009e1 to
470abe6
Compare
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title "perf(browser): throttle idle guests between active operations" matches the main change: browsing guests now throttle when idle and are temporarily held active for commands, captures, and st… Description check The description follows the repository template. It includes ELI5, What Changed, Why (with measurements and reasoning), Visual Proof (N/A with reason and a fixture image), Testing with commands and re… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "perf(browser): throttle idle guests between active operations" matches the main change: browsing guests now throttle when idle and are temporarily held active for commands, captures, and streams. It is specific, uses a conventional commit prefix, and avoids vague wording. The title does not mention the capture-only paint hold or the queue-level lease, but it does not need to cover every detail.
Full details: Description check
-
Explanation
The description follows the repository template. It includes ELI5, What Changed, Why (with measurements and reasoning), Visual Proof (N/A with reason and a fixture image), Testing with commands and results, Review, Agent skill upstream boundary, Notes, and Checklist. Two gaps exist. The Linked Issue section says 'None' and has no 'Fixes #' reference, which the template says is required for outside contributors. The AI Disclosure section is missing, although the template says it is only skipped for StablyAI team members and the author's status is not stated. The final checklist item for pnpm lint/typecheck/test/build is unchecked, with a note that CI will cover it.
Resolution
Consider these additions before merge: link an existing issue in the Linked Issue section (the template states that outside contributors need one, and 'None' does not meet this), or confirm that the author is a StablyAI team member. Fill in the AI Disclosure section if AI tools were used. Confirm the status of the unchecked lint/typecheck/test/build item.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 14 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
ELI5
Idle browser pages keep running their timers after Orca is hidden. Let idle pages rest, and temporarily wake them while a browser command, screenshot or live stream needs them.
What Changed
holdGuestActive) when its queue turn starts, after resolving the current guest. Commands do not take the renderer paint hold, host unthrottle or host re-hide readback; fix(browser): let pixel capture hold its own page drawn, without the desktop window #22534's capture-only paint design stays intact. The fullholdPaintForCapture(renderer paint hint, host lease, guest lease) remains on the capture paths only: CDP screenshots, the first-paint gate, selection screenshots and extensiontabs.captureVisibleTab.capturePagewith an empty rect andstayHidden) after each command. This is the same mechanism screencasts and captures already use.browser:capturePaintHoldnotification never throws (for example, from a disposed frame), so it cannot fail the capture it accompanies.Why
Measured on Linux with Electron 43.8.1, a hidden native host and one real registered webview running a 16ms interval plus requestAnimationFrame:
This removes about 98% of the measured idle timer callbacks. Reusing the existing owner lets concurrent commands, captures and streams keep the page active until the last owner finishes. CPU, RAM, GPU and battery savings were not measured.
Linked Issue
None; found during the Electron reliability and performance audit.
Visual Proof
N/A
The change affects hidden-page scheduling and has no visible UI change. The before/after native callback counts are above. This captured fixture image verifies that hidden screenshots still contain the expected rendered page:
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
ORCA_BACKGROUND_LAUNCH=1 pnpm check:changed --maxWorkers=2: 20 checks passed (835.5s).Targeted runs: 164 bridge/stream/throttle tests; 25 final lease/queue/lifecycle tests; 36 initial manager/grab/extension tests; 12 popup-policy tests; existing native streamed-page destruction test, covering three cycles with a surviving host. Node typecheck passes.
Three regression checks fail against unchanged production sources: permanent idle exemption, guest ownership without a renderer and queued command ownership/replacement.
Real AgentBrowserBridge evaluation awaits two animation frames and a timer, then returns to idle. HTTP navigation succeeds. A first click after the existing bounded first-paint gate reaches its target on a fresh hidden document.
Viewport, full-page and streamed images match expected pixels. Two real runtime stream subscribers plus a simultaneous capture share ownership; leaving one subscriber keeps both contents active, and leaving the last restores idle throttling automatically.
Review follow-up: after the switch to guest-only command leases, the native harness was re-run on an isolated hidden Xvfb display. Idle callbacks were 3/3/3 timers per 3s. Agent rAF+timer evaluation, HTTP navigation, the first-paint click, viewport/full-page/stream pixels and automatic restore all passed. Bridge tests assert that non-capture commands never call
holdPaintForCapture.Failure/teardown controls cover queued replacement, dropped queued work, killed running work, setup/capture failure, duplicate release, shared/offscreen ownership, renderer replacement/destruction and notification/acquire failures.
Review
Review the queue execution boundary, per-renderer capture accounting and final release paths. Native checks use production managers and capture/stream code in an isolated hidden shell, with CDP screenshot inspection.
Agent skill upstream boundary
Notes
Linux/Xvfb native validation only. CPU/GPU cost of the per-command guest re-hide readback was not measured. Full Orca navigation/focus flows, macOS, Windows, remote live sessions and a physical mobile client were not executed. The code uses platform-neutral Electron APIs and changes no remote/mobile protocol. Hidden Xvfb records no paint-timing entry within the existing first-paint deadline, so the input control asserts actual click delivery.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)The changed-file gate result is recorded above; CI will cover the full build/test and platform matrix.