fix(favicon): prevent swap on pool reuse and add fallback for title bars
Bug 1: Favicons swapped between sites when closing panes quickly because GLib notify::favicon signals survived WebView pool reuse. The old page's queued signal would fire with the new pane's callback, delivering the wrong favicon. Fixed by: - Disconnecting GLib signals in ResetForPoolReuse() - Reconnecting signals in pool Acquire() - Adding generation counter to detect stale closures - Verifying WebView identity in onFaviconChanged before updating Bug 2: Subpath URLs (e.g. github.com/bnema) showed no favicon in stacked title bars because they relied solely on cache preload + WebKit signal. Unlike the omnibox, there was no fallback fetch. Fixed by adding resolveCommittedFavicon() on load commit which checks WebKit's existing favicon first, then falls back to the full GetOrFetch pipeline (cache + WebKit DB + DuckDuckGo API).
- commit
- 4b76d6b024653f36548581b3b285b1e8fb4365ad
- author
- brice · · 2026-03-13 05:47 UTC
- parent
- b4c833b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +118 -19
M internal/infrastructure/webkit/pool.go +1 -0
| @@ -171,6 +171,7 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||
| 171 | 171 | if p.filterApplier != nil { | |
| 172 | 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | |
| 173 | 173 | } | |
| 174 | + | wv.ReconnectSignals() | |
| 174 | 175 | p.acquireCount.Add(1) | |
| 175 | 176 | p.acquirePoolHits.Add(1) | |
| 176 | 177 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | |
| @@ -171,6 +171,7 @@ func (p *WebViewPool) Acquire(ctx context.Context) (*WebView, error) { | |||||
| 171 | if p.filterApplier != nil { | 171 | if p.filterApplier != nil { | ||
| 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | 172 | p.filterApplier.ApplyTo(ctx, wv.ucm) | ||
| 173 | } | 173 | } | ||
| 174 | + | wv.ReconnectSignals() | |||
| 174 | p.acquireCount.Add(1) | 175 | p.acquireCount.Add(1) | ||
| 175 | p.acquirePoolHits.Add(1) | 176 | p.acquirePoolHits.Add(1) | ||
| 176 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | 177 | p.acquireTotalNanos.Add(time.Since(start).Nanoseconds()) | ||
M internal/infrastructure/webkit/webview.go +27 -1
| @@ -137,7 +137,8 @@ type WebView struct { | |||
| 137 | 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | |
| 138 | 138 | ||
| 139 | 139 | // Signal handler IDs for disconnection | |
| 140 | - | signalIDs []uintptr | |
| 140 | + | signalIDs []uintptr | |
| 141 | + | generation atomic.Uint64 | |
| 141 | 142 | ||
| 142 | 143 | // Callbacks (set by UI layer) | |
| 143 | 144 | OnLoadChanged func(LoadEvent) | |
| @@ -1405,6 +1406,12 @@ func (wv *WebView) Favicon() *gdk.Texture { | |||
| 1405 | 1406 | return wv.inner.GetFavicon() | |
| 1406 | 1407 | } | |
| 1407 | 1408 | ||
| 1409 | + | // Generation returns the current reuse generation of this WebView. | |
| 1410 | + | // Used by callback holders to detect stale closures after pool reuse. | |
| 1411 | + | func (wv *WebView) Generation() uint64 { | |
| 1412 | + | return wv.generation.Load() | |
| 1413 | + | } | |
| 1414 | + | ||
| 1408 | 1415 | // IsLoading returns true if a page is currently loading. | |
| 1409 | 1416 | func (wv *WebView) IsLoading() bool { | |
| 1410 | 1417 | wv.mu.RLock() | |
| @@ -1623,6 +1630,20 @@ func (wv *WebView) DisconnectSignals() { | |||
| 1623 | 1630 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | |
| 1624 | 1631 | } | |
| 1625 | 1632 | ||
| 1633 | + | // ReconnectSignals re-establishes GLib signal handlers after pool reuse. | |
| 1634 | + | // Must be called after acquiring a WebView from the pool and before setting up callbacks. | |
| 1635 | + | func (wv *WebView) ReconnectSignals() { | |
| 1636 | + | if wv == nil || wv.destroyed.Load() || wv.inner == nil { | |
| 1637 | + | return | |
| 1638 | + | } | |
| 1639 | + | // Only reconnect if signals were previously disconnected | |
| 1640 | + | if len(wv.signalIDs) > 0 { | |
| 1641 | + | return // Already connected | |
| 1642 | + | } | |
| 1643 | + | wv.connectSignals() | |
| 1644 | + | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals reconnected after pool reuse") | |
| 1645 | + | } | |
| 1646 | + | ||
| 1626 | 1647 | // Destroy cleans up the WebView resources and terminates the web process. | |
| 1627 | 1648 | // This must be called when a WebView is no longer needed to free GPU resources, | |
| 1628 | 1649 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | |
| @@ -1697,6 +1718,11 @@ func (wv *WebView) ResetForPoolReuse() { | |||
| 1697 | 1718 | if wv == nil || wv.destroyed.Load() { | |
| 1698 | 1719 | return | |
| 1699 | 1720 | } | |
| 1721 | + | wv.generation.Add(1) | |
| 1722 | + | ||
| 1723 | + | // Disconnect GLib signals to prevent stale callbacks from firing on reused WebView. | |
| 1724 | + | // They will be reconnected when the WebView is re-acquired from the pool. | |
| 1725 | + | wv.DisconnectSignals() | |
| 1700 | 1726 | ||
| 1701 | 1727 | wv.OnLoadChanged = nil | |
| 1702 | 1728 | wv.OnTitleChanged = nil | |
| @@ -137,7 +137,8 @@ type WebView struct { | |||||
| 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | 137 | lastProgressUpdate atomic.Int64 // Unix nanoseconds | ||
| 138 | 138 | ||||
| 139 | // Signal handler IDs for disconnection | 139 | // Signal handler IDs for disconnection | ||
| 140 | - | signalIDs []uintptr | 140 | + | signalIDs []uintptr |
| 141 | + | generation atomic.Uint64 | |||
| 141 | 142 | ||||
| 142 | // Callbacks (set by UI layer) | 143 | // Callbacks (set by UI layer) | ||
| 143 | OnLoadChanged func(LoadEvent) | 144 | OnLoadChanged func(LoadEvent) | ||
| @@ -1405,6 +1406,12 @@ func (wv *WebView) Favicon() *gdk.Texture { | |||||
| 1405 | return wv.inner.GetFavicon() | 1406 | return wv.inner.GetFavicon() | ||
| 1406 | } | 1407 | } | ||
| 1407 | 1408 | ||||
| 1409 | + | // Generation returns the current reuse generation of this WebView. | |||
| 1410 | + | // Used by callback holders to detect stale closures after pool reuse. | |||
| 1411 | + | func (wv *WebView) Generation() uint64 { | |||
| 1412 | + | return wv.generation.Load() | |||
| 1413 | + | } | |||
| 1414 | + | ||||
| 1408 | // IsLoading returns true if a page is currently loading. | 1415 | // IsLoading returns true if a page is currently loading. | ||
| 1409 | func (wv *WebView) IsLoading() bool { | 1416 | func (wv *WebView) IsLoading() bool { | ||
| 1410 | wv.mu.RLock() | 1417 | wv.mu.RLock() | ||
| @@ -1623,6 +1630,20 @@ func (wv *WebView) DisconnectSignals() { | |||||
| 1623 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | 1630 | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals disconnected") | ||
| 1624 | } | 1631 | } | ||
| 1625 | 1632 | ||||
| 1633 | + | // ReconnectSignals re-establishes GLib signal handlers after pool reuse. | |||
| 1634 | + | // Must be called after acquiring a WebView from the pool and before setting up callbacks. | |||
| 1635 | + | func (wv *WebView) ReconnectSignals() { | |||
| 1636 | + | if wv == nil || wv.destroyed.Load() || wv.inner == nil { | |||
| 1637 | + | return | |||
| 1638 | + | } | |||
| 1639 | + | // Only reconnect if signals were previously disconnected | |||
| 1640 | + | if len(wv.signalIDs) > 0 { | |||
| 1641 | + | return // Already connected | |||
| 1642 | + | } | |||
| 1643 | + | wv.connectSignals() | |||
| 1644 | + | wv.logger.Debug().Uint64("id", uint64(wv.id)).Msg("signals reconnected after pool reuse") | |||
| 1645 | + | } | |||
| 1646 | + | ||||
| 1626 | // Destroy cleans up the WebView resources and terminates the web process. | 1647 | // Destroy cleans up the WebView resources and terminates the web process. | ||
| 1627 | // This must be called when a WebView is no longer needed to free GPU resources, | 1648 | // This must be called when a WebView is no longer needed to free GPU resources, | ||
| 1628 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | 1649 | // VA-API decoder contexts, and DMA-BUF buffers held by the web process. | ||
| @@ -1697,6 +1718,11 @@ func (wv *WebView) ResetForPoolReuse() { | |||||
| 1697 | if wv == nil || wv.destroyed.Load() { | 1718 | if wv == nil || wv.destroyed.Load() { | ||
| 1698 | return | 1719 | return | ||
| 1699 | } | 1720 | } | ||
| 1721 | + | wv.generation.Add(1) | |||
| 1722 | + | ||||
| 1723 | + | // Disconnect GLib signals to prevent stale callbacks from firing on reused WebView. | |||
| 1724 | + | // They will be reconnected when the WebView is re-acquired from the pool. | |||
| 1725 | + | wv.DisconnectSignals() | |||
| 1700 | 1726 | ||||
| 1701 | wv.OnLoadChanged = nil | 1727 | wv.OnLoadChanged = nil | ||
| 1702 | wv.OnTitleChanged = nil | 1728 | wv.OnTitleChanged = nil | ||
M internal/ui/coordinator/content.go +90 -18
| @@ -982,15 +982,19 @@ func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity | |||
| 982 | 982 | } | |
| 983 | 983 | ||
| 984 | 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | |
| 985 | - | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | |
| 985 | + | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, emittingWV *webkit.WebView, favicon *gdk.Texture) { | |
| 986 | 986 | log := logging.FromContext(ctx) | |
| 987 | 987 | ||
| 988 | - | // Get current URI to extract domain for caching | |
| 989 | - | wv := c.getWebViewLocked(paneID) | |
| 990 | - | if wv == nil { | |
| 988 | + | // Verify this WebView is still bound to the expected pane | |
| 989 | + | currentWV := c.getWebViewLocked(paneID) | |
| 990 | + | if currentWV == nil || currentWV != emittingWV { | |
| 991 | + | log.Debug(). | |
| 992 | + | Str("pane_id", string(paneID)). | |
| 993 | + | Msg("ignoring favicon change from unbound webview") | |
| 991 | 994 | return | |
| 992 | 995 | } | |
| 993 | - | uri := wv.URI() | |
| 996 | + | ||
| 997 | + | uri := emittingWV.URI() | |
| 994 | 998 | ||
| 995 | 999 | // Update favicon cache with domain key (handles cross-domain redirects) | |
| 996 | 1000 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | |
| @@ -1001,16 +1005,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||
| 1001 | 1005 | } | |
| 1002 | 1006 | ||
| 1003 | 1007 | // Update StackedView favicon if this pane is in a stack | |
| 1004 | - | _, wsView := c.getActiveWS() | |
| 1005 | - | if wsView != nil { | |
| 1006 | - | tr := wsView.TreeRenderer() | |
| 1007 | - | if tr != nil { | |
| 1008 | - | stackedView := tr.GetStackedViewForPane(string(paneID)) | |
| 1009 | - | if stackedView != nil { | |
| 1010 | - | c.updateStackedPaneFavicon(ctx, stackedView, paneID, favicon) | |
| 1011 | - | } | |
| 1012 | - | } | |
| 1013 | - | } | |
| 1008 | + | c.updateStackedFaviconForPane(ctx, paneID, favicon) | |
| 1014 | 1009 | ||
| 1015 | 1010 | log.Debug(). | |
| 1016 | 1011 | Str("pane_id", string(paneID)). | |
| @@ -1042,6 +1037,72 @@ func (c *ContentCoordinator) updateStackedPaneFavicon( | |||
| 1042 | 1037 | } | |
| 1043 | 1038 | } | |
| 1044 | 1039 | ||
| 1040 | + | // updateStackedFaviconForPane updates the stacked title bar favicon for a pane. | |
| 1041 | + | func (c *ContentCoordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture *gdk.Texture) { | |
| 1042 | + | _, wsView := c.getActiveWS() | |
| 1043 | + | if wsView == nil { | |
| 1044 | + | return | |
| 1045 | + | } | |
| 1046 | + | tr := wsView.TreeRenderer() | |
| 1047 | + | if tr == nil { | |
| 1048 | + | return | |
| 1049 | + | } | |
| 1050 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |
| 1051 | + | c.updateStackedPaneFavicon(ctx, sv, paneID, texture) | |
| 1052 | + | } | |
| 1053 | + | } | |
| 1054 | + | ||
| 1055 | + | // resolveCommittedFavicon ensures the stacked title bar has a favicon after navigation commits. | |
| 1056 | + | // First checks if WebKit already has a favicon for the page (common for subpath URLs). | |
| 1057 | + | // Falls back to the full GetOrFetch pipeline (cache + WebKit DB + DuckDuckGo API). | |
| 1058 | + | // This closes the gap where title bars relied solely on the notify::favicon signal. | |
| 1059 | + | func (c *ContentCoordinator) resolveCommittedFavicon(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1060 | + | if c.faviconAdapter == nil || wv == nil { | |
| 1061 | + | return | |
| 1062 | + | } | |
| 1063 | + | ||
| 1064 | + | uri := wv.URI() | |
| 1065 | + | if uri == "" || strings.HasPrefix(uri, "about:") || strings.HasPrefix(uri, "dumb://") { | |
| 1066 | + | return | |
| 1067 | + | } | |
| 1068 | + | ||
| 1069 | + | log := logging.FromContext(ctx) | |
| 1070 | + | ||
| 1071 | + | // Check if we already have a texture cached for this domain | |
| 1072 | + | if texture := c.faviconAdapter.GetTextureByURL(uri); texture != nil { | |
| 1073 | + | // Already cached - just ensure the stacked view is updated | |
| 1074 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |
| 1075 | + | return | |
| 1076 | + | } | |
| 1077 | + | ||
| 1078 | + | // Check if WebKit already has a favicon for this page | |
| 1079 | + | if icon := wv.Favicon(); icon != nil { | |
| 1080 | + | log.Debug().Str("pane_id", string(paneID)).Str("uri", uri).Msg("using existing WebKit favicon for committed page") | |
| 1081 | + | // Store and update through the normal path | |
| 1082 | + | c.navOriginMu.RLock() | |
| 1083 | + | originURL := c.navOrigins[paneID] | |
| 1084 | + | c.navOriginMu.RUnlock() | |
| 1085 | + | c.faviconAdapter.StoreFromWebKitWithOrigin(ctx, uri, originURL, icon) | |
| 1086 | + | c.updateStackedFaviconForPane(ctx, paneID, icon) | |
| 1087 | + | return | |
| 1088 | + | } | |
| 1089 | + | ||
| 1090 | + | // Fall back to GetOrFetch (checks service cache, WebKit DB, then DuckDuckGo API) | |
| 1091 | + | // Capture current generation to guard against stale callbacks | |
| 1092 | + | gen := wv.Generation() | |
| 1093 | + | c.faviconAdapter.GetOrFetch(ctx, uri, func(texture *gdk.Texture) { | |
| 1094 | + | // Verify WebView is still bound to pane and hasn't been reused | |
| 1095 | + | if wv.Generation() != gen { | |
| 1096 | + | return | |
| 1097 | + | } | |
| 1098 | + | currentWV := c.getWebViewLocked(paneID) | |
| 1099 | + | if currentWV != wv { | |
| 1100 | + | return | |
| 1101 | + | } | |
| 1102 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |
| 1103 | + | }) | |
| 1104 | + | } | |
| 1105 | + | ||
| 1045 | 1106 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | |
| 1046 | 1107 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | |
| 1047 | 1108 | return c.faviconAdapter | |
| @@ -1087,7 +1148,11 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||
| 1087 | 1148 | if c.getActiveWS == nil { | |
| 1088 | 1149 | return false | |
| 1089 | 1150 | } | |
| 1090 | - | // A nil texture triggers the default icon fallback, which avoids stale favicons. | |
| 1151 | + | // Only update if we found a cached texture. | |
| 1152 | + | // If nil, let resolveCommittedFavicon handle it after load commits. | |
| 1153 | + | if texture == nil { | |
| 1154 | + | return false | |
| 1155 | + | } | |
| 1091 | 1156 | _, wsView := c.getActiveWS() | |
| 1092 | 1157 | if wsView == nil { | |
| 1093 | 1158 | return false | |
| @@ -1190,6 +1255,9 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||
| 1190 | 1255 | c.onHistoryRecord(ctx, paneID, uri) | |
| 1191 | 1256 | } | |
| 1192 | 1257 | ||
| 1258 | + | // Resolve favicon for stacked title bar (fills gap when WebKit doesn't emit notify::favicon) | |
| 1259 | + | c.resolveCommittedFavicon(ctx, paneID, wv) | |
| 1260 | + | ||
| 1193 | 1261 | // Notify active pane navigation for permission indicator reset. | |
| 1194 | 1262 | c.notifyActiveNavigation(paneID, uri) | |
| 1195 | 1263 | ||
| @@ -1675,9 +1743,13 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1675 | 1743 | c.onTitleChanged(ctx, paneID, title) | |
| 1676 | 1744 | } | |
| 1677 | 1745 | ||
| 1678 | - | // Favicon changes | |
| 1746 | + | // Favicon changes - capture generation to detect stale callbacks after pool reuse | |
| 1747 | + | faviconGen := wv.Generation() | |
| 1679 | 1748 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | |
| 1680 | - | c.onFaviconChanged(ctx, paneID, favicon) | |
| 1749 | + | if wv.Generation() != faviconGen { | |
| 1750 | + | return // WebView was reused, ignore stale signal | |
| 1751 | + | } | |
| 1752 | + | c.onFaviconChanged(ctx, paneID, wv, favicon) | |
| 1681 | 1753 | } | |
| 1682 | 1754 | ||
| 1683 | 1755 | // Load events | |
| @@ -982,15 +982,19 @@ func (c *ContentCoordinator) syncStackedTitle(ctx context.Context, paneID entity | |||||
| 982 | } | 982 | } | ||
| 983 | 983 | ||||
| 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | 984 | // onFaviconChanged updates favicon tracking when a WebView's favicon changes. | ||
| 985 | - | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, favicon *gdk.Texture) { | 985 | + | func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity.PaneID, emittingWV *webkit.WebView, favicon *gdk.Texture) { |
| 986 | log := logging.FromContext(ctx) | 986 | log := logging.FromContext(ctx) | ||
| 987 | 987 | ||||
| 988 | - | // Get current URI to extract domain for caching | 988 | + | // Verify this WebView is still bound to the expected pane |
| 989 | - | wv := c.getWebViewLocked(paneID) | 989 | + | currentWV := c.getWebViewLocked(paneID) |
| 990 | - | if wv == nil { | 990 | + | if currentWV == nil || currentWV != emittingWV { |
| 991 | + | log.Debug(). | |||
| 992 | + | Str("pane_id", string(paneID)). | |||
| 993 | + | Msg("ignoring favicon change from unbound webview") | |||
| 991 | return | 994 | return | ||
| 992 | } | 995 | } | ||
| 993 | - | uri := wv.URI() | 996 | + | |
| 997 | + | uri := emittingWV.URI() | |||
| 994 | 998 | ||||
| 995 | // Update favicon cache with domain key (handles cross-domain redirects) | 999 | // Update favicon cache with domain key (handles cross-domain redirects) | ||
| 996 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | 1000 | if c.faviconAdapter != nil && favicon != nil && uri != "" { | ||
| @@ -1001,16 +1005,7 @@ func (c *ContentCoordinator) onFaviconChanged(ctx context.Context, paneID entity | |||||
| 1001 | } | 1005 | } | ||
| 1002 | 1006 | ||||
| 1003 | // Update StackedView favicon if this pane is in a stack | 1007 | // Update StackedView favicon if this pane is in a stack | ||
| 1004 | - | _, wsView := c.getActiveWS() | 1008 | + | c.updateStackedFaviconForPane(ctx, paneID, favicon) |
| 1005 | - | if wsView != nil { | |||
| 1006 | - | tr := wsView.TreeRenderer() | |||
| 1007 | - | if tr != nil { | |||
| 1008 | - | stackedView := tr.GetStackedViewForPane(string(paneID)) | |||
| 1009 | - | if stackedView != nil { | |||
| 1010 | - | c.updateStackedPaneFavicon(ctx, stackedView, paneID, favicon) | |||
| 1011 | - | } | |||
| 1012 | - | } | |||
| 1013 | - | } | |||
| 1014 | 1009 | ||||
| 1015 | log.Debug(). | 1010 | log.Debug(). | ||
| 1016 | Str("pane_id", string(paneID)). | 1011 | Str("pane_id", string(paneID)). | ||
| @@ -1042,6 +1037,72 @@ func (c *ContentCoordinator) updateStackedPaneFavicon( | |||||
| 1042 | } | 1037 | } | ||
| 1043 | } | 1038 | } | ||
| 1044 | 1039 | ||||
| 1040 | + | // updateStackedFaviconForPane updates the stacked title bar favicon for a pane. | |||
| 1041 | + | func (c *ContentCoordinator) updateStackedFaviconForPane(ctx context.Context, paneID entity.PaneID, texture *gdk.Texture) { | |||
| 1042 | + | _, wsView := c.getActiveWS() | |||
| 1043 | + | if wsView == nil { | |||
| 1044 | + | return | |||
| 1045 | + | } | |||
| 1046 | + | tr := wsView.TreeRenderer() | |||
| 1047 | + | if tr == nil { | |||
| 1048 | + | return | |||
| 1049 | + | } | |||
| 1050 | + | if sv := tr.GetStackedViewForPane(string(paneID)); sv != nil { | |||
| 1051 | + | c.updateStackedPaneFavicon(ctx, sv, paneID, texture) | |||
| 1052 | + | } | |||
| 1053 | + | } | |||
| 1054 | + | ||||
| 1055 | + | // resolveCommittedFavicon ensures the stacked title bar has a favicon after navigation commits. | |||
| 1056 | + | // First checks if WebKit already has a favicon for the page (common for subpath URLs). | |||
| 1057 | + | // Falls back to the full GetOrFetch pipeline (cache + WebKit DB + DuckDuckGo API). | |||
| 1058 | + | // This closes the gap where title bars relied solely on the notify::favicon signal. | |||
| 1059 | + | func (c *ContentCoordinator) resolveCommittedFavicon(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |||
| 1060 | + | if c.faviconAdapter == nil || wv == nil { | |||
| 1061 | + | return | |||
| 1062 | + | } | |||
| 1063 | + | ||||
| 1064 | + | uri := wv.URI() | |||
| 1065 | + | if uri == "" || strings.HasPrefix(uri, "about:") || strings.HasPrefix(uri, "dumb://") { | |||
| 1066 | + | return | |||
| 1067 | + | } | |||
| 1068 | + | ||||
| 1069 | + | log := logging.FromContext(ctx) | |||
| 1070 | + | ||||
| 1071 | + | // Check if we already have a texture cached for this domain | |||
| 1072 | + | if texture := c.faviconAdapter.GetTextureByURL(uri); texture != nil { | |||
| 1073 | + | // Already cached - just ensure the stacked view is updated | |||
| 1074 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |||
| 1075 | + | return | |||
| 1076 | + | } | |||
| 1077 | + | ||||
| 1078 | + | // Check if WebKit already has a favicon for this page | |||
| 1079 | + | if icon := wv.Favicon(); icon != nil { | |||
| 1080 | + | log.Debug().Str("pane_id", string(paneID)).Str("uri", uri).Msg("using existing WebKit favicon for committed page") | |||
| 1081 | + | // Store and update through the normal path | |||
| 1082 | + | c.navOriginMu.RLock() | |||
| 1083 | + | originURL := c.navOrigins[paneID] | |||
| 1084 | + | c.navOriginMu.RUnlock() | |||
| 1085 | + | c.faviconAdapter.StoreFromWebKitWithOrigin(ctx, uri, originURL, icon) | |||
| 1086 | + | c.updateStackedFaviconForPane(ctx, paneID, icon) | |||
| 1087 | + | return | |||
| 1088 | + | } | |||
| 1089 | + | ||||
| 1090 | + | // Fall back to GetOrFetch (checks service cache, WebKit DB, then DuckDuckGo API) | |||
| 1091 | + | // Capture current generation to guard against stale callbacks | |||
| 1092 | + | gen := wv.Generation() | |||
| 1093 | + | c.faviconAdapter.GetOrFetch(ctx, uri, func(texture *gdk.Texture) { | |||
| 1094 | + | // Verify WebView is still bound to pane and hasn't been reused | |||
| 1095 | + | if wv.Generation() != gen { | |||
| 1096 | + | return | |||
| 1097 | + | } | |||
| 1098 | + | currentWV := c.getWebViewLocked(paneID) | |||
| 1099 | + | if currentWV != wv { | |||
| 1100 | + | return | |||
| 1101 | + | } | |||
| 1102 | + | c.updateStackedFaviconForPane(ctx, paneID, texture) | |||
| 1103 | + | }) | |||
| 1104 | + | } | |||
| 1105 | + | ||||
| 1045 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | 1106 | // FaviconAdapter returns the favicon adapter for external use (e.g., omnibox). | ||
| 1046 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | 1107 | func (c *ContentCoordinator) FaviconAdapter() *adapter.FaviconAdapter { | ||
| 1047 | return c.faviconAdapter | 1108 | return c.faviconAdapter | ||
| @@ -1087,7 +1148,11 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||||
| 1087 | if c.getActiveWS == nil { | 1148 | if c.getActiveWS == nil { | ||
| 1088 | return false | 1149 | return false | ||
| 1089 | } | 1150 | } | ||
| 1090 | - | // A nil texture triggers the default icon fallback, which avoids stale favicons. | 1151 | + | // Only update if we found a cached texture. |
| 1152 | + | // If nil, let resolveCommittedFavicon handle it after load commits. | |||
| 1153 | + | if texture == nil { | |||
| 1154 | + | return false | |||
| 1155 | + | } | |||
| 1091 | _, wsView := c.getActiveWS() | 1156 | _, wsView := c.getActiveWS() | ||
| 1092 | if wsView == nil { | 1157 | if wsView == nil { | ||
| 1093 | return false | 1158 | return false | ||
| @@ -1190,6 +1255,9 @@ func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity. | |||||
| 1190 | c.onHistoryRecord(ctx, paneID, uri) | 1255 | c.onHistoryRecord(ctx, paneID, uri) | ||
| 1191 | } | 1256 | } | ||
| 1192 | 1257 | ||||
| 1258 | + | // Resolve favicon for stacked title bar (fills gap when WebKit doesn't emit notify::favicon) | |||
| 1259 | + | c.resolveCommittedFavicon(ctx, paneID, wv) | |||
| 1260 | + | ||||
| 1193 | // Notify active pane navigation for permission indicator reset. | 1261 | // Notify active pane navigation for permission indicator reset. | ||
| 1194 | c.notifyActiveNavigation(paneID, uri) | 1262 | c.notifyActiveNavigation(paneID, uri) | ||
| 1195 | 1263 | ||||
| @@ -1675,9 +1743,13 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1675 | c.onTitleChanged(ctx, paneID, title) | 1743 | c.onTitleChanged(ctx, paneID, title) | ||
| 1676 | } | 1744 | } | ||
| 1677 | 1745 | ||||
| 1678 | - | // Favicon changes | 1746 | + | // Favicon changes - capture generation to detect stale callbacks after pool reuse |
| 1747 | + | faviconGen := wv.Generation() | |||
| 1679 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | 1748 | wv.OnFaviconChanged = func(favicon *gdk.Texture) { | ||
| 1680 | - | c.onFaviconChanged(ctx, paneID, favicon) | 1749 | + | if wv.Generation() != faviconGen { |
| 1750 | + | return // WebView was reused, ignore stale signal | |||
| 1751 | + | } | |||
| 1752 | + | c.onFaviconChanged(ctx, paneID, wv, favicon) | |||
| 1681 | } | 1753 | } | ||
| 1682 | 1754 | ||||
| 1683 | // Load events | 1755 | // Load events | ||