refactor(content): trim named popup cache state
Drop cached popup IDs that are already available from the stored WebView. Keep the named popup reuse test aligned with the extra ID lookup in the reuse path.
- commit
- bfd66cc38db4713c89c23726efc213236896e114
- author
- brice · · 2026-04-11 10:25 UTC
- parent
- 5dc11a5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +5 -9
M internal/ui/coordinator/content/popup.go +4 -8
| @@ -85,8 +85,6 @@ type namedPopupKey struct { | |||
| 85 | 85 | } | |
| 86 | 86 | ||
| 87 | 87 | type namedPopupState struct { | |
| 88 | - | PaneID entity.PaneID | |
| 89 | - | PopupID port.WebViewID | |
| 90 | 88 | WebView port.WebView | |
| 91 | 89 | } | |
| 92 | 90 | ||
| @@ -123,8 +121,6 @@ func (c *Coordinator) lookupReusableNamedPopup(parentPaneID entity.PaneID, frame | |||
| 123 | 121 | func (c *Coordinator) storeReusableNamedPopup( | |
| 124 | 122 | parentPaneID entity.PaneID, | |
| 125 | 123 | frameName string, | |
| 126 | - | paneID entity.PaneID, | |
| 127 | - | popupID port.WebViewID, | |
| 128 | 124 | wv port.WebView, | |
| 129 | 125 | ) { | |
| 130 | 126 | if !isReusableNamedPopupFrame(frameName) || wv == nil { | |
| @@ -137,7 +133,7 @@ func (c *Coordinator) storeReusableNamedPopup( | |||
| 137 | 133 | if c.namedPopups == nil { | |
| 138 | 134 | c.namedPopups = make(map[namedPopupKey]*namedPopupState) | |
| 139 | 135 | } | |
| 140 | - | c.namedPopups[key] = &namedPopupState{PaneID: paneID, PopupID: popupID, WebView: wv} | |
| 136 | + | c.namedPopups[key] = &namedPopupState{WebView: wv} | |
| 141 | 137 | c.popupMu.Unlock() | |
| 142 | 138 | } | |
| 143 | 139 | ||
| @@ -152,7 +148,7 @@ func (c *Coordinator) updatePendingPopupTarget(popupID port.WebViewID, targetURI | |||
| 152 | 148 | func (c *Coordinator) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | |
| 153 | 149 | c.popupMu.Lock() | |
| 154 | 150 | for key, state := range c.namedPopups { | |
| 155 | - | if state != nil && state.PopupID == popupID { | |
| 151 | + | if state != nil && state.WebView != nil && state.WebView.ID() == popupID { | |
| 156 | 152 | delete(c.namedPopups, key) | |
| 157 | 153 | } | |
| 158 | 154 | } | |
| @@ -306,7 +302,7 @@ func (c *Coordinator) handlePopupCreate( | |||
| 306 | 302 | ||
| 307 | 303 | parentID := parentWV.ID() | |
| 308 | 304 | if existing, ok := c.lookupReusableNamedPopup(parentPaneID, req.FrameName); ok { | |
| 309 | - | c.updatePendingPopupTarget(existing.PopupID, req.TargetURI) | |
| 305 | + | c.updatePendingPopupTarget(existing.WebView.ID(), req.TargetURI) | |
| 310 | 306 | if err := existing.WebView.LoadURI(ctx, req.TargetURI); err != nil { | |
| 311 | 307 | log.Warn().Err(err). | |
| 312 | 308 | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| @@ -374,7 +370,7 @@ func (c *Coordinator) handlePopupCreate( | |||
| 374 | 370 | ||
| 375 | 371 | // Register WebView in our map (after successful insertion) | |
| 376 | 372 | c.setWebViewLocked(paneID, popupWV) | |
| 377 | - | c.storeReusableNamedPopup(parentPaneID, req.FrameName, paneID, popupID, popupWV) | |
| 373 | + | c.storeReusableNamedPopup(parentPaneID, req.FrameName, popupWV) | |
| 378 | 374 | ||
| 379 | 375 | // Setup standard callbacks (after successful insertion to avoid leak) | |
| 380 | 376 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | |
| @@ -85,8 +85,6 @@ type namedPopupKey struct { | |||||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
| 87 | type namedPopupState struct { | 87 | type namedPopupState struct { | ||
| 88 | - | PaneID entity.PaneID | |||
| 89 | - | PopupID port.WebViewID | |||
| 90 | WebView port.WebView | 88 | WebView port.WebView | ||
| 91 | } | 89 | } | ||
| 92 | 90 | ||||
| @@ -123,8 +121,6 @@ func (c *Coordinator) lookupReusableNamedPopup(parentPaneID entity.PaneID, frame | |||||
| 123 | func (c *Coordinator) storeReusableNamedPopup( | 121 | func (c *Coordinator) storeReusableNamedPopup( | ||
| 124 | parentPaneID entity.PaneID, | 122 | parentPaneID entity.PaneID, | ||
| 125 | frameName string, | 123 | frameName string, | ||
| 126 | - | paneID entity.PaneID, | |||
| 127 | - | popupID port.WebViewID, | |||
| 128 | wv port.WebView, | 124 | wv port.WebView, | ||
| 129 | ) { | 125 | ) { | ||
| 130 | if !isReusableNamedPopupFrame(frameName) || wv == nil { | 126 | if !isReusableNamedPopupFrame(frameName) || wv == nil { | ||
| @@ -137,7 +133,7 @@ func (c *Coordinator) storeReusableNamedPopup( | |||||
| 137 | if c.namedPopups == nil { | 133 | if c.namedPopups == nil { | ||
| 138 | c.namedPopups = make(map[namedPopupKey]*namedPopupState) | 134 | c.namedPopups = make(map[namedPopupKey]*namedPopupState) | ||
| 139 | } | 135 | } | ||
| 140 | - | c.namedPopups[key] = &namedPopupState{PaneID: paneID, PopupID: popupID, WebView: wv} | 136 | + | c.namedPopups[key] = &namedPopupState{WebView: wv} |
| 141 | c.popupMu.Unlock() | 137 | c.popupMu.Unlock() | ||
| 142 | } | 138 | } | ||
| 143 | 139 | ||||
| @@ -152,7 +148,7 @@ func (c *Coordinator) updatePendingPopupTarget(popupID port.WebViewID, targetURI | |||||
| 152 | func (c *Coordinator) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | 148 | func (c *Coordinator) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | ||
| 153 | c.popupMu.Lock() | 149 | c.popupMu.Lock() | ||
| 154 | for key, state := range c.namedPopups { | 150 | for key, state := range c.namedPopups { | ||
| 155 | - | if state != nil && state.PopupID == popupID { | 151 | + | if state != nil && state.WebView != nil && state.WebView.ID() == popupID { |
| 156 | delete(c.namedPopups, key) | 152 | delete(c.namedPopups, key) | ||
| 157 | } | 153 | } | ||
| 158 | } | 154 | } | ||
| @@ -306,7 +302,7 @@ func (c *Coordinator) handlePopupCreate( | |||||
| 306 | 302 | ||||
| 307 | parentID := parentWV.ID() | 303 | parentID := parentWV.ID() | ||
| 308 | if existing, ok := c.lookupReusableNamedPopup(parentPaneID, req.FrameName); ok { | 304 | if existing, ok := c.lookupReusableNamedPopup(parentPaneID, req.FrameName); ok { | ||
| 309 | - | c.updatePendingPopupTarget(existing.PopupID, req.TargetURI) | 305 | + | c.updatePendingPopupTarget(existing.WebView.ID(), req.TargetURI) |
| 310 | if err := existing.WebView.LoadURI(ctx, req.TargetURI); err != nil { | 306 | if err := existing.WebView.LoadURI(ctx, req.TargetURI); err != nil { | ||
| 311 | log.Warn().Err(err). | 307 | log.Warn().Err(err). | ||
| 312 | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | 308 | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | ||
| @@ -374,7 +370,7 @@ func (c *Coordinator) handlePopupCreate( | |||||
| 374 | 370 | ||||
| 375 | // Register WebView in our map (after successful insertion) | 371 | // Register WebView in our map (after successful insertion) | ||
| 376 | c.setWebViewLocked(paneID, popupWV) | 372 | c.setWebViewLocked(paneID, popupWV) | ||
| 377 | - | c.storeReusableNamedPopup(parentPaneID, req.FrameName, paneID, popupID, popupWV) | 373 | + | c.storeReusableNamedPopup(parentPaneID, req.FrameName, popupWV) |
| 378 | 374 | ||||
| 379 | // Setup standard callbacks (after successful insertion to avoid leak) | 375 | // Setup standard callbacks (after successful insertion to avoid leak) | ||
| 380 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | 376 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | ||
M internal/ui/coordinator/content/popup_test.go +1 -1
| @@ -132,7 +132,7 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |||
| 132 | 132 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 133 | 133 | ||
| 134 | 134 | popupWV := mocks.NewMockWebView(t) | |
| 135 | - | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | |
| 135 | + | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Twice() | |
| 136 | 136 | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 137 | 137 | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 138 | 138 | popupWV.EXPECT().IsLoading().Return(false).Maybe() | |
| @@ -132,7 +132,7 @@ func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |||||
| 132 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | 132 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | ||
| 133 | 133 | ||||
| 134 | popupWV := mocks.NewMockWebView(t) | 134 | popupWV := mocks.NewMockWebView(t) | ||
| 135 | - | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | 135 | + | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Twice() |
| 136 | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | 136 | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | ||
| 137 | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | 137 | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | ||
| 138 | popupWV.EXPECT().IsLoading().Return(false).Maybe() | 138 | popupWV.EXPECT().IsLoading().Return(false).Maybe() | ||