fix(cef): reuse named popup panes for OAuth flows
Reuse named popup frames from the same parent pane so Google sign-in updates the existing popup instead of opening a second split. Add a regression test for repeated named popup requests in the content coordinator.
- commit
- 5dc11a583d1d79abc844b5f7eaa5393f92363a11
- author
- brice · · 2026-04-11 10:18 UTC
- parent
- 622e14a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +160 -1
M internal/ui/coordinator/content/coordinator.go +2 -0
| @@ -85,6 +85,7 @@ type Coordinator struct { | |||
| 85 | 85 | factory port.WebViewFactory | |
| 86 | 86 | popupConfig *entity.PopupBehaviorConfig | |
| 87 | 87 | pendingPopups map[port.WebViewID]*PendingPopup | |
| 88 | + | namedPopups map[namedPopupKey]*namedPopupState | |
| 88 | 89 | popupOAuth map[port.WebViewID]*popupOAuthState | |
| 89 | 90 | popupRefresh map[entity.PaneID]*time.Timer | |
| 90 | 91 | popupMu sync.RWMutex | |
| @@ -168,6 +169,7 @@ func NewCoordinator( | |||
| 168 | 169 | pendingThemePanes: make(map[entity.PaneID]bool), | |
| 169 | 170 | getActiveWS: getActiveWS, | |
| 170 | 171 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | |
| 172 | + | namedPopups: make(map[namedPopupKey]*namedPopupState), | |
| 171 | 173 | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 172 | 174 | popupRefresh: make(map[entity.PaneID]*time.Timer), | |
| 173 | 175 | } | |
| @@ -85,6 +85,7 @@ type Coordinator struct { | |||||
| 85 | factory port.WebViewFactory | 85 | factory port.WebViewFactory | ||
| 86 | popupConfig *entity.PopupBehaviorConfig | 86 | popupConfig *entity.PopupBehaviorConfig | ||
| 87 | pendingPopups map[port.WebViewID]*PendingPopup | 87 | pendingPopups map[port.WebViewID]*PendingPopup | ||
| 88 | + | namedPopups map[namedPopupKey]*namedPopupState | |||
| 88 | popupOAuth map[port.WebViewID]*popupOAuthState | 89 | popupOAuth map[port.WebViewID]*popupOAuthState | ||
| 89 | popupRefresh map[entity.PaneID]*time.Timer | 90 | popupRefresh map[entity.PaneID]*time.Timer | ||
| 90 | popupMu sync.RWMutex | 91 | popupMu sync.RWMutex | ||
| @@ -168,6 +169,7 @@ func NewCoordinator( | |||||
| 168 | pendingThemePanes: make(map[entity.PaneID]bool), | 169 | pendingThemePanes: make(map[entity.PaneID]bool), | ||
| 169 | getActiveWS: getActiveWS, | 170 | getActiveWS: getActiveWS, | ||
| 170 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | 171 | pendingPopups: make(map[port.WebViewID]*PendingPopup), | ||
| 172 | + | namedPopups: make(map[namedPopupKey]*namedPopupState), | |||
| 171 | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | 173 | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | ||
| 172 | popupRefresh: make(map[entity.PaneID]*time.Timer), | 174 | popupRefresh: make(map[entity.PaneID]*time.Timer), | ||
| 173 | } | 175 | } | ||
M internal/ui/coordinator/content/popup.go +100 -1
| @@ -79,6 +79,86 @@ type PendingPopup struct { | |||
| 79 | 79 | CreatedAt time.Time | |
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | + | type namedPopupKey struct { | |
| 83 | + | ParentPaneID entity.PaneID | |
| 84 | + | FrameName string | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | type namedPopupState struct { | |
| 88 | + | PaneID entity.PaneID | |
| 89 | + | PopupID port.WebViewID | |
| 90 | + | WebView port.WebView | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | func isReusableNamedPopupFrame(frameName string) bool { | |
| 94 | + | return frameName != "" && frameName != "_blank" | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func (c *Coordinator) lookupReusableNamedPopup(parentPaneID entity.PaneID, frameName string) (*namedPopupState, bool) { | |
| 98 | + | if !isReusableNamedPopupFrame(frameName) { | |
| 99 | + | return nil, false | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |
| 103 | + | ||
| 104 | + | c.popupMu.RLock() | |
| 105 | + | state, ok := c.namedPopups[key] | |
| 106 | + | c.popupMu.RUnlock() | |
| 107 | + | if !ok || state == nil || state.WebView == nil { | |
| 108 | + | return nil, false | |
| 109 | + | } | |
| 110 | + | ||
| 111 | + | if state.WebView.IsDestroyed() { | |
| 112 | + | c.popupMu.Lock() | |
| 113 | + | if current, ok := c.namedPopups[key]; ok && current == state { | |
| 114 | + | delete(c.namedPopups, key) | |
| 115 | + | } | |
| 116 | + | c.popupMu.Unlock() | |
| 117 | + | return nil, false | |
| 118 | + | } | |
| 119 | + | ||
| 120 | + | return state, true | |
| 121 | + | } | |
| 122 | + | ||
| 123 | + | func (c *Coordinator) storeReusableNamedPopup( | |
| 124 | + | parentPaneID entity.PaneID, | |
| 125 | + | frameName string, | |
| 126 | + | paneID entity.PaneID, | |
| 127 | + | popupID port.WebViewID, | |
| 128 | + | wv port.WebView, | |
| 129 | + | ) { | |
| 130 | + | if !isReusableNamedPopupFrame(frameName) || wv == nil { | |
| 131 | + | return | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |
| 135 | + | ||
| 136 | + | c.popupMu.Lock() | |
| 137 | + | if c.namedPopups == nil { | |
| 138 | + | c.namedPopups = make(map[namedPopupKey]*namedPopupState) | |
| 139 | + | } | |
| 140 | + | c.namedPopups[key] = &namedPopupState{PaneID: paneID, PopupID: popupID, WebView: wv} | |
| 141 | + | c.popupMu.Unlock() | |
| 142 | + | } | |
| 143 | + | ||
| 144 | + | func (c *Coordinator) updatePendingPopupTarget(popupID port.WebViewID, targetURI string) { | |
| 145 | + | c.popupMu.Lock() | |
| 146 | + | if pending, ok := c.pendingPopups[popupID]; ok && pending != nil { | |
| 147 | + | pending.TargetURI = targetURI | |
| 148 | + | } | |
| 149 | + | c.popupMu.Unlock() | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | func (c *Coordinator) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | |
| 153 | + | c.popupMu.Lock() | |
| 154 | + | for key, state := range c.namedPopups { | |
| 155 | + | if state != nil && state.PopupID == popupID { | |
| 156 | + | delete(c.namedPopups, key) | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | c.popupMu.Unlock() | |
| 160 | + | } | |
| 161 | + | ||
| 82 | 162 | // InsertPopupInput contains the data needed to insert a popup into the workspace. | |
| 83 | 163 | type InsertPopupInput struct { | |
| 84 | 164 | // ParentPaneID is the pane that spawned this popup. | |
| @@ -224,8 +304,23 @@ func (c *Coordinator) handlePopupCreate( | |||
| 224 | 304 | return nil | |
| 225 | 305 | } | |
| 226 | 306 | ||
| 227 | - | // Create related WebView for session sharing (created hidden) | |
| 228 | 307 | parentID := parentWV.ID() | |
| 308 | + | if existing, ok := c.lookupReusableNamedPopup(parentPaneID, req.FrameName); ok { | |
| 309 | + | c.updatePendingPopupTarget(existing.PopupID, req.TargetURI) | |
| 310 | + | if err := existing.WebView.LoadURI(ctx, req.TargetURI); err != nil { | |
| 311 | + | log.Warn().Err(err). | |
| 312 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| 313 | + | Msg("failed to load target URI in reused popup") | |
| 314 | + | } | |
| 315 | + | log.Info(). | |
| 316 | + | Str("parent_pane", string(parentPaneID)). | |
| 317 | + | Str("frame_name", req.FrameName). | |
| 318 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |
| 319 | + | Msg("reused named popup") | |
| 320 | + | return existing.WebView | |
| 321 | + | } | |
| 322 | + | ||
| 323 | + | // Create related WebView for session sharing (created hidden) | |
| 229 | 324 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | |
| 230 | 325 | if err != nil { | |
| 231 | 326 | log.Error().Err(err).Msg("failed to create related webview for popup") | |
| @@ -279,6 +374,7 @@ func (c *Coordinator) handlePopupCreate( | |||
| 279 | 374 | ||
| 280 | 375 | // Register WebView in our map (after successful insertion) | |
| 281 | 376 | c.setWebViewLocked(paneID, popupWV) | |
| 377 | + | c.storeReusableNamedPopup(parentPaneID, req.FrameName, paneID, popupID, popupWV) | |
| 282 | 378 | ||
| 283 | 379 | // Setup standard callbacks (after successful insertion to avoid leak) | |
| 284 | 380 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | |
| @@ -404,6 +500,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 404 | 500 | } | |
| 405 | 501 | } | |
| 406 | 502 | c.handlePopupOAuthClose(ctx, popupID) | |
| 503 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |
| 407 | 504 | c.ReleaseWebView(ctx, pending.PaneID) | |
| 408 | 505 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | |
| 409 | 506 | return | |
| @@ -422,6 +519,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 422 | 519 | ||
| 423 | 520 | if paneID == "" { | |
| 424 | 521 | c.handlePopupOAuthClose(ctx, popupID) | |
| 522 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |
| 425 | 523 | log.Warn().Msg("popup close: could not find pane for webview") | |
| 426 | 524 | return | |
| 427 | 525 | } | |
| @@ -434,6 +532,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 434 | 532 | } | |
| 435 | 533 | ||
| 436 | 534 | c.handlePopupOAuthClose(ctx, popupID) | |
| 535 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |
| 437 | 536 | ||
| 438 | 537 | // Release the WebView (this will clean up tracking) | |
| 439 | 538 | c.ReleaseWebView(ctx, paneID) | |
| @@ -79,6 +79,86 @@ type PendingPopup struct { | |||||
| 79 | CreatedAt time.Time | 79 | CreatedAt time.Time | ||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | + | type namedPopupKey struct { | |||
| 83 | + | ParentPaneID entity.PaneID | |||
| 84 | + | FrameName string | |||
| 85 | + | } | |||
| 86 | + | ||||
| 87 | + | type namedPopupState struct { | |||
| 88 | + | PaneID entity.PaneID | |||
| 89 | + | PopupID port.WebViewID | |||
| 90 | + | WebView port.WebView | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | func isReusableNamedPopupFrame(frameName string) bool { | |||
| 94 | + | return frameName != "" && frameName != "_blank" | |||
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | func (c *Coordinator) lookupReusableNamedPopup(parentPaneID entity.PaneID, frameName string) (*namedPopupState, bool) { | |||
| 98 | + | if !isReusableNamedPopupFrame(frameName) { | |||
| 99 | + | return nil, false | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |||
| 103 | + | ||||
| 104 | + | c.popupMu.RLock() | |||
| 105 | + | state, ok := c.namedPopups[key] | |||
| 106 | + | c.popupMu.RUnlock() | |||
| 107 | + | if !ok || state == nil || state.WebView == nil { | |||
| 108 | + | return nil, false | |||
| 109 | + | } | |||
| 110 | + | ||||
| 111 | + | if state.WebView.IsDestroyed() { | |||
| 112 | + | c.popupMu.Lock() | |||
| 113 | + | if current, ok := c.namedPopups[key]; ok && current == state { | |||
| 114 | + | delete(c.namedPopups, key) | |||
| 115 | + | } | |||
| 116 | + | c.popupMu.Unlock() | |||
| 117 | + | return nil, false | |||
| 118 | + | } | |||
| 119 | + | ||||
| 120 | + | return state, true | |||
| 121 | + | } | |||
| 122 | + | ||||
| 123 | + | func (c *Coordinator) storeReusableNamedPopup( | |||
| 124 | + | parentPaneID entity.PaneID, | |||
| 125 | + | frameName string, | |||
| 126 | + | paneID entity.PaneID, | |||
| 127 | + | popupID port.WebViewID, | |||
| 128 | + | wv port.WebView, | |||
| 129 | + | ) { | |||
| 130 | + | if !isReusableNamedPopupFrame(frameName) || wv == nil { | |||
| 131 | + | return | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | key := namedPopupKey{ParentPaneID: parentPaneID, FrameName: frameName} | |||
| 135 | + | ||||
| 136 | + | c.popupMu.Lock() | |||
| 137 | + | if c.namedPopups == nil { | |||
| 138 | + | c.namedPopups = make(map[namedPopupKey]*namedPopupState) | |||
| 139 | + | } | |||
| 140 | + | c.namedPopups[key] = &namedPopupState{PaneID: paneID, PopupID: popupID, WebView: wv} | |||
| 141 | + | c.popupMu.Unlock() | |||
| 142 | + | } | |||
| 143 | + | ||||
| 144 | + | func (c *Coordinator) updatePendingPopupTarget(popupID port.WebViewID, targetURI string) { | |||
| 145 | + | c.popupMu.Lock() | |||
| 146 | + | if pending, ok := c.pendingPopups[popupID]; ok && pending != nil { | |||
| 147 | + | pending.TargetURI = targetURI | |||
| 148 | + | } | |||
| 149 | + | c.popupMu.Unlock() | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | func (c *Coordinator) clearReusableNamedPopupByWebViewID(popupID port.WebViewID) { | |||
| 153 | + | c.popupMu.Lock() | |||
| 154 | + | for key, state := range c.namedPopups { | |||
| 155 | + | if state != nil && state.PopupID == popupID { | |||
| 156 | + | delete(c.namedPopups, key) | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | c.popupMu.Unlock() | |||
| 160 | + | } | |||
| 161 | + | ||||
| 82 | // InsertPopupInput contains the data needed to insert a popup into the workspace. | 162 | // InsertPopupInput contains the data needed to insert a popup into the workspace. | ||
| 83 | type InsertPopupInput struct { | 163 | type InsertPopupInput struct { | ||
| 84 | // ParentPaneID is the pane that spawned this popup. | 164 | // ParentPaneID is the pane that spawned this popup. | ||
| @@ -224,8 +304,23 @@ func (c *Coordinator) handlePopupCreate( | |||||
| 224 | return nil | 304 | return nil | ||
| 225 | } | 305 | } | ||
| 226 | 306 | ||||
| 227 | - | // Create related WebView for session sharing (created hidden) | |||
| 228 | parentID := parentWV.ID() | 307 | parentID := parentWV.ID() | ||
| 308 | + | if existing, ok := c.lookupReusableNamedPopup(parentPaneID, req.FrameName); ok { | |||
| 309 | + | c.updatePendingPopupTarget(existing.PopupID, req.TargetURI) | |||
| 310 | + | if err := existing.WebView.LoadURI(ctx, req.TargetURI); err != nil { | |||
| 311 | + | log.Warn().Err(err). | |||
| 312 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |||
| 313 | + | Msg("failed to load target URI in reused popup") | |||
| 314 | + | } | |||
| 315 | + | log.Info(). | |||
| 316 | + | Str("parent_pane", string(parentPaneID)). | |||
| 317 | + | Str("frame_name", req.FrameName). | |||
| 318 | + | Str("target_uri", logging.TruncateURL(req.TargetURI, logURLMaxLen)). | |||
| 319 | + | Msg("reused named popup") | |||
| 320 | + | return existing.WebView | |||
| 321 | + | } | |||
| 322 | + | ||||
| 323 | + | // Create related WebView for session sharing (created hidden) | |||
| 229 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | 324 | popupWV, err := c.factory.CreateRelated(ctx, parentID) | ||
| 230 | if err != nil { | 325 | if err != nil { | ||
| 231 | log.Error().Err(err).Msg("failed to create related webview for popup") | 326 | log.Error().Err(err).Msg("failed to create related webview for popup") | ||
| @@ -279,6 +374,7 @@ func (c *Coordinator) handlePopupCreate( | |||||
| 279 | 374 | ||||
| 280 | // Register WebView in our map (after successful insertion) | 375 | // Register WebView in our map (after successful insertion) | ||
| 281 | c.setWebViewLocked(paneID, popupWV) | 376 | c.setWebViewLocked(paneID, popupWV) | ||
| 377 | + | c.storeReusableNamedPopup(parentPaneID, req.FrameName, paneID, popupID, popupWV) | |||
| 282 | 378 | ||||
| 283 | // Setup standard callbacks (after successful insertion to avoid leak) | 379 | // Setup standard callbacks (after successful insertion to avoid leak) | ||
| 284 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | 380 | c.setupWebViewCallbacks(ctx, paneID, popupWV) | ||
| @@ -404,6 +500,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 404 | } | 500 | } | ||
| 405 | } | 501 | } | ||
| 406 | c.handlePopupOAuthClose(ctx, popupID) | 502 | c.handlePopupOAuthClose(ctx, popupID) | ||
| 503 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |||
| 407 | c.ReleaseWebView(ctx, pending.PaneID) | 504 | c.ReleaseWebView(ctx, pending.PaneID) | ||
| 408 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | 505 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | ||
| 409 | return | 506 | return | ||
| @@ -422,6 +519,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 422 | 519 | ||||
| 423 | if paneID == "" { | 520 | if paneID == "" { | ||
| 424 | c.handlePopupOAuthClose(ctx, popupID) | 521 | c.handlePopupOAuthClose(ctx, popupID) | ||
| 522 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |||
| 425 | log.Warn().Msg("popup close: could not find pane for webview") | 523 | log.Warn().Msg("popup close: could not find pane for webview") | ||
| 426 | return | 524 | return | ||
| 427 | } | 525 | } | ||
| @@ -434,6 +532,7 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 434 | } | 532 | } | ||
| 435 | 533 | ||||
| 436 | c.handlePopupOAuthClose(ctx, popupID) | 534 | c.handlePopupOAuthClose(ctx, popupID) | ||
| 535 | + | c.clearReusableNamedPopupByWebViewID(popupID) | |||
| 437 | 536 | ||||
| 438 | // Release the WebView (this will clean up tracking) | 537 | // Release the WebView (this will clean up tracking) | ||
| 439 | c.ReleaseWebView(ctx, paneID) | 538 | c.ReleaseWebView(ctx, paneID) | ||
M internal/ui/coordinator/content/popup_test.go +58 -0
| @@ -6,9 +6,11 @@ import ( | |||
| 6 | 6 | "testing" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 9 | 10 | "github.com/stretchr/testify/require" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/bnema/dumber/internal/application/port" | |
| 13 | + | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 12 | 14 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | 15 | ) | |
| 14 | 16 | ||
| @@ -123,6 +125,62 @@ func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) { | |||
| 123 | 125 | assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg)) | |
| 124 | 126 | } | |
| 125 | 127 | ||
| 128 | + | func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |
| 129 | + | ctx := context.Background() | |
| 130 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 131 | + | parentWV := mocks.NewMockWebView(t) | |
| 132 | + | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |
| 133 | + | ||
| 134 | + | popupWV := mocks.NewMockWebView(t) | |
| 135 | + | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | |
| 136 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 137 | + | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 138 | + | popupWV.EXPECT().IsLoading().Return(false).Maybe() | |
| 139 | + | currentURI := "" | |
| 140 | + | popupWV.EXPECT().URI().RunAndReturn(func() string { return currentURI }).Maybe() | |
| 141 | + | popupWV.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 142 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?first").RunAndReturn(func(context.Context, string) error { | |
| 143 | + | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?first" | |
| 144 | + | return nil | |
| 145 | + | }).Once() | |
| 146 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?second").RunAndReturn(func(context.Context, string) error { | |
| 147 | + | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?second" | |
| 148 | + | return nil | |
| 149 | + | }).Once() | |
| 150 | + | ||
| 151 | + | factory := mocks.NewMockWebViewFactory(t) | |
| 152 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| 153 | + | ||
| 154 | + | insertCalls := 0 | |
| 155 | + | c := &Coordinator{ | |
| 156 | + | webViews: make(map[entity.PaneID]port.WebView), | |
| 157 | + | pendingPopups: make(map[port.WebViewID]*PendingPopup), | |
| 158 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 159 | + | } | |
| 160 | + | c.SetPopupConfig(factory, nil, nil) | |
| 161 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { | |
| 162 | + | insertCalls++ | |
| 163 | + | return nil | |
| 164 | + | }) | |
| 165 | + | ||
| 166 | + | first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 167 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?first", | |
| 168 | + | FrameName: "g_credential_picker_x", | |
| 169 | + | IsUserGesture: true, | |
| 170 | + | }) | |
| 171 | + | require.Same(t, popupWV, first) | |
| 172 | + | ||
| 173 | + | second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |
| 174 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?second", | |
| 175 | + | FrameName: "g_credential_picker_x", | |
| 176 | + | IsUserGesture: true, | |
| 177 | + | }) | |
| 178 | + | require.Same(t, popupWV, second) | |
| 179 | + | ||
| 180 | + | assert.Equal(t, 1, insertCalls) | |
| 181 | + | assert.Equal(t, "https://accounts.google.com/o/oauth2/v2/auth?second", currentURI) | |
| 182 | + | } | |
| 183 | + | ||
| 126 | 184 | // --------------------------------------------------------------------------- | |
| 127 | 185 | // trackOAuthPopup / capturePopupOAuthState | |
| 128 | 186 | // --------------------------------------------------------------------------- | |
| @@ -6,9 +6,11 @@ import ( | |||||
| 6 | "testing" | 6 | "testing" | ||
| 7 | 7 | ||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 9 | "github.com/stretchr/testify/require" | 10 | "github.com/stretchr/testify/require" | ||
| 10 | 11 | ||||
| 11 | "github.com/bnema/dumber/internal/application/port" | 12 | "github.com/bnema/dumber/internal/application/port" | ||
| 13 | + | "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 12 | "github.com/bnema/dumber/internal/domain/entity" | 14 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 13 | ) | 15 | ) | ||
| 14 | 16 | ||||
| @@ -123,6 +125,62 @@ func TestGetBehavior_TabPopup_BlocksWhenOpenInNewPaneFalse(t *testing.T) { | |||||
| 123 | assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg)) | 125 | assert.Equal(t, entity.PopupBehaviorSplit, GetBehavior(PopupTypeTab, cfg)) | ||
| 124 | } | 126 | } | ||
| 125 | 127 | ||||
| 128 | + | func TestHandlePopupCreate_ReusesNamedPopup(t *testing.T) { | |||
| 129 | + | ctx := context.Background() | |||
| 130 | + | parentPaneID := entity.PaneID("parent-pane") | |||
| 131 | + | parentWV := mocks.NewMockWebView(t) | |||
| 132 | + | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Twice() | |||
| 133 | + | ||||
| 134 | + | popupWV := mocks.NewMockWebView(t) | |||
| 135 | + | popupWV.EXPECT().ID().Return(port.WebViewID(202)).Once() | |||
| 136 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |||
| 137 | + | popupWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |||
| 138 | + | popupWV.EXPECT().IsLoading().Return(false).Maybe() | |||
| 139 | + | currentURI := "" | |||
| 140 | + | popupWV.EXPECT().URI().RunAndReturn(func() string { return currentURI }).Maybe() | |||
| 141 | + | popupWV.EXPECT().IsDestroyed().Return(false).Maybe() | |||
| 142 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?first").RunAndReturn(func(context.Context, string) error { | |||
| 143 | + | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?first" | |||
| 144 | + | return nil | |||
| 145 | + | }).Once() | |||
| 146 | + | popupWV.EXPECT().LoadURI(mock.Anything, "https://accounts.google.com/o/oauth2/v2/auth?second").RunAndReturn(func(context.Context, string) error { | |||
| 147 | + | currentURI = "https://accounts.google.com/o/oauth2/v2/auth?second" | |||
| 148 | + | return nil | |||
| 149 | + | }).Once() | |||
| 150 | + | ||||
| 151 | + | factory := mocks.NewMockWebViewFactory(t) | |||
| 152 | + | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |||
| 153 | + | ||||
| 154 | + | insertCalls := 0 | |||
| 155 | + | c := &Coordinator{ | |||
| 156 | + | webViews: make(map[entity.PaneID]port.WebView), | |||
| 157 | + | pendingPopups: make(map[port.WebViewID]*PendingPopup), | |||
| 158 | + | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |||
| 159 | + | } | |||
| 160 | + | c.SetPopupConfig(factory, nil, nil) | |||
| 161 | + | c.SetOnInsertPopup(func(context.Context, InsertPopupInput) error { | |||
| 162 | + | insertCalls++ | |||
| 163 | + | return nil | |||
| 164 | + | }) | |||
| 165 | + | ||||
| 166 | + | first := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |||
| 167 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?first", | |||
| 168 | + | FrameName: "g_credential_picker_x", | |||
| 169 | + | IsUserGesture: true, | |||
| 170 | + | }) | |||
| 171 | + | require.Same(t, popupWV, first) | |||
| 172 | + | ||||
| 173 | + | second := c.handlePopupCreate(ctx, parentPaneID, parentWV, port.PopupRequest{ | |||
| 174 | + | TargetURI: "https://accounts.google.com/o/oauth2/v2/auth?second", | |||
| 175 | + | FrameName: "g_credential_picker_x", | |||
| 176 | + | IsUserGesture: true, | |||
| 177 | + | }) | |||
| 178 | + | require.Same(t, popupWV, second) | |||
| 179 | + | ||||
| 180 | + | assert.Equal(t, 1, insertCalls) | |||
| 181 | + | assert.Equal(t, "https://accounts.google.com/o/oauth2/v2/auth?second", currentURI) | |||
| 182 | + | } | |||
| 183 | + | ||||
| 126 | // --------------------------------------------------------------------------- | 184 | // --------------------------------------------------------------------------- | ||
| 127 | // trackOAuthPopup / capturePopupOAuthState | 185 | // trackOAuthPopup / capturePopupOAuthState | ||
| 128 | // --------------------------------------------------------------------------- | 186 | // --------------------------------------------------------------------------- | ||