Repository navigation
Fix browser freeze after pane split - #1852
Conversation
…g state When a pane split occurs, SwiftUI recreates host views and the portal system rebinds the WKWebView to a new container. However, the bind path never called BrowserWindowPortalRegistry.refresh(), so WebKit's internal rendering state (_exitInWindow/_enterInWindow) was never cycled. This left the WKWebView frozen in the original pane after a split. Add refresh() calls after every portal bind that changes the host, in three code paths: the main update path (shouldBindNow), onDidMoveToWindow, and onGeometryChanged. The refresh is a no-op when no reattach is needed (browserPortalNeedsRenderingStateReattach == false), so normal rendering is unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@busihoward-gpu is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdded refresh calls to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable the changed files summary in the walkthrough.Disable the |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Greptile SummaryThis PR fixes a Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant SwiftUI
participant WebViewRepresentable
participant BrowserWindowPortalRegistry
participant WKWebView
Note over SwiftUI,WKWebView: Pane split — SwiftUI recreates host view
SwiftUI->>WebViewRepresentable: updateNSView (shouldBindNow=true)
WebViewRepresentable->>BrowserWindowPortalRegistry: bind(webView, to: newAnchor)
BrowserWindowPortalRegistry->>WKWebView: reparent to new container
WebViewRepresentable->>BrowserWindowPortalRegistry: refresh(reason: "portalHostBind") ✨ NEW
BrowserWindowPortalRegistry->>WKWebView: _exitInWindow (immediate)
BrowserWindowPortalRegistry->>WKWebView: _enterInWindow (immediate)
BrowserWindowPortalRegistry-->>BrowserWindowPortalRegistry: schedule async pass (+0ms)
BrowserWindowPortalRegistry-->>BrowserWindowPortalRegistry: schedule delayed pass (+30ms)
Note over SwiftUI,WKWebView: Host view enters window
SwiftUI->>WebViewRepresentable: onDidMoveToWindow fires
WebViewRepresentable->>BrowserWindowPortalRegistry: bind(webView, to: anchor)
WebViewRepresentable->>BrowserWindowPortalRegistry: refresh(reason: "portalHostBind.didMoveToWindow") ✨ NEW
BrowserWindowPortalRegistry->>WKWebView: _exitInWindow/_enterInWindow (3 passes)
Note over SwiftUI,WKWebView: Geometry changes with new host
SwiftUI->>WebViewRepresentable: onGeometryChanged fires
WebViewRepresentable->>WebViewRepresentable: check lastPortalHostId != hostId
WebViewRepresentable->>BrowserWindowPortalRegistry: bind(webView, to: anchor)
WebViewRepresentable->>BrowserWindowPortalRegistry: refresh(reason: "portalHostBind.geometryChanged") ✨ NEW
BrowserWindowPortalRegistry->>WKWebView: _exitInWindow/_enterInWindow (3 passes)
Last reviewed commit: "Fix browser freeze a..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| BrowserWindowPortalRegistry.refresh( | ||
| webView: webView, | ||
| reason: "portalHostBind.didMoveToWindow" | ||
| ) |
There was a problem hiding this comment.
onDidMoveToWindow refresh fires unconditionally on every window-entry event
Unlike the onGeometryChanged handler (which guards with coordinator.lastPortalHostId != hostId || !isWebView(boundTo:) before calling bind + refresh), this path calls bind() + refresh() on every invocation that passes the generation/ownership/window guards — even when the host hasn't changed.
refreshHostedWebViewPresentation schedules three layout + browserPortalReattachRenderingState passes (immediate, async, and asyncAfter +30 ms), guarded only by containerView.isHidden. If the container is visible at the moment onDidMoveToWindow fires (e.g., after SwiftUI recreates the host view during a workspace transition while the pane is in view), all three passes run unnecessarily.
Consider adding an early-exit guard consistent with onGeometryChanged:
| BrowserWindowPortalRegistry.refresh( | |
| webView: webView, | |
| reason: "portalHostBind.didMoveToWindow" | |
| ) | |
| Self.installPortalAnchorView(portalAnchorView, in: host) | |
| let hostId = ObjectIdentifier(host) | |
| let needsRebind = coordinator.lastPortalHostId != hostId || | |
| !BrowserWindowPortalRegistry.isWebView(webView, boundTo: portalAnchorView) | |
| BrowserWindowPortalRegistry.bind( | |
| webView: webView, | |
| to: portalAnchorView, | |
| visibleInUI: coordinator.desiredPortalVisibleInUI, | |
| zPriority: coordinator.desiredPortalZPriority | |
| ) | |
| if needsRebind { | |
| BrowserWindowPortalRegistry.refresh( | |
| webView: webView, | |
| reason: "portalHostBind.didMoveToWindow" | |
| ) | |
| } |
This mirrors the guard already present in onGeometryChanged and avoids the three-pass refresh cycle on ordinary re-presentations where the host–anchor binding is already correct.
| BrowserWindowPortalRegistry.refresh( | ||
| webView: webView, | ||
| reason: "portalHostBind.geometryChanged" | ||
| ) |
There was a problem hiding this comment.
onGeometryChanged refresh path
The shouldBindNow path has a detailed comment explaining why refresh() is required (cycling _exitInWindow/_enterInWindow after a pane split reparents the WKWebView). The same rationale applies here — a new hostId means the web view was reparented — but there's no comment, which can make this call look like unnecessary churn to a future reader.
| BrowserWindowPortalRegistry.refresh( | |
| webView: webView, | |
| reason: "portalHostBind.geometryChanged" | |
| ) | |
| BrowserWindowPortalRegistry.refresh( | |
| webView: webView, | |
| reason: "portalHostBind.geometryChanged" | |
| ) | |
| // Force a rendering-state reattach after portal host replacement | |
| // (e.g. after a pane split). Without this, WKWebView can freeze | |
| // because _exitInWindow/_enterInWindow are never cycled when the | |
| // web view is reparented to a new container during bind. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/Panels/BrowserPanelView.swift (1)
6143-6150: Consider centralizing thebind()+refresh()sequence.This fix is correct here, but the same invariant now lives in three call sites. A small helper for portal rebinding would make it much harder for a future bind path to miss the refresh and reintroduce this freeze.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanelView.swift` around lines 6143 - 6150, Extract the repeated bind() + BrowserWindowPortalRegistry.refresh(...) sequence into a single helper and replace the three call sites with it; for example add a method (either a static on BrowserWindowPortalRegistry like bindPortalHost(webView:reason:) or an instance/helper on BrowserPanelView such as rebindPortal(for webView:reason:)) that performs the bind and then calls BrowserWindowPortalRegistry.refresh(webView:reason:), and update all locations that currently call bind() followed by refresh() to call this new helper to ensure the refresh invariant is always applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 6143-6150: Extract the repeated bind() +
BrowserWindowPortalRegistry.refresh(...) sequence into a single helper and
replace the three call sites with it; for example add a method (either a static
on BrowserWindowPortalRegistry like bindPortalHost(webView:reason:) or an
instance/helper on BrowserPanelView such as rebindPortal(for webView:reason:))
that performs the bind and then calls
BrowserWindowPortalRegistry.refresh(webView:reason:), and update all locations
that currently call bind() followed by refresh() to call this new helper to
ensure the refresh invariant is always applied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c74c70e-ffbf-4a73-8984-bd885f002102
📒 Files selected for processing (1)
Sources/Panels/BrowserPanelView.swift
…g state (manaflow-ai#1852) When a pane split occurs, SwiftUI recreates host views and the portal system rebinds the WKWebView to a new container. However, the bind path never called BrowserWindowPortalRegistry.refresh(), so WebKit's internal rendering state (_exitInWindow/_enterInWindow) was never cycled. This left the WKWebView frozen in the original pane after a split. Add refresh() calls after every portal bind that changes the host, in three code paths: the main update path (shouldBindNow), onDidMoveToWindow, and onGeometryChanged. The refresh is a no-op when no reattach is needed (browserPortalNeedsRenderingStateReattach == false), so normal rendering is unaffected. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
BrowserWindowPortalRegistry.refresh()calls after portal host rebinds_exitInWindow/_enterInWindow) was never cycled — causing the web view to freezeRoot cause
In
BrowserPanelView, the three portal bind paths (shouldBindNow,onDidMoveToWindow,onGeometryChanged) callBrowserWindowPortalRegistry.bind()but never follow up withrefresh(). After a split reparents the WKWebView to a new container, the stale rendering state leaves the web view frozen.Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes the built-in browser freezing after a pane split by reattaching
WKWebViewrendering state when the portal rebinds to a new host. Keeps rendering alive after splits without affecting normal behavior.BrowserWindowPortalRegistry.refresh(webView:reason:)after portal host bind in three paths:shouldBindNow,onDidMoveToWindow, andonGeometryChanged._exitInWindow/_enterInWindowafter reparenting to prevent freezes.Written for commit 9323f12. Summary will update on new commits.
Summary by CodeRabbit