fix(cef): resume parent oauth flow from popup callbacks
- commit
- bc78f4e1dbd6943135ecdd08eab90bcb81233209
- author
- brice · · 2026-04-21 14:37 UTC
- parent
- 3101cce
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +97 -29
M internal/infrastructure/cef/webview.go +9 -4
| @@ -1179,11 +1179,16 @@ func (wv *WebView) runCloseCallbacks() { | |||
| 1179 | 1179 | wv.mu.RLock() | |
| 1180 | 1180 | callbacks := append([]func(){}, wv.closeCallbacks...) | |
| 1181 | 1181 | wv.mu.RUnlock() | |
| 1182 | - | for _, fn := range callbacks { | |
| 1183 | - | if fn != nil { | |
| 1184 | - | fn() | |
| 1185 | - | } | |
| 1182 | + | if len(callbacks) == 0 { | |
| 1183 | + | return | |
| 1186 | 1184 | } | |
| 1185 | + | wv.runOnGTK(func() { | |
| 1186 | + | for _, fn := range callbacks { | |
| 1187 | + | if fn != nil { | |
| 1188 | + | fn() | |
| 1189 | + | } | |
| 1190 | + | } | |
| 1191 | + | }) | |
| 1187 | 1192 | } | |
| 1188 | 1193 | ||
| 1189 | 1194 | func (wv *WebView) runNavigationCallbacks(uri string) { | |
| @@ -1179,11 +1179,16 @@ func (wv *WebView) runCloseCallbacks() { | |||||
| 1179 | wv.mu.RLock() | 1179 | wv.mu.RLock() | ||
| 1180 | callbacks := append([]func(){}, wv.closeCallbacks...) | 1180 | callbacks := append([]func(){}, wv.closeCallbacks...) | ||
| 1181 | wv.mu.RUnlock() | 1181 | wv.mu.RUnlock() | ||
| 1182 | - | for _, fn := range callbacks { | 1182 | + | if len(callbacks) == 0 { |
| 1183 | - | if fn != nil { | 1183 | + | return |
| 1184 | - | fn() | |||
| 1185 | - | } | |||
| 1186 | } | 1184 | } | ||
| 1185 | + | wv.runOnGTK(func() { | |||
| 1186 | + | for _, fn := range callbacks { | |||
| 1187 | + | if fn != nil { | |||
| 1188 | + | fn() | |||
| 1189 | + | } | |||
| 1190 | + | } | |||
| 1191 | + | }) | |||
| 1187 | } | 1192 | } | ||
| 1188 | 1193 | ||||
| 1189 | func (wv *WebView) runNavigationCallbacks(uri string) { | 1194 | func (wv *WebView) runNavigationCallbacks(uri string) { | ||
M internal/ui/coordinator/content/oauth.go +47 -7
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| 11 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | + | urlutil "github.com/bnema/dumber/internal/domain/url" | |
| 12 | 13 | "github.com/bnema/dumber/internal/logging" | |
| 13 | 14 | "github.com/bnema/puregotk/v4/glib" | |
| 14 | 15 | ) | |
| @@ -349,6 +350,15 @@ func (c *Coordinator) capturePopupOAuthState(popupID port.WebViewID, uri string) | |||
| 349 | 350 | state.Error = IsOAuthError(uri) | |
| 350 | 351 | } | |
| 351 | 352 | ||
| 353 | + | func shouldLoadOAuthCallbackInParent(parentURI, callbackURI string) bool { | |
| 354 | + | if callbackURI == "" || !IsOAuthCallback(callbackURI) { | |
| 355 | + | return false | |
| 356 | + | } | |
| 357 | + | parentDomain := urlutil.ExtractDomain(parentURI) | |
| 358 | + | callbackDomain := urlutil.ExtractDomain(callbackURI) | |
| 359 | + | return parentDomain != "" && parentDomain == callbackDomain | |
| 360 | + | } | |
| 361 | + | ||
| 352 | 362 | func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.WebViewID) { | |
| 353 | 363 | log := logging.FromContext(ctx) | |
| 354 | 364 | ||
| @@ -370,17 +380,26 @@ func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.We | |||
| 370 | 380 | Bool("oauth_error", state.Error). | |
| 371 | 381 | Msg("popup oauth result captured on close") | |
| 372 | 382 | ||
| 373 | - | if !state.Success || state.ParentPaneID == "" { | |
| 383 | + | if state.ParentPaneID == "" { | |
| 384 | + | return | |
| 385 | + | } | |
| 386 | + | ||
| 387 | + | if strings.TrimSpace(state.CallbackURI) != "" { | |
| 388 | + | c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, state.CallbackURI) | |
| 389 | + | return | |
| 390 | + | } | |
| 391 | + | if !state.Success { | |
| 374 | 392 | return | |
| 375 | 393 | } | |
| 376 | 394 | ||
| 377 | - | c.scheduleParentPaneRefresh(ctx, state.ParentPaneID, popupID) | |
| 395 | + | c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, "") | |
| 378 | 396 | } | |
| 379 | 397 | ||
| 380 | - | func (c *Coordinator) scheduleParentPaneRefresh( | |
| 398 | + | func (c *Coordinator) scheduleParentPaneOAuthResume( | |
| 381 | 399 | ctx context.Context, | |
| 382 | 400 | parentPaneID entity.PaneID, | |
| 383 | 401 | popupID port.WebViewID, | |
| 402 | + | callbackURI string, | |
| 384 | 403 | ) { | |
| 385 | 404 | c.popupMu.Lock() | |
| 386 | 405 | if c.popupRefresh == nil { | |
| @@ -394,7 +413,7 @@ func (c *Coordinator) scheduleParentPaneRefresh( | |||
| 394 | 413 | delete(c.popupRefresh, parentPaneID) | |
| 395 | 414 | c.popupMu.Unlock() | |
| 396 | 415 | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 397 | - | c.refreshPaneAfterOAuth(ctx, parentPaneID, popupID) | |
| 416 | + | c.resumeParentPaneAfterOAuth(ctx, parentPaneID, popupID, callbackURI) | |
| 398 | 417 | return false | |
| 399 | 418 | }) | |
| 400 | 419 | glib.IdleAdd(&cb, 0) | |
| @@ -402,10 +421,11 @@ func (c *Coordinator) scheduleParentPaneRefresh( | |||
| 402 | 421 | c.popupMu.Unlock() | |
| 403 | 422 | } | |
| 404 | 423 | ||
| 405 | - | func (c *Coordinator) refreshPaneAfterOAuth( | |
| 424 | + | func (c *Coordinator) resumeParentPaneAfterOAuth( | |
| 406 | 425 | ctx context.Context, | |
| 407 | 426 | parentPaneID entity.PaneID, | |
| 408 | 427 | popupID port.WebViewID, | |
| 428 | + | callbackURI string, | |
| 409 | 429 | ) { | |
| 410 | 430 | log := logging.FromContext(ctx) | |
| 411 | 431 | wv := c.getWebViewLocked(parentPaneID) | |
| @@ -413,7 +433,27 @@ func (c *Coordinator) refreshPaneAfterOAuth( | |||
| 413 | 433 | log.Debug(). | |
| 414 | 434 | Str("parent_pane_id", string(parentPaneID)). | |
| 415 | 435 | Uint64("popup_id", uint64(popupID)). | |
| 416 | - | Msg("skipping parent pane refresh after oauth close: parent webview unavailable") | |
| 436 | + | Msg("skipping parent pane oauth resume: parent webview unavailable") | |
| 437 | + | return | |
| 438 | + | } | |
| 439 | + | ||
| 440 | + | parentURI := wv.URI() | |
| 441 | + | if shouldLoadOAuthCallbackInParent(parentURI, callbackURI) { | |
| 442 | + | if err := wv.LoadURI(ctx, callbackURI); err != nil { | |
| 443 | + | log.Warn(). | |
| 444 | + | Err(err). | |
| 445 | + | Str("parent_pane_id", string(parentPaneID)). | |
| 446 | + | Uint64("popup_id", uint64(popupID)). | |
| 447 | + | Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)). | |
| 448 | + | Msg("failed to load oauth callback URI in parent pane") | |
| 449 | + | return | |
| 450 | + | } | |
| 451 | + | ||
| 452 | + | log.Info(). | |
| 453 | + | Str("parent_pane_id", string(parentPaneID)). | |
| 454 | + | Uint64("popup_id", uint64(popupID)). | |
| 455 | + | Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)). | |
| 456 | + | Msg("loaded oauth callback URI in parent pane") | |
| 417 | 457 | return | |
| 418 | 458 | } | |
| 419 | 459 | ||
| @@ -429,5 +469,5 @@ func (c *Coordinator) refreshPaneAfterOAuth( | |||
| 429 | 469 | log.Info(). | |
| 430 | 470 | Str("parent_pane_id", string(parentPaneID)). | |
| 431 | 471 | Uint64("popup_id", uint64(popupID)). | |
| 432 | - | Msg("refreshed parent pane after oauth popup success") | |
| 472 | + | Msg("refreshed parent pane after oauth popup close") | |
| 433 | 473 | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | "github.com/bnema/dumber/internal/application/port" | 10 | "github.com/bnema/dumber/internal/application/port" | ||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 11 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 12 | + | urlutil "github.com/bnema/dumber/internal/domain/url" | |||
| 12 | "github.com/bnema/dumber/internal/logging" | 13 | "github.com/bnema/dumber/internal/logging" | ||
| 13 | "github.com/bnema/puregotk/v4/glib" | 14 | "github.com/bnema/puregotk/v4/glib" | ||
| 14 | ) | 15 | ) | ||
| @@ -349,6 +350,15 @@ func (c *Coordinator) capturePopupOAuthState(popupID port.WebViewID, uri string) | |||||
| 349 | state.Error = IsOAuthError(uri) | 350 | state.Error = IsOAuthError(uri) | ||
| 350 | } | 351 | } | ||
| 351 | 352 | ||||
| 353 | + | func shouldLoadOAuthCallbackInParent(parentURI, callbackURI string) bool { | |||
| 354 | + | if callbackURI == "" || !IsOAuthCallback(callbackURI) { | |||
| 355 | + | return false | |||
| 356 | + | } | |||
| 357 | + | parentDomain := urlutil.ExtractDomain(parentURI) | |||
| 358 | + | callbackDomain := urlutil.ExtractDomain(callbackURI) | |||
| 359 | + | return parentDomain != "" && parentDomain == callbackDomain | |||
| 360 | + | } | |||
| 361 | + | ||||
| 352 | func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.WebViewID) { | 362 | func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.WebViewID) { | ||
| 353 | log := logging.FromContext(ctx) | 363 | log := logging.FromContext(ctx) | ||
| 354 | 364 | ||||
| @@ -370,17 +380,26 @@ func (c *Coordinator) handlePopupOAuthClose(ctx context.Context, popupID port.We | |||||
| 370 | Bool("oauth_error", state.Error). | 380 | Bool("oauth_error", state.Error). | ||
| 371 | Msg("popup oauth result captured on close") | 381 | Msg("popup oauth result captured on close") | ||
| 372 | 382 | ||||
| 373 | - | if !state.Success || state.ParentPaneID == "" { | 383 | + | if state.ParentPaneID == "" { |
| 384 | + | return | |||
| 385 | + | } | |||
| 386 | + | ||||
| 387 | + | if strings.TrimSpace(state.CallbackURI) != "" { | |||
| 388 | + | c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, state.CallbackURI) | |||
| 389 | + | return | |||
| 390 | + | } | |||
| 391 | + | if !state.Success { | |||
| 374 | return | 392 | return | ||
| 375 | } | 393 | } | ||
| 376 | 394 | ||||
| 377 | - | c.scheduleParentPaneRefresh(ctx, state.ParentPaneID, popupID) | 395 | + | c.scheduleParentPaneOAuthResume(ctx, state.ParentPaneID, popupID, "") |
| 378 | } | 396 | } | ||
| 379 | 397 | ||||
| 380 | - | func (c *Coordinator) scheduleParentPaneRefresh( | 398 | + | func (c *Coordinator) scheduleParentPaneOAuthResume( |
| 381 | ctx context.Context, | 399 | ctx context.Context, | ||
| 382 | parentPaneID entity.PaneID, | 400 | parentPaneID entity.PaneID, | ||
| 383 | popupID port.WebViewID, | 401 | popupID port.WebViewID, | ||
| 402 | + | callbackURI string, | |||
| 384 | ) { | 403 | ) { | ||
| 385 | c.popupMu.Lock() | 404 | c.popupMu.Lock() | ||
| 386 | if c.popupRefresh == nil { | 405 | if c.popupRefresh == nil { | ||
| @@ -394,7 +413,7 @@ func (c *Coordinator) scheduleParentPaneRefresh( | |||||
| 394 | delete(c.popupRefresh, parentPaneID) | 413 | delete(c.popupRefresh, parentPaneID) | ||
| 395 | c.popupMu.Unlock() | 414 | c.popupMu.Unlock() | ||
| 396 | cb := glib.SourceFunc(func(_ uintptr) bool { | 415 | cb := glib.SourceFunc(func(_ uintptr) bool { | ||
| 397 | - | c.refreshPaneAfterOAuth(ctx, parentPaneID, popupID) | 416 | + | c.resumeParentPaneAfterOAuth(ctx, parentPaneID, popupID, callbackURI) |
| 398 | return false | 417 | return false | ||
| 399 | }) | 418 | }) | ||
| 400 | glib.IdleAdd(&cb, 0) | 419 | glib.IdleAdd(&cb, 0) | ||
| @@ -402,10 +421,11 @@ func (c *Coordinator) scheduleParentPaneRefresh( | |||||
| 402 | c.popupMu.Unlock() | 421 | c.popupMu.Unlock() | ||
| 403 | } | 422 | } | ||
| 404 | 423 | ||||
| 405 | - | func (c *Coordinator) refreshPaneAfterOAuth( | 424 | + | func (c *Coordinator) resumeParentPaneAfterOAuth( |
| 406 | ctx context.Context, | 425 | ctx context.Context, | ||
| 407 | parentPaneID entity.PaneID, | 426 | parentPaneID entity.PaneID, | ||
| 408 | popupID port.WebViewID, | 427 | popupID port.WebViewID, | ||
| 428 | + | callbackURI string, | |||
| 409 | ) { | 429 | ) { | ||
| 410 | log := logging.FromContext(ctx) | 430 | log := logging.FromContext(ctx) | ||
| 411 | wv := c.getWebViewLocked(parentPaneID) | 431 | wv := c.getWebViewLocked(parentPaneID) | ||
| @@ -413,7 +433,27 @@ func (c *Coordinator) refreshPaneAfterOAuth( | |||||
| 413 | log.Debug(). | 433 | log.Debug(). | ||
| 414 | Str("parent_pane_id", string(parentPaneID)). | 434 | Str("parent_pane_id", string(parentPaneID)). | ||
| 415 | Uint64("popup_id", uint64(popupID)). | 435 | Uint64("popup_id", uint64(popupID)). | ||
| 416 | - | Msg("skipping parent pane refresh after oauth close: parent webview unavailable") | 436 | + | Msg("skipping parent pane oauth resume: parent webview unavailable") |
| 437 | + | return | |||
| 438 | + | } | |||
| 439 | + | ||||
| 440 | + | parentURI := wv.URI() | |||
| 441 | + | if shouldLoadOAuthCallbackInParent(parentURI, callbackURI) { | |||
| 442 | + | if err := wv.LoadURI(ctx, callbackURI); err != nil { | |||
| 443 | + | log.Warn(). | |||
| 444 | + | Err(err). | |||
| 445 | + | Str("parent_pane_id", string(parentPaneID)). | |||
| 446 | + | Uint64("popup_id", uint64(popupID)). | |||
| 447 | + | Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)). | |||
| 448 | + | Msg("failed to load oauth callback URI in parent pane") | |||
| 449 | + | return | |||
| 450 | + | } | |||
| 451 | + | ||||
| 452 | + | log.Info(). | |||
| 453 | + | Str("parent_pane_id", string(parentPaneID)). | |||
| 454 | + | Uint64("popup_id", uint64(popupID)). | |||
| 455 | + | Str("callback_uri", logging.TruncateURL(callbackURI, logURLMaxLen)). | |||
| 456 | + | Msg("loaded oauth callback URI in parent pane") | |||
| 417 | return | 457 | return | ||
| 418 | } | 458 | } | ||
| 419 | 459 | ||||
| @@ -429,5 +469,5 @@ func (c *Coordinator) refreshPaneAfterOAuth( | |||||
| 429 | log.Info(). | 469 | log.Info(). | ||
| 430 | Str("parent_pane_id", string(parentPaneID)). | 470 | Str("parent_pane_id", string(parentPaneID)). | ||
| 431 | Uint64("popup_id", uint64(popupID)). | 471 | Uint64("popup_id", uint64(popupID)). | ||
| 432 | - | Msg("refreshed parent pane after oauth popup success") | 472 | + | Msg("refreshed parent pane after oauth popup close") |
| 433 | } | 473 | } | ||
M internal/ui/coordinator/content/oauth_test.go +30 -13
| @@ -6,8 +6,10 @@ import ( | |||
| 6 | 6 | "time" | |
| 7 | 7 | ||
| 8 | 8 | "github.com/stretchr/testify/assert" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/bnema/dumber/internal/application/port" | |
| 12 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | 13 | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | 14 | ) | |
| 13 | 15 | ||
| @@ -47,7 +49,7 @@ func TestComposeOnLoadChanged_Order(t *testing.T) { | |||
| 47 | 49 | assert.Equal(t, []string{"existing", "next"}, calls) | |
| 48 | 50 | } | |
| 49 | 51 | ||
| 50 | - | func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |
| 52 | + | func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) { | |
| 51 | 53 | parentPaneID := entity.PaneID("parent-pane") | |
| 52 | 54 | popupID := port.WebViewID(101) | |
| 53 | 55 | ||
| @@ -58,7 +60,7 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||
| 58 | 60 | } | |
| 59 | 61 | ||
| 60 | 62 | c.trackOAuthPopup(popupID, parentPaneID) | |
| 61 | - | c.capturePopupOAuthState(popupID, "https://app/callback?code=123") | |
| 63 | + | c.capturePopupOAuthState(popupID, "https://www.notion.so/googlepopupcallback?code=123") | |
| 62 | 64 | c.handlePopupOAuthClose(context.Background(), popupID) | |
| 63 | 65 | ||
| 64 | 66 | c.popupMu.RLock() | |
| @@ -67,7 +69,7 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||
| 67 | 69 | c.popupMu.RUnlock() | |
| 68 | 70 | ||
| 69 | 71 | assert.False(t, exists, "oauth state should be removed after close handling") | |
| 70 | - | assert.NotNil(t, refreshTimer, "successful oauth close should schedule parent refresh") | |
| 72 | + | assert.NotNil(t, refreshTimer, "oauth callback should schedule parent resume") | |
| 71 | 73 | ||
| 72 | 74 | waitFor(t, time.Second, func() bool { | |
| 73 | 75 | c.popupMu.RLock() | |
| @@ -76,23 +78,38 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||
| 76 | 78 | }) | |
| 77 | 79 | } | |
| 78 | 80 | ||
| 79 | - | func TestHandlePopupOAuthClose_ErrorDoesNotScheduleParentRefresh(t *testing.T) { | |
| 81 | + | func TestResumeParentPaneAfterOAuth_SameDomainLoadsCallbackURI(t *testing.T) { | |
| 80 | 82 | parentPaneID := entity.PaneID("parent-pane") | |
| 81 | 83 | popupID := port.WebViewID(102) | |
| 84 | + | parentWV := portmocks.NewMockWebView(t) | |
| 85 | + | parentWV.EXPECT().IsDestroyed().Return(false).Once() | |
| 86 | + | parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once() | |
| 87 | + | parentWV.EXPECT().LoadURI(mock.Anything, "https://www.notion.so/googlepopupcallback?code=123").Return(nil).Once() | |
| 82 | 88 | ||
| 83 | 89 | c := &Coordinator{ | |
| 84 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 85 | - | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | |
| 86 | - | popupRefresh: make(map[entity.PaneID]*time.Timer), | |
| 90 | + | webViews: map[entity.PaneID]port.WebView{ | |
| 91 | + | parentPaneID: parentWV, | |
| 92 | + | }, | |
| 87 | 93 | } | |
| 88 | 94 | ||
| 89 | - | c.trackOAuthPopup(popupID, parentPaneID) | |
| 90 | - | c.capturePopupOAuthState(popupID, "https://app/callback?error=access_denied") | |
| 91 | - | c.handlePopupOAuthClose(context.Background(), popupID) | |
| 95 | + | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/googlepopupcallback?code=123") | |
| 96 | + | } | |
| 92 | 97 | ||
| 93 | - | c.popupMu.RLock() | |
| 94 | - | defer c.popupMu.RUnlock() | |
| 95 | - | assert.Nil(t, c.popupRefresh[parentPaneID], "oauth errors should not refresh parent pane") | |
| 98 | + | func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) { | |
| 99 | + | parentPaneID := entity.PaneID("parent-pane") | |
| 100 | + | popupID := port.WebViewID(103) | |
| 101 | + | parentWV := portmocks.NewMockWebView(t) | |
| 102 | + | parentWV.EXPECT().IsDestroyed().Return(false).Once() | |
| 103 | + | parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once() | |
| 104 | + | parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once() | |
| 105 | + | ||
| 106 | + | c := &Coordinator{ | |
| 107 | + | webViews: map[entity.PaneID]port.WebView{ | |
| 108 | + | parentPaneID: parentWV, | |
| 109 | + | }, | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://accounts.example.com/callback?code=123") | |
| 96 | 113 | } | |
| 97 | 114 | ||
| 98 | 115 | func TestIsOAuthURL_DoesNotTreatGenericStateParamAsOAuth(t *testing.T) { | |
| @@ -6,8 +6,10 @@ import ( | |||||
| 6 | "time" | 6 | "time" | ||
| 7 | 7 | ||||
| 8 | "github.com/stretchr/testify/assert" | 8 | "github.com/stretchr/testify/assert" | ||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 9 | 10 | ||||
| 10 | "github.com/bnema/dumber/internal/application/port" | 11 | "github.com/bnema/dumber/internal/application/port" | ||
| 12 | + | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 13 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 12 | ) | 14 | ) | ||
| 13 | 15 | ||||
| @@ -47,7 +49,7 @@ func TestComposeOnLoadChanged_Order(t *testing.T) { | |||||
| 47 | assert.Equal(t, []string{"existing", "next"}, calls) | 49 | assert.Equal(t, []string{"existing", "next"}, calls) | ||
| 48 | } | 50 | } | ||
| 49 | 51 | ||||
| 50 | - | func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | 52 | + | func TestHandlePopupOAuthClose_SuccessSchedulesParentResume(t *testing.T) { |
| 51 | parentPaneID := entity.PaneID("parent-pane") | 53 | parentPaneID := entity.PaneID("parent-pane") | ||
| 52 | popupID := port.WebViewID(101) | 54 | popupID := port.WebViewID(101) | ||
| 53 | 55 | ||||
| @@ -58,7 +60,7 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||||
| 58 | } | 60 | } | ||
| 59 | 61 | ||||
| 60 | c.trackOAuthPopup(popupID, parentPaneID) | 62 | c.trackOAuthPopup(popupID, parentPaneID) | ||
| 61 | - | c.capturePopupOAuthState(popupID, "https://app/callback?code=123") | 63 | + | c.capturePopupOAuthState(popupID, "https://www.notion.so/googlepopupcallback?code=123") |
| 62 | c.handlePopupOAuthClose(context.Background(), popupID) | 64 | c.handlePopupOAuthClose(context.Background(), popupID) | ||
| 63 | 65 | ||||
| 64 | c.popupMu.RLock() | 66 | c.popupMu.RLock() | ||
| @@ -67,7 +69,7 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||||
| 67 | c.popupMu.RUnlock() | 69 | c.popupMu.RUnlock() | ||
| 68 | 70 | ||||
| 69 | assert.False(t, exists, "oauth state should be removed after close handling") | 71 | assert.False(t, exists, "oauth state should be removed after close handling") | ||
| 70 | - | assert.NotNil(t, refreshTimer, "successful oauth close should schedule parent refresh") | 72 | + | assert.NotNil(t, refreshTimer, "oauth callback should schedule parent resume") |
| 71 | 73 | ||||
| 72 | waitFor(t, time.Second, func() bool { | 74 | waitFor(t, time.Second, func() bool { | ||
| 73 | c.popupMu.RLock() | 75 | c.popupMu.RLock() | ||
| @@ -76,23 +78,38 @@ func TestHandlePopupOAuthClose_SuccessSchedulesParentRefresh(t *testing.T) { | |||||
| 76 | }) | 78 | }) | ||
| 77 | } | 79 | } | ||
| 78 | 80 | ||||
| 79 | - | func TestHandlePopupOAuthClose_ErrorDoesNotScheduleParentRefresh(t *testing.T) { | 81 | + | func TestResumeParentPaneAfterOAuth_SameDomainLoadsCallbackURI(t *testing.T) { |
| 80 | parentPaneID := entity.PaneID("parent-pane") | 82 | parentPaneID := entity.PaneID("parent-pane") | ||
| 81 | popupID := port.WebViewID(102) | 83 | popupID := port.WebViewID(102) | ||
| 84 | + | parentWV := portmocks.NewMockWebView(t) | |||
| 85 | + | parentWV.EXPECT().IsDestroyed().Return(false).Once() | |||
| 86 | + | parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once() | |||
| 87 | + | parentWV.EXPECT().LoadURI(mock.Anything, "https://www.notion.so/googlepopupcallback?code=123").Return(nil).Once() | |||
| 82 | 88 | ||||
| 83 | c := &Coordinator{ | 89 | c := &Coordinator{ | ||
| 84 | - | webViews: make(map[entity.PaneID]port.WebView), | 90 | + | webViews: map[entity.PaneID]port.WebView{ |
| 85 | - | popupOAuth: make(map[port.WebViewID]*popupOAuthState), | 91 | + | parentPaneID: parentWV, |
| 86 | - | popupRefresh: make(map[entity.PaneID]*time.Timer), | 92 | + | }, |
| 87 | } | 93 | } | ||
| 88 | 94 | ||||
| 89 | - | c.trackOAuthPopup(popupID, parentPaneID) | 95 | + | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/googlepopupcallback?code=123") |
| 90 | - | c.capturePopupOAuthState(popupID, "https://app/callback?error=access_denied") | 96 | + | } |
| 91 | - | c.handlePopupOAuthClose(context.Background(), popupID) | |||
| 92 | 97 | ||||
| 93 | - | c.popupMu.RLock() | 98 | + | func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing.T) { |
| 94 | - | defer c.popupMu.RUnlock() | 99 | + | parentPaneID := entity.PaneID("parent-pane") |
| 95 | - | assert.Nil(t, c.popupRefresh[parentPaneID], "oauth errors should not refresh parent pane") | 100 | + | popupID := port.WebViewID(103) |
| 101 | + | parentWV := portmocks.NewMockWebView(t) | |||
| 102 | + | parentWV.EXPECT().IsDestroyed().Return(false).Once() | |||
| 103 | + | parentWV.EXPECT().URI().Return("https://www.notion.so/login").Once() | |||
| 104 | + | parentWV.EXPECT().Reload(mock.Anything).Return(nil).Once() | |||
| 105 | + | ||||
| 106 | + | c := &Coordinator{ | |||
| 107 | + | webViews: map[entity.PaneID]port.WebView{ | |||
| 108 | + | parentPaneID: parentWV, | |||
| 109 | + | }, | |||
| 110 | + | } | |||
| 111 | + | ||||
| 112 | + | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://accounts.example.com/callback?code=123") | |||
| 96 | } | 113 | } | ||
| 97 | 114 | ||||
| 98 | func TestIsOAuthURL_DoesNotTreatGenericStateParamAsOAuth(t *testing.T) { | 115 | func TestIsOAuthURL_DoesNotTreatGenericStateParamAsOAuth(t *testing.T) { | ||
M internal/ui/coordinator/content/popup.go +11 -5
| @@ -676,14 +676,16 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 676 | 676 | c.popupMu.Unlock() | |
| 677 | 677 | ||
| 678 | 678 | if wasPending && pending != nil { | |
| 679 | + | c.handlePopupOAuthClose(ctx, popupID) | |
| 679 | 680 | if c.onClosePane != nil { | |
| 680 | 681 | if err := c.onClosePane(ctx, pending.PaneID); err != nil { | |
| 681 | 682 | log.Warn().Err(err).Str("pane_id", string(pending.PaneID)).Msg("failed to close pending popup pane") | |
| 682 | 683 | } | |
| 683 | 684 | } | |
| 684 | - | c.handlePopupOAuthClose(ctx, popupID) | |
| 685 | 685 | c.clearReusableNamedPopupByWebViewID(popupID) | |
| 686 | - | c.ReleaseWebView(ctx, pending.PaneID) | |
| 686 | + | if c.getWebViewLocked(pending.PaneID) != nil { | |
| 687 | + | c.ReleaseWebView(ctx, pending.PaneID) | |
| 688 | + | } | |
| 687 | 689 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | |
| 688 | 690 | return | |
| 689 | 691 | } | |
| @@ -706,6 +708,8 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 706 | 708 | return | |
| 707 | 709 | } | |
| 708 | 710 | ||
| 711 | + | c.handlePopupOAuthClose(ctx, popupID) | |
| 712 | + | ||
| 709 | 713 | // Close the pane in workspace (this removes the UI element) | |
| 710 | 714 | if c.onClosePane != nil { | |
| 711 | 715 | if err := c.onClosePane(ctx, paneID); err != nil { | |
| @@ -713,11 +717,13 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||
| 713 | 717 | } | |
| 714 | 718 | } | |
| 715 | 719 | ||
| 716 | - | c.handlePopupOAuthClose(ctx, popupID) | |
| 717 | 720 | c.clearReusableNamedPopupByWebViewID(popupID) | |
| 718 | 721 | ||
| 719 | - | // Release the WebView (this will clean up tracking) | |
| 720 | - | c.ReleaseWebView(ctx, paneID) | |
| 722 | + | // ClosePaneByID usually releases the WebView already; only release here if it | |
| 723 | + | // is still registered. | |
| 724 | + | if c.getWebViewLocked(paneID) != nil { | |
| 725 | + | c.ReleaseWebView(ctx, paneID) | |
| 726 | + | } | |
| 721 | 727 | ||
| 722 | 728 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | |
| 723 | 729 | } | |
| @@ -676,14 +676,16 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 676 | c.popupMu.Unlock() | 676 | c.popupMu.Unlock() | ||
| 677 | 677 | ||||
| 678 | if wasPending && pending != nil { | 678 | if wasPending && pending != nil { | ||
| 679 | + | c.handlePopupOAuthClose(ctx, popupID) | |||
| 679 | if c.onClosePane != nil { | 680 | if c.onClosePane != nil { | ||
| 680 | if err := c.onClosePane(ctx, pending.PaneID); err != nil { | 681 | if err := c.onClosePane(ctx, pending.PaneID); err != nil { | ||
| 681 | log.Warn().Err(err).Str("pane_id", string(pending.PaneID)).Msg("failed to close pending popup pane") | 682 | log.Warn().Err(err).Str("pane_id", string(pending.PaneID)).Msg("failed to close pending popup pane") | ||
| 682 | } | 683 | } | ||
| 683 | } | 684 | } | ||
| 684 | - | c.handlePopupOAuthClose(ctx, popupID) | |||
| 685 | c.clearReusableNamedPopupByWebViewID(popupID) | 685 | c.clearReusableNamedPopupByWebViewID(popupID) | ||
| 686 | - | c.ReleaseWebView(ctx, pending.PaneID) | 686 | + | if c.getWebViewLocked(pending.PaneID) != nil { |
| 687 | + | c.ReleaseWebView(ctx, pending.PaneID) | |||
| 688 | + | } | |||
| 687 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | 689 | log.Debug().Str("pane_id", string(pending.PaneID)).Msg("cleaned up pending popup that was never shown") | ||
| 688 | return | 690 | return | ||
| 689 | } | 691 | } | ||
| @@ -706,6 +708,8 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 706 | return | 708 | return | ||
| 707 | } | 709 | } | ||
| 708 | 710 | ||||
| 711 | + | c.handlePopupOAuthClose(ctx, popupID) | |||
| 712 | + | ||||
| 709 | // Close the pane in workspace (this removes the UI element) | 713 | // Close the pane in workspace (this removes the UI element) | ||
| 710 | if c.onClosePane != nil { | 714 | if c.onClosePane != nil { | ||
| 711 | if err := c.onClosePane(ctx, paneID); err != nil { | 715 | if err := c.onClosePane(ctx, paneID); err != nil { | ||
| @@ -713,11 +717,13 @@ func (c *Coordinator) handlePopupClose(ctx context.Context, popupID port.WebView | |||||
| 713 | } | 717 | } | ||
| 714 | } | 718 | } | ||
| 715 | 719 | ||||
| 716 | - | c.handlePopupOAuthClose(ctx, popupID) | |||
| 717 | c.clearReusableNamedPopupByWebViewID(popupID) | 720 | c.clearReusableNamedPopupByWebViewID(popupID) | ||
| 718 | 721 | ||||
| 719 | - | // Release the WebView (this will clean up tracking) | 722 | + | // ClosePaneByID usually releases the WebView already; only release here if it |
| 720 | - | c.ReleaseWebView(ctx, paneID) | 723 | + | // is still registered. |
| 724 | + | if c.getWebViewLocked(paneID) != nil { | |||
| 725 | + | c.ReleaseWebView(ctx, paneID) | |||
| 726 | + | } | |||
| 721 | 727 | ||||
| 722 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | 728 | log.Info().Str("pane_id", string(paneID)).Msg("popup closed") | ||
| 723 | } | 729 | } | ||