Repository navigation
Protect visible browser web views from discard #6191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3132,6 +3132,7 @@ final class BrowserPanel: Panel, ObservableObject { | |
| private var pendingInteractiveBrowserPrompts: [PendingInteractiveBrowserPrompt] = [] | ||
| private var isPresentingPendingInteractiveBrowserPrompt = false | ||
| private var isWebViewVisibleInUI: Bool = false | ||
| private var isProtectedByWorkspaceVisibilityForResourceCleanup: Bool = false | ||
| private var isClosingWebViewLifecycle: Bool = false | ||
|
|
||
| /// True while a canvas pane hosts this browser's webview inline (in the | ||
|
|
@@ -3453,6 +3454,23 @@ final class BrowserPanel: Panel, ObservableObject { | |
| } | ||
| } | ||
|
|
||
| @discardableResult | ||
| func setWorkspaceVisibilityProtectsHiddenWebViewDiscard( | ||
| _ isProtected: Bool, | ||
| reason: String, | ||
| now: Date = Date() | ||
| ) -> Bool { | ||
| guard isProtectedByWorkspaceVisibilityForResourceCleanup != isProtected else { return false } | ||
| isProtectedByWorkspaceVisibilityForResourceCleanup = isProtected | ||
| if !isProtected && !isWebViewVisibleInUI { | ||
| webViewLastHiddenAt = now | ||
| webViewLastVisibilityChangeAt = now | ||
| webViewLastVisibilityChangeReason = "workspace_visibility.\(reason)" | ||
| } | ||
| reevaluateHiddenWebViewDiscardScheduling(reason: "workspace_visibility.\(reason)") | ||
| return true | ||
| } | ||
|
|
||
| func webViewLifecycleTopPayload(now: Date = Date()) -> [String: Any] { | ||
| let discardBlockers = hiddenWebViewDiscardBlockers() | ||
| return [ | ||
|
|
@@ -6105,6 +6123,7 @@ extension BrowserPanel: BrowserHiddenWebViewDiscardManagerDelegate { | |
| var hiddenWebViewDiscardSnapshot: BrowserHiddenWebViewDiscardManager.BlockerSnapshot { | ||
| BrowserHiddenWebViewDiscardManager.BlockerSnapshot( | ||
| isClosing: isClosingWebViewLifecycle, | ||
| isProtectedByWorkspaceVisibility: isProtectedByWorkspaceVisibilityForResourceCleanup, | ||
| isVisibleInUI: isWebViewVisibleInUI, | ||
| shouldRenderWebView: shouldRenderWebView, | ||
| hasPendingRemoteNavigation: pendingRemoteNavigation != nil, | ||
|
|
@@ -8426,6 +8445,10 @@ private extension BrowserPanel { | |
|
|
||
| extension BrowserPanel { | ||
| func hideBrowserPortalView(source: String) { | ||
| _ = setWorkspaceVisibilityProtectsHiddenWebViewDiscard( | ||
| false, | ||
| reason: "portal.\(source)" | ||
| ) | ||
|
Comment on lines
+8448
to
+8451
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t clear workspace protection from the generic portal-hide path. Line 8442 removes the new blocker before recording the webview as hidden, so a workspace-visible browser can still become discard-eligible during the transient hide this PR is meant to tolerate. Clear this only from the authoritative workspace-retirement/removal path, not every portal hide. 🐛 Proposed fix- func hideBrowserPortalView(source: String) {
- _ = setWorkspaceVisibilityProtectsHiddenWebViewDiscard(
- false,
- reason: "portal.\(source)"
- )
+ func hideBrowserPortalView(
+ source: String,
+ clearWorkspaceVisibilityProtection: Bool = false
+ ) {
+ if clearWorkspaceVisibilityProtection {
+ _ = setWorkspaceVisibilityProtectsHiddenWebViewDiscard(
+ false,
+ reason: "portal.\(source)"
+ )
+ }
noteWebViewVisibility(
false,
reason: "portal.\(source)",
recordIfUnchanged: true
)Update the workspace-retirement caller to pass 🤖 Prompt for AI Agents |
||
| noteWebViewVisibility( | ||
| false, | ||
| reason: "portal.\(source)", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3656,6 +3656,7 @@ final class Workspace: Identifiable, ObservableObject { | |
| private var layoutFollowUpStalledAttemptCount = 0 | ||
| private var pendingReparentFocusSuppressionViews: [ObjectIdentifier: GhosttySurfaceScrollView] = [:] | ||
| private var portalRenderingEnabled = true | ||
| private var workspacePresentationVisible = false | ||
| private var agentHibernationAutoResumePresentationVisible = true | ||
| private var isAttemptingLayoutFollowUp = false | ||
| private var isNormalizingPinnedTabOrder = false | ||
|
|
@@ -9882,6 +9883,12 @@ final class Workspace: Identifiable, ObservableObject { | |
| reconcileTerminalPortalVisibilityForCurrentRenderedLayout() | ||
| } | ||
|
|
||
| func setWorkspacePresentationVisible(_ isVisible: Bool) { | ||
| guard workspacePresentationVisible != isVisible else { return } | ||
| workspacePresentationVisible = isVisible | ||
| reconcileBrowserPortalVisibilityForCurrentRenderedLayout(reason: "workspace.presentationVisibility") | ||
| } | ||
|
|
||
| // MARK: - Utility | ||
|
|
||
| /// Writes a small shell wrapper that keeps a disconnected remote terminal visible. | ||
|
|
@@ -10611,6 +10618,10 @@ final class Workspace: Identifiable, ObservableObject { | |
| return renderedVisiblePanelIdsForCurrentLayout() | ||
| } | ||
|
|
||
| private func presentationVisiblePanelIdsForCurrentLayout() -> Set<UUID> { | ||
| workspacePresentationVisible ? renderedVisiblePanelIdsForCurrentLayout() : [] | ||
| } | ||
|
|
||
| @discardableResult | ||
| func reconcileTerminalPortalVisibilityForCurrentRenderedLayout() -> Bool { | ||
| let visiblePanelIds = renderedVisiblePanelIdsForCurrentLayout() | ||
|
|
@@ -10674,15 +10685,29 @@ final class Workspace: Identifiable, ObservableObject { | |
|
|
||
| @discardableResult | ||
| func reconcileBrowserPortalVisibilityForCurrentRenderedLayout(reason: String) -> Bool { | ||
| let visiblePanelIds = renderedVisiblePanelIdsForCurrentLayout() | ||
| let visiblePanelIds = presentationVisiblePanelIdsForCurrentLayout() | ||
| var didChange = false | ||
|
|
||
| for panel in panels.values { | ||
| guard let browserPanel = panel as? BrowserPanel else { continue } | ||
| // Canvas-inline-hosted webviews live in the pane hierarchy; portal | ||
| // rebinds/refreshes here would steal them back into the portal. | ||
| if browserPanel.canvasInlineHostingActive { continue } | ||
| if browserPanel.canvasInlineHostingActive { | ||
| if browserPanel.setWorkspaceVisibilityProtectsHiddenWebViewDiscard( | ||
| false, | ||
| reason: "canvasInline.\(reason)" | ||
| ) { | ||
| didChange = true | ||
| } | ||
| continue | ||
|
Comment on lines
+10695
to
+10702
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Skip canvas-inline browsers in the follow-up predicate too. Line 10665 now intentionally bypasses portal reconciliation for Proposed fix private func browserPortalVisibilityNeedsFollowUp() -> Bool {
let visiblePanelIds = renderedVisiblePanelIdsForCurrentLayout()
for panel in panels.values {
guard let browserPanel = panel as? BrowserPanel else { continue }
+ if browserPanel.canvasInlineHostingActive { continue }
guard visiblePanelIds.contains(browserPanel.id) else { continue }
let anchorView = browserPanel.portalAnchorViewAlso applies to: 10737-10750 🤖 Prompt for AI Agents |
||
| } | ||
| let shouldBeVisible = visiblePanelIds.contains(browserPanel.id) | ||
| if browserPanel.setWorkspaceVisibilityProtectsHiddenWebViewDiscard( | ||
| shouldBeVisible, | ||
| reason: reason | ||
| ) { | ||
| didChange = true | ||
| } | ||
|
Comment on lines
+10704
to
+10710
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clear the workspace-visibility blocker when a tab is deselected.
Proposed fix private func hideBrowserPortalsForDeselectedTabs(inPane pane: PaneID, selectedTabId: TabID) {
for tab in bonsplitController.tabs(inPane: pane) {
guard tab.id != selectedTabId else { continue }
guard let panelId = panelIdFromSurfaceId(tab.id),
let browserPanel = panels[panelId] as? BrowserPanel else { continue }
+ _ = browserPanel.setWorkspaceVisibilityProtectsHiddenWebViewDiscard(
+ false,
+ reason: "tabDeselected"
+ )
browserPanel.hideBrowserPortalView(source: "tabDeselected")
}
}Also applies to: 11541-11547 🤖 Prompt for AI Agents |
||
| let anchorView = browserPanel.portalAnchorView | ||
| let snapshot = BrowserWindowPortalRegistry.debugSnapshot(for: browserPanel.webView) | ||
| if shouldBeVisible { | ||
|
|
@@ -10737,7 +10762,7 @@ final class Workspace: Identifiable, ObservableObject { | |
| } | ||
|
|
||
| private func browserPortalVisibilityNeedsFollowUp() -> Bool { | ||
| let visiblePanelIds = renderedVisiblePanelIdsForCurrentLayout() | ||
| let visiblePanelIds = presentationVisiblePanelIdsForCurrentLayout() | ||
|
|
||
| for panel in panels.values { | ||
| guard let browserPanel = panel as? BrowserPanel else { continue } | ||
|
|
@@ -10759,6 +10784,12 @@ final class Workspace: Identifiable, ObservableObject { | |
| return false | ||
| } | ||
|
|
||
| #if DEBUG | ||
| func debugBrowserPortalVisibilityNeedsFollowUpForTesting() -> Bool { | ||
| browserPortalVisibilityNeedsFollowUp() | ||
| } | ||
| #endif | ||
|
|
||
| private func scheduleMovedTerminalRefresh(panelId: UUID) { | ||
| guard terminalPanel(for: panelId) != nil else { return } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isProtectedByWorkspaceVisibilityForResourceCleanupis a cached copy of workspace-level state kept onBrowserPanel. Its correctness depends entirely on every layout transition eventually callingreconcileBrowserPortalVisibilityForCurrentRenderedLayout. There are currently three call sites (layout change,toggleSplitZoom, canvas layout), but if a future transition is added and the reconcile call is missed — or if a panel is moved between windows in a multi-window path that lacks its own reconcile call — the flag remainstrueand the panel is silently exempt from discard indefinitely. The source of truth (renderedVisiblePanelIdsForCurrentLayout) lives inWorkspace, not inBrowserPanel, so the panel can represent the wrong visibility without any compile-time or runtime signal.Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)