Repository navigation
Evict hidden browser WebViews under memory pressure - #6585
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OS memory-pressure eviction for hidden browser WebViews. ChangesSystem memory pressure hidden WebView eviction
Sequence Diagram(s)sequenceDiagram
rect rgba(200, 220, 255, 0.5)
Note over OS,PaneMemoryGuardrail: OS pressure signal (timerQueue)
participant OS as OS DispatchSource
participant PaneMemoryGuardrail
OS->>PaneMemoryGuardrail: .warning / .critical event
PaneMemoryGuardrail->>PaneMemoryGuardrail: handleSystemMemoryPressure()
end
rect rgba(220, 255, 220, 0.5)
Note over PaneMemoryGuardrail,BrowserHiddenWebViewDiscardManager: Eviction cascade (main actor)
participant AppDelegate
participant TabManager
participant Workspace
participant BrowserPanel
participant BrowserHiddenWebViewDiscardManager
PaneMemoryGuardrail->>AppDelegate: onSystemMemoryPressure()
AppDelegate->>TabManager: discardHiddenBrowserWebViewsForSystemMemoryPressure(now)
TabManager->>Workspace: discardHiddenBrowserWebViewsForSystemMemoryPressure(now)
Workspace->>BrowserPanel: discardHiddenWebViewForSystemMemoryPressure(now)
BrowserPanel->>BrowserHiddenWebViewDiscardManager: requestImmediateDiscardIfSafe(reason: "system_memory_pressure", now)
BrowserHiddenWebViewDiscardManager-->>BrowserPanel: Bool (discarded?)
BrowserPanel-->>Workspace: Bool
Workspace-->>TabManager: count
TabManager-->>AppDelegate: total count
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 22 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (22 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Greptile SummaryThis PR adds OS memory pressure eviction for hidden browser
Confidence Score: 5/5Safe to merge; the memory-pressure path reuses all existing discard safety blockers and the post-wake crash guard, so no visible pane or pane in a fragile state is at risk. The implementation follows established patterns (same DispatchSource + Task @mainactor hop as the poll timer, same blockers checklist, same post-wake guard), threads a single No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant OS as macOS Kernel
participant Src as DispatchSourceMemoryPressure
participant MA as MainActor Task
participant PMG as PaneMemoryGuardrail
participant AD as AppDelegate
participant TM as TabManager
participant WS as Workspace
participant BP as BrowserPanel
participant MGR as BrowserHiddenWebViewDiscardManager
OS->>Src: .warning / .critical event
Src->>MA: "Task @MainActor (weak self)"
MA->>PMG: handleSystemMemoryPressure()
PMG->>AD: onSystemMemoryPressure()
AD->>AD: "let now = Date()"
loop each TabManager
AD->>TM: discardHiddenBrowserWebViewsForSystemMemoryPressure(now:)
loop each Workspace
TM->>WS: discardHiddenBrowserWebViewsForSystemMemoryPressure(now:)
loop each BrowserPanel
WS->>BP: discardHiddenWebViewForSystemMemoryPressure(now:)
BP->>MGR: requestImmediateDiscardIfSafe(reason:now:)
alt blockers empty AND hiddenAt set AND not in post-wake window
MGR-->>BP: true - discard immediately
else hiddenAt nil OR in post-wake window
MGR->>MGR: scheduleIfNeeded(reason:now:)
MGR-->>BP: false - scheduled
else blockers present
MGR-->>BP: false - skipped
end
end
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant OS as macOS Kernel
participant Src as DispatchSourceMemoryPressure
participant MA as MainActor Task
participant PMG as PaneMemoryGuardrail
participant AD as AppDelegate
participant TM as TabManager
participant WS as Workspace
participant BP as BrowserPanel
participant MGR as BrowserHiddenWebViewDiscardManager
OS->>Src: .warning / .critical event
Src->>MA: "Task @MainActor (weak self)"
MA->>PMG: handleSystemMemoryPressure()
PMG->>AD: onSystemMemoryPressure()
AD->>AD: "let now = Date()"
loop each TabManager
AD->>TM: discardHiddenBrowserWebViewsForSystemMemoryPressure(now:)
loop each Workspace
TM->>WS: discardHiddenBrowserWebViewsForSystemMemoryPressure(now:)
loop each BrowserPanel
WS->>BP: discardHiddenWebViewForSystemMemoryPressure(now:)
BP->>MGR: requestImmediateDiscardIfSafe(reason:now:)
alt blockers empty AND hiddenAt set AND not in post-wake window
MGR-->>BP: true - discard immediately
else hiddenAt nil OR in post-wake window
MGR->>MGR: scheduleIfNeeded(reason:now:)
MGR-->>BP: false - scheduled
else blockers present
MGR-->>BP: false - skipped
end
end
end
end
Reviews (23): Last reviewed commit: "Merge origin/main into browser memory ev..." | Re-trigger Greptile |
…bview-mem-eviction
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/BrowserHiddenWebViewDiscardMemoryPressureTests.swift`:
- Around line 74-86: The withMemoryPressureHiddenWebViewDiscardPolicyEnabled
method only snapshots and restores the enabledKey from UserDefaults, but it does
not handle hiddenDelayKey. This allows hiddenDelayKey state from other tests to
leak into this test, making hasScheduledDiscard flaky. Add snapshot-and-restore
logic for BrowserHiddenWebViewDiscardPolicy.hiddenDelayKey in addition to
enabledKey by capturing its previous value before calling body() and then either
restoring it or removing it in the defer block, following the same pattern
already used for enabledKey.
In `@cmuxTests/GhosttyConfigTests.swift`:
- Around line 2166-2188: The test
testSystemMemoryPressureDoesNotDiscardVisibleWebView is passing vacuously
because it does not explicitly enable the discard policy before testing that
visibility blocks the discard behavior. Before calling
discardHiddenWebViewForSystemMemoryPressure, explicitly set the policy state to
ensure the test exercises the intended "visible blocks discard" path rather than
passing simply because the policy is disabled by default. Avoid relying on
shared or global UserDefaults state; instead, configure the discard policy state
directly on the panel or through the appropriate configuration mechanism before
invoking the discard method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0b0aaf9b-3a56-43f4-b4d0-b457c6912753
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (10)
Sources/AppDelegate+PaneMemoryGuardrail.swiftSources/AppDelegate.swiftSources/PaneMemoryGuardrail.swiftSources/Panels/BrowserHiddenWebViewDiscardManager.swiftSources/Panels/BrowserPanel.swiftSources/TabManager.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserHiddenWebViewDiscardMemoryPressureTests.swiftcmuxTests/GhosttyConfigTests.swift
…bview-mem-eviction
…bview-mem-eviction
…bview-mem-eviction # Conflicts: # .github/swift-file-length-budget.tsv
Closes #6584
Summary
PaneMemoryGuardrailcovers pane process trees while WKWebView web-content processes are not part of the hidden browser discard timing pathDispatchSourceMemoryPressurehook that immediately fans out to browser panels across registered window tab managersWKWebView, then reload on returnTests
Notes
Observed: hidden browser panes only used the timer-based 5-minute discard path, while system memory pressure had no immediate browser WebView eviction route.
Expected: warning/critical OS memory pressure promptly frees eligible non-visible embedded browser WebViews without changing visible-pane behavior.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Evicts hidden browser
WKWebViews on warning/critical system memory pressure while keeping visible panes intact. If a hidden timestamp isn’t recorded or we’re in the post‑wake safety window, we schedule a discard instead and honor the memory‑saver policy.New Features
PaneMemoryGuardraillistens for.warning/.criticalDispatchSourceMemoryPressureand invokesonSystemMemoryPressureon the main actor.TabManager→Workspaces and callsBrowserHiddenWebViewDiscardManager.requestImmediateDiscardIfSafe("system_memory_pressure", now); a singlenowis forwarded and the total discard count is logged in debug.now. The post‑wake guard remains in place. Policy is read via injectedpolicyDefaults(UserDefaults).Tests
BrowserHiddenWebViewDiscardMemoryPressureTestscovering immediate discard, scheduling when hidden state is missing, and post‑wake deferral with isolatedUserDefaults.Written for commit 9e3a5d2. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Tests