Merge pull request #1224 from manaflow-ai/issue-1223-cmd-d-browser-refresh

Fix browser pane refreshes on split and resize churn

.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

.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
 

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,36 @@ final class WindowBrowserPortal: NSObject {
             scrollView.displayIfNeeded()
         }
         webView.layoutSubtreeIfNeeded()
-        webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)")
+        if reattachRenderingState {
+            webView.browserPortalReattachRenderingState(reason: "\(reason):\(phase)")
+        }
         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 +2453,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 +2476,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<String> = [
+            "frame",
+            "bounds",
+            "webFrame",
+            "webFrameBottomDock",
+        ]
+
+        private static let refreshReasons: Set<String> = [
+            "syncAttachContainer",
+            "syncAttachWebView",
+            "reveal",
+            "transientRecovery",
+            "anchor",
+        ]
+
+        static func resolve(reasons: [String]) -> Self {
+            guard !reasons.isEmpty else { return .none }
+            let reasonSet = Set(reasons)
+            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 +3332,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(
+            reasons: refreshReasons
+        )
+        if !shouldHide, containerOwnsWebView, presentationUpdateKind != .none {
+            if presentationUpdateKind == .refresh &&
+                hostedInspectorAdjustedDuringSync &&
+                !recoveredFromTransientGeometry {
 #if DEBUG
                 dlog(
                     "browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " +
@@ -3282,11 +3347,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 {

cmuxTests/CmuxWebViewKeyEquivalentTests.swift

@@ -11441,11 +11441,22 @@ 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()
         }
+
+        @objc(_enterInWindow)
+        func cmuxUnitTestEnterInWindow() {
+            reattachRenderingStateCount += 1
+        }
+
+        @objc(_endDeferringViewInWindowChangesSync)
+        func cmuxUnitTestEndDeferringViewInWindowChangesSync() {
+            reattachRenderingStateCount += 1
+        }
     }
 
     private final class WKInspectorProbeView: NSView {}
@@ -11784,6 +11795,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),
@@ -12071,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() {