From 9ba495a750ba8f22e07e0a50b2a41662356a419e Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Wed, 11 Mar 2026 21:03:22 -0700 Subject: [PATCH 1/3] Add regression tests for browser portal resize refresh --- cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 40fde753..c2ec9996 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -11441,11 +11441,27 @@ final class TerminalWindowPortalLifecycleTests: XCTestCase { final class BrowserWindowPortalLifecycleTests: XCTestCase { private final class TrackingPortalWebView: WKWebView { private(set) var displayIfNeededCount = 0 + private(set) var reattachRenderingStateCount = 0 override func displayIfNeeded() { displayIfNeededCount += 1 super.displayIfNeeded() } + + override func viewDidUnhide() { + reattachRenderingStateCount += 1 + super.viewDidUnhide() + } + + @objc(_enterInWindow) + func cmuxUnitTestEnterInWindow() { + reattachRenderingStateCount += 1 + } + + @objc(_endDeferringViewInWindowChangesSync) + func cmuxUnitTestEndDeferringViewInWindowChangesSync() { + reattachRenderingStateCount += 1 + } } private final class WKInspectorProbeView: NSView {} @@ -11784,6 +11800,138 @@ final class BrowserWindowPortalLifecycleTests: XCTestCase { ) } + func testPortalAnchorResizeDoesNotForceHostedWebViewPresentationRefresh() { + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 520, height: 320), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + realizeWindowLayout(window) + let portal = WindowBrowserPortal(window: window) + guard let contentView = window.contentView else { + XCTFail("Expected content view") + return + } + + let anchor = NSView(frame: NSRect(x: 40, y: 24, width: 220, height: 160)) + contentView.addSubview(anchor) + + let webView = TrackingPortalWebView(frame: .zero, configuration: WKWebViewConfiguration()) + portal.bind(webView: webView, to: anchor, visibleInUI: true) + contentView.layoutSubtreeIfNeeded() + portal.synchronizeWebViewForAnchor(anchor) + advanceAnimations() + + guard let slot = webView.superview as? WindowBrowserSlotView else { + XCTFail("Expected browser slot") + return + } + + let initialDisplayCount = webView.displayIfNeededCount + let initialReattachCount = webView.reattachRenderingStateCount + anchor.frame = NSRect(x: 52, y: 30, width: 248, height: 178) + contentView.layoutSubtreeIfNeeded() + portal.synchronizeWebViewForAnchor(anchor) + advanceAnimations() + + XCTAssertFalse(slot.isHidden, "Anchor resize should keep the portal-hosted browser visible") + XCTAssertEqual(slot.frame.origin.x, 52, accuracy: 0.5) + XCTAssertEqual(slot.frame.origin.y, 30, accuracy: 0.5) + XCTAssertEqual(slot.frame.size.width, 248, accuracy: 0.5) + XCTAssertEqual(slot.frame.size.height, 178, accuracy: 0.5) + XCTAssertGreaterThan( + webView.displayIfNeededCount, + initialDisplayCount, + "Pure anchor geometry updates should still repaint the hosted browser" + ) + XCTAssertEqual( + webView.reattachRenderingStateCount, + initialReattachCount, + "Pure anchor geometry updates should not trigger the WebKit reattach path" + ) + } + + func testExternalSplitResizeDoesNotForceHostedWebViewPresentationRefresh() { + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 360), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + defer { window.orderOut(nil) } + realizeWindowLayout(window) + let portal = WindowBrowserPortal(window: window) + guard let contentView = window.contentView else { + XCTFail("Expected content view") + return + } + + let splitView = NSSplitView(frame: contentView.bounds) + splitView.autoresizingMask = [.width, .height] + splitView.isVertical = true + + let leadingPane = NSView( + frame: NSRect(x: 0, y: 0, width: 220, height: contentView.bounds.height) + ) + leadingPane.autoresizingMask = [.height] + let trailingPane = NSView( + frame: NSRect( + x: 221, + y: 0, + width: contentView.bounds.width - 221, + height: contentView.bounds.height + ) + ) + trailingPane.autoresizingMask = [.width, .height] + splitView.addSubview(leadingPane) + splitView.addSubview(trailingPane) + contentView.addSubview(splitView) + splitView.adjustSubviews() + + let anchor = NSView(frame: trailingPane.bounds.insetBy(dx: 12, dy: 12)) + anchor.autoresizingMask = [.width, .height] + trailingPane.addSubview(anchor) + + let webView = TrackingPortalWebView(frame: .zero, configuration: WKWebViewConfiguration()) + portal.bind(webView: webView, to: anchor, visibleInUI: true) + contentView.layoutSubtreeIfNeeded() + portal.synchronizeWebViewForAnchor(anchor) + advanceAnimations() + + guard let slot = webView.superview as? WindowBrowserSlotView else { + XCTFail("Expected browser slot") + return + } + + let initialDisplayCount = webView.displayIfNeededCount + let initialReattachCount = webView.reattachRenderingStateCount + let initialWidth = slot.frame.width + + splitView.setPosition(280, ofDividerAt: 0) + contentView.layoutSubtreeIfNeeded() + NotificationCenter.default.post(name: NSSplitView.didResizeSubviewsNotification, object: splitView) + advanceAnimations() + + XCTAssertFalse(slot.isHidden, "App split resize should keep the browser slot visible") + XCTAssertLessThan( + slot.frame.width, + initialWidth, + "Moving the app split divider should shrink the hosted browser slot" + ) + XCTAssertGreaterThan( + webView.displayIfNeededCount, + initialDisplayCount, + "External split resize should still repaint the hosted browser" + ) + XCTAssertEqual( + webView.reattachRenderingStateCount, + initialReattachCount, + "External split resize should not trigger the WebKit reattach path" + ) + } + func testPortalSyncRepairsBottomDockedInspectorOverflowedPageFrame() { let window = NSWindow( contentRect: NSRect(x: 0, y: 0, width: 520, height: 320), From 7c296529715dbf7f69e5e5cbe52055ec85294f89 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Wed, 11 Mar 2026 21:03:26 -0700 Subject: [PATCH 2/3] Avoid browser portal refresh on geometry churn --- Sources/BrowserWindowPortal.swift | 117 +++++++++++++++++++++++++----- 1 file changed, 99 insertions(+), 18 deletions(-) diff --git a/Sources/BrowserWindowPortal.swift b/Sources/BrowserWindowPortal.swift index 6bf9a18d..2d85866c 100644 --- a/Sources/BrowserWindowPortal.swift +++ b/Sources/BrowserWindowPortal.swift @@ -2101,7 +2101,7 @@ final class WindowBrowserPortal: NSObject { let containerView = entry.containerView, !containerView.isHidden else { continue } guard webView.superview === containerView else { continue } - refreshHostedWebViewPresentation( + invalidateHostedWebViewGeometry( webView, in: containerView, reason: "externalGeometry" @@ -2378,14 +2378,16 @@ final class WindowBrowserPortal: NSObject { _ webView: WKWebView, in containerView: WindowBrowserSlotView, reason: String, - phase: String + phase: String, + reattachRenderingState: Bool ) { guard !containerView.isHidden else { return } guard !containerView.isHostedInspectorDividerDragActive else { #if DEBUG dlog( "browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " + - "container=\(browserPortalDebugToken(containerView)) reason=\(reason) phase=\(phase) drag=1" + "container=\(browserPortalDebugToken(containerView)) reason=\(reason) phase=\(phase) " + + "drag=1 reattach=\(reattachRenderingState ? 1 : 0)" ) #endif return @@ -2414,19 +2416,40 @@ final class WindowBrowserPortal: NSObject { scrollView.displayIfNeeded() } webView.layoutSubtreeIfNeeded() - webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") - containerView.displayIfNeeded() - webView.displayIfNeeded() - (webView.window ?? hostView.window)?.displayIfNeeded() + if reattachRenderingState { + webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") + containerView.displayIfNeeded() + webView.displayIfNeeded() + (webView.window ?? hostView.window)?.displayIfNeeded() + } else { + containerView.displayIfNeeded() + webView.displayIfNeeded() + (webView.window ?? hostView.window)?.displayIfNeeded() + } #if DEBUG dlog( - "browser.portal.refresh web=\(browserPortalDebugToken(webView)) " + + "\(reattachRenderingState ? "browser.portal.refresh" : "browser.portal.invalidate") " + + "web=\(browserPortalDebugToken(webView)) " + "container=\(browserPortalDebugToken(containerView)) reason=\(reason) " + "phase=\(phase) frame=\(browserPortalDebugFrame(containerView.frame))" ) #endif } + private func invalidateHostedWebViewGeometry( + _ webView: WKWebView, + in containerView: WindowBrowserSlotView, + reason: String + ) { + runHostedWebViewRefreshPass( + webView, + in: containerView, + reason: reason, + phase: "geometry", + reattachRenderingState: false + ) + } + private func refreshHostedWebViewPresentation( _ webView: WKWebView, in containerView: WindowBrowserSlotView, @@ -2434,14 +2457,21 @@ final class WindowBrowserPortal: NSObject { ) { guard !containerView.isHidden else { return } - runHostedWebViewRefreshPass(webView, in: containerView, reason: reason, phase: "immediate") + runHostedWebViewRefreshPass( + webView, + in: containerView, + reason: reason, + phase: "immediate", + reattachRenderingState: true + ) DispatchQueue.main.async { [weak self, weak webView, weak containerView] in guard let self, let webView, let containerView else { return } self.runHostedWebViewRefreshPass( webView, in: containerView, reason: reason, - phase: "async" + phase: "async", + reattachRenderingState: true ) } DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { [weak self, weak webView, weak containerView] in @@ -2450,11 +2480,45 @@ final class WindowBrowserPortal: NSObject { webView, in: containerView, reason: reason, - phase: "delayed" + phase: "delayed", + reattachRenderingState: true ) } } + private enum HostedWebViewPresentationUpdateKind { + case none + case geometryOnly + case refresh + + private static let geometryOnlyReasons: Set = [ + "frame", + "bounds", + "webFrame", + "webFrameBottomDock", + ] + + private static let refreshReasons: Set = [ + "syncAttachContainer", + "syncAttachWebView", + "reveal", + "transientRecovery", + "anchor", + ] + + static func resolve(refreshReasons: [String]) -> Self { + guard !refreshReasons.isEmpty else { return .none } + let reasonSet = Set(refreshReasons) + if !reasonSet.isDisjoint(with: Self.refreshReasons) { + return .refresh + } + if reasonSet.isSubset(of: Self.geometryOnlyReasons) { + return .geometryOnly + } + return .refresh + } + } + private func moveWebKitRelatedSubviewsIfNeeded( from sourceSuperview: NSView, to containerView: WindowBrowserSlotView, @@ -3272,8 +3336,13 @@ final class WindowBrowserPortal: NSObject { let hostedInspectorAdjustedDuringSync = containerOwnsWebView && hostView.reapplyHostedInspectorDividerIfNeeded(in: containerView, reason: "portal.sync") - if !shouldHide, containerOwnsWebView, !refreshReasons.isEmpty { - if hostedInspectorAdjustedDuringSync && !recoveredFromTransientGeometry { + let presentationUpdateKind = HostedWebViewPresentationUpdateKind.resolve( + refreshReasons: refreshReasons + ) + if !shouldHide, containerOwnsWebView, presentationUpdateKind != .none { + if presentationUpdateKind == .refresh && + hostedInspectorAdjustedDuringSync && + !recoveredFromTransientGeometry { #if DEBUG dlog( "browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " + @@ -3282,11 +3351,23 @@ final class WindowBrowserPortal: NSObject { ) #endif } else { - refreshHostedWebViewPresentation( - webView, - in: containerView, - reason: "\(source):" + refreshReasons.joined(separator: ",") - ) + let refreshReason = "\(source):" + refreshReasons.joined(separator: ",") + switch presentationUpdateKind { + case .none: + break + case .geometryOnly: + invalidateHostedWebViewGeometry( + webView, + in: containerView, + reason: refreshReason + ) + case .refresh: + refreshHostedWebViewPresentation( + webView, + in: containerView, + reason: refreshReason + ) + } } } if containerOwnsWebView, !hostedInspectorAdjustedDuringSync { From 27e598ca5a9d0ebd0a12b2c5b694ee59b384ef89 Mon Sep 17 00:00:00 2001 From: austinpower1258 Date: Wed, 11 Mar 2026 21:13:25 -0700 Subject: [PATCH 3/3] Address CI blockers and review follow-ups --- .github/workflows/ci-macos-compat.yml | 3 ++- .github/workflows/ci.yml | 15 +++++++++------ Sources/BrowserWindowPortal.swift | 18 +++++++----------- cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 17 ++++++++++++----- 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci-macos-compat.yml b/.github/workflows/ci-macos-compat.yml index b6ba18dc..250fe061 100644 --- a/.github/workflows/ci-macos-compat.yml +++ b/.github/workflows/ci-macos-compat.yml @@ -17,8 +17,9 @@ jobs: runs-on: ${{ matrix.os }} steps: - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} submodules: recursive - name: Select Xcode diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7315d8ed..967a4758 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Validate Depot runner guards run: ./tests/test_ci_self_hosted_guard.sh @@ -35,10 +35,13 @@ jobs: working-directory: web steps: - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Setup Bun - uses: oven-sh/setup-bun@3d267786b128fe76c2f16a390aa2448b815359f3 # v2 + - name: Install Bun + run: | + set -euo pipefail + curl -fsSL https://bun.sh/install | bash -s -- bun-v1.3.10 + echo "$HOME/.bun/bin" >> "$GITHUB_PATH" - name: Install dependencies run: bun install --frozen-lockfile @@ -50,7 +53,7 @@ jobs: runs-on: macos-15 steps: - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: recursive @@ -157,7 +160,7 @@ jobs: runs-on: depot-macos-latest steps: - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: recursive diff --git a/Sources/BrowserWindowPortal.swift b/Sources/BrowserWindowPortal.swift index 2d85866c..f1a9830f 100644 --- a/Sources/BrowserWindowPortal.swift +++ b/Sources/BrowserWindowPortal.swift @@ -2418,14 +2418,10 @@ final class WindowBrowserPortal: NSObject { webView.layoutSubtreeIfNeeded() if reattachRenderingState { webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)") - containerView.displayIfNeeded() - webView.displayIfNeeded() - (webView.window ?? hostView.window)?.displayIfNeeded() - } else { - containerView.displayIfNeeded() - webView.displayIfNeeded() - (webView.window ?? hostView.window)?.displayIfNeeded() } + containerView.displayIfNeeded() + webView.displayIfNeeded() + (webView.window ?? hostView.window)?.displayIfNeeded() #if DEBUG dlog( "\(reattachRenderingState ? "browser.portal.refresh" : "browser.portal.invalidate") " + @@ -2506,9 +2502,9 @@ final class WindowBrowserPortal: NSObject { "anchor", ] - static func resolve(refreshReasons: [String]) -> Self { - guard !refreshReasons.isEmpty else { return .none } - let reasonSet = Set(refreshReasons) + static func resolve(reasons: [String]) -> Self { + guard !reasons.isEmpty else { return .none } + let reasonSet = Set(reasons) if !reasonSet.isDisjoint(with: Self.refreshReasons) { return .refresh } @@ -3337,7 +3333,7 @@ final class WindowBrowserPortal: NSObject { containerOwnsWebView && hostView.reapplyHostedInspectorDividerIfNeeded(in: containerView, reason: "portal.sync") let presentationUpdateKind = HostedWebViewPresentationUpdateKind.resolve( - refreshReasons: refreshReasons + reasons: refreshReasons ) if !shouldHide, containerOwnsWebView, presentationUpdateKind != .none { if presentationUpdateKind == .refresh && diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index c2ec9996..8fb6ee1f 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -11448,11 +11448,6 @@ final class BrowserWindowPortalLifecycleTests: XCTestCase { super.displayIfNeeded() } - override func viewDidUnhide() { - reattachRenderingStateCount += 1 - super.viewDidUnhide() - } - @objc(_enterInWindow) func cmuxUnitTestEnterInWindow() { reattachRenderingStateCount += 1 @@ -12219,22 +12214,34 @@ final class BrowserWindowPortalLifecycleTests: XCTestCase { portal.synchronizeWebViewForAnchor(anchor) advanceAnimations() let initialDisplayCount = webView.displayIfNeededCount + let initialReattachCount = webView.reattachRenderingStateCount portal.updateEntryVisibility(forWebViewId: ObjectIdentifier(webView), visibleInUI: false, zPriority: 0) portal.synchronizeWebViewForAnchor(anchor) advanceAnimations() let hiddenDisplayCount = webView.displayIfNeededCount + let hiddenReattachCount = webView.reattachRenderingStateCount portal.updateEntryVisibility(forWebViewId: ObjectIdentifier(webView), visibleInUI: true, zPriority: 0) portal.synchronizeWebViewForAnchor(anchor) advanceAnimations() XCTAssertGreaterThanOrEqual(hiddenDisplayCount, initialDisplayCount) + XCTAssertEqual( + hiddenReattachCount, + initialReattachCount, + "Hiding a portal-hosted browser should not itself trigger the WebKit reattach path" + ) XCTAssertGreaterThan( webView.displayIfNeededCount, hiddenDisplayCount, "Revealing an existing portal-hosted browser should refresh WebKit presentation immediately" ) + XCTAssertGreaterThan( + webView.reattachRenderingStateCount, + hiddenReattachCount, + "Revealing an existing portal-hosted browser should trigger the WebKit reattach path" + ) } func testVisiblePortalEntryHidesWithoutDetachingDuringTransientAnchorRemovalUntilRebind() {