Skip to content
13 changes: 13 additions & 0 deletions Sources/Panels/BrowserPanel+MediaPlayback.swift
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,19 @@ extension BrowserPanel {
)
}

func teardownMediaPlaybackMessageHandler(for webView: WKWebView) {
guard mediaPlaybackMessageHandler != nil else {
resetMediaPlaybackTracking()
return
}
webView.configuration.userContentController.removeScriptMessageHandler(
forName: mediaPlaybackMessageHandlerName,
contentWorld: Self.mediaPlaybackContentWorld
)
mediaPlaybackMessageHandler = nil
resetMediaPlaybackTracking()
}
Comment thread
austinywang marked this conversation as resolved.

/// Applies a per-frame playback report from the injected hook, aggregating
/// across the main frame and any iframes. Reports from a superseded webview
/// generation are dropped.
Expand Down
68 changes: 65 additions & 3 deletions Sources/Panels/BrowserPanel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3547,6 +3547,10 @@ final class BrowserPanel: Panel, ObservableObject {
private var downloadDelegate: BrowserDownloadDelegate?
private var webViewObservers: [NSKeyValueObservation] = []
private var activeDownloadCount: Int = 0
/// Browser close is a one-shot resource teardown: it severs WebKit callbacks
/// and swaps the heavy page web view for an inert placeholder, so repeated
/// close requests from stale SwiftUI/AppKit teardown paths must be ignored.
private var isClosed = false

// Avoid flickering the loading indicator for very fast navigations.
private let minLoadingIndicatorDuration: TimeInterval = 0.35
Expand Down Expand Up @@ -4223,6 +4227,48 @@ final class BrowserPanel: Panel, ObservableObject {
applyMuteState(to: webView, reason: "bindWebView")
}

private func releaseWebViewForTeardown(_ targetWebView: WKWebView, reason: String) {
if let window = targetWebView.window,
Self.responderChainContains(window.firstResponder, target: targetWebView) {
window.makeFirstResponder(nil)
}

BrowserWindowPortalRegistry.detach(webView: targetWebView)
BrowserWindowPortalRegistry.discard(
webView: targetWebView,
source: reason,
preserveCurrentSuperview: false
)
targetWebView.stopLoading()
targetWebView.navigationDelegate = nil
targetWebView.uiDelegate = nil
targetWebView.configuration.userContentController.removeAllUserScripts()
teardownReactGrabMessageHandler(for: targetWebView)
teardownMediaPlaybackMessageHandler(for: targetWebView)
Comment thread
austinywang marked this conversation as resolved.

if let cmuxWebView = targetWebView as? CmuxWebView {
cmuxWebView.onContextMenuDownloadStateChanged = nil
cmuxWebView.onContextMenuOpenLinkInNewTab = nil
cmuxWebView.contextMenuCanMoveTabToNewWorkspace = nil
cmuxWebView.contextMenuMoveTabToNewWorkspace = nil
cmuxWebView.contextMenuLinkURLProvider = nil
cmuxWebView.contextMenuDefaultBrowserOpener = nil
cmuxWebView.allowsFirstResponderAcquisition = false
}

targetWebView.removeFromSuperview()
}
Comment thread
cursor[bot] marked this conversation as resolved.

private func installClosedPlaceholderWebView() {
let replacement = Self.makeWebView(
profileID: profileID,
websiteDataStore: websiteDataStore
)
replacement.allowsFirstResponderAcquisition = false
webViewInstanceID = UUID()
webView = replacement
}

private func configureNavigationDelegateCallbacks() {
guard let navigationDelegate else { return }
let boundWebViewInstanceID = webViewInstanceID
Expand Down Expand Up @@ -5623,6 +5669,9 @@ final class BrowserPanel: Panel, ObservableObject {
}

func close() {
guard !isClosed else { return }
isClosed = true
Comment thread
austinywang marked this conversation as resolved.

cancelHiddenWebViewDiscard()
isClosingWebViewLifecycle = true
refreshWebViewLifecycleState()
Expand All @@ -5645,17 +5694,30 @@ final class BrowserPanel: Panel, ObservableObject {
popup.closePopup()
}

webView.stopLoading()
let closedWebView = webView
releaseWebViewForTeardown(closedWebView, reason: "panel.close")
installClosedPlaceholderWebView()
Comment thread
austinywang marked this conversation as resolved.

isMainFrameProvisionalNavigationActive = false
webView.navigationDelegate = nil
webView.uiDelegate = nil
navigationDelegate = nil
uiDelegate = nil
downloadDelegate = nil
webViewDidRequestClose = nil
webViewObservers.removeAll()
webViewCancellables.removeAll()
faviconTask?.cancel()
faviconTask = nil
Comment thread
austinywang marked this conversation as resolved.
loadingEndWorkItem?.cancel()
loadingEndWorkItem = nil
activeDownloadCount = 0
isDownloading = false
isLoading = false
estimatedProgress = 0
shouldRenderWebView = false
activePortalHostLease = nil
pendingDistinctPortalHostReplacementPaneId = nil
lockedPortalHost = nil
resetReactGrabState(reason: "panel.close")
}

// MARK: - Popup window management
Expand Down
6 changes: 6 additions & 0 deletions Sources/Panels/ReactGrab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ extension BrowserPanel {
webView.configuration.userContentController.add(handler, name: reactGrabMessageHandlerName)
}

func teardownReactGrabMessageHandler(for webView: WKWebView) {
guard reactGrabMessageHandler != nil else { return }
webView.configuration.userContentController.removeScriptMessageHandler(forName: reactGrabMessageHandlerName)
reactGrabMessageHandler = nil
}
Comment thread
austinywang marked this conversation as resolved.

func armReactGrabRoundTrip(returnTo panelId: UUID) {
let token = UUID().uuidString
#if DEBUG
Expand Down
56 changes: 56 additions & 0 deletions cmuxTests/BrowserPanelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,62 @@ final class BrowserPanelInitialNavigationTests: XCTestCase {
XCTAssertFalse(panel.shouldRenderWebViewForSessionSnapshot())
}

func testCloseDetachesPortalHostedWebViewImmediately() {
let panel = BrowserPanel(workspaceId: UUID())
let webView = panel.webView
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 800, height: 600),
styleMask: [.titled],
backing: .buffered,
defer: false
)
let anchorView = NSView(frame: NSRect(x: 0, y: 0, width: 800, height: 600))
window.contentView = anchorView

BrowserWindowPortalRegistry.bind(
webView: webView,
to: anchorView,
visibleInUI: true
)
XCTAssertNotNil(webView.superview, "Test precondition: portal bind should host the web view")
XCTAssertNotNil(
panel.mediaPlaybackMessageHandler,
"Test precondition: binding should install the media playback handler"
)

panel.close()

XCTAssertNil(webView.superview, "Closing a browser panel must release the old WKWebView immediately")
XCTAssertNil(
panel.mediaPlaybackMessageHandler,
"Closing a browser panel must release the media playback message handler"
)
}

func testCloseToleratesAlreadyTornDownScriptHandlers() {
let panel = BrowserPanel(workspaceId: UUID())
let webView = panel.webView
XCTAssertNotNil(
panel.reactGrabMessageHandler,
"Test precondition: binding should install the ReactGrab handler"
)
XCTAssertNotNil(
panel.mediaPlaybackMessageHandler,
"Test precondition: binding should install the media playback handler"
)

panel.teardownReactGrabMessageHandler(for: webView)
panel.teardownMediaPlaybackMessageHandler(for: webView)

XCTAssertNil(panel.reactGrabMessageHandler)
XCTAssertNil(panel.mediaPlaybackMessageHandler)

panel.close()

XCTAssertNil(panel.reactGrabMessageHandler)
XCTAssertNil(panel.mediaPlaybackMessageHandler)
}

func testDiffViewerURLIsNotPersistedForSessionRestore() throws {
let schemeURL = try XCTUnwrap(URL(string: "\(CmuxDiffViewerURLSchemeHandler.scheme)://token/index.html"))
let schemePanel = BrowserPanel(
Expand Down
Loading