Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 73 additions & 16 deletions Sources/Panels/BrowserPanel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2483,7 +2483,7 @@ actor BrowserSearchSuggestionService {
}

/// BrowserPanel provides a WKWebView-based browser panel.
/// All browser panels share a WKProcessPool for cookie sharing.
/// Each browser panel owns a WKProcessPool so WebContent crashes stay isolated.
private enum BrowserInsecureHTTPNavigationIntent {
case currentTab
case newTab
Expand Down Expand Up @@ -2853,9 +2853,6 @@ final class BrowserPortalAnchorView: NSView {

@MainActor
final class BrowserPanel: Panel, ObservableObject {
/// Shared process pool for cookie sharing across all browser panels
private static let sharedProcessPool = WKProcessPool()

/// Popup windows owned by this panel (for lifecycle cleanup)
private var popupControllers: [BrowserPopupWindowController] = []

Expand Down Expand Up @@ -3004,11 +3001,15 @@ final class BrowserPanel: Panel, ObservableObject {
/// The underlying web view
private(set) var webView: WKWebView
private var websiteDataStore: WKWebsiteDataStore
/// Isolate WebKit crashes to this browser panel while keeping popups in the same browsing context.
private let processPool: WKProcessPool
var webViewDidRequestClose: (() -> Void)?

/// Monotonic identity for the current WKWebView instance.
/// Incremented whenever we replace the underlying WKWebView after a process crash.
@Published private(set) var webViewInstanceID: UUID = UUID()
@Published private(set) var hasRecoverableWebContentTermination = false
private var pendingWebContentRecoveryURL: URL?

/// Prevent the omnibar from auto-focusing for a short window after explicit programmatic focus.
/// This avoids races where SwiftUI focus state steals first responder back from WebKit.
Expand Down Expand Up @@ -3646,7 +3647,8 @@ final class BrowserPanel: Panel, ObservableObject {

let replacement = Self.makeWebView(
profileID: profileID,
websiteDataStore: websiteDataStore
websiteDataStore: websiteDataStore,
processPool: processPool
)
replacement.pageZoom = desiredZoom
webViewInstanceID = UUID()
Expand Down Expand Up @@ -3884,12 +3886,14 @@ final class BrowserPanel: Panel, ObservableObject {

private static func makeWebView(
profileID: UUID,
websiteDataStore: WKWebsiteDataStore? = nil
websiteDataStore: WKWebsiteDataStore? = nil,
processPool: WKProcessPool
) -> CmuxWebView {
let config = WKWebViewConfiguration()
configureWebViewConfiguration(
config,
websiteDataStore: websiteDataStore ?? BrowserProfileStore.shared.websiteDataStore(for: profileID)
websiteDataStore: websiteDataStore ?? BrowserProfileStore.shared.websiteDataStore(for: profileID),
processPool: processPool
)

let webView = CmuxWebView(frame: .zero, configuration: config)
Expand All @@ -3909,7 +3913,7 @@ final class BrowserPanel: Panel, ObservableObject {
static func configureWebViewConfiguration(
_ configuration: WKWebViewConfiguration,
websiteDataStore: WKWebsiteDataStore,
processPool: WKProcessPool = BrowserPanel.sharedProcessPool
processPool: WKProcessPool
) {
configuration.processPool = processPool
configuration.mediaTypesRequiringUserActionForPlayback = []
Expand Down Expand Up @@ -4090,10 +4094,13 @@ final class BrowserPanel: Panel, ObservableObject {
self.websiteDataStore = isRemoteWorkspace
? WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? workspaceId)
: BrowserProfileStore.shared.websiteDataStore(for: resolvedProfileID)
let processPool = WKProcessPool()
self.processPool = processPool

let webView = Self.makeWebView(
profileID: resolvedProfileID,
websiteDataStore: websiteDataStore
websiteDataStore: websiteDataStore,
processPool: processPool
)
self.webView = webView
self.insecureHTTPAlertFactory = { NSAlert() }
Expand Down Expand Up @@ -4538,7 +4545,8 @@ final class BrowserPanel: Panel, ObservableObject {

let replacement = Self.makeWebView(
profileID: resolvedProfileID,
websiteDataStore: websiteDataStore
websiteDataStore: websiteDataStore,
processPool: processPool
)
replacement.pageZoom = desiredZoom
webViewInstanceID = UUID()
Expand Down Expand Up @@ -4889,21 +4897,28 @@ final class BrowserPanel: Panel, ObservableObject {
replaceWebViewPreservingState(
from: terminatedWebView,
websiteDataStore: websiteDataStore,
reason: "webcontent_process_terminated"
reason: "webcontent_process_terminated",
waitForManualRecovery: true
)
}

private func replaceWebViewPreservingState(
from oldWebView: WKWebView,
websiteDataStore: WKWebsiteDataStore,
reason: String
reason: String,
waitForManualRecovery: Bool = false
) {
guard oldWebView === webView else { return }

let wasRenderable = shouldRenderWebView
let restoreURL = Self.remoteProxyDisplayURL(for: oldWebView.url) ?? currentURL
let restoreURL = Self.remoteProxyDisplayURL(for: oldWebView.url)
?? currentURL
?? Self.remoteProxyDisplayURL(for: navigationDelegate?.lastAttemptedURL)
?? navigationDelegate?.lastAttemptedURL
?? resolvedCurrentSessionHistoryURL()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
let restoreURLString = restoreURL?.absoluteString
let shouldRestoreURL = wasRenderable && restoreURLString != nil && restoreURLString != blankURLString
let shouldShowManualRecovery = waitForManualRecovery && wasRenderable
let history = sessionNavigationHistorySnapshot()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
let historyCurrentURL = preferredURLStringForOmnibar()
let desiredZoom = max(minPageZoom, min(maxPageZoom, oldWebView.pageZoom))
Expand Down Expand Up @@ -4937,7 +4952,8 @@ final class BrowserPanel: Panel, ObservableObject {

let replacement = Self.makeWebView(
profileID: profileID,
websiteDataStore: websiteDataStore
websiteDataStore: websiteDataStore,
processPool: processPool
)
replacement.pageZoom = desiredZoom
webViewInstanceID = UUID()
Expand All @@ -4957,7 +4973,15 @@ final class BrowserPanel: Panel, ObservableObject {
)
}

if shouldRestoreURL, let restoreURL {
if shouldShowManualRecovery {
pendingWebContentRecoveryURL = restoreURL
hasRecoverableWebContentTermination = true
refreshNavigationAvailability()
} else {
clearWebContentTerminationRecovery()
}

if !shouldShowManualRecovery, shouldRestoreURL, let restoreURL {
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
navigateWithoutInsecureHTTPPrompt(
to: restoreURL,
recordTypedNavigation: false,
Expand All @@ -4981,6 +5005,34 @@ final class BrowserPanel: Panel, ObservableObject {
#endif
}

@discardableResult
func recoverTerminatedWebContent(reason: String = "manual") -> Bool {
guard hasRecoverableWebContentTermination else { return false }
let recoveryURL = pendingWebContentRecoveryURL
clearWebContentTerminationRecovery()
#if DEBUG
cmuxDebugLog(
"browser.webcontent.recover panel=\(id.uuidString.prefix(5)) " +
"reason=\(reason) url=\(recoveryURL?.absoluteString ?? "nil")"
)
#endif
guard let recoveryURL else {
refreshNavigationAvailability()
return true
}
navigateWithoutInsecureHTTPPrompt(
to: recoveryURL,
recordTypedNavigation: false,
preserveRestoredSessionHistory: true
)
return true
}

private func clearWebContentTerminationRecovery() {
pendingWebContentRecoveryURL = nil
hasRecoverableWebContentTermination = false
}

#if DEBUG
func debugSimulateWebContentProcessTermination() {
replaceWebViewAfterContentProcessTermination(for: webView)
Expand Down Expand Up @@ -5527,6 +5579,7 @@ final class BrowserPanel: Panel, ObservableObject {
preserveRestoredSessionHistory: Bool
) {
cancelHiddenWebViewDiscard()
clearWebContentTerminationRecovery()
if !preserveRestoredSessionHistory {
abandonRestoredSessionHistoryIfNeeded()
}
Expand Down Expand Up @@ -5892,7 +5945,8 @@ extension BrowserPanel {

let replacement = Self.makeWebView(
profileID: profileID,
websiteDataStore: websiteDataStore
websiteDataStore: websiteDataStore,
processPool: processPool
Comment thread
cursor[bot] marked this conversation as resolved.
)
webViewInstanceID = UUID()
webView = replacement
Expand Down Expand Up @@ -6114,6 +6168,9 @@ extension BrowserPanel {

/// Reload the current page
func reload() {
if recoverTerminatedWebContent(reason: "reload") {
return
}
if restoreDiscardedWebViewIfNeeded(reason: "reload") {
return
}
Expand Down
31 changes: 31 additions & 0 deletions Sources/Panels/BrowserPanelView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,10 @@ struct BrowserPanelView: View {
return
}

if panel.recoverTerminatedWebContent(reason: "toolbarReload") {
return
}

if currentEventIsCommandPointerActivation {
#if DEBUG
cmuxDebugLog("browser.reload.commandClickDuplicate panel=\(panel.id.uuidString.prefix(5))")
Expand Down Expand Up @@ -1645,10 +1649,37 @@ struct BrowserPanelView: View {
}
}
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
.overlay {
if panel.hasRecoverableWebContentTermination {
webContentRecoveryOverlay
}
}
.layoutPriority(1)
.zIndex(0)
}

private var webContentRecoveryOverlay: some View {
ZStack {
Color(nsColor: browserChromeBackgroundColor)
.opacity(0.92)
Button(action: {
panel.recoverTerminatedWebContent(reason: "overlayButton")
}) {
Label(
String(localized: "browser.error.reload", defaultValue: "Reload"),
systemImage: "arrow.clockwise"
)
.font(.system(size: 13, weight: .medium))
.padding(.horizontal, 6)
}
.buttonStyle(.borderedProminent)
.controlSize(.regular)
.safeHelp(String(localized: "browser.reload", defaultValue: "Reload"))
.accessibilityIdentifier("BrowserWebContentRecoveryButton")
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private func triggerFocusFlashAnimation() {
focusFlashAnimationGeneration &+= 1
let generation = focusFlashAnimationGeneration
Expand Down
12 changes: 12 additions & 0 deletions Sources/Panels/BrowserPopupWindowController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,14 @@ final class BrowserPopupWindowController: NSObject, NSWindowDelegate {
}
}

fileprivate func handleWebContentProcessTermination(for terminatedWebView: WKWebView) {
guard terminatedWebView === webView else { return }
#if DEBUG
cmuxDebugLog("popup.webcontent.terminated depth=\(nestingDepth)")
#endif
closePopup()
}

fileprivate func requestNavigation(_ request: URLRequest, in webView: WKWebView) {
guard let url = request.url else { return }

Expand Down Expand Up @@ -666,6 +674,10 @@ private class PopupNavigationDelegate: NSObject, WKNavigationDelegate {
completionHandler(.performDefaultHandling, nil)
}

func webViewWebContentProcessDidTerminate(_ webView: WKWebView) {
controller?.handleWebContentProcessTermination(for: webView)
}

func webView(_ webView: WKWebView, navigationAction: WKNavigationAction, didBecome download: WKDownload) {
#if DEBUG
cmuxDebugLog("popup.download.didBecome source=navigationAction")
Expand Down
36 changes: 36 additions & 0 deletions cmuxTests/BrowserConfigTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2809,6 +2809,22 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
XCTAssertEqual(panel.pageTitle, "Race A")
}

func testBrowserPanelsUseSeparateWebContentProcessPools() {
let first = BrowserPanel(workspaceId: UUID())
let second = BrowserPanel(workspaceId: UUID())
defer {
first.close()
second.close()
}

XCTAssertFalse(
first.webView.configuration.processPool === second.webView.configuration.processPool
)
XCTAssertTrue(
first.webView.configuration.websiteDataStore === second.webView.configuration.websiteDataStore
)
}

func testWebViewReplacementAfterProcessTerminationUpdatesInstanceIdentity() {
let panel = BrowserPanel(
workspaceId: UUID(),
Expand All @@ -2817,15 +2833,34 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
defer { panel.close() }
let oldWebView = panel.webView
let oldInstanceID = panel.webViewInstanceID
let oldProcessPool = oldWebView.configuration.processPool

panel.debugSimulateWebContentProcessTermination()

XCTAssertFalse(panel.webView === oldWebView)
XCTAssertNotEqual(panel.webViewInstanceID, oldInstanceID)
XCTAssertTrue(panel.webView.configuration.processPool === oldProcessPool)
XCTAssertTrue(panel.hasRecoverableWebContentTermination)
XCTAssertNotNil(panel.webView.navigationDelegate)
XCTAssertNotNil(panel.webView.uiDelegate)
}

func testReloadRecoversTerminatedWebView() {
let panel = BrowserPanel(
workspaceId: UUID(),
initialURL: URL(string: "https://example.com")
)
defer { panel.close() }

panel.debugSimulateWebContentProcessTermination()
XCTAssertTrue(panel.hasRecoverableWebContentTermination)

panel.reload()

XCTAssertFalse(panel.hasRecoverableWebContentTermination)
XCTAssertTrue(panel.shouldRenderWebView)
}

func testWebViewReplacementPreservesEmptyNewTabRenderState() {
let panel = BrowserPanel(workspaceId: UUID())
defer { panel.close() }
Expand All @@ -2834,6 +2869,7 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
panel.debugSimulateWebContentProcessTermination()

XCTAssertFalse(panel.shouldRenderWebView)
XCTAssertFalse(panel.hasRecoverableWebContentTermination)
}

func testResetSidebarContextClearsBrowserPanelsIntoNewTabState() throws {
Expand Down
3 changes: 2 additions & 1 deletion cmuxTests/BrowserPanelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ final class BrowserPanelDiffViewerSchemeTests: XCTestCase {

BrowserPanel.configureWebViewConfiguration(
config,
websiteDataStore: .nonPersistent()
websiteDataStore: .nonPersistent(),
processPool: WKProcessPool()
)

XCTAssertNotNil(config.urlSchemeHandler(forURLScheme: CmuxDiffViewerURLSchemeHandler.scheme))
Expand Down
18 changes: 18 additions & 0 deletions cmuxTests/GhosttyConfigTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1814,6 +1814,24 @@ final class BrowserPanelPopupContextTests: XCTestCase {
)
}

func testFloatingPopupClosesWhenWebContentProcessTerminates() throws {
let panel = BrowserPanel(workspaceId: UUID(), isRemoteWorkspace: false)
defer { panel.close() }
let popupWebView = try XCTUnwrap(
panel.createFloatingPopup(
configuration: WKWebViewConfiguration(),
windowFeatures: WKWindowFeatures()
)
)
let popupWindow = try XCTUnwrap(popupWebView.window)

popupWebView.navigationDelegate?.webViewWebContentProcessDidTerminate?(popupWebView)

XCTAssertNil(popupWebView.navigationDelegate)
XCTAssertNil(popupWebView.uiDelegate)
XCTAssertFalse(popupWindow.isVisible)
}

func testFloatingPopupInheritsRemoteWorkspaceWebsiteDataStore() throws {
let remoteWorkspaceId = UUID()
let panel = BrowserPanel(
Expand Down
Loading