perf(browser): throttle idle guests with owned activity exceptions - #282
Conversation
Hermes Review BotConfidence: 5 Engine: SummaryEmbedded browser guest Confidence Score: 5/55/5 — All changed implementation files, error handling paths, lifecycle boundaries (cancellation, crash, and close), unit tests, and native standalone e2e fixtures were completely traced and verified against Electron contracts and existing patterns. 📁 Important Files Changed
FindingsNo findings. 📊 Sequence DiagramsequenceDiagram
autonumber
participant Q as BrowserActionQueue
participant S as BrowserService
participant T as BrowserBackgroundThrottling
participant WC as Guest WebContents
participant R as Recorder Window
Note over WC: Initial state: backgroundThrottling = true
rect rgb(240, 248, 255)
Note over Q,WC: Action Queue Execution (e.g. evaluate / click)
Q->>T: acquire(signal)
T->>WC: setBackgroundThrottling(false)
Q->>WC: execute action
Q->>T: release()
T->>WC: setBackgroundThrottling(true)
end
rect rgb(255, 250, 240)
Note over S,R: Recording Lifecycle
S->>T: acquire() [recording lease]
T->>WC: setBackgroundThrottling(false)
S->>R: start recording session
Note over S,R: Recording in progress (unthrottled)
S->>R: stop / auto-stop / close / crash
R-->>T: "closed" event triggers release()
T->>WC: setBackgroundThrottling(true)
end
Machine-Readable Findings[]
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
A guest activity lease can keep the entire host BrowserWindow and its other displayed contents drawing frames while backgrounded or minimized; please account for this window-wide impact before merge.
Reviewed changes This review covers head 0a56d0c, which changes browser guest throttling and adds lease lifecycle tests and a native Electron fixture.
- Default policy and ownership. New guests use normal background throttling, with scoped leases around queued actions, captures, and recordings.
- Lifecycle and evidence. Abort, close, and crash release or reset ownership; the added tests cover these policy and lifecycle paths.
Local app and E2E type checks and Playwright discovery pass. npm run test:browser reports 153/154 because the Playwright Chromium binary is unavailable; the focused native fixture cannot start because Electron's Linux SUID sandbox is not configured.
GPT Luna ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes This incremental review covers the two commits since the prior Pullfrog review, documenting the accepted window-wide throttling exception and extending native coverage for attached guests, background operation, overlapping ownership, and recording cleanup.
- Documented the host-window effect. Added pinned Electron aggregation evidence and clarified that operation ownership, rather than a universal timer, bounds the active exception.
- Extended native lifecycle coverage. Exercised attached host and sibling views through automation, overlapping cancellation, explicit stop, and deterministic auto-stop cleanup.
GPT Luna ｜ 𝕏
2 participants
Embedded browser guests previously disabled background throttling for their entire lifetime. Ordinary guests now use throttling, with reference-counted exceptions owned by active automation, screenshots and recording. Abort, recording settlement, crash and close release the appropriate exception.
Validation: 154 browser tests; native Electron lifecycle test covering hidden capture, automation, cancellation, real recording/crash and close; app/E2E type checks; scoped lint; full build; CI discovery/policy checks. The original native policy check fails and the changed policy passes for three hidden guests. Fresh-context GPT-6 Astra medium review found no actionable issues and passed 16 focused tests.
Attached-window activity exception: Electron combines throttling sources at the containing window, so an active guest lease can also permit host/sibling drawing. This is accepted during explicit browser work; ownership, not a universal five-minute timer, bounds the lease. Documentation identifies individual deadlines and unbounded debugger/export waits. Extended native coverage exercises attached guests, actual macOS minimization, overlapping cancellation, explicit recording stop and automatic-stop cleanup. Fresh-context follow-up review independently passed 154 browser tests and the native fixture and verified pinned Electron source; no actionable findings remain. Linux uses hide/show in the fixture and has not been executed locally.
Evidence:
docs/performance/browser-throttling.mdand its raw attached-window JSON. Policy and functional behavior are measured; CPU/GPU/energy savings and Linux native behavior remain unmeasured. Existing detached-page animation-frame behavior is unchanged.