content: harden async popup callbacks
- commit
- 1f4013cb0129941f616bf67b99a96b45c4d966e1
- author
- brice · · 2026-04-23 14:31 UTC
- parent
- 5ef1bed
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +15 -6
M internal/infrastructure/cef/factory.go +9 -1
| @@ -293,7 +293,15 @@ func (f *WebViewFactory) schedulePendingBrowserCreateRetry( | |||
| 293 | 293 | w, h int32, | |
| 294 | 294 | pc *pendingBrowserCreate, | |
| 295 | 295 | ) { | |
| 296 | - | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() || pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | |
| 296 | + | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() { | |
| 297 | + | return | |
| 298 | + | } | |
| 299 | + | if pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | |
| 300 | + | logging.FromContext(ctx).Warn(). | |
| 301 | + | Uint64("webview_id", uint64(wv.id)). | |
| 302 | + | Int("retries", pc.postTaskRetries). | |
| 303 | + | Int("max_retries", pendingBrowserCreateMaxPostRetries). | |
| 304 | + | Msg("cef: browser creation retries exhausted") | |
| 297 | 305 | return | |
| 298 | 306 | } | |
| 299 | 307 | cefScheduleAfter(pendingBrowserCreateRetryDelay, func() { | |
| @@ -293,7 +293,15 @@ func (f *WebViewFactory) schedulePendingBrowserCreateRetry( | |||||
| 293 | w, h int32, | 293 | w, h int32, | ||
| 294 | pc *pendingBrowserCreate, | 294 | pc *pendingBrowserCreate, | ||
| 295 | ) { | 295 | ) { | ||
| 296 | - | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() || pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | 296 | + | if f == nil || wv == nil || pc == nil || wv.destroyed.Load() { |
| 297 | + | return | |||
| 298 | + | } | |||
| 299 | + | if pc.postTaskRetries > pendingBrowserCreateMaxPostRetries { | |||
| 300 | + | logging.FromContext(ctx).Warn(). | |||
| 301 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 302 | + | Int("retries", pc.postTaskRetries). | |||
| 303 | + | Int("max_retries", pendingBrowserCreateMaxPostRetries). | |||
| 304 | + | Msg("cef: browser creation retries exhausted") | |||
| 297 | return | 305 | return | ||
| 298 | } | 306 | } | ||
| 299 | cefScheduleAfter(pendingBrowserCreateRetryDelay, func() { | 307 | cefScheduleAfter(pendingBrowserCreateRetryDelay, func() { | ||
M internal/ui/coordinator/content/oauth_test.go +1 -0
| @@ -246,6 +246,7 @@ func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing. | |||
| 246 | 246 | webViews: map[entity.PaneID]port.WebView{ | |
| 247 | 247 | parentPaneID: parentWV, | |
| 248 | 248 | }, | |
| 249 | + | popups: newPopupManager(), | |
| 249 | 250 | } | |
| 250 | 251 | ||
| 251 | 252 | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://accounts.example.com/callback?code=123") | |
| @@ -246,6 +246,7 @@ func TestResumeParentPaneAfterOAuth_DifferentDomainFallsBackToReload(t *testing. | |||||
| 246 | webViews: map[entity.PaneID]port.WebView{ | 246 | webViews: map[entity.PaneID]port.WebView{ | ||
| 247 | parentPaneID: parentWV, | 247 | parentPaneID: parentWV, | ||
| 248 | }, | 248 | }, | ||
| 249 | + | popups: newPopupManager(), | |||
| 249 | } | 250 | } | ||
| 250 | 251 | ||||
| 251 | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://accounts.example.com/callback?code=123") | 252 | c.resumeParentPaneAfterOAuth(context.Background(), parentPaneID, popupID, "https://www.notion.so/login", "https://accounts.example.com/callback?code=123") | ||
M internal/ui/coordinator/content/popup_manager.go +5 -5
| @@ -354,7 +354,7 @@ func (pm *popupManager) createPopupWebView( | |||
| 354 | 354 | } | |
| 355 | 355 | ||
| 356 | 356 | log := logging.FromContext(ctx) | |
| 357 | - | relatedErr := error(nil) | |
| 357 | + | var relatedErr error | |
| 358 | 358 | ||
| 359 | 359 | if !noJavaScriptAccess && !pm.relatedPopupSupportDisabled() { | |
| 360 | 360 | popupWV, err := pm.factory.CreateRelated(ctx, parentID) | |
| @@ -579,7 +579,7 @@ func (pm *popupManager) finishPopupCreate( | |||
| 579 | 579 | if _, hasNativePopupLifecycle := create.PopupWebView.(port.PopupLifecycleCapable); !hasNativePopupLifecycle { | |
| 580 | 580 | if closeCapable, ok := create.PopupWebView.(port.OAuthCallbackCapable); ok { | |
| 581 | 581 | closeCapable.AddCloseCallback(func() { | |
| 582 | - | pm.handlePopupClose(ctx, hooks, create.PopupID) | |
| 582 | + | pm.handlePopupClose(context.Background(), hooks, create.PopupID) | |
| 583 | 583 | }) | |
| 584 | 584 | } | |
| 585 | 585 | } | |
| @@ -608,10 +608,10 @@ func (pm *popupManager) finishPopupCreate( | |||
| 608 | 608 | ||
| 609 | 609 | if lifecycle, ok := create.PopupWebView.(port.PopupLifecycleCapable); ok { | |
| 610 | 610 | lifecycle.SetOnReadyToShow(func() { | |
| 611 | - | pm.handlePopupReadyToShow(ctx, create.PopupID) | |
| 611 | + | pm.handlePopupReadyToShow(context.Background(), create.PopupID) | |
| 612 | 612 | }) | |
| 613 | 613 | lifecycle.SetOnClose(func() { | |
| 614 | - | pm.handlePopupClose(ctx, hooks, create.PopupID) | |
| 614 | + | pm.handlePopupClose(context.Background(), hooks, create.PopupID) | |
| 615 | 615 | }) | |
| 616 | 616 | log.Info(). | |
| 617 | 617 | Uint64("popup_id", uint64(create.PopupID)). | |
| @@ -626,7 +626,7 @@ func (pm *popupManager) finishPopupCreate( | |||
| 626 | 626 | Str("popup_type", create.PopupType.String()). | |
| 627 | 627 | Str("target_uri", logging.TruncateURL(create.Request.TargetURI, logURLMaxLen)). | |
| 628 | 628 | Msg("popup inserted, immediately ready (no PopupLifecycleCapable)") | |
| 629 | - | pm.handlePopupReadyToShow(ctx, create.PopupID) | |
| 629 | + | pm.handlePopupReadyToShow(context.Background(), create.PopupID) | |
| 630 | 630 | } | |
| 631 | 631 | ||
| 632 | 632 | return create.PopupWebView | |
| @@ -354,7 +354,7 @@ func (pm *popupManager) createPopupWebView( | |||||
| 354 | } | 354 | } | ||
| 355 | 355 | ||||
| 356 | log := logging.FromContext(ctx) | 356 | log := logging.FromContext(ctx) | ||
| 357 | - | relatedErr := error(nil) | 357 | + | var relatedErr error |
| 358 | 358 | ||||
| 359 | if !noJavaScriptAccess && !pm.relatedPopupSupportDisabled() { | 359 | if !noJavaScriptAccess && !pm.relatedPopupSupportDisabled() { | ||
| 360 | popupWV, err := pm.factory.CreateRelated(ctx, parentID) | 360 | popupWV, err := pm.factory.CreateRelated(ctx, parentID) | ||
| @@ -579,7 +579,7 @@ func (pm *popupManager) finishPopupCreate( | |||||
| 579 | if _, hasNativePopupLifecycle := create.PopupWebView.(port.PopupLifecycleCapable); !hasNativePopupLifecycle { | 579 | if _, hasNativePopupLifecycle := create.PopupWebView.(port.PopupLifecycleCapable); !hasNativePopupLifecycle { | ||
| 580 | if closeCapable, ok := create.PopupWebView.(port.OAuthCallbackCapable); ok { | 580 | if closeCapable, ok := create.PopupWebView.(port.OAuthCallbackCapable); ok { | ||
| 581 | closeCapable.AddCloseCallback(func() { | 581 | closeCapable.AddCloseCallback(func() { | ||
| 582 | - | pm.handlePopupClose(ctx, hooks, create.PopupID) | 582 | + | pm.handlePopupClose(context.Background(), hooks, create.PopupID) |
| 583 | }) | 583 | }) | ||
| 584 | } | 584 | } | ||
| 585 | } | 585 | } | ||
| @@ -608,10 +608,10 @@ func (pm *popupManager) finishPopupCreate( | |||||
| 608 | 608 | ||||
| 609 | if lifecycle, ok := create.PopupWebView.(port.PopupLifecycleCapable); ok { | 609 | if lifecycle, ok := create.PopupWebView.(port.PopupLifecycleCapable); ok { | ||
| 610 | lifecycle.SetOnReadyToShow(func() { | 610 | lifecycle.SetOnReadyToShow(func() { | ||
| 611 | - | pm.handlePopupReadyToShow(ctx, create.PopupID) | 611 | + | pm.handlePopupReadyToShow(context.Background(), create.PopupID) |
| 612 | }) | 612 | }) | ||
| 613 | lifecycle.SetOnClose(func() { | 613 | lifecycle.SetOnClose(func() { | ||
| 614 | - | pm.handlePopupClose(ctx, hooks, create.PopupID) | 614 | + | pm.handlePopupClose(context.Background(), hooks, create.PopupID) |
| 615 | }) | 615 | }) | ||
| 616 | log.Info(). | 616 | log.Info(). | ||
| 617 | Uint64("popup_id", uint64(create.PopupID)). | 617 | Uint64("popup_id", uint64(create.PopupID)). | ||
| @@ -626,7 +626,7 @@ func (pm *popupManager) finishPopupCreate( | |||||
| 626 | Str("popup_type", create.PopupType.String()). | 626 | Str("popup_type", create.PopupType.String()). | ||
| 627 | Str("target_uri", logging.TruncateURL(create.Request.TargetURI, logURLMaxLen)). | 627 | Str("target_uri", logging.TruncateURL(create.Request.TargetURI, logURLMaxLen)). | ||
| 628 | Msg("popup inserted, immediately ready (no PopupLifecycleCapable)") | 628 | Msg("popup inserted, immediately ready (no PopupLifecycleCapable)") | ||
| 629 | - | pm.handlePopupReadyToShow(ctx, create.PopupID) | 629 | + | pm.handlePopupReadyToShow(context.Background(), create.PopupID) |
| 630 | } | 630 | } | ||
| 631 | 631 | ||||
| 632 | return create.PopupWebView | 632 | return create.PopupWebView | ||