Merge pull request #273 from bnema/fix/cef-vulkan-dispatch-hardening
fix(cef): harden Vulkan navigation handoff
20 files · +1439 -139
M internal/infrastructure/cef/cef2gtk_boundary_test.go +1 -1
| @@ -19,7 +19,7 @@ func TestCEF2GTKImportStaysInInfrastructureCEF(t *testing.T) { | |||
| 19 | 19 | } | |
| 20 | 20 | if d.IsDir() { | |
| 21 | 21 | switch d.Name() { | |
| 22 | - | case ".git", "vendor", "dist", "tmp": | |
| 22 | + | case ".git", ".worktrees", "vendor", "dist", "tmp": | |
| 23 | 23 | return filepath.SkipDir | |
| 24 | 24 | } | |
| 25 | 25 | return nil | |
| @@ -19,7 +19,7 @@ func TestCEF2GTKImportStaysInInfrastructureCEF(t *testing.T) { | |||||
| 19 | } | 19 | } | ||
| 20 | if d.IsDir() { | 20 | if d.IsDir() { | ||
| 21 | switch d.Name() { | 21 | switch d.Name() { | ||
| 22 | - | case ".git", "vendor", "dist", "tmp": | 22 | + | case ".git", ".worktrees", "vendor", "dist", "tmp": |
| 23 | return filepath.SkipDir | 23 | return filepath.SkipDir | ||
| 24 | } | 24 | } | ||
| 25 | return nil | 25 | return nil | ||
M internal/infrastructure/cef/factory.go +31 -8
| @@ -2,6 +2,7 @@ package cef | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "sync/atomic" | |
| 7 | 8 | "time" | |
| @@ -102,7 +103,10 @@ func (f *WebViewFactory) Create(ctx context.Context) (port.WebView, error) { | |||
| 102 | 103 | // Defer browser creation until the bridge view has a non-zero size. | |
| 103 | 104 | // CEF requires GetViewRect to return a non-empty rect, but the GL area | |
| 104 | 105 | // is not yet realized at this point. | |
| 105 | - | f.configureInitialBrowserCreation(ctx, wv, wv.client, &windowInfo, &settings, nil) | |
| 106 | + | if err := f.configureInitialBrowserCreation(ctx, wv, wv.client, &windowInfo, &settings, nil); err != nil { | |
| 107 | + | wv.Destroy() | |
| 108 | + | return nil, err | |
| 109 | + | } | |
| 106 | 110 | ||
| 107 | 111 | return wv, nil | |
| 108 | 112 | } | |
| @@ -128,7 +132,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||
| 128 | 132 | inputConfig: f.inputConfig, | |
| 129 | 133 | backgroundColor: f.bgColor.Load(), | |
| 130 | 134 | } | |
| 131 | - | wv.runOnGTKSync(func() { | |
| 135 | + | result := wv.runOnGTKSyncLabel("cef.new_webview.init_widgets", func() { | |
| 132 | 136 | nativeWidget := viewBridge.Widget() | |
| 133 | 137 | popupSurface := newPopupBridgeSurface(ctx, nativeWidget, f.engine.renderStackPlan, applicationScale) | |
| 134 | 138 | wv.nativeWidget = nativeWidget | |
| @@ -139,6 +143,10 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||
| 139 | 143 | } | |
| 140 | 144 | } | |
| 141 | 145 | }) | |
| 146 | + | if !result.Completed() { | |
| 147 | + | wv.destroyViewBridgeOnGTKAsync() | |
| 148 | + | return nil, errGTKSyncDispatchIncomplete("initialize CEF GTK widgets", result) | |
| 149 | + | } | |
| 142 | 150 | ||
| 143 | 151 | handlers := &handlerSet{wv: wv} | |
| 144 | 152 | wv.handlers = handlers | |
| @@ -152,7 +160,11 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||
| 152 | 160 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | |
| 153 | 161 | } | |
| 154 | 162 | } | |
| 155 | - | wv.installViewportSyncHooks() | |
| 163 | + | viewportHooksResult := wv.installViewportSyncHooks() | |
| 164 | + | if !viewportHooksResult.Completed() { | |
| 165 | + | wv.destroyViewBridgeOnGTKAsync() | |
| 166 | + | return nil, errGTKSyncDispatchIncomplete("install CEF viewport sync hooks", viewportHooksResult) | |
| 167 | + | } | |
| 156 | 168 | wv.startRenderStallWatchdog() | |
| 157 | 169 | return wv, nil | |
| 158 | 170 | } | |
| @@ -161,7 +173,10 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||
| 161 | 173 | ctx context.Context, wv *WebView, client purecef.RawClient, | |
| 162 | 174 | windowInfo *purecef.WindowInfo, settings *purecef.BrowserSettings, | |
| 163 | 175 | onFirstResize func(w, h int32), | |
| 164 | - | ) { | |
| 176 | + | ) error { | |
| 177 | + | if wv == nil { | |
| 178 | + | return errors.New("configure initial browser creation: webview is nil") | |
| 179 | + | } | |
| 165 | 180 | wv.pendingCreate = &pendingBrowserCreate{ | |
| 166 | 181 | windowInfo: windowInfo, | |
| 167 | 182 | client: client, | |
| @@ -174,9 +189,9 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||
| 174 | 189 | } | |
| 175 | 190 | } | |
| 176 | 191 | if wv.viewBridge == nil { | |
| 177 | - | return | |
| 192 | + | return nil | |
| 178 | 193 | } | |
| 179 | - | wv.runOnGTKSync(func() { | |
| 194 | + | result := wv.runOnGTKSyncLabel("cef.configure_initial_browser_creation", func() { | |
| 180 | 195 | wv.removeSizeObserver = wv.viewBridge.AddSizeObserver(func(w, h int32) { | |
| 181 | 196 | // Size observers run on the GTK thread. Prepare the GtkGLArea only for | |
| 182 | 197 | // the one-shot initial resize path; once that path has been consumed, | |
| @@ -195,6 +210,11 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||
| 195 | 210 | Msg("cef: viewport sync handled after GTK size change") | |
| 196 | 211 | }) | |
| 197 | 212 | }) | |
| 213 | + | if !result.Completed() { | |
| 214 | + | wv.pendingCreate = nil | |
| 215 | + | return errGTKSyncDispatchIncomplete("configure initial browser creation", result) | |
| 216 | + | } | |
| 217 | + | return nil | |
| 198 | 218 | } | |
| 199 | 219 | ||
| 200 | 220 | func (f *WebViewFactory) handleInitialBrowserCreateSizeObserver( | |
| @@ -365,11 +385,14 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie | |||
| 365 | 385 | settings.BackgroundColor = bg | |
| 366 | 386 | } | |
| 367 | 387 | ||
| 368 | - | f.configureInitialBrowserCreation(ctx, popupWV, popupWV.client, &windowInfo, &settings, func(w, h int32) { | |
| 388 | + | if err := f.configureInitialBrowserCreation(ctx, popupWV, popupWV.client, &windowInfo, &settings, func(w, h int32) { | |
| 369 | 389 | f.handlePopupShellInitialResize(ctx, popupWV, func(w, h int32) { | |
| 370 | 390 | f.postPendingBrowserCreate(ctx, popupWV, w, h) | |
| 371 | 391 | }, w, h) | |
| 372 | - | }) | |
| 392 | + | }); err != nil { | |
| 393 | + | popupWV.Destroy() | |
| 394 | + | return nil, err | |
| 395 | + | } | |
| 373 | 396 | return popupWV, nil | |
| 374 | 397 | } | |
| 375 | 398 | ||
| @@ -2,6 +2,7 @@ package cef | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "sync/atomic" | 7 | "sync/atomic" | ||
| 7 | "time" | 8 | "time" | ||
| @@ -102,7 +103,10 @@ func (f *WebViewFactory) Create(ctx context.Context) (port.WebView, error) { | |||||
| 102 | // Defer browser creation until the bridge view has a non-zero size. | 103 | // Defer browser creation until the bridge view has a non-zero size. | ||
| 103 | // CEF requires GetViewRect to return a non-empty rect, but the GL area | 104 | // CEF requires GetViewRect to return a non-empty rect, but the GL area | ||
| 104 | // is not yet realized at this point. | 105 | // is not yet realized at this point. | ||
| 105 | - | f.configureInitialBrowserCreation(ctx, wv, wv.client, &windowInfo, &settings, nil) | 106 | + | if err := f.configureInitialBrowserCreation(ctx, wv, wv.client, &windowInfo, &settings, nil); err != nil { |
| 107 | + | wv.Destroy() | |||
| 108 | + | return nil, err | |||
| 109 | + | } | |||
| 106 | 110 | ||||
| 107 | return wv, nil | 111 | return wv, nil | ||
| 108 | } | 112 | } | ||
| @@ -128,7 +132,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||||
| 128 | inputConfig: f.inputConfig, | 132 | inputConfig: f.inputConfig, | ||
| 129 | backgroundColor: f.bgColor.Load(), | 133 | backgroundColor: f.bgColor.Load(), | ||
| 130 | } | 134 | } | ||
| 131 | - | wv.runOnGTKSync(func() { | 135 | + | result := wv.runOnGTKSyncLabel("cef.new_webview.init_widgets", func() { |
| 132 | nativeWidget := viewBridge.Widget() | 136 | nativeWidget := viewBridge.Widget() | ||
| 133 | popupSurface := newPopupBridgeSurface(ctx, nativeWidget, f.engine.renderStackPlan, applicationScale) | 137 | popupSurface := newPopupBridgeSurface(ctx, nativeWidget, f.engine.renderStackPlan, applicationScale) | ||
| 134 | wv.nativeWidget = nativeWidget | 138 | wv.nativeWidget = nativeWidget | ||
| @@ -139,6 +143,10 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||||
| 139 | } | 143 | } | ||
| 140 | } | 144 | } | ||
| 141 | }) | 145 | }) | ||
| 146 | + | if !result.Completed() { | |||
| 147 | + | wv.destroyViewBridgeOnGTKAsync() | |||
| 148 | + | return nil, errGTKSyncDispatchIncomplete("initialize CEF GTK widgets", result) | |||
| 149 | + | } | |||
| 142 | 150 | ||||
| 143 | handlers := &handlerSet{wv: wv} | 151 | handlers := &handlerSet{wv: wv} | ||
| 144 | wv.handlers = handlers | 152 | wv.handlers = handlers | ||
| @@ -152,7 +160,11 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||||
| 152 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | 160 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | ||
| 153 | } | 161 | } | ||
| 154 | } | 162 | } | ||
| 155 | - | wv.installViewportSyncHooks() | 163 | + | viewportHooksResult := wv.installViewportSyncHooks() |
| 164 | + | if !viewportHooksResult.Completed() { | |||
| 165 | + | wv.destroyViewBridgeOnGTKAsync() | |||
| 166 | + | return nil, errGTKSyncDispatchIncomplete("install CEF viewport sync hooks", viewportHooksResult) | |||
| 167 | + | } | |||
| 156 | wv.startRenderStallWatchdog() | 168 | wv.startRenderStallWatchdog() | ||
| 157 | return wv, nil | 169 | return wv, nil | ||
| 158 | } | 170 | } | ||
| @@ -161,7 +173,10 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||||
| 161 | ctx context.Context, wv *WebView, client purecef.RawClient, | 173 | ctx context.Context, wv *WebView, client purecef.RawClient, | ||
| 162 | windowInfo *purecef.WindowInfo, settings *purecef.BrowserSettings, | 174 | windowInfo *purecef.WindowInfo, settings *purecef.BrowserSettings, | ||
| 163 | onFirstResize func(w, h int32), | 175 | onFirstResize func(w, h int32), | ||
| 164 | - | ) { | 176 | + | ) error { |
| 177 | + | if wv == nil { | |||
| 178 | + | return errors.New("configure initial browser creation: webview is nil") | |||
| 179 | + | } | |||
| 165 | wv.pendingCreate = &pendingBrowserCreate{ | 180 | wv.pendingCreate = &pendingBrowserCreate{ | ||
| 166 | windowInfo: windowInfo, | 181 | windowInfo: windowInfo, | ||
| 167 | client: client, | 182 | client: client, | ||
| @@ -174,9 +189,9 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||||
| 174 | } | 189 | } | ||
| 175 | } | 190 | } | ||
| 176 | if wv.viewBridge == nil { | 191 | if wv.viewBridge == nil { | ||
| 177 | - | return | 192 | + | return nil |
| 178 | } | 193 | } | ||
| 179 | - | wv.runOnGTKSync(func() { | 194 | + | result := wv.runOnGTKSyncLabel("cef.configure_initial_browser_creation", func() { |
| 180 | wv.removeSizeObserver = wv.viewBridge.AddSizeObserver(func(w, h int32) { | 195 | wv.removeSizeObserver = wv.viewBridge.AddSizeObserver(func(w, h int32) { | ||
| 181 | // Size observers run on the GTK thread. Prepare the GtkGLArea only for | 196 | // Size observers run on the GTK thread. Prepare the GtkGLArea only for | ||
| 182 | // the one-shot initial resize path; once that path has been consumed, | 197 | // the one-shot initial resize path; once that path has been consumed, | ||
| @@ -195,6 +210,11 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||||
| 195 | Msg("cef: viewport sync handled after GTK size change") | 210 | Msg("cef: viewport sync handled after GTK size change") | ||
| 196 | }) | 211 | }) | ||
| 197 | }) | 212 | }) | ||
| 213 | + | if !result.Completed() { | |||
| 214 | + | wv.pendingCreate = nil | |||
| 215 | + | return errGTKSyncDispatchIncomplete("configure initial browser creation", result) | |||
| 216 | + | } | |||
| 217 | + | return nil | |||
| 198 | } | 218 | } | ||
| 199 | 219 | ||||
| 200 | func (f *WebViewFactory) handleInitialBrowserCreateSizeObserver( | 220 | func (f *WebViewFactory) handleInitialBrowserCreateSizeObserver( | ||
| @@ -365,11 +385,14 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie | |||||
| 365 | settings.BackgroundColor = bg | 385 | settings.BackgroundColor = bg | ||
| 366 | } | 386 | } | ||
| 367 | 387 | ||||
| 368 | - | f.configureInitialBrowserCreation(ctx, popupWV, popupWV.client, &windowInfo, &settings, func(w, h int32) { | 388 | + | if err := f.configureInitialBrowserCreation(ctx, popupWV, popupWV.client, &windowInfo, &settings, func(w, h int32) { |
| 369 | f.handlePopupShellInitialResize(ctx, popupWV, func(w, h int32) { | 389 | f.handlePopupShellInitialResize(ctx, popupWV, func(w, h int32) { | ||
| 370 | f.postPendingBrowserCreate(ctx, popupWV, w, h) | 390 | f.postPendingBrowserCreate(ctx, popupWV, w, h) | ||
| 371 | }, w, h) | 391 | }, w, h) | ||
| 372 | - | }) | 392 | + | }); err != nil { |
| 393 | + | popupWV.Destroy() | |||
| 394 | + | return nil, err | |||
| 395 | + | } | |||
| 373 | return popupWV, nil | 396 | return popupWV, nil | ||
| 374 | } | 397 | } | ||
| 375 | 398 | ||||
A internal/infrastructure/cef/gtk_sync_required_test.go +104 -0
| @@ -0,0 +1,104 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 9 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestConfigureInitialBrowserCreationClearsPendingCreateWhenGTKDispatchTimesOut(t *testing.T) { | |
| 14 | + | factory := &WebViewFactory{} | |
| 15 | + | wv := &WebView{ | |
| 16 | + | ctx: context.Background(), | |
| 17 | + | viewBridge: &Cef2gtkAdapter{}, | |
| 18 | + | gtkSyncTimeout: 5 * time.Millisecond, | |
| 19 | + | gtkSyncIsOwner: func() bool { return false }, | |
| 20 | + | gtkSyncDispatch: func(func()) { | |
| 21 | + | // Simulate a GTK main loop that does not service the callback. | |
| 22 | + | }, | |
| 23 | + | } | |
| 24 | + | windowInfo := purecef.NewWindowInfo() | |
| 25 | + | settings := purecef.NewBrowserSettings() | |
| 26 | + | ||
| 27 | + | err := factory.configureInitialBrowserCreation(context.Background(), wv, nil, &windowInfo, &settings, nil) | |
| 28 | + | ||
| 29 | + | require.Error(t, err) | |
| 30 | + | require.Contains(t, err.Error(), "configure initial browser creation") | |
| 31 | + | require.Nil(t, wv.pendingCreate) | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | func TestInstallViewportSyncHooksReportsIncompleteGTKDispatch(t *testing.T) { | |
| 35 | + | wv := &WebView{ | |
| 36 | + | ctx: context.Background(), | |
| 37 | + | viewBridge: &Cef2gtkAdapter{}, | |
| 38 | + | gtkSyncTimeout: 5 * time.Millisecond, | |
| 39 | + | gtkSyncIsOwner: func() bool { return false }, | |
| 40 | + | gtkSyncDispatch: func(func()) { | |
| 41 | + | // Simulate a GTK main loop that does not service the callback. | |
| 42 | + | }, | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | result := wv.installViewportSyncHooks() | |
| 46 | + | ||
| 47 | + | require.False(t, result.Completed()) | |
| 48 | + | require.Equal(t, syncdispatch.SyncDispatchTimedOut, result.Status) | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func TestDestroyViewBridgeOnGTKSyncLeavesCleanupQueuedAfterTimeout(t *testing.T) { | |
| 52 | + | delayed := make(chan func(), 1) | |
| 53 | + | wv := &WebView{ | |
| 54 | + | ctx: context.Background(), | |
| 55 | + | engine: &Engine{}, | |
| 56 | + | viewBridge: &Cef2gtkAdapter{}, | |
| 57 | + | gtkSyncTimeout: 5 * time.Millisecond, | |
| 58 | + | gtkSyncIsOwner: func() bool { return false }, | |
| 59 | + | gtkSyncDispatch: func(fn func()) { | |
| 60 | + | delayed <- fn | |
| 61 | + | }, | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | wv.destroyViewBridgeOnGTKSync() | |
| 65 | + | ||
| 66 | + | require.NotNil(t, wv.viewBridge) | |
| 67 | + | select { | |
| 68 | + | case fn := <-delayed: | |
| 69 | + | fn() | |
| 70 | + | case <-time.After(time.Second): | |
| 71 | + | t.Fatal("cleanup callback was not captured") | |
| 72 | + | } | |
| 73 | + | require.Nil(t, wv.viewBridge) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | func TestRunOnGTKSyncCompletedAfterTimeoutStillCountsAsCompleted(t *testing.T) { | |
| 77 | + | started := make(chan struct{}) | |
| 78 | + | release := make(chan struct{}) | |
| 79 | + | wv := &WebView{ | |
| 80 | + | ctx: context.Background(), | |
| 81 | + | engine: &Engine{}, | |
| 82 | + | gtkSyncTimeout: 5 * time.Millisecond, | |
| 83 | + | gtkSyncIsOwner: func() bool { return false }, | |
| 84 | + | gtkSyncDispatch: func(fn func()) { | |
| 85 | + | go fn() | |
| 86 | + | }, | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | resultCh := make(chan syncdispatch.SyncDispatchResult, 1) | |
| 90 | + | go func() { | |
| 91 | + | resultCh <- wv.runOnGTKSync(func() { | |
| 92 | + | close(started) | |
| 93 | + | <-release | |
| 94 | + | }) | |
| 95 | + | }() | |
| 96 | + | ||
| 97 | + | <-started | |
| 98 | + | time.Sleep(20 * time.Millisecond) | |
| 99 | + | close(release) | |
| 100 | + | ||
| 101 | + | result := <-resultCh | |
| 102 | + | require.Equal(t, syncdispatch.SyncDispatchCompletedAfterTimeout, result.Status) | |
| 103 | + | require.True(t, result.Completed()) | |
| 104 | + | } | |
| @@ -0,0 +1,104 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 9 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestConfigureInitialBrowserCreationClearsPendingCreateWhenGTKDispatchTimesOut(t *testing.T) { | |||
| 14 | + | factory := &WebViewFactory{} | |||
| 15 | + | wv := &WebView{ | |||
| 16 | + | ctx: context.Background(), | |||
| 17 | + | viewBridge: &Cef2gtkAdapter{}, | |||
| 18 | + | gtkSyncTimeout: 5 * time.Millisecond, | |||
| 19 | + | gtkSyncIsOwner: func() bool { return false }, | |||
| 20 | + | gtkSyncDispatch: func(func()) { | |||
| 21 | + | // Simulate a GTK main loop that does not service the callback. | |||
| 22 | + | }, | |||
| 23 | + | } | |||
| 24 | + | windowInfo := purecef.NewWindowInfo() | |||
| 25 | + | settings := purecef.NewBrowserSettings() | |||
| 26 | + | ||||
| 27 | + | err := factory.configureInitialBrowserCreation(context.Background(), wv, nil, &windowInfo, &settings, nil) | |||
| 28 | + | ||||
| 29 | + | require.Error(t, err) | |||
| 30 | + | require.Contains(t, err.Error(), "configure initial browser creation") | |||
| 31 | + | require.Nil(t, wv.pendingCreate) | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | func TestInstallViewportSyncHooksReportsIncompleteGTKDispatch(t *testing.T) { | |||
| 35 | + | wv := &WebView{ | |||
| 36 | + | ctx: context.Background(), | |||
| 37 | + | viewBridge: &Cef2gtkAdapter{}, | |||
| 38 | + | gtkSyncTimeout: 5 * time.Millisecond, | |||
| 39 | + | gtkSyncIsOwner: func() bool { return false }, | |||
| 40 | + | gtkSyncDispatch: func(func()) { | |||
| 41 | + | // Simulate a GTK main loop that does not service the callback. | |||
| 42 | + | }, | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | result := wv.installViewportSyncHooks() | |||
| 46 | + | ||||
| 47 | + | require.False(t, result.Completed()) | |||
| 48 | + | require.Equal(t, syncdispatch.SyncDispatchTimedOut, result.Status) | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func TestDestroyViewBridgeOnGTKSyncLeavesCleanupQueuedAfterTimeout(t *testing.T) { | |||
| 52 | + | delayed := make(chan func(), 1) | |||
| 53 | + | wv := &WebView{ | |||
| 54 | + | ctx: context.Background(), | |||
| 55 | + | engine: &Engine{}, | |||
| 56 | + | viewBridge: &Cef2gtkAdapter{}, | |||
| 57 | + | gtkSyncTimeout: 5 * time.Millisecond, | |||
| 58 | + | gtkSyncIsOwner: func() bool { return false }, | |||
| 59 | + | gtkSyncDispatch: func(fn func()) { | |||
| 60 | + | delayed <- fn | |||
| 61 | + | }, | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | wv.destroyViewBridgeOnGTKSync() | |||
| 65 | + | ||||
| 66 | + | require.NotNil(t, wv.viewBridge) | |||
| 67 | + | select { | |||
| 68 | + | case fn := <-delayed: | |||
| 69 | + | fn() | |||
| 70 | + | case <-time.After(time.Second): | |||
| 71 | + | t.Fatal("cleanup callback was not captured") | |||
| 72 | + | } | |||
| 73 | + | require.Nil(t, wv.viewBridge) | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | func TestRunOnGTKSyncCompletedAfterTimeoutStillCountsAsCompleted(t *testing.T) { | |||
| 77 | + | started := make(chan struct{}) | |||
| 78 | + | release := make(chan struct{}) | |||
| 79 | + | wv := &WebView{ | |||
| 80 | + | ctx: context.Background(), | |||
| 81 | + | engine: &Engine{}, | |||
| 82 | + | gtkSyncTimeout: 5 * time.Millisecond, | |||
| 83 | + | gtkSyncIsOwner: func() bool { return false }, | |||
| 84 | + | gtkSyncDispatch: func(fn func()) { | |||
| 85 | + | go fn() | |||
| 86 | + | }, | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | resultCh := make(chan syncdispatch.SyncDispatchResult, 1) | |||
| 90 | + | go func() { | |||
| 91 | + | resultCh <- wv.runOnGTKSync(func() { | |||
| 92 | + | close(started) | |||
| 93 | + | <-release | |||
| 94 | + | }) | |||
| 95 | + | }() | |||
| 96 | + | ||||
| 97 | + | <-started | |||
| 98 | + | time.Sleep(20 * time.Millisecond) | |||
| 99 | + | close(release) | |||
| 100 | + | ||||
| 101 | + | result := <-resultCh | |||
| 102 | + | require.Equal(t, syncdispatch.SyncDispatchCompletedAfterTimeout, result.Status) | |||
| 103 | + | require.True(t, result.Completed()) | |||
| 104 | + | } | |||
M internal/infrastructure/cef/webview.go +175 -25
| @@ -21,6 +21,7 @@ import ( | |||
| 21 | 21 | "github.com/bnema/dumber/internal/application/dto" | |
| 22 | 22 | "github.com/bnema/dumber/internal/application/port" | |
| 23 | 23 | "github.com/bnema/dumber/internal/logging" | |
| 24 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 24 | 25 | ) | |
| 25 | 26 | ||
| 26 | 27 | // Compile-time interface checks. | |
| @@ -60,6 +61,11 @@ var cefLoadWatchdogDelays = []time.Duration{ | |||
| 60 | 61 | 30 * time.Second, | |
| 61 | 62 | } | |
| 62 | 63 | ||
| 64 | + | const ( | |
| 65 | + | cefGTKSyncDispatchTimeout = 2 * time.Second | |
| 66 | + | cefGTKSyncDispatchSlowThreshold = 250 * time.Millisecond | |
| 67 | + | ) | |
| 68 | + | ||
| 63 | 69 | var ( | |
| 64 | 70 | cefNewTask = purecef.NewTask | |
| 65 | 71 | cefPostTask = purecef.PostTask | |
| @@ -124,7 +130,15 @@ type WebView struct { | |||
| 124 | 130 | initialBrowserCreateResizeHandled bool | |
| 125 | 131 | ||
| 126 | 132 | // pendingURI is set when LoadURI is called before the browser exists. | |
| 127 | - | pendingURI string | |
| 133 | + | pendingURI string | |
| 134 | + | pendingURISetAt time.Time | |
| 135 | + | pendingURIStartedAt time.Time | |
| 136 | + | ||
| 137 | + | // GTK sync dispatch hooks are injectable for tests. Production uses the GTK | |
| 138 | + | // default main context through runOnGTK and isOnGTKThread. | |
| 139 | + | gtkSyncDispatch func(func()) | |
| 140 | + | gtkSyncIsOwner func() bool | |
| 141 | + | gtkSyncTimeout time.Duration | |
| 128 | 142 | ||
| 129 | 143 | // crashCount tracks consecutive renderer crashes to prevent infinite | |
| 130 | 144 | // crash → redirect → crash loops. | |
| @@ -259,7 +273,7 @@ func (wv *WebView) LoadURI(_ context.Context, uri string) error { | |||
| 259 | 273 | browser := wv.browser | |
| 260 | 274 | // Always remember the latest requested URI so a browser/main-frame race | |
| 261 | 275 | // cannot strand startup on about:blank. | |
| 262 | - | wv.pendingURI = actualURI | |
| 276 | + | wv.setPendingNavigationLocked(actualURI, time.Now()) | |
| 263 | 277 | wv.mu.Unlock() | |
| 264 | 278 | if browser == nil { | |
| 265 | 279 | return nil | |
| @@ -604,7 +618,7 @@ func (wv *WebView) PrimePopupNavigation(uri string) { | |||
| 604 | 618 | } | |
| 605 | 619 | wv.mu.Lock() | |
| 606 | 620 | browser := wv.browser | |
| 607 | - | wv.pendingURI = actualURI | |
| 621 | + | wv.setPendingNavigationLocked(actualURI, time.Now()) | |
| 608 | 622 | wv.mu.Unlock() | |
| 609 | 623 | if browser != nil { | |
| 610 | 624 | wv.schedulePendingNavigationReplay(0) | |
| @@ -965,7 +979,7 @@ func (wv *WebView) activateNativePopup(popupID int32, targetURL string) (purecef | |||
| 965 | 979 | wv.nativePopupID = popupID | |
| 966 | 980 | wv.nativePopupFallbackStarted = false | |
| 967 | 981 | if strings.TrimSpace(wv.pendingURI) == "" { | |
| 968 | - | wv.pendingURI = toActualInternalURL(targetURL) | |
| 982 | + | wv.setPendingNavigationLocked(toActualInternalURL(targetURL), time.Now()) | |
| 969 | 983 | } | |
| 970 | 984 | if strings.TrimSpace(wv.pendingURI) != "" { | |
| 971 | 985 | wv.uri = toConceptualInternalURL(wv.pendingURI) | |
| @@ -1147,7 +1161,7 @@ func (wv *WebView) destroyViewBridgeOnGTKSync() { | |||
| 1147 | 1161 | if wv == nil || wv.viewBridge == nil { | |
| 1148 | 1162 | return | |
| 1149 | 1163 | } | |
| 1150 | - | wv.runOnGTKSync(func() { | |
| 1164 | + | wv.runOnGTKSyncLabelAllowLateStart("cef.destroy_view_bridge", true, func() { | |
| 1151 | 1165 | wv.destroyViewBridgeOnGTKThread() | |
| 1152 | 1166 | }) | |
| 1153 | 1167 | } | |
| @@ -1212,10 +1226,7 @@ func (wv *WebView) updateURI(uri string) { | |||
| 1212 | 1226 | wv.uri = uri | |
| 1213 | 1227 | wv.loadDiagLastAddressAt = now | |
| 1214 | 1228 | cb := wv.callbacks | |
| 1215 | - | pendingMatched := pendingURIEquivalent(wv.pendingURI, uri) | |
| 1216 | - | if pendingMatched { | |
| 1217 | - | wv.pendingURI = "" | |
| 1218 | - | } | |
| 1229 | + | pendingMatched := wv.clearPendingNavigationIfEquivalentLocked(uri) | |
| 1219 | 1230 | wv.mu.Unlock() | |
| 1220 | 1231 | ||
| 1221 | 1232 | if pendingMatched && wv.ctx != nil { | |
| @@ -1276,8 +1287,20 @@ func (wv *WebView) updateLoadState(loading, back, fwd bool) { | |||
| 1276 | 1287 | wv.loadDiagLastAddressAt = time.Time{} | |
| 1277 | 1288 | loadDiagSeq = wv.loadDiagSeq | |
| 1278 | 1289 | } | |
| 1290 | + | clearedPendingURI := "" | |
| 1291 | + | currentURI := "" | |
| 1292 | + | if !loading && wv.hasObservedAddressForPendingNavigationLocked() { | |
| 1293 | + | clearedPendingURI = wv.clearPendingNavigationLocked() | |
| 1294 | + | currentURI = wv.uri | |
| 1295 | + | } | |
| 1279 | 1296 | wv.mu.Unlock() | |
| 1280 | 1297 | ||
| 1298 | + | if clearedPendingURI != "" && wv.ctx != nil { | |
| 1299 | + | logging.FromContext(wv.ctx).Debug(). | |
| 1300 | + | Str("pending_uri", logging.TruncateURL(clearedPendingURI, logging.PermissionLogURLMaxLen)). | |
| 1301 | + | Str("uri", logging.TruncateURL(currentURI, logging.PermissionLogURLMaxLen)). | |
| 1302 | + | Msg("cef: cleared pending navigation after load finished at final address") | |
| 1303 | + | } | |
| 1281 | 1304 | if loadDiagSeq != 0 { | |
| 1282 | 1305 | wv.scheduleLoadWatchdogs(loadDiagSeq) | |
| 1283 | 1306 | } | |
| @@ -1675,6 +1698,49 @@ func (wv *WebView) pendingNavigationURI() string { | |||
| 1675 | 1698 | return wv.pendingURI | |
| 1676 | 1699 | } | |
| 1677 | 1700 | ||
| 1701 | + | func (wv *WebView) setPendingNavigationLocked(uri string, at time.Time) { | |
| 1702 | + | wv.pendingURI = uri | |
| 1703 | + | wv.pendingURIStartedAt = time.Time{} | |
| 1704 | + | if uri == "" { | |
| 1705 | + | wv.pendingURISetAt = time.Time{} | |
| 1706 | + | return | |
| 1707 | + | } | |
| 1708 | + | wv.pendingURISetAt = at | |
| 1709 | + | } | |
| 1710 | + | ||
| 1711 | + | func (wv *WebView) markPendingNavigationStartedLocked(uri string, at time.Time) { | |
| 1712 | + | if !pendingURIEquivalent(wv.pendingURI, uri) { | |
| 1713 | + | return | |
| 1714 | + | } | |
| 1715 | + | wv.pendingURIStartedAt = at | |
| 1716 | + | } | |
| 1717 | + | ||
| 1718 | + | func (wv *WebView) clearPendingNavigationLocked() string { | |
| 1719 | + | cleared := wv.pendingURI | |
| 1720 | + | wv.pendingURI = "" | |
| 1721 | + | wv.pendingURISetAt = time.Time{} | |
| 1722 | + | wv.pendingURIStartedAt = time.Time{} | |
| 1723 | + | return cleared | |
| 1724 | + | } | |
| 1725 | + | ||
| 1726 | + | func (wv *WebView) clearPendingNavigationIfEquivalentLocked(uri string) bool { | |
| 1727 | + | if !pendingURIEquivalent(wv.pendingURI, uri) { | |
| 1728 | + | return false | |
| 1729 | + | } | |
| 1730 | + | wv.clearPendingNavigationLocked() | |
| 1731 | + | return true | |
| 1732 | + | } | |
| 1733 | + | ||
| 1734 | + | func (wv *WebView) hasObservedAddressForPendingNavigationLocked() bool { | |
| 1735 | + | if strings.TrimSpace(wv.pendingURI) == "" || strings.TrimSpace(wv.uri) == "" { | |
| 1736 | + | return false | |
| 1737 | + | } | |
| 1738 | + | if wv.loadDiagLastAddressAt.IsZero() || wv.pendingURIStartedAt.IsZero() { | |
| 1739 | + | return false | |
| 1740 | + | } | |
| 1741 | + | return !wv.loadDiagLastAddressAt.Before(wv.pendingURIStartedAt) | |
| 1742 | + | } | |
| 1743 | + | ||
| 1678 | 1744 | func (wv *WebView) schedulePendingNavigationReplay(attempt int) { | |
| 1679 | 1745 | if wv == nil || wv.destroyed.Load() { | |
| 1680 | 1746 | return | |
| @@ -1746,9 +1812,7 @@ func (wv *WebView) replayPendingNavigation(attempt int) { | |||
| 1746 | 1812 | currentURL := frame.GetURL() | |
| 1747 | 1813 | if pendingURIEquivalent(currentURL, uri) { | |
| 1748 | 1814 | wv.mu.Lock() | |
| 1749 | - | if pendingURIEquivalent(wv.pendingURI, uri) { | |
| 1750 | - | wv.pendingURI = "" | |
| 1751 | - | } | |
| 1815 | + | wv.clearPendingNavigationIfEquivalentLocked(uri) | |
| 1752 | 1816 | wv.mu.Unlock() | |
| 1753 | 1817 | if wv.ctx != nil { | |
| 1754 | 1818 | logging.FromContext(wv.ctx).Debug(). | |
| @@ -1758,6 +1822,9 @@ func (wv *WebView) replayPendingNavigation(attempt int) { | |||
| 1758 | 1822 | } | |
| 1759 | 1823 | return | |
| 1760 | 1824 | } | |
| 1825 | + | wv.mu.Lock() | |
| 1826 | + | wv.markPendingNavigationStartedLocked(uri, time.Now()) | |
| 1827 | + | wv.mu.Unlock() | |
| 1761 | 1828 | frame.LoadURL(uri) | |
| 1762 | 1829 | if wv.ctx != nil { | |
| 1763 | 1830 | logging.FromContext(wv.ctx).Debug(). | |
| @@ -1973,23 +2040,106 @@ func (wv *WebView) isOnGTKThread() bool { | |||
| 1973 | 2040 | } | |
| 1974 | 2041 | ||
| 1975 | 2042 | // runOnGTKSync executes fn on the GTK main context and waits for completion. | |
| 1976 | - | // Callers already running on the GTK thread must execute inline to avoid | |
| 1977 | - | // self-deadlocking while waiting for an IdleAddOnce callback. | |
| 1978 | - | func (wv *WebView) runOnGTKSync(fn func()) { | |
| 1979 | - | if fn == nil { | |
| 1980 | - | return | |
| 2043 | + | // Callers already running on the GTK thread execute inline to avoid | |
| 2044 | + | // self-deadlocking while waiting for an IdleAddOnce callback. Calls are bounded | |
| 2045 | + | // so a wedged GTK loop cannot block CEF callbacks forever. | |
| 2046 | + | func (wv *WebView) runOnGTKSync(fn func()) syncdispatch.SyncDispatchResult { | |
| 2047 | + | return wv.runOnGTKSyncLabel("cef.gtk_sync", fn) | |
| 2048 | + | } | |
| 2049 | + | ||
| 2050 | + | func (wv *WebView) runOnGTKSyncLabel(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 2051 | + | return wv.runOnGTKSyncLabelAllowLateStart(label, false, fn) | |
| 2052 | + | } | |
| 2053 | + | ||
| 2054 | + | func (wv *WebView) runOnGTKSyncLabelAllowLateStart( | |
| 2055 | + | label string, | |
| 2056 | + | allowLateStartAfterTimeout bool, | |
| 2057 | + | fn func(), | |
| 2058 | + | ) syncdispatch.SyncDispatchResult { | |
| 2059 | + | if wv == nil { | |
| 2060 | + | if fn != nil { | |
| 2061 | + | fn() | |
| 2062 | + | } | |
| 2063 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 1981 | 2064 | } | |
| 1982 | - | if wv == nil || wv.engine == nil || wv.isOnGTKThread() { | |
| 1983 | - | fn() | |
| 2065 | + | if wv.engine == nil && wv.gtkSyncDispatch == nil && wv.gtkSyncIsOwner == nil { | |
| 2066 | + | if fn != nil { | |
| 2067 | + | fn() | |
| 2068 | + | } | |
| 2069 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 2070 | + | } | |
| 2071 | + | ||
| 2072 | + | isOwner := wv.gtkSyncIsOwner | |
| 2073 | + | if isOwner == nil { | |
| 2074 | + | isOwner = wv.isOnGTKThread | |
| 2075 | + | } | |
| 2076 | + | dispatch := wv.gtkSyncDispatch | |
| 2077 | + | if dispatch == nil { | |
| 2078 | + | dispatch = wv.runOnGTK | |
| 2079 | + | } | |
| 2080 | + | timeout := wv.gtkSyncTimeout | |
| 2081 | + | if timeout <= 0 { | |
| 2082 | + | timeout = cefGTKSyncDispatchTimeout | |
| 2083 | + | } | |
| 2084 | + | ||
| 2085 | + | result := syncdispatch.RunSynchronousDispatch(syncdispatch.SyncDispatchOptions{ | |
| 2086 | + | Label: label, | |
| 2087 | + | Timeout: timeout, | |
| 2088 | + | IsOwner: isOwner, | |
| 2089 | + | Dispatch: dispatch, | |
| 2090 | + | AllowLateStartAfterTimeout: allowLateStartAfterTimeout, | |
| 2091 | + | }, fn) | |
| 2092 | + | wv.logGTKSyncDispatchResult(result) | |
| 2093 | + | return result | |
| 2094 | + | } | |
| 2095 | + | ||
| 2096 | + | func (wv *WebView) logGTKSyncDispatchResult(result syncdispatch.SyncDispatchResult) { | |
| 2097 | + | if wv == nil { | |
| 1984 | 2098 | return | |
| 1985 | 2099 | } | |
| 2100 | + | logger := logging.FromContext(wv.ctx) | |
| 2101 | + | switch result.Status { | |
| 2102 | + | case syncdispatch.SyncDispatchTimedOut: | |
| 2103 | + | logger.Warn(). | |
| 2104 | + | Str("dispatch_label", result.Label). | |
| 2105 | + | Dur("elapsed", result.Elapsed). | |
| 2106 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |
| 2107 | + | Uint64("webview_id", uint64(wv.id)). | |
| 2108 | + | Msg("cef: GTK synchronous dispatch timed out before callback started") | |
| 2109 | + | case syncdispatch.SyncDispatchCompletedAfterTimeout: | |
| 2110 | + | logger.Warn(). | |
| 2111 | + | Str("dispatch_label", result.Label). | |
| 2112 | + | Dur("elapsed", result.Elapsed). | |
| 2113 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |
| 2114 | + | Uint64("webview_id", uint64(wv.id)). | |
| 2115 | + | Msg("cef: GTK synchronous dispatch completed after timeout") | |
| 2116 | + | case syncdispatch.SyncDispatchQueuedAfterTimeout: | |
| 2117 | + | logger.Warn(). | |
| 2118 | + | Str("dispatch_label", result.Label). | |
| 2119 | + | Dur("elapsed", result.Elapsed). | |
| 2120 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |
| 2121 | + | Uint64("webview_id", uint64(wv.id)). | |
| 2122 | + | Msg("cef: GTK synchronous dispatch left queued after timeout") | |
| 2123 | + | case syncdispatch.SyncDispatchCompleted: | |
| 2124 | + | if result.Elapsed >= cefGTKSyncDispatchSlowThreshold { | |
| 2125 | + | logger.Debug(). | |
| 2126 | + | Str("dispatch_label", result.Label). | |
| 2127 | + | Dur("elapsed", result.Elapsed). | |
| 2128 | + | Uint64("webview_id", uint64(wv.id)). | |
| 2129 | + | Msg("cef: GTK synchronous dispatch completed slowly") | |
| 2130 | + | } | |
| 2131 | + | } | |
| 2132 | + | } | |
| 1986 | 2133 | ||
| 1987 | - | done := make(chan struct{}) | |
| 1988 | - | wv.runOnGTK(func() { | |
| 1989 | - | defer close(done) | |
| 1990 | - | fn() | |
| 1991 | - | }) | |
| 1992 | - | <-done | |
| 2134 | + | func (wv *WebView) effectiveGTKSyncTimeout() time.Duration { | |
| 2135 | + | if wv != nil && wv.gtkSyncTimeout > 0 { | |
| 2136 | + | return wv.gtkSyncTimeout | |
| 2137 | + | } | |
| 2138 | + | return cefGTKSyncDispatchTimeout | |
| 2139 | + | } | |
| 2140 | + | ||
| 2141 | + | func errGTKSyncDispatchIncomplete(operation string, result syncdispatch.SyncDispatchResult) error { | |
| 2142 | + | return fmt.Errorf("%s: GTK dispatch did not complete: %s", operation, result.Status) | |
| 1993 | 2143 | } | |
| 1994 | 2144 | ||
| 1995 | 2145 | func (wv *WebView) runCloseCallbacks() { | |
| @@ -21,6 +21,7 @@ import ( | |||||
| 21 | "github.com/bnema/dumber/internal/application/dto" | 21 | "github.com/bnema/dumber/internal/application/dto" | ||
| 22 | "github.com/bnema/dumber/internal/application/port" | 22 | "github.com/bnema/dumber/internal/application/port" | ||
| 23 | "github.com/bnema/dumber/internal/logging" | 23 | "github.com/bnema/dumber/internal/logging" | ||
| 24 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 24 | ) | 25 | ) | ||
| 25 | 26 | ||||
| 26 | // Compile-time interface checks. | 27 | // Compile-time interface checks. | ||
| @@ -60,6 +61,11 @@ var cefLoadWatchdogDelays = []time.Duration{ | |||||
| 60 | 30 * time.Second, | 61 | 30 * time.Second, | ||
| 61 | } | 62 | } | ||
| 62 | 63 | ||||
| 64 | + | const ( | |||
| 65 | + | cefGTKSyncDispatchTimeout = 2 * time.Second | |||
| 66 | + | cefGTKSyncDispatchSlowThreshold = 250 * time.Millisecond | |||
| 67 | + | ) | |||
| 68 | + | ||||
| 63 | var ( | 69 | var ( | ||
| 64 | cefNewTask = purecef.NewTask | 70 | cefNewTask = purecef.NewTask | ||
| 65 | cefPostTask = purecef.PostTask | 71 | cefPostTask = purecef.PostTask | ||
| @@ -124,7 +130,15 @@ type WebView struct { | |||||
| 124 | initialBrowserCreateResizeHandled bool | 130 | initialBrowserCreateResizeHandled bool | ||
| 125 | 131 | ||||
| 126 | // pendingURI is set when LoadURI is called before the browser exists. | 132 | // pendingURI is set when LoadURI is called before the browser exists. | ||
| 127 | - | pendingURI string | 133 | + | pendingURI string |
| 134 | + | pendingURISetAt time.Time | |||
| 135 | + | pendingURIStartedAt time.Time | |||
| 136 | + | ||||
| 137 | + | // GTK sync dispatch hooks are injectable for tests. Production uses the GTK | |||
| 138 | + | // default main context through runOnGTK and isOnGTKThread. | |||
| 139 | + | gtkSyncDispatch func(func()) | |||
| 140 | + | gtkSyncIsOwner func() bool | |||
| 141 | + | gtkSyncTimeout time.Duration | |||
| 128 | 142 | ||||
| 129 | // crashCount tracks consecutive renderer crashes to prevent infinite | 143 | // crashCount tracks consecutive renderer crashes to prevent infinite | ||
| 130 | // crash → redirect → crash loops. | 144 | // crash → redirect → crash loops. | ||
| @@ -259,7 +273,7 @@ func (wv *WebView) LoadURI(_ context.Context, uri string) error { | |||||
| 259 | browser := wv.browser | 273 | browser := wv.browser | ||
| 260 | // Always remember the latest requested URI so a browser/main-frame race | 274 | // Always remember the latest requested URI so a browser/main-frame race | ||
| 261 | // cannot strand startup on about:blank. | 275 | // cannot strand startup on about:blank. | ||
| 262 | - | wv.pendingURI = actualURI | 276 | + | wv.setPendingNavigationLocked(actualURI, time.Now()) |
| 263 | wv.mu.Unlock() | 277 | wv.mu.Unlock() | ||
| 264 | if browser == nil { | 278 | if browser == nil { | ||
| 265 | return nil | 279 | return nil | ||
| @@ -604,7 +618,7 @@ func (wv *WebView) PrimePopupNavigation(uri string) { | |||||
| 604 | } | 618 | } | ||
| 605 | wv.mu.Lock() | 619 | wv.mu.Lock() | ||
| 606 | browser := wv.browser | 620 | browser := wv.browser | ||
| 607 | - | wv.pendingURI = actualURI | 621 | + | wv.setPendingNavigationLocked(actualURI, time.Now()) |
| 608 | wv.mu.Unlock() | 622 | wv.mu.Unlock() | ||
| 609 | if browser != nil { | 623 | if browser != nil { | ||
| 610 | wv.schedulePendingNavigationReplay(0) | 624 | wv.schedulePendingNavigationReplay(0) | ||
| @@ -965,7 +979,7 @@ func (wv *WebView) activateNativePopup(popupID int32, targetURL string) (purecef | |||||
| 965 | wv.nativePopupID = popupID | 979 | wv.nativePopupID = popupID | ||
| 966 | wv.nativePopupFallbackStarted = false | 980 | wv.nativePopupFallbackStarted = false | ||
| 967 | if strings.TrimSpace(wv.pendingURI) == "" { | 981 | if strings.TrimSpace(wv.pendingURI) == "" { | ||
| 968 | - | wv.pendingURI = toActualInternalURL(targetURL) | 982 | + | wv.setPendingNavigationLocked(toActualInternalURL(targetURL), time.Now()) |
| 969 | } | 983 | } | ||
| 970 | if strings.TrimSpace(wv.pendingURI) != "" { | 984 | if strings.TrimSpace(wv.pendingURI) != "" { | ||
| 971 | wv.uri = toConceptualInternalURL(wv.pendingURI) | 985 | wv.uri = toConceptualInternalURL(wv.pendingURI) | ||
| @@ -1147,7 +1161,7 @@ func (wv *WebView) destroyViewBridgeOnGTKSync() { | |||||
| 1147 | if wv == nil || wv.viewBridge == nil { | 1161 | if wv == nil || wv.viewBridge == nil { | ||
| 1148 | return | 1162 | return | ||
| 1149 | } | 1163 | } | ||
| 1150 | - | wv.runOnGTKSync(func() { | 1164 | + | wv.runOnGTKSyncLabelAllowLateStart("cef.destroy_view_bridge", true, func() { |
| 1151 | wv.destroyViewBridgeOnGTKThread() | 1165 | wv.destroyViewBridgeOnGTKThread() | ||
| 1152 | }) | 1166 | }) | ||
| 1153 | } | 1167 | } | ||
| @@ -1212,10 +1226,7 @@ func (wv *WebView) updateURI(uri string) { | |||||
| 1212 | wv.uri = uri | 1226 | wv.uri = uri | ||
| 1213 | wv.loadDiagLastAddressAt = now | 1227 | wv.loadDiagLastAddressAt = now | ||
| 1214 | cb := wv.callbacks | 1228 | cb := wv.callbacks | ||
| 1215 | - | pendingMatched := pendingURIEquivalent(wv.pendingURI, uri) | 1229 | + | pendingMatched := wv.clearPendingNavigationIfEquivalentLocked(uri) |
| 1216 | - | if pendingMatched { | |||
| 1217 | - | wv.pendingURI = "" | |||
| 1218 | - | } | |||
| 1219 | wv.mu.Unlock() | 1230 | wv.mu.Unlock() | ||
| 1220 | 1231 | ||||
| 1221 | if pendingMatched && wv.ctx != nil { | 1232 | if pendingMatched && wv.ctx != nil { | ||
| @@ -1276,8 +1287,20 @@ func (wv *WebView) updateLoadState(loading, back, fwd bool) { | |||||
| 1276 | wv.loadDiagLastAddressAt = time.Time{} | 1287 | wv.loadDiagLastAddressAt = time.Time{} | ||
| 1277 | loadDiagSeq = wv.loadDiagSeq | 1288 | loadDiagSeq = wv.loadDiagSeq | ||
| 1278 | } | 1289 | } | ||
| 1290 | + | clearedPendingURI := "" | |||
| 1291 | + | currentURI := "" | |||
| 1292 | + | if !loading && wv.hasObservedAddressForPendingNavigationLocked() { | |||
| 1293 | + | clearedPendingURI = wv.clearPendingNavigationLocked() | |||
| 1294 | + | currentURI = wv.uri | |||
| 1295 | + | } | |||
| 1279 | wv.mu.Unlock() | 1296 | wv.mu.Unlock() | ||
| 1280 | 1297 | ||||
| 1298 | + | if clearedPendingURI != "" && wv.ctx != nil { | |||
| 1299 | + | logging.FromContext(wv.ctx).Debug(). | |||
| 1300 | + | Str("pending_uri", logging.TruncateURL(clearedPendingURI, logging.PermissionLogURLMaxLen)). | |||
| 1301 | + | Str("uri", logging.TruncateURL(currentURI, logging.PermissionLogURLMaxLen)). | |||
| 1302 | + | Msg("cef: cleared pending navigation after load finished at final address") | |||
| 1303 | + | } | |||
| 1281 | if loadDiagSeq != 0 { | 1304 | if loadDiagSeq != 0 { | ||
| 1282 | wv.scheduleLoadWatchdogs(loadDiagSeq) | 1305 | wv.scheduleLoadWatchdogs(loadDiagSeq) | ||
| 1283 | } | 1306 | } | ||
| @@ -1675,6 +1698,49 @@ func (wv *WebView) pendingNavigationURI() string { | |||||
| 1675 | return wv.pendingURI | 1698 | return wv.pendingURI | ||
| 1676 | } | 1699 | } | ||
| 1677 | 1700 | ||||
| 1701 | + | func (wv *WebView) setPendingNavigationLocked(uri string, at time.Time) { | |||
| 1702 | + | wv.pendingURI = uri | |||
| 1703 | + | wv.pendingURIStartedAt = time.Time{} | |||
| 1704 | + | if uri == "" { | |||
| 1705 | + | wv.pendingURISetAt = time.Time{} | |||
| 1706 | + | return | |||
| 1707 | + | } | |||
| 1708 | + | wv.pendingURISetAt = at | |||
| 1709 | + | } | |||
| 1710 | + | ||||
| 1711 | + | func (wv *WebView) markPendingNavigationStartedLocked(uri string, at time.Time) { | |||
| 1712 | + | if !pendingURIEquivalent(wv.pendingURI, uri) { | |||
| 1713 | + | return | |||
| 1714 | + | } | |||
| 1715 | + | wv.pendingURIStartedAt = at | |||
| 1716 | + | } | |||
| 1717 | + | ||||
| 1718 | + | func (wv *WebView) clearPendingNavigationLocked() string { | |||
| 1719 | + | cleared := wv.pendingURI | |||
| 1720 | + | wv.pendingURI = "" | |||
| 1721 | + | wv.pendingURISetAt = time.Time{} | |||
| 1722 | + | wv.pendingURIStartedAt = time.Time{} | |||
| 1723 | + | return cleared | |||
| 1724 | + | } | |||
| 1725 | + | ||||
| 1726 | + | func (wv *WebView) clearPendingNavigationIfEquivalentLocked(uri string) bool { | |||
| 1727 | + | if !pendingURIEquivalent(wv.pendingURI, uri) { | |||
| 1728 | + | return false | |||
| 1729 | + | } | |||
| 1730 | + | wv.clearPendingNavigationLocked() | |||
| 1731 | + | return true | |||
| 1732 | + | } | |||
| 1733 | + | ||||
| 1734 | + | func (wv *WebView) hasObservedAddressForPendingNavigationLocked() bool { | |||
| 1735 | + | if strings.TrimSpace(wv.pendingURI) == "" || strings.TrimSpace(wv.uri) == "" { | |||
| 1736 | + | return false | |||
| 1737 | + | } | |||
| 1738 | + | if wv.loadDiagLastAddressAt.IsZero() || wv.pendingURIStartedAt.IsZero() { | |||
| 1739 | + | return false | |||
| 1740 | + | } | |||
| 1741 | + | return !wv.loadDiagLastAddressAt.Before(wv.pendingURIStartedAt) | |||
| 1742 | + | } | |||
| 1743 | + | ||||
| 1678 | func (wv *WebView) schedulePendingNavigationReplay(attempt int) { | 1744 | func (wv *WebView) schedulePendingNavigationReplay(attempt int) { | ||
| 1679 | if wv == nil || wv.destroyed.Load() { | 1745 | if wv == nil || wv.destroyed.Load() { | ||
| 1680 | return | 1746 | return | ||
| @@ -1746,9 +1812,7 @@ func (wv *WebView) replayPendingNavigation(attempt int) { | |||||
| 1746 | currentURL := frame.GetURL() | 1812 | currentURL := frame.GetURL() | ||
| 1747 | if pendingURIEquivalent(currentURL, uri) { | 1813 | if pendingURIEquivalent(currentURL, uri) { | ||
| 1748 | wv.mu.Lock() | 1814 | wv.mu.Lock() | ||
| 1749 | - | if pendingURIEquivalent(wv.pendingURI, uri) { | 1815 | + | wv.clearPendingNavigationIfEquivalentLocked(uri) |
| 1750 | - | wv.pendingURI = "" | |||
| 1751 | - | } | |||
| 1752 | wv.mu.Unlock() | 1816 | wv.mu.Unlock() | ||
| 1753 | if wv.ctx != nil { | 1817 | if wv.ctx != nil { | ||
| 1754 | logging.FromContext(wv.ctx).Debug(). | 1818 | logging.FromContext(wv.ctx).Debug(). | ||
| @@ -1758,6 +1822,9 @@ func (wv *WebView) replayPendingNavigation(attempt int) { | |||||
| 1758 | } | 1822 | } | ||
| 1759 | return | 1823 | return | ||
| 1760 | } | 1824 | } | ||
| 1825 | + | wv.mu.Lock() | |||
| 1826 | + | wv.markPendingNavigationStartedLocked(uri, time.Now()) | |||
| 1827 | + | wv.mu.Unlock() | |||
| 1761 | frame.LoadURL(uri) | 1828 | frame.LoadURL(uri) | ||
| 1762 | if wv.ctx != nil { | 1829 | if wv.ctx != nil { | ||
| 1763 | logging.FromContext(wv.ctx).Debug(). | 1830 | logging.FromContext(wv.ctx).Debug(). | ||
| @@ -1973,23 +2040,106 @@ func (wv *WebView) isOnGTKThread() bool { | |||||
| 1973 | } | 2040 | } | ||
| 1974 | 2041 | ||||
| 1975 | // runOnGTKSync executes fn on the GTK main context and waits for completion. | 2042 | // runOnGTKSync executes fn on the GTK main context and waits for completion. | ||
| 1976 | - | // Callers already running on the GTK thread must execute inline to avoid | 2043 | + | // Callers already running on the GTK thread execute inline to avoid |
| 1977 | - | // self-deadlocking while waiting for an IdleAddOnce callback. | 2044 | + | // self-deadlocking while waiting for an IdleAddOnce callback. Calls are bounded |
| 1978 | - | func (wv *WebView) runOnGTKSync(fn func()) { | 2045 | + | // so a wedged GTK loop cannot block CEF callbacks forever. |
| 1979 | - | if fn == nil { | 2046 | + | func (wv *WebView) runOnGTKSync(fn func()) syncdispatch.SyncDispatchResult { |
| 1980 | - | return | 2047 | + | return wv.runOnGTKSyncLabel("cef.gtk_sync", fn) |
| 2048 | + | } | |||
| 2049 | + | ||||
| 2050 | + | func (wv *WebView) runOnGTKSyncLabel(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 2051 | + | return wv.runOnGTKSyncLabelAllowLateStart(label, false, fn) | |||
| 2052 | + | } | |||
| 2053 | + | ||||
| 2054 | + | func (wv *WebView) runOnGTKSyncLabelAllowLateStart( | |||
| 2055 | + | label string, | |||
| 2056 | + | allowLateStartAfterTimeout bool, | |||
| 2057 | + | fn func(), | |||
| 2058 | + | ) syncdispatch.SyncDispatchResult { | |||
| 2059 | + | if wv == nil { | |||
| 2060 | + | if fn != nil { | |||
| 2061 | + | fn() | |||
| 2062 | + | } | |||
| 2063 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 1981 | } | 2064 | } | ||
| 1982 | - | if wv == nil || wv.engine == nil || wv.isOnGTKThread() { | 2065 | + | if wv.engine == nil && wv.gtkSyncDispatch == nil && wv.gtkSyncIsOwner == nil { |
| 1983 | - | fn() | 2066 | + | if fn != nil { |
| 2067 | + | fn() | |||
| 2068 | + | } | |||
| 2069 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 2070 | + | } | |||
| 2071 | + | ||||
| 2072 | + | isOwner := wv.gtkSyncIsOwner | |||
| 2073 | + | if isOwner == nil { | |||
| 2074 | + | isOwner = wv.isOnGTKThread | |||
| 2075 | + | } | |||
| 2076 | + | dispatch := wv.gtkSyncDispatch | |||
| 2077 | + | if dispatch == nil { | |||
| 2078 | + | dispatch = wv.runOnGTK | |||
| 2079 | + | } | |||
| 2080 | + | timeout := wv.gtkSyncTimeout | |||
| 2081 | + | if timeout <= 0 { | |||
| 2082 | + | timeout = cefGTKSyncDispatchTimeout | |||
| 2083 | + | } | |||
| 2084 | + | ||||
| 2085 | + | result := syncdispatch.RunSynchronousDispatch(syncdispatch.SyncDispatchOptions{ | |||
| 2086 | + | Label: label, | |||
| 2087 | + | Timeout: timeout, | |||
| 2088 | + | IsOwner: isOwner, | |||
| 2089 | + | Dispatch: dispatch, | |||
| 2090 | + | AllowLateStartAfterTimeout: allowLateStartAfterTimeout, | |||
| 2091 | + | }, fn) | |||
| 2092 | + | wv.logGTKSyncDispatchResult(result) | |||
| 2093 | + | return result | |||
| 2094 | + | } | |||
| 2095 | + | ||||
| 2096 | + | func (wv *WebView) logGTKSyncDispatchResult(result syncdispatch.SyncDispatchResult) { | |||
| 2097 | + | if wv == nil { | |||
| 1984 | return | 2098 | return | ||
| 1985 | } | 2099 | } | ||
| 2100 | + | logger := logging.FromContext(wv.ctx) | |||
| 2101 | + | switch result.Status { | |||
| 2102 | + | case syncdispatch.SyncDispatchTimedOut: | |||
| 2103 | + | logger.Warn(). | |||
| 2104 | + | Str("dispatch_label", result.Label). | |||
| 2105 | + | Dur("elapsed", result.Elapsed). | |||
| 2106 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |||
| 2107 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 2108 | + | Msg("cef: GTK synchronous dispatch timed out before callback started") | |||
| 2109 | + | case syncdispatch.SyncDispatchCompletedAfterTimeout: | |||
| 2110 | + | logger.Warn(). | |||
| 2111 | + | Str("dispatch_label", result.Label). | |||
| 2112 | + | Dur("elapsed", result.Elapsed). | |||
| 2113 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |||
| 2114 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 2115 | + | Msg("cef: GTK synchronous dispatch completed after timeout") | |||
| 2116 | + | case syncdispatch.SyncDispatchQueuedAfterTimeout: | |||
| 2117 | + | logger.Warn(). | |||
| 2118 | + | Str("dispatch_label", result.Label). | |||
| 2119 | + | Dur("elapsed", result.Elapsed). | |||
| 2120 | + | Dur("timeout", wv.effectiveGTKSyncTimeout()). | |||
| 2121 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 2122 | + | Msg("cef: GTK synchronous dispatch left queued after timeout") | |||
| 2123 | + | case syncdispatch.SyncDispatchCompleted: | |||
| 2124 | + | if result.Elapsed >= cefGTKSyncDispatchSlowThreshold { | |||
| 2125 | + | logger.Debug(). | |||
| 2126 | + | Str("dispatch_label", result.Label). | |||
| 2127 | + | Dur("elapsed", result.Elapsed). | |||
| 2128 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 2129 | + | Msg("cef: GTK synchronous dispatch completed slowly") | |||
| 2130 | + | } | |||
| 2131 | + | } | |||
| 2132 | + | } | |||
| 1986 | 2133 | ||||
| 1987 | - | done := make(chan struct{}) | 2134 | + | func (wv *WebView) effectiveGTKSyncTimeout() time.Duration { |
| 1988 | - | wv.runOnGTK(func() { | 2135 | + | if wv != nil && wv.gtkSyncTimeout > 0 { |
| 1989 | - | defer close(done) | 2136 | + | return wv.gtkSyncTimeout |
| 1990 | - | fn() | 2137 | + | } |
| 1991 | - | }) | 2138 | + | return cefGTKSyncDispatchTimeout |
| 1992 | - | <-done | 2139 | + | } |
| 2140 | + | ||||
| 2141 | + | func errGTKSyncDispatchIncomplete(operation string, result syncdispatch.SyncDispatchResult) error { | |||
| 2142 | + | return fmt.Errorf("%s: GTK dispatch did not complete: %s", operation, result.Status) | |||
| 1993 | } | 2143 | } | ||
| 1994 | 2144 | ||||
| 1995 | func (wv *WebView) runCloseCallbacks() { | 2145 | func (wv *WebView) runCloseCallbacks() { | ||
M internal/infrastructure/cef/webview_handlers.go +10 -1
| @@ -494,7 +494,7 @@ func (h *handlerSet) OnBeforePopup( | |||
| 494 | 494 | sourceFrameURL = frame.GetURL() | |
| 495 | 495 | } | |
| 496 | 496 | ||
| 497 | - | h.wv.runOnGTKSync(func() { | |
| 497 | + | dispatchResult := h.wv.runOnGTKSyncLabel("cef.on_before_popup", func() { | |
| 498 | 498 | popup = cb.OnCreate(port.PopupRequest{ | |
| 499 | 499 | TargetURI: targetURL, | |
| 500 | 500 | FrameName: targetFrameName, | |
| @@ -507,6 +507,15 @@ func (h *handlerSet) OnBeforePopup( | |||
| 507 | 507 | TargetDisposition: mapCEFWindowDisposition(targetDisposition), | |
| 508 | 508 | }) | |
| 509 | 509 | }) | |
| 510 | + | if !dispatchResult.Completed() { | |
| 511 | + | logging.FromContext(h.currentContext()).Warn(). | |
| 512 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |
| 513 | + | Str("target_frame", targetFrameName). | |
| 514 | + | Dur("elapsed", dispatchResult.Elapsed). | |
| 515 | + | Str("dispatch_status", string(dispatchResult.Status)). | |
| 516 | + | Msg("cef: blocked popup after GTK dispatch did not complete") | |
| 517 | + | return true | |
| 518 | + | } | |
| 510 | 519 | ||
| 511 | 520 | cefPopup, ok := popup.(*WebView) | |
| 512 | 521 | if !ok || cefPopup == nil { | |
| @@ -494,7 +494,7 @@ func (h *handlerSet) OnBeforePopup( | |||||
| 494 | sourceFrameURL = frame.GetURL() | 494 | sourceFrameURL = frame.GetURL() | ||
| 495 | } | 495 | } | ||
| 496 | 496 | ||||
| 497 | - | h.wv.runOnGTKSync(func() { | 497 | + | dispatchResult := h.wv.runOnGTKSyncLabel("cef.on_before_popup", func() { |
| 498 | popup = cb.OnCreate(port.PopupRequest{ | 498 | popup = cb.OnCreate(port.PopupRequest{ | ||
| 499 | TargetURI: targetURL, | 499 | TargetURI: targetURL, | ||
| 500 | FrameName: targetFrameName, | 500 | FrameName: targetFrameName, | ||
| @@ -507,6 +507,15 @@ func (h *handlerSet) OnBeforePopup( | |||||
| 507 | TargetDisposition: mapCEFWindowDisposition(targetDisposition), | 507 | TargetDisposition: mapCEFWindowDisposition(targetDisposition), | ||
| 508 | }) | 508 | }) | ||
| 509 | }) | 509 | }) | ||
| 510 | + | if !dispatchResult.Completed() { | |||
| 511 | + | logging.FromContext(h.currentContext()).Warn(). | |||
| 512 | + | Str("target_url", logging.TruncateURL(targetURL, logging.PermissionLogURLMaxLen)). | |||
| 513 | + | Str("target_frame", targetFrameName). | |||
| 514 | + | Dur("elapsed", dispatchResult.Elapsed). | |||
| 515 | + | Str("dispatch_status", string(dispatchResult.Status)). | |||
| 516 | + | Msg("cef: blocked popup after GTK dispatch did not complete") | |||
| 517 | + | return true | |||
| 518 | + | } | |||
| 510 | 519 | ||||
| 511 | cefPopup, ok := popup.(*WebView) | 520 | cefPopup, ok := popup.(*WebView) | ||
| 512 | if !ok || cefPopup == nil { | 521 | if !ok || cefPopup == nil { | ||
M internal/infrastructure/cef/webview_handlers_test.go +52 -0
| @@ -3,6 +3,7 @@ package cef | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "sync" | |
| 6 | + | "sync/atomic" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "time" | |
| 8 | 9 | ||
| @@ -215,6 +216,27 @@ func TestStartNativePopupFallback_RejectsDestroyedWebView(t *testing.T) { | |||
| 215 | 216 | require.False(t, wv.startNativePopupFallback()) | |
| 216 | 217 | } | |
| 217 | 218 | ||
| 219 | + | func TestNativePopupActivationDoesNotLetInitialBlankLoadClearPendingNavigationBeforeReplay(t *testing.T) { | |
| 220 | + | parent := &WebView{} | |
| 221 | + | wv := &WebView{ | |
| 222 | + | ctx: context.Background(), | |
| 223 | + | nativePopupCandidate: true, | |
| 224 | + | nativePopupParent: parent, | |
| 225 | + | client: cefmocks.NewMockRawClient(t), | |
| 226 | + | pendingCreate: &pendingBrowserCreate{}, | |
| 227 | + | } | |
| 228 | + | ||
| 229 | + | rawClient, ok := wv.activateNativePopup(55, "https://example.com/oauth") | |
| 230 | + | require.True(t, ok) | |
| 231 | + | require.NotNil(t, rawClient) | |
| 232 | + | require.Equal(t, "https://example.com/oauth", wv.pendingNavigationURI()) | |
| 233 | + | ||
| 234 | + | wv.updateURI("about:blank") | |
| 235 | + | wv.updateLoadState(false, false, false) | |
| 236 | + | ||
| 237 | + | require.Equal(t, "https://example.com/oauth", wv.pendingNavigationURI()) | |
| 238 | + | } | |
| 239 | + | ||
| 218 | 240 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | |
| 219 | 241 | closed := false | |
| 220 | 242 | parent := &WebView{} | |
| @@ -240,6 +262,36 @@ func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testin | |||
| 240 | 262 | require.True(t, wv.isLoading) | |
| 241 | 263 | } | |
| 242 | 264 | ||
| 265 | + | func TestOnBeforePopup_TimesOutGTKDispatchAndBlocksPopup(t *testing.T) { | |
| 266 | + | var delayed func() | |
| 267 | + | parentWV := &WebView{ | |
| 268 | + | ctx: context.Background(), | |
| 269 | + | id: 16, | |
| 270 | + | gtkSyncTimeout: 5 * time.Millisecond, | |
| 271 | + | gtkSyncIsOwner: func() bool { return false }, | |
| 272 | + | gtkSyncDispatch: func(fn func()) { | |
| 273 | + | delayed = fn | |
| 274 | + | }, | |
| 275 | + | } | |
| 276 | + | var createCalls atomic.Int32 | |
| 277 | + | parentWV.SetCallbacks(&port.WebViewCallbacks{ | |
| 278 | + | OnCreate: func(_ port.PopupRequest) port.WebView { | |
| 279 | + | createCalls.Add(1) | |
| 280 | + | return &WebView{ctx: context.Background(), id: 24} | |
| 281 | + | }, | |
| 282 | + | }) | |
| 283 | + | ||
| 284 | + | h := &handlerSet{wv: parentWV} | |
| 285 | + | blocked := h.OnBeforePopup(nil, nil, 90, "https://example.com/slow-popup", "slow-popup", 0, 1, nil, nil, nil, nil, nil, nil) | |
| 286 | + | ||
| 287 | + | require.True(t, blocked) | |
| 288 | + | require.Zero(t, createCalls.Load()) | |
| 289 | + | require.NotNil(t, delayed) | |
| 290 | + | ||
| 291 | + | delayed() | |
| 292 | + | require.Zero(t, createCalls.Load()) | |
| 293 | + | } | |
| 294 | + | ||
| 243 | 295 | func TestOnBeforePopup_PrimesPopupNavigationWhenCEFPopupBlocksNativeCreation(t *testing.T) { | |
| 244 | 296 | parentWV := &WebView{ctx: context.Background(), id: 17} | |
| 245 | 297 | popupWV := &WebView{ctx: context.Background(), id: 23} | |
| @@ -3,6 +3,7 @@ package cef | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "sync" | 5 | "sync" | ||
| 6 | + | "sync/atomic" | |||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "time" | 8 | "time" | ||
| 8 | 9 | ||||
| @@ -215,6 +216,27 @@ func TestStartNativePopupFallback_RejectsDestroyedWebView(t *testing.T) { | |||||
| 215 | require.False(t, wv.startNativePopupFallback()) | 216 | require.False(t, wv.startNativePopupFallback()) | ||
| 216 | } | 217 | } | ||
| 217 | 218 | ||||
| 219 | + | func TestNativePopupActivationDoesNotLetInitialBlankLoadClearPendingNavigationBeforeReplay(t *testing.T) { | |||
| 220 | + | parent := &WebView{} | |||
| 221 | + | wv := &WebView{ | |||
| 222 | + | ctx: context.Background(), | |||
| 223 | + | nativePopupCandidate: true, | |||
| 224 | + | nativePopupParent: parent, | |||
| 225 | + | client: cefmocks.NewMockRawClient(t), | |||
| 226 | + | pendingCreate: &pendingBrowserCreate{}, | |||
| 227 | + | } | |||
| 228 | + | ||||
| 229 | + | rawClient, ok := wv.activateNativePopup(55, "https://example.com/oauth") | |||
| 230 | + | require.True(t, ok) | |||
| 231 | + | require.NotNil(t, rawClient) | |||
| 232 | + | require.Equal(t, "https://example.com/oauth", wv.pendingNavigationURI()) | |||
| 233 | + | ||||
| 234 | + | wv.updateURI("about:blank") | |||
| 235 | + | wv.updateLoadState(false, false, false) | |||
| 236 | + | ||||
| 237 | + | require.Equal(t, "https://example.com/oauth", wv.pendingNavigationURI()) | |||
| 238 | + | } | |||
| 239 | + | ||||
| 218 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | 240 | func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testing.T) { | ||
| 219 | closed := false | 241 | closed := false | ||
| 220 | parent := &WebView{} | 242 | parent := &WebView{} | ||
| @@ -240,6 +262,36 @@ func TestHandleNativePopupAborted_PreservesPrimedNavigationForFallback(t *testin | |||||
| 240 | require.True(t, wv.isLoading) | 262 | require.True(t, wv.isLoading) | ||
| 241 | } | 263 | } | ||
| 242 | 264 | ||||
| 265 | + | func TestOnBeforePopup_TimesOutGTKDispatchAndBlocksPopup(t *testing.T) { | |||
| 266 | + | var delayed func() | |||
| 267 | + | parentWV := &WebView{ | |||
| 268 | + | ctx: context.Background(), | |||
| 269 | + | id: 16, | |||
| 270 | + | gtkSyncTimeout: 5 * time.Millisecond, | |||
| 271 | + | gtkSyncIsOwner: func() bool { return false }, | |||
| 272 | + | gtkSyncDispatch: func(fn func()) { | |||
| 273 | + | delayed = fn | |||
| 274 | + | }, | |||
| 275 | + | } | |||
| 276 | + | var createCalls atomic.Int32 | |||
| 277 | + | parentWV.SetCallbacks(&port.WebViewCallbacks{ | |||
| 278 | + | OnCreate: func(_ port.PopupRequest) port.WebView { | |||
| 279 | + | createCalls.Add(1) | |||
| 280 | + | return &WebView{ctx: context.Background(), id: 24} | |||
| 281 | + | }, | |||
| 282 | + | }) | |||
| 283 | + | ||||
| 284 | + | h := &handlerSet{wv: parentWV} | |||
| 285 | + | blocked := h.OnBeforePopup(nil, nil, 90, "https://example.com/slow-popup", "slow-popup", 0, 1, nil, nil, nil, nil, nil, nil) | |||
| 286 | + | ||||
| 287 | + | require.True(t, blocked) | |||
| 288 | + | require.Zero(t, createCalls.Load()) | |||
| 289 | + | require.NotNil(t, delayed) | |||
| 290 | + | ||||
| 291 | + | delayed() | |||
| 292 | + | require.Zero(t, createCalls.Load()) | |||
| 293 | + | } | |||
| 294 | + | ||||
| 243 | func TestOnBeforePopup_PrimesPopupNavigationWhenCEFPopupBlocksNativeCreation(t *testing.T) { | 295 | func TestOnBeforePopup_PrimesPopupNavigationWhenCEFPopupBlocksNativeCreation(t *testing.T) { | ||
| 244 | parentWV := &WebView{ctx: context.Background(), id: 17} | 296 | parentWV := &WebView{ctx: context.Background(), id: 17} | ||
| 245 | popupWV := &WebView{ctx: context.Background(), id: 23} | 297 | popupWV := &WebView{ctx: context.Background(), id: 23} | ||
M internal/infrastructure/cef/webview_navigation_test.go +59 -0
| @@ -162,3 +162,62 @@ func TestWebViewUpdateURI_ClearsMatchingPendingNavigation(t *testing.T) { | |||
| 162 | 162 | ||
| 163 | 163 | require.Empty(t, wv.pendingNavigationURI()) | |
| 164 | 164 | } | |
| 165 | + | ||
| 166 | + | func TestWebViewUpdateLoadState_ClearsStalePendingNavigationAfterStartedRedirectCompletes(t *testing.T) { | |
| 167 | + | wv := &WebView{ctx: context.Background()} | |
| 168 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |
| 169 | + | startedAt := time.Now().Add(-time.Second) | |
| 170 | + | wv.mu.Lock() | |
| 171 | + | wv.setPendingNavigationLocked(pending, startedAt.Add(-time.Millisecond)) | |
| 172 | + | wv.markPendingNavigationStartedLocked(pending, startedAt) | |
| 173 | + | wv.mu.Unlock() | |
| 174 | + | wv.updateURI("https://www.reddit.com/r/somewhere/") | |
| 175 | + | ||
| 176 | + | wv.updateLoadState(false, true, false) | |
| 177 | + | ||
| 178 | + | require.Empty(t, wv.pendingNavigationURI()) | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | func TestWebViewUpdateLoadState_KeepsPendingNavigationBeforeReplayStarts(t *testing.T) { | |
| 182 | + | wv := &WebView{ctx: context.Background()} | |
| 183 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |
| 184 | + | wv.updateURI("https://old.example/still-finishing") | |
| 185 | + | wv.mu.Lock() | |
| 186 | + | wv.setPendingNavigationLocked(pending, time.Now()) | |
| 187 | + | wv.mu.Unlock() | |
| 188 | + | ||
| 189 | + | wv.updateLoadState(false, true, false) | |
| 190 | + | ||
| 191 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |
| 192 | + | } | |
| 193 | + | ||
| 194 | + | func TestWebViewUpdateLoadState_DoesNotClearPendingNavigationWhenOnlyOlderAddressWasObserved(t *testing.T) { | |
| 195 | + | wv := &WebView{ctx: context.Background()} | |
| 196 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |
| 197 | + | observedAt := time.Now() | |
| 198 | + | wv.mu.Lock() | |
| 199 | + | wv.uri = "https://old.example/final" | |
| 200 | + | wv.loadDiagLastAddressAt = observedAt | |
| 201 | + | wv.setPendingNavigationLocked(pending, observedAt.Add(time.Millisecond)) | |
| 202 | + | wv.markPendingNavigationStartedLocked(pending, observedAt.Add(time.Millisecond)) | |
| 203 | + | wv.mu.Unlock() | |
| 204 | + | ||
| 205 | + | wv.updateLoadState(false, true, false) | |
| 206 | + | ||
| 207 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | func TestWebViewUpdateLoadState_KeepsPendingNavigationWhileLoading(t *testing.T) { | |
| 211 | + | wv := &WebView{ctx: context.Background()} | |
| 212 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |
| 213 | + | startedAt := time.Now().Add(-time.Second) | |
| 214 | + | wv.mu.Lock() | |
| 215 | + | wv.setPendingNavigationLocked(pending, startedAt.Add(-time.Millisecond)) | |
| 216 | + | wv.markPendingNavigationStartedLocked(pending, startedAt) | |
| 217 | + | wv.mu.Unlock() | |
| 218 | + | wv.updateURI("https://www.reddit.com/r/somewhere/") | |
| 219 | + | ||
| 220 | + | wv.updateLoadState(true, true, false) | |
| 221 | + | ||
| 222 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |
| 223 | + | } | |
| @@ -162,3 +162,62 @@ func TestWebViewUpdateURI_ClearsMatchingPendingNavigation(t *testing.T) { | |||||
| 162 | 162 | ||||
| 163 | require.Empty(t, wv.pendingNavigationURI()) | 163 | require.Empty(t, wv.pendingNavigationURI()) | ||
| 164 | } | 164 | } | ||
| 165 | + | ||||
| 166 | + | func TestWebViewUpdateLoadState_ClearsStalePendingNavigationAfterStartedRedirectCompletes(t *testing.T) { | |||
| 167 | + | wv := &WebView{ctx: context.Background()} | |||
| 168 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |||
| 169 | + | startedAt := time.Now().Add(-time.Second) | |||
| 170 | + | wv.mu.Lock() | |||
| 171 | + | wv.setPendingNavigationLocked(pending, startedAt.Add(-time.Millisecond)) | |||
| 172 | + | wv.markPendingNavigationStartedLocked(pending, startedAt) | |||
| 173 | + | wv.mu.Unlock() | |||
| 174 | + | wv.updateURI("https://www.reddit.com/r/somewhere/") | |||
| 175 | + | ||||
| 176 | + | wv.updateLoadState(false, true, false) | |||
| 177 | + | ||||
| 178 | + | require.Empty(t, wv.pendingNavigationURI()) | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | func TestWebViewUpdateLoadState_KeepsPendingNavigationBeforeReplayStarts(t *testing.T) { | |||
| 182 | + | wv := &WebView{ctx: context.Background()} | |||
| 183 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |||
| 184 | + | wv.updateURI("https://old.example/still-finishing") | |||
| 185 | + | wv.mu.Lock() | |||
| 186 | + | wv.setPendingNavigationLocked(pending, time.Now()) | |||
| 187 | + | wv.mu.Unlock() | |||
| 188 | + | ||||
| 189 | + | wv.updateLoadState(false, true, false) | |||
| 190 | + | ||||
| 191 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |||
| 192 | + | } | |||
| 193 | + | ||||
| 194 | + | func TestWebViewUpdateLoadState_DoesNotClearPendingNavigationWhenOnlyOlderAddressWasObserved(t *testing.T) { | |||
| 195 | + | wv := &WebView{ctx: context.Background()} | |||
| 196 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |||
| 197 | + | observedAt := time.Now() | |||
| 198 | + | wv.mu.Lock() | |||
| 199 | + | wv.uri = "https://old.example/final" | |||
| 200 | + | wv.loadDiagLastAddressAt = observedAt | |||
| 201 | + | wv.setPendingNavigationLocked(pending, observedAt.Add(time.Millisecond)) | |||
| 202 | + | wv.markPendingNavigationStartedLocked(pending, observedAt.Add(time.Millisecond)) | |||
| 203 | + | wv.mu.Unlock() | |||
| 204 | + | ||||
| 205 | + | wv.updateLoadState(false, true, false) | |||
| 206 | + | ||||
| 207 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | func TestWebViewUpdateLoadState_KeepsPendingNavigationWhileLoading(t *testing.T) { | |||
| 211 | + | wv := &WebView{ctx: context.Background()} | |||
| 212 | + | pending := "https://duckduckgo.com/?q=pi+agent+reddit" | |||
| 213 | + | startedAt := time.Now().Add(-time.Second) | |||
| 214 | + | wv.mu.Lock() | |||
| 215 | + | wv.setPendingNavigationLocked(pending, startedAt.Add(-time.Millisecond)) | |||
| 216 | + | wv.markPendingNavigationStartedLocked(pending, startedAt) | |||
| 217 | + | wv.mu.Unlock() | |||
| 218 | + | wv.updateURI("https://www.reddit.com/r/somewhere/") | |||
| 219 | + | ||||
| 220 | + | wv.updateLoadState(true, true, false) | |||
| 221 | + | ||||
| 222 | + | require.Equal(t, pending, wv.pendingNavigationURI()) | |||
| 223 | + | } | |||
M internal/infrastructure/cef/webview_viewport.go +4 -3
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | "github.com/bnema/puregotk/v4/gtk" | |
| 11 | 11 | ||
| 12 | 12 | "github.com/bnema/dumber/internal/logging" | |
| 13 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 13 | 14 | ) | |
| 14 | 15 | ||
| 15 | 16 | type viewportSyncBrowserHost interface { | |
| @@ -402,11 +403,11 @@ func (wv *WebView) schedulePendingBrowserCreateObservedSizeRetry(ctx context.Con | |||
| 402 | 403 | }) | |
| 403 | 404 | } | |
| 404 | 405 | ||
| 405 | - | func (wv *WebView) installViewportSyncHooks() { | |
| 406 | + | func (wv *WebView) installViewportSyncHooks() syncdispatch.SyncDispatchResult { | |
| 406 | 407 | if wv == nil || wv.viewBridge == nil { | |
| 407 | - | return | |
| 408 | + | return syncdispatch.SyncDispatchResult{Label: "cef.install_viewport_sync_hooks", Status: syncdispatch.SyncDispatchInline} | |
| 408 | 409 | } | |
| 409 | - | wv.runOnGTKSync(func() { | |
| 410 | + | return wv.runOnGTKSyncLabel("cef.install_viewport_sync_hooks", func() { | |
| 410 | 411 | wv.installViewportSyncHooksOnGTKThread() | |
| 411 | 412 | }) | |
| 412 | 413 | } | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | "github.com/bnema/puregotk/v4/gtk" | 10 | "github.com/bnema/puregotk/v4/gtk" | ||
| 11 | 11 | ||||
| 12 | "github.com/bnema/dumber/internal/logging" | 12 | "github.com/bnema/dumber/internal/logging" | ||
| 13 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 13 | ) | 14 | ) | ||
| 14 | 15 | ||||
| 15 | type viewportSyncBrowserHost interface { | 16 | type viewportSyncBrowserHost interface { | ||
| @@ -402,11 +403,11 @@ func (wv *WebView) schedulePendingBrowserCreateObservedSizeRetry(ctx context.Con | |||||
| 402 | }) | 403 | }) | ||
| 403 | } | 404 | } | ||
| 404 | 405 | ||||
| 405 | - | func (wv *WebView) installViewportSyncHooks() { | 406 | + | func (wv *WebView) installViewportSyncHooks() syncdispatch.SyncDispatchResult { |
| 406 | if wv == nil || wv.viewBridge == nil { | 407 | if wv == nil || wv.viewBridge == nil { | ||
| 407 | - | return | 408 | + | return syncdispatch.SyncDispatchResult{Label: "cef.install_viewport_sync_hooks", Status: syncdispatch.SyncDispatchInline} |
| 408 | } | 409 | } | ||
| 409 | - | wv.runOnGTKSync(func() { | 410 | + | return wv.runOnGTKSyncLabel("cef.install_viewport_sync_hooks", func() { |
| 410 | wv.installViewportSyncHooksOnGTKThread() | 411 | wv.installViewportSyncHooksOnGTKThread() | ||
| 411 | }) | 412 | }) | ||
| 412 | } | 413 | } | ||
M internal/infrastructure/desktop/browser_launch_relay.go +76 -8
| @@ -2,6 +2,8 @@ package desktop | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "crypto/rand" | |
| 6 | + | "encoding/hex" | |
| 5 | 7 | "encoding/json" | |
| 6 | 8 | "errors" | |
| 7 | 9 | "fmt" | |
| @@ -37,11 +39,14 @@ type browserLaunchRelay struct { | |||
| 37 | 39 | } | |
| 38 | 40 | ||
| 39 | 41 | type browserLaunchRequest struct { | |
| 40 | - | URL string `json:"url"` | |
| 42 | + | RequestID string `json:"request_id,omitempty"` | |
| 43 | + | URL string `json:"url"` | |
| 41 | 44 | } | |
| 42 | 45 | ||
| 43 | 46 | type browserLaunchResponse struct { | |
| 44 | - | Error string `json:"error,omitempty"` | |
| 47 | + | RequestID string `json:"request_id,omitempty"` | |
| 48 | + | Accepted bool `json:"accepted,omitempty"` | |
| 49 | + | Error string `json:"error,omitempty"` | |
| 45 | 50 | } | |
| 46 | 51 | ||
| 47 | 52 | type browserLaunchRelayListener struct { | |
| @@ -51,6 +56,14 @@ type browserLaunchRelayListener struct { | |||
| 51 | 56 | err error | |
| 52 | 57 | } | |
| 53 | 58 | ||
| 59 | + | var newBrowserLaunchRequestID = func() string { | |
| 60 | + | var buf [8]byte | |
| 61 | + | if _, err := rand.Read(buf[:]); err == nil { | |
| 62 | + | return "blr-" + hex.EncodeToString(buf[:]) | |
| 63 | + | } | |
| 64 | + | return fmt.Sprintf("blr-%d", time.Now().UnixNano()) | |
| 65 | + | } | |
| 66 | + | ||
| 54 | 67 | func NewBrowserLaunchRelay(ipc runtimeprofile.IPCPaths) port.BrowserLaunchRelay { | |
| 55 | 68 | return &browserLaunchRelay{ipc: ipc} | |
| 56 | 69 | } | |
| @@ -70,10 +83,17 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 70 | 83 | } | |
| 71 | 84 | defer func() { _ = conn.Close() }() | |
| 72 | 85 | ||
| 86 | + | requestID := newBrowserLaunchRequestID() | |
| 87 | + | log := logging.FromContext(ctx) | |
| 88 | + | log.Debug(). | |
| 89 | + | Str("request_id", requestID). | |
| 90 | + | Str("url_host", safeURLHost(url)). | |
| 91 | + | Msg("browser launch relay delivery started") | |
| 92 | + | ||
| 73 | 93 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | |
| 74 | 94 | return false, err | |
| 75 | 95 | } | |
| 76 | - | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{URL: url}); err != nil { | |
| 96 | + | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url}); err != nil { | |
| 77 | 97 | return false, err | |
| 78 | 98 | } | |
| 79 | 99 | ||
| @@ -90,7 +110,8 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 90 | 110 | return false, ctxErr | |
| 91 | 111 | } | |
| 92 | 112 | if _, ok := ctx.Deadline(); !ok { | |
| 93 | - | logging.FromContext(ctx).Warn(). | |
| 113 | + | log.Warn(). | |
| 114 | + | Str("request_id", requestID). | |
| 94 | 115 | Str("url_host", safeURLHost(url)). | |
| 95 | 116 | Dur("timeout", browserLaunchIOTimeout). | |
| 96 | 117 | Msg("browser launch relay response timed out without caller deadline; delivery is unconfirmed") | |
| @@ -105,10 +126,23 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||
| 105 | 126 | } | |
| 106 | 127 | break | |
| 107 | 128 | } | |
| 129 | + | if response.RequestID != "" && response.RequestID != requestID { | |
| 130 | + | return true, fmt.Errorf("mismatched browser launch relay response request id: got %q, want %q", response.RequestID, requestID) | |
| 131 | + | } | |
| 108 | 132 | if response.Error != "" { | |
| 133 | + | log.Warn(). | |
| 134 | + | Str("request_id", requestID). | |
| 135 | + | Str("url_host", safeURLHost(url)). | |
| 136 | + | Str("relay_error", response.Error). | |
| 137 | + | Msg("browser launch relay delivery rejected") | |
| 109 | 138 | return true, errors.New(response.Error) | |
| 110 | 139 | } | |
| 111 | 140 | ||
| 141 | + | log.Debug(). | |
| 142 | + | Str("request_id", requestID). | |
| 143 | + | Str("url_host", safeURLHost(url)). | |
| 144 | + | Bool("accepted", response.Accepted || response.RequestID == ""). | |
| 145 | + | Msg("browser launch relay delivery acknowledged") | |
| 112 | 146 | return true, nil | |
| 113 | 147 | } | |
| 114 | 148 | ||
| @@ -240,6 +274,7 @@ func (l *browserLaunchRelayListener) serve(ctx context.Context, opener port.Brow | |||
| 240 | 274 | ||
| 241 | 275 | func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *net.UnixConn, opener port.BrowserWindowOpener) { | |
| 242 | 276 | defer func() { _ = conn.Close() }() | |
| 277 | + | log := logging.FromContext(ctx) | |
| 243 | 278 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | |
| 244 | 279 | return | |
| 245 | 280 | } | |
| @@ -248,20 +283,53 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||
| 248 | 283 | if err := json.NewDecoder(conn).Decode(&request); err != nil { | |
| 249 | 284 | return | |
| 250 | 285 | } | |
| 286 | + | requestID := request.RequestID | |
| 287 | + | if requestID == "" { | |
| 288 | + | requestID = newBrowserLaunchRequestID() | |
| 289 | + | } | |
| 290 | + | log.Debug(). | |
| 291 | + | Str("request_id", requestID). | |
| 292 | + | Str("url_host", safeURLHost(request.URL)). | |
| 293 | + | Msg("browser launch relay request received") | |
| 251 | 294 | ||
| 252 | 295 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | |
| 253 | 296 | return | |
| 254 | 297 | } | |
| 255 | - | if err := json.NewEncoder(conn).Encode(browserLaunchResponse{}); err != nil { | |
| 256 | - | log := logging.FromContext(ctx) | |
| 257 | - | log.Warn().Err(err).Str("url", request.URL).Msg("failed to encode browser launch response") | |
| 298 | + | if err := json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: requestID, Accepted: true}); err != nil { | |
| 299 | + | log.Warn().Err(err). | |
| 300 | + | Str("request_id", requestID). | |
| 301 | + | Str("url_host", safeURLHost(request.URL)). | |
| 302 | + | Msg("failed to encode browser launch response") | |
| 258 | 303 | return | |
| 259 | 304 | } | |
| 305 | + | log.Debug(). | |
| 306 | + | Str("request_id", requestID). | |
| 307 | + | Str("url_host", safeURLHost(request.URL)). | |
| 308 | + | Msg("browser launch relay request accepted") | |
| 260 | 309 | ||
| 261 | 310 | go func() { | |
| 311 | + | if opener == nil { | |
| 312 | + | log.Warn(). | |
| 313 | + | Str("request_id", requestID). | |
| 314 | + | Str("url_host", safeURLHost(request.URL)). | |
| 315 | + | Msg("browser launch relay accepted request without opener") | |
| 316 | + | return | |
| 317 | + | } | |
| 318 | + | log.Debug(). | |
| 319 | + | Str("request_id", requestID). | |
| 320 | + | Str("url_host", safeURLHost(request.URL)). | |
| 321 | + | Msg("browser launch relay opening fresh window") | |
| 262 | 322 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | |
| 263 | - | logging.FromContext(ctx).Warn().Err(err).Str("url", request.URL).Msg("failed to open fresh browser window") | |
| 323 | + | log.Warn().Err(err). | |
| 324 | + | Str("request_id", requestID). | |
| 325 | + | Str("url_host", safeURLHost(request.URL)). | |
| 326 | + | Msg("failed to open fresh browser window") | |
| 327 | + | return | |
| 264 | 328 | } | |
| 329 | + | log.Debug(). | |
| 330 | + | Str("request_id", requestID). | |
| 331 | + | Str("url_host", safeURLHost(request.URL)). | |
| 332 | + | Msg("browser launch relay opened fresh window") | |
| 265 | 333 | }() | |
| 266 | 334 | } | |
| 267 | 335 | ||
| @@ -2,6 +2,8 @@ package desktop | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "crypto/rand" | |||
| 6 | + | "encoding/hex" | |||
| 5 | "encoding/json" | 7 | "encoding/json" | ||
| 6 | "errors" | 8 | "errors" | ||
| 7 | "fmt" | 9 | "fmt" | ||
| @@ -37,11 +39,14 @@ type browserLaunchRelay struct { | |||||
| 37 | } | 39 | } | ||
| 38 | 40 | ||||
| 39 | type browserLaunchRequest struct { | 41 | type browserLaunchRequest struct { | ||
| 40 | - | URL string `json:"url"` | 42 | + | RequestID string `json:"request_id,omitempty"` |
| 43 | + | URL string `json:"url"` | |||
| 41 | } | 44 | } | ||
| 42 | 45 | ||||
| 43 | type browserLaunchResponse struct { | 46 | type browserLaunchResponse struct { | ||
| 44 | - | Error string `json:"error,omitempty"` | 47 | + | RequestID string `json:"request_id,omitempty"` |
| 48 | + | Accepted bool `json:"accepted,omitempty"` | |||
| 49 | + | Error string `json:"error,omitempty"` | |||
| 45 | } | 50 | } | ||
| 46 | 51 | ||||
| 47 | type browserLaunchRelayListener struct { | 52 | type browserLaunchRelayListener struct { | ||
| @@ -51,6 +56,14 @@ type browserLaunchRelayListener struct { | |||||
| 51 | err error | 56 | err error | ||
| 52 | } | 57 | } | ||
| 53 | 58 | ||||
| 59 | + | var newBrowserLaunchRequestID = func() string { | |||
| 60 | + | var buf [8]byte | |||
| 61 | + | if _, err := rand.Read(buf[:]); err == nil { | |||
| 62 | + | return "blr-" + hex.EncodeToString(buf[:]) | |||
| 63 | + | } | |||
| 64 | + | return fmt.Sprintf("blr-%d", time.Now().UnixNano()) | |||
| 65 | + | } | |||
| 66 | + | ||||
| 54 | func NewBrowserLaunchRelay(ipc runtimeprofile.IPCPaths) port.BrowserLaunchRelay { | 67 | func NewBrowserLaunchRelay(ipc runtimeprofile.IPCPaths) port.BrowserLaunchRelay { | ||
| 55 | return &browserLaunchRelay{ipc: ipc} | 68 | return &browserLaunchRelay{ipc: ipc} | ||
| 56 | } | 69 | } | ||
| @@ -70,10 +83,17 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 70 | } | 83 | } | ||
| 71 | defer func() { _ = conn.Close() }() | 84 | defer func() { _ = conn.Close() }() | ||
| 72 | 85 | ||||
| 86 | + | requestID := newBrowserLaunchRequestID() | |||
| 87 | + | log := logging.FromContext(ctx) | |||
| 88 | + | log.Debug(). | |||
| 89 | + | Str("request_id", requestID). | |||
| 90 | + | Str("url_host", safeURLHost(url)). | |||
| 91 | + | Msg("browser launch relay delivery started") | |||
| 92 | + | ||||
| 73 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | 93 | if err := setBrowserLaunchConnDeadline(ctx, conn); err != nil { | ||
| 74 | return false, err | 94 | return false, err | ||
| 75 | } | 95 | } | ||
| 76 | - | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{URL: url}); err != nil { | 96 | + | if err := json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: requestID, URL: url}); err != nil { |
| 77 | return false, err | 97 | return false, err | ||
| 78 | } | 98 | } | ||
| 79 | 99 | ||||
| @@ -90,7 +110,8 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 90 | return false, ctxErr | 110 | return false, ctxErr | ||
| 91 | } | 111 | } | ||
| 92 | if _, ok := ctx.Deadline(); !ok { | 112 | if _, ok := ctx.Deadline(); !ok { | ||
| 93 | - | logging.FromContext(ctx).Warn(). | 113 | + | log.Warn(). |
| 114 | + | Str("request_id", requestID). | |||
| 94 | Str("url_host", safeURLHost(url)). | 115 | Str("url_host", safeURLHost(url)). | ||
| 95 | Dur("timeout", browserLaunchIOTimeout). | 116 | Dur("timeout", browserLaunchIOTimeout). | ||
| 96 | Msg("browser launch relay response timed out without caller deadline; delivery is unconfirmed") | 117 | Msg("browser launch relay response timed out without caller deadline; delivery is unconfirmed") | ||
| @@ -105,10 +126,23 @@ func (r *browserLaunchRelay) DeliverOpenFreshWindow(ctx context.Context, url str | |||||
| 105 | } | 126 | } | ||
| 106 | break | 127 | break | ||
| 107 | } | 128 | } | ||
| 129 | + | if response.RequestID != "" && response.RequestID != requestID { | |||
| 130 | + | return true, fmt.Errorf("mismatched browser launch relay response request id: got %q, want %q", response.RequestID, requestID) | |||
| 131 | + | } | |||
| 108 | if response.Error != "" { | 132 | if response.Error != "" { | ||
| 133 | + | log.Warn(). | |||
| 134 | + | Str("request_id", requestID). | |||
| 135 | + | Str("url_host", safeURLHost(url)). | |||
| 136 | + | Str("relay_error", response.Error). | |||
| 137 | + | Msg("browser launch relay delivery rejected") | |||
| 109 | return true, errors.New(response.Error) | 138 | return true, errors.New(response.Error) | ||
| 110 | } | 139 | } | ||
| 111 | 140 | ||||
| 141 | + | log.Debug(). | |||
| 142 | + | Str("request_id", requestID). | |||
| 143 | + | Str("url_host", safeURLHost(url)). | |||
| 144 | + | Bool("accepted", response.Accepted || response.RequestID == ""). | |||
| 145 | + | Msg("browser launch relay delivery acknowledged") | |||
| 112 | return true, nil | 146 | return true, nil | ||
| 113 | } | 147 | } | ||
| 114 | 148 | ||||
| @@ -240,6 +274,7 @@ func (l *browserLaunchRelayListener) serve(ctx context.Context, opener port.Brow | |||||
| 240 | 274 | ||||
| 241 | func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *net.UnixConn, opener port.BrowserWindowOpener) { | 275 | func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *net.UnixConn, opener port.BrowserWindowOpener) { | ||
| 242 | defer func() { _ = conn.Close() }() | 276 | defer func() { _ = conn.Close() }() | ||
| 277 | + | log := logging.FromContext(ctx) | |||
| 243 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | 278 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | ||
| 244 | return | 279 | return | ||
| 245 | } | 280 | } | ||
| @@ -248,20 +283,53 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||||
| 248 | if err := json.NewDecoder(conn).Decode(&request); err != nil { | 283 | if err := json.NewDecoder(conn).Decode(&request); err != nil { | ||
| 249 | return | 284 | return | ||
| 250 | } | 285 | } | ||
| 286 | + | requestID := request.RequestID | |||
| 287 | + | if requestID == "" { | |||
| 288 | + | requestID = newBrowserLaunchRequestID() | |||
| 289 | + | } | |||
| 290 | + | log.Debug(). | |||
| 291 | + | Str("request_id", requestID). | |||
| 292 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 293 | + | Msg("browser launch relay request received") | |||
| 251 | 294 | ||||
| 252 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | 295 | if err := conn.SetDeadline(time.Now().Add(browserLaunchIOTimeout)); err != nil { | ||
| 253 | return | 296 | return | ||
| 254 | } | 297 | } | ||
| 255 | - | if err := json.NewEncoder(conn).Encode(browserLaunchResponse{}); err != nil { | 298 | + | if err := json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: requestID, Accepted: true}); err != nil { |
| 256 | - | log := logging.FromContext(ctx) | 299 | + | log.Warn().Err(err). |
| 257 | - | log.Warn().Err(err).Str("url", request.URL).Msg("failed to encode browser launch response") | 300 | + | Str("request_id", requestID). |
| 301 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 302 | + | Msg("failed to encode browser launch response") | |||
| 258 | return | 303 | return | ||
| 259 | } | 304 | } | ||
| 305 | + | log.Debug(). | |||
| 306 | + | Str("request_id", requestID). | |||
| 307 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 308 | + | Msg("browser launch relay request accepted") | |||
| 260 | 309 | ||||
| 261 | go func() { | 310 | go func() { | ||
| 311 | + | if opener == nil { | |||
| 312 | + | log.Warn(). | |||
| 313 | + | Str("request_id", requestID). | |||
| 314 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 315 | + | Msg("browser launch relay accepted request without opener") | |||
| 316 | + | return | |||
| 317 | + | } | |||
| 318 | + | log.Debug(). | |||
| 319 | + | Str("request_id", requestID). | |||
| 320 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 321 | + | Msg("browser launch relay opening fresh window") | |||
| 262 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | 322 | if err := opener.OpenFreshWindow(ctx, request.URL); err != nil { | ||
| 263 | - | logging.FromContext(ctx).Warn().Err(err).Str("url", request.URL).Msg("failed to open fresh browser window") | 323 | + | log.Warn().Err(err). |
| 324 | + | Str("request_id", requestID). | |||
| 325 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 326 | + | Msg("failed to open fresh browser window") | |||
| 327 | + | return | |||
| 264 | } | 328 | } | ||
| 329 | + | log.Debug(). | |||
| 330 | + | Str("request_id", requestID). | |||
| 331 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 332 | + | Msg("browser launch relay opened fresh window") | |||
| 265 | }() | 333 | }() | ||
| 266 | } | 334 | } | ||
| 267 | 335 | ||||
M internal/infrastructure/desktop/browser_launch_relay_test.go +119 -1
| @@ -3,6 +3,7 @@ package desktop | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 6 | 7 | "net" | |
| 7 | 8 | "os" | |
| 8 | 9 | "path/filepath" | |
| @@ -66,6 +67,13 @@ func testNamespacedIPC(root, engine string) runtimeprofile.IPCPaths { | |||
| 66 | 67 | } | |
| 67 | 68 | } | |
| 68 | 69 | ||
| 70 | + | func overrideBrowserLaunchRequestID(t *testing.T, id string) func() { | |
| 71 | + | t.Helper() | |
| 72 | + | previous := newBrowserLaunchRequestID | |
| 73 | + | newBrowserLaunchRequestID = func() string { return id } | |
| 74 | + | return func() { newBrowserLaunchRequestID = previous } | |
| 75 | + | } | |
| 76 | + | ||
| 69 | 77 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_MissingListenerReturnsFalseNil(t *testing.T) { | |
| 70 | 78 | relay := NewBrowserLaunchRelay(testIPC(shortTempDir(t))) | |
| 71 | 79 | ||
| @@ -106,6 +114,78 @@ func TestBrowserLaunchRelay_DevCEFAndDevWebKit_UseDifferentSockets(t *testing.T) | |||
| 106 | 114 | require.NotEqual(t, cefIPC.BrowserLaunchSocket, wkIPC.BrowserLaunchSocket) | |
| 107 | 115 | } | |
| 108 | 116 | ||
| 117 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_SendsRequestIDAndAcceptsMatchingResponse(t *testing.T) { | |
| 118 | + | ipc := testIPC(shortTempDir(t)) | |
| 119 | + | relay := NewBrowserLaunchRelay(ipc) | |
| 120 | + | restore := overrideBrowserLaunchRequestID(t, "request-test-1") | |
| 121 | + | defer restore() | |
| 122 | + | ||
| 123 | + | receivedRequest := make(chan browserLaunchRequest, 1) | |
| 124 | + | require.NoError(t, os.MkdirAll(ipc.RuntimeDir, 0o700)) | |
| 125 | + | listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: ipc.BrowserLaunchSocket, Net: "unix"}) | |
| 126 | + | require.NoError(t, err) | |
| 127 | + | defer func() { | |
| 128 | + | _ = listener.Close() | |
| 129 | + | _ = os.Remove(ipc.BrowserLaunchSocket) | |
| 130 | + | }() | |
| 131 | + | ||
| 132 | + | go func() { | |
| 133 | + | conn, acceptErr := listener.AcceptUnix() | |
| 134 | + | if acceptErr != nil { | |
| 135 | + | return | |
| 136 | + | } | |
| 137 | + | defer conn.Close() | |
| 138 | + | var request browserLaunchRequest | |
| 139 | + | if decodeErr := json.NewDecoder(conn).Decode(&request); decodeErr != nil { | |
| 140 | + | return | |
| 141 | + | } | |
| 142 | + | receivedRequest <- request | |
| 143 | + | _ = json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: request.RequestID, Accepted: true}) | |
| 144 | + | }() | |
| 145 | + | ||
| 146 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/request-id") | |
| 147 | + | ||
| 148 | + | require.NoError(t, err) | |
| 149 | + | require.True(t, delivered) | |
| 150 | + | request := <-receivedRequest | |
| 151 | + | require.Equal(t, "request-test-1", request.RequestID) | |
| 152 | + | require.Equal(t, "https://example.com/request-id", request.URL) | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RejectsMismatchedResponseRequestID(t *testing.T) { | |
| 156 | + | ipc := testIPC(shortTempDir(t)) | |
| 157 | + | relay := NewBrowserLaunchRelay(ipc) | |
| 158 | + | restore := overrideBrowserLaunchRequestID(t, "request-test-2") | |
| 159 | + | defer restore() | |
| 160 | + | ||
| 161 | + | require.NoError(t, os.MkdirAll(ipc.RuntimeDir, 0o700)) | |
| 162 | + | listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: ipc.BrowserLaunchSocket, Net: "unix"}) | |
| 163 | + | require.NoError(t, err) | |
| 164 | + | defer func() { | |
| 165 | + | _ = listener.Close() | |
| 166 | + | _ = os.Remove(ipc.BrowserLaunchSocket) | |
| 167 | + | }() | |
| 168 | + | ||
| 169 | + | go func() { | |
| 170 | + | conn, acceptErr := listener.AcceptUnix() | |
| 171 | + | if acceptErr != nil { | |
| 172 | + | return | |
| 173 | + | } | |
| 174 | + | defer conn.Close() | |
| 175 | + | var request browserLaunchRequest | |
| 176 | + | if decodeErr := json.NewDecoder(conn).Decode(&request); decodeErr != nil { | |
| 177 | + | return | |
| 178 | + | } | |
| 179 | + | _ = json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: "different-request", Accepted: true}) | |
| 180 | + | }() | |
| 181 | + | ||
| 182 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/request-id-mismatch") | |
| 183 | + | ||
| 184 | + | require.Error(t, err) | |
| 185 | + | require.True(t, delivered) | |
| 186 | + | require.Contains(t, err.Error(), "mismatched browser launch relay response") | |
| 187 | + | } | |
| 188 | + | ||
| 109 | 189 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RoundTrip(t *testing.T) { | |
| 110 | 190 | ipc := testIPC(shortTempDir(t)) | |
| 111 | 191 | relay := NewBrowserLaunchRelay(ipc) | |
| @@ -385,11 +465,13 @@ func TestBrowserLaunchRelay_AcknowledgesBeforeOpenerReturns(t *testing.T) { | |||
| 385 | 465 | require.NoError(t, err) | |
| 386 | 466 | defer conn.Close() | |
| 387 | 467 | require.NoError(t, conn.SetDeadline(time.Now().Add(300*time.Millisecond))) | |
| 388 | - | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{URL: "https://example.com/slow-but-healthy"})) | |
| 468 | + | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: "raw-request-1", URL: "https://example.com/slow-but-healthy"})) | |
| 389 | 469 | ||
| 390 | 470 | var response browserLaunchResponse | |
| 391 | 471 | require.NoError(t, json.NewDecoder(conn).Decode(&response)) | |
| 392 | 472 | assert.Empty(t, response.Error) | |
| 473 | + | assert.True(t, response.Accepted) | |
| 474 | + | assert.Equal(t, "raw-request-1", response.RequestID) | |
| 393 | 475 | ||
| 394 | 476 | select { | |
| 395 | 477 | case <-started: | |
| @@ -399,6 +481,42 @@ func TestBrowserLaunchRelay_AcknowledgesBeforeOpenerReturns(t *testing.T) { | |||
| 399 | 481 | close(release) | |
| 400 | 482 | } | |
| 401 | 483 | ||
| 484 | + | func TestBrowserLaunchRelay_AcknowledgesAcceptedEvenWhenOpenerReturnsLateError(t *testing.T) { | |
| 485 | + | ipc := testIPC(shortTempDir(t)) | |
| 486 | + | relay := NewBrowserLaunchRelay(ipc) | |
| 487 | + | ||
| 488 | + | started := make(chan struct{}) | |
| 489 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 490 | + | defer cancel() | |
| 491 | + | ||
| 492 | + | closer, err := relay.Listen(ctx, browserWindowOpenerFunc(func(_ context.Context, _ string) error { | |
| 493 | + | close(started) | |
| 494 | + | return errors.New("late open failure") | |
| 495 | + | })) | |
| 496 | + | require.NoError(t, err) | |
| 497 | + | defer closer.Close() | |
| 498 | + | ||
| 499 | + | waitForSocket(t, ipc.BrowserLaunchSocket) | |
| 500 | + | ||
| 501 | + | conn, err := net.Dial("unix", ipc.BrowserLaunchSocket) | |
| 502 | + | require.NoError(t, err) | |
| 503 | + | defer conn.Close() | |
| 504 | + | require.NoError(t, conn.SetDeadline(time.Now().Add(300*time.Millisecond))) | |
| 505 | + | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: "raw-request-2", URL: "https://example.com/fails-later"})) | |
| 506 | + | ||
| 507 | + | var response browserLaunchResponse | |
| 508 | + | require.NoError(t, json.NewDecoder(conn).Decode(&response)) | |
| 509 | + | assert.Empty(t, response.Error) | |
| 510 | + | assert.True(t, response.Accepted) | |
| 511 | + | assert.Equal(t, "raw-request-2", response.RequestID) | |
| 512 | + | ||
| 513 | + | select { | |
| 514 | + | case <-started: | |
| 515 | + | case <-time.After(time.Second): | |
| 516 | + | t.Fatal("expected opener to be invoked after accepted acknowledgement") | |
| 517 | + | } | |
| 518 | + | } | |
| 519 | + | ||
| 402 | 520 | func TestBrowserLaunchRelay_SilentClientDoesNotStallListener(t *testing.T) { | |
| 403 | 521 | ipc := testIPC(shortTempDir(t)) | |
| 404 | 522 | relay := NewBrowserLaunchRelay(ipc) | |
| @@ -3,6 +3,7 @@ package desktop | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 6 | "net" | 7 | "net" | ||
| 7 | "os" | 8 | "os" | ||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| @@ -66,6 +67,13 @@ func testNamespacedIPC(root, engine string) runtimeprofile.IPCPaths { | |||||
| 66 | } | 67 | } | ||
| 67 | } | 68 | } | ||
| 68 | 69 | ||||
| 70 | + | func overrideBrowserLaunchRequestID(t *testing.T, id string) func() { | |||
| 71 | + | t.Helper() | |||
| 72 | + | previous := newBrowserLaunchRequestID | |||
| 73 | + | newBrowserLaunchRequestID = func() string { return id } | |||
| 74 | + | return func() { newBrowserLaunchRequestID = previous } | |||
| 75 | + | } | |||
| 76 | + | ||||
| 69 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_MissingListenerReturnsFalseNil(t *testing.T) { | 77 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_MissingListenerReturnsFalseNil(t *testing.T) { | ||
| 70 | relay := NewBrowserLaunchRelay(testIPC(shortTempDir(t))) | 78 | relay := NewBrowserLaunchRelay(testIPC(shortTempDir(t))) | ||
| 71 | 79 | ||||
| @@ -106,6 +114,78 @@ func TestBrowserLaunchRelay_DevCEFAndDevWebKit_UseDifferentSockets(t *testing.T) | |||||
| 106 | require.NotEqual(t, cefIPC.BrowserLaunchSocket, wkIPC.BrowserLaunchSocket) | 114 | require.NotEqual(t, cefIPC.BrowserLaunchSocket, wkIPC.BrowserLaunchSocket) | ||
| 107 | } | 115 | } | ||
| 108 | 116 | ||||
| 117 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_SendsRequestIDAndAcceptsMatchingResponse(t *testing.T) { | |||
| 118 | + | ipc := testIPC(shortTempDir(t)) | |||
| 119 | + | relay := NewBrowserLaunchRelay(ipc) | |||
| 120 | + | restore := overrideBrowserLaunchRequestID(t, "request-test-1") | |||
| 121 | + | defer restore() | |||
| 122 | + | ||||
| 123 | + | receivedRequest := make(chan browserLaunchRequest, 1) | |||
| 124 | + | require.NoError(t, os.MkdirAll(ipc.RuntimeDir, 0o700)) | |||
| 125 | + | listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: ipc.BrowserLaunchSocket, Net: "unix"}) | |||
| 126 | + | require.NoError(t, err) | |||
| 127 | + | defer func() { | |||
| 128 | + | _ = listener.Close() | |||
| 129 | + | _ = os.Remove(ipc.BrowserLaunchSocket) | |||
| 130 | + | }() | |||
| 131 | + | ||||
| 132 | + | go func() { | |||
| 133 | + | conn, acceptErr := listener.AcceptUnix() | |||
| 134 | + | if acceptErr != nil { | |||
| 135 | + | return | |||
| 136 | + | } | |||
| 137 | + | defer conn.Close() | |||
| 138 | + | var request browserLaunchRequest | |||
| 139 | + | if decodeErr := json.NewDecoder(conn).Decode(&request); decodeErr != nil { | |||
| 140 | + | return | |||
| 141 | + | } | |||
| 142 | + | receivedRequest <- request | |||
| 143 | + | _ = json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: request.RequestID, Accepted: true}) | |||
| 144 | + | }() | |||
| 145 | + | ||||
| 146 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/request-id") | |||
| 147 | + | ||||
| 148 | + | require.NoError(t, err) | |||
| 149 | + | require.True(t, delivered) | |||
| 150 | + | request := <-receivedRequest | |||
| 151 | + | require.Equal(t, "request-test-1", request.RequestID) | |||
| 152 | + | require.Equal(t, "https://example.com/request-id", request.URL) | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RejectsMismatchedResponseRequestID(t *testing.T) { | |||
| 156 | + | ipc := testIPC(shortTempDir(t)) | |||
| 157 | + | relay := NewBrowserLaunchRelay(ipc) | |||
| 158 | + | restore := overrideBrowserLaunchRequestID(t, "request-test-2") | |||
| 159 | + | defer restore() | |||
| 160 | + | ||||
| 161 | + | require.NoError(t, os.MkdirAll(ipc.RuntimeDir, 0o700)) | |||
| 162 | + | listener, err := net.ListenUnix("unix", &net.UnixAddr{Name: ipc.BrowserLaunchSocket, Net: "unix"}) | |||
| 163 | + | require.NoError(t, err) | |||
| 164 | + | defer func() { | |||
| 165 | + | _ = listener.Close() | |||
| 166 | + | _ = os.Remove(ipc.BrowserLaunchSocket) | |||
| 167 | + | }() | |||
| 168 | + | ||||
| 169 | + | go func() { | |||
| 170 | + | conn, acceptErr := listener.AcceptUnix() | |||
| 171 | + | if acceptErr != nil { | |||
| 172 | + | return | |||
| 173 | + | } | |||
| 174 | + | defer conn.Close() | |||
| 175 | + | var request browserLaunchRequest | |||
| 176 | + | if decodeErr := json.NewDecoder(conn).Decode(&request); decodeErr != nil { | |||
| 177 | + | return | |||
| 178 | + | } | |||
| 179 | + | _ = json.NewEncoder(conn).Encode(browserLaunchResponse{RequestID: "different-request", Accepted: true}) | |||
| 180 | + | }() | |||
| 181 | + | ||||
| 182 | + | delivered, err := relay.DeliverOpenFreshWindow(context.Background(), "https://example.com/request-id-mismatch") | |||
| 183 | + | ||||
| 184 | + | require.Error(t, err) | |||
| 185 | + | require.True(t, delivered) | |||
| 186 | + | require.Contains(t, err.Error(), "mismatched browser launch relay response") | |||
| 187 | + | } | |||
| 188 | + | ||||
| 109 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RoundTrip(t *testing.T) { | 189 | func TestBrowserLaunchRelay_DeliverOpenFreshWindow_RoundTrip(t *testing.T) { | ||
| 110 | ipc := testIPC(shortTempDir(t)) | 190 | ipc := testIPC(shortTempDir(t)) | ||
| 111 | relay := NewBrowserLaunchRelay(ipc) | 191 | relay := NewBrowserLaunchRelay(ipc) | ||
| @@ -385,11 +465,13 @@ func TestBrowserLaunchRelay_AcknowledgesBeforeOpenerReturns(t *testing.T) { | |||||
| 385 | require.NoError(t, err) | 465 | require.NoError(t, err) | ||
| 386 | defer conn.Close() | 466 | defer conn.Close() | ||
| 387 | require.NoError(t, conn.SetDeadline(time.Now().Add(300*time.Millisecond))) | 467 | require.NoError(t, conn.SetDeadline(time.Now().Add(300*time.Millisecond))) | ||
| 388 | - | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{URL: "https://example.com/slow-but-healthy"})) | 468 | + | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: "raw-request-1", URL: "https://example.com/slow-but-healthy"})) |
| 389 | 469 | ||||
| 390 | var response browserLaunchResponse | 470 | var response browserLaunchResponse | ||
| 391 | require.NoError(t, json.NewDecoder(conn).Decode(&response)) | 471 | require.NoError(t, json.NewDecoder(conn).Decode(&response)) | ||
| 392 | assert.Empty(t, response.Error) | 472 | assert.Empty(t, response.Error) | ||
| 473 | + | assert.True(t, response.Accepted) | |||
| 474 | + | assert.Equal(t, "raw-request-1", response.RequestID) | |||
| 393 | 475 | ||||
| 394 | select { | 476 | select { | ||
| 395 | case <-started: | 477 | case <-started: | ||
| @@ -399,6 +481,42 @@ func TestBrowserLaunchRelay_AcknowledgesBeforeOpenerReturns(t *testing.T) { | |||||
| 399 | close(release) | 481 | close(release) | ||
| 400 | } | 482 | } | ||
| 401 | 483 | ||||
| 484 | + | func TestBrowserLaunchRelay_AcknowledgesAcceptedEvenWhenOpenerReturnsLateError(t *testing.T) { | |||
| 485 | + | ipc := testIPC(shortTempDir(t)) | |||
| 486 | + | relay := NewBrowserLaunchRelay(ipc) | |||
| 487 | + | ||||
| 488 | + | started := make(chan struct{}) | |||
| 489 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 490 | + | defer cancel() | |||
| 491 | + | ||||
| 492 | + | closer, err := relay.Listen(ctx, browserWindowOpenerFunc(func(_ context.Context, _ string) error { | |||
| 493 | + | close(started) | |||
| 494 | + | return errors.New("late open failure") | |||
| 495 | + | })) | |||
| 496 | + | require.NoError(t, err) | |||
| 497 | + | defer closer.Close() | |||
| 498 | + | ||||
| 499 | + | waitForSocket(t, ipc.BrowserLaunchSocket) | |||
| 500 | + | ||||
| 501 | + | conn, err := net.Dial("unix", ipc.BrowserLaunchSocket) | |||
| 502 | + | require.NoError(t, err) | |||
| 503 | + | defer conn.Close() | |||
| 504 | + | require.NoError(t, conn.SetDeadline(time.Now().Add(300*time.Millisecond))) | |||
| 505 | + | require.NoError(t, json.NewEncoder(conn).Encode(browserLaunchRequest{RequestID: "raw-request-2", URL: "https://example.com/fails-later"})) | |||
| 506 | + | ||||
| 507 | + | var response browserLaunchResponse | |||
| 508 | + | require.NoError(t, json.NewDecoder(conn).Decode(&response)) | |||
| 509 | + | assert.Empty(t, response.Error) | |||
| 510 | + | assert.True(t, response.Accepted) | |||
| 511 | + | assert.Equal(t, "raw-request-2", response.RequestID) | |||
| 512 | + | ||||
| 513 | + | select { | |||
| 514 | + | case <-started: | |||
| 515 | + | case <-time.After(time.Second): | |||
| 516 | + | t.Fatal("expected opener to be invoked after accepted acknowledgement") | |||
| 517 | + | } | |||
| 518 | + | } | |||
| 519 | + | ||||
| 402 | func TestBrowserLaunchRelay_SilentClientDoesNotStallListener(t *testing.T) { | 520 | func TestBrowserLaunchRelay_SilentClientDoesNotStallListener(t *testing.T) { | ||
| 403 | ipc := testIPC(shortTempDir(t)) | 521 | ipc := testIPC(shortTempDir(t)) | ||
| 404 | relay := NewBrowserLaunchRelay(ipc) | 522 | relay := NewBrowserLaunchRelay(ipc) | ||
M internal/infrastructure/snapshot/service.go +7 -0
| @@ -172,6 +172,13 @@ func (s *Service) saveSnapshot(ctx context.Context) error { | |||
| 172 | 172 | } | |
| 173 | 173 | ||
| 174 | 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | |
| 175 | + | // A nil window list with no active index means the snapshot is truly unavailable, | |
| 176 | + | // not merely empty. Keep the session dirty so a later capture can retry. | |
| 177 | + | if windows == nil && activeWindowIndex < 0 { | |
| 178 | + | s.markDirty() | |
| 179 | + | logging.FromContext(ctx).Warn().Msg("window snapshot unavailable; keeping session snapshot dirty") | |
| 180 | + | return nil | |
| 181 | + | } | |
| 175 | 182 | if windows == nil { | |
| 176 | 183 | windows = make([]entity.WindowTabListState, 0) | |
| 177 | 184 | } | |
| @@ -172,6 +172,13 @@ func (s *Service) saveSnapshot(ctx context.Context) error { | |||||
| 172 | } | 172 | } | ||
| 173 | 173 | ||||
| 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | ||
| 175 | + | // A nil window list with no active index means the snapshot is truly unavailable, | |||
| 176 | + | // not merely empty. Keep the session dirty so a later capture can retry. | |||
| 177 | + | if windows == nil && activeWindowIndex < 0 { | |||
| 178 | + | s.markDirty() | |||
| 179 | + | logging.FromContext(ctx).Warn().Msg("window snapshot unavailable; keeping session snapshot dirty") | |||
| 180 | + | return nil | |||
| 181 | + | } | |||
| 175 | if windows == nil { | 182 | if windows == nil { | ||
| 176 | windows = make([]entity.WindowTabListState, 0) | 183 | windows = make([]entity.WindowTabListState, 0) | ||
| 177 | } | 184 | } | ||
M internal/infrastructure/snapshot/service_test.go +16 -0
| @@ -176,6 +176,22 @@ func TestService_SaveNowPassesWindowSnapshots(t *testing.T) { | |||
| 176 | 176 | assert.False(t, svc.dirty) | |
| 177 | 177 | } | |
| 178 | 178 | ||
| 179 | + | func TestService_SaveSnapshotKeepsDirtyWhenWindowSnapshotUnavailable(t *testing.T) { | |
| 180 | + | repo := repomocks.NewMockSessionStateRepository(t) | |
| 181 | + | uc := usecase.NewSnapshotSessionUseCase(repo) | |
| 182 | + | provider := mocks.NewMockWindowStateProvider(t) | |
| 183 | + | provider.EXPECT().GetSessionID().Return(entity.SessionID("20260501_snapshot_unavailable")).Once() | |
| 184 | + | provider.EXPECT().GetWindowSnapshotState().Return(nil, -1).Once() | |
| 185 | + | ||
| 186 | + | svc := NewService(uc, provider, 1) | |
| 187 | + | svc.ready = true | |
| 188 | + | svc.dirty = true | |
| 189 | + | ||
| 190 | + | err := svc.saveSnapshot(context.Background()) | |
| 191 | + | require.NoError(t, err) | |
| 192 | + | assert.True(t, svc.dirty) | |
| 193 | + | } | |
| 194 | + | ||
| 179 | 195 | func TestService_SaveNowPersistsEmptyWindowSnapshotAsV2(t *testing.T) { | |
| 180 | 196 | repo := repomocks.NewMockSessionStateRepository(t) | |
| 181 | 197 | sessionID := entity.SessionID("20260501_empty_window_snap") | |
| @@ -176,6 +176,22 @@ func TestService_SaveNowPassesWindowSnapshots(t *testing.T) { | |||||
| 176 | assert.False(t, svc.dirty) | 176 | assert.False(t, svc.dirty) | ||
| 177 | } | 177 | } | ||
| 178 | 178 | ||||
| 179 | + | func TestService_SaveSnapshotKeepsDirtyWhenWindowSnapshotUnavailable(t *testing.T) { | |||
| 180 | + | repo := repomocks.NewMockSessionStateRepository(t) | |||
| 181 | + | uc := usecase.NewSnapshotSessionUseCase(repo) | |||
| 182 | + | provider := mocks.NewMockWindowStateProvider(t) | |||
| 183 | + | provider.EXPECT().GetSessionID().Return(entity.SessionID("20260501_snapshot_unavailable")).Once() | |||
| 184 | + | provider.EXPECT().GetWindowSnapshotState().Return(nil, -1).Once() | |||
| 185 | + | ||||
| 186 | + | svc := NewService(uc, provider, 1) | |||
| 187 | + | svc.ready = true | |||
| 188 | + | svc.dirty = true | |||
| 189 | + | ||||
| 190 | + | err := svc.saveSnapshot(context.Background()) | |||
| 191 | + | require.NoError(t, err) | |||
| 192 | + | assert.True(t, svc.dirty) | |||
| 193 | + | } | |||
| 194 | + | ||||
| 179 | func TestService_SaveNowPersistsEmptyWindowSnapshotAsV2(t *testing.T) { | 195 | func TestService_SaveNowPersistsEmptyWindowSnapshotAsV2(t *testing.T) { | ||
| 180 | repo := repomocks.NewMockSessionStateRepository(t) | 196 | repo := repomocks.NewMockSessionStateRepository(t) | ||
| 181 | sessionID := entity.SessionID("20260501_empty_window_snap") | 197 | sessionID := entity.SessionID("20260501_empty_window_snap") | ||
A internal/shared/syncdispatch/sync_dispatch.go +191 -0
| @@ -0,0 +1,191 @@ | |||
| 1 | + | package syncdispatch | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "time" | |
| 6 | + | ) | |
| 7 | + | ||
| 8 | + | // SyncDispatchStatus describes how a synchronous event-loop dispatch finished. | |
| 9 | + | type SyncDispatchStatus string | |
| 10 | + | ||
| 11 | + | const ( | |
| 12 | + | // SyncDispatchInline means the callback ran immediately because the caller | |
| 13 | + | // already owned the target main context. | |
| 14 | + | SyncDispatchInline SyncDispatchStatus = "inline" | |
| 15 | + | // SyncDispatchCompleted means the callback was dispatched and completed before | |
| 16 | + | // the configured timeout elapsed. | |
| 17 | + | SyncDispatchCompleted SyncDispatchStatus = "completed" | |
| 18 | + | // SyncDispatchTimedOut means the callback did not start before the timeout and | |
| 19 | + | // was canceled before running side effects. | |
| 20 | + | SyncDispatchTimedOut SyncDispatchStatus = "timed_out" | |
| 21 | + | // SyncDispatchCompletedAfterTimeout means the callback started before the | |
| 22 | + | // timeout and completed after the timeout elapsed. Callers must treat it as | |
| 23 | + | // completed because side effects have run. | |
| 24 | + | SyncDispatchCompletedAfterTimeout SyncDispatchStatus = "completed_after_timeout" | |
| 25 | + | // SyncDispatchQueuedAfterTimeout means the callback did not start before the | |
| 26 | + | // timeout, but the dispatch was intentionally left queued so it may run later. | |
| 27 | + | SyncDispatchQueuedAfterTimeout SyncDispatchStatus = "queued_after_timeout" | |
| 28 | + | // SyncDispatchSkipped means required inputs were missing, so no callback ran. | |
| 29 | + | SyncDispatchSkipped SyncDispatchStatus = "skipped" | |
| 30 | + | ) | |
| 31 | + | ||
| 32 | + | // SyncDispatchOptions configures RunSynchronousDispatch. | |
| 33 | + | type SyncDispatchOptions struct { | |
| 34 | + | // Label identifies the caller in diagnostics. The helper does not log by | |
| 35 | + | // itself, but callers can include this label when reporting the returned | |
| 36 | + | // result. | |
| 37 | + | Label string | |
| 38 | + | // Timeout bounds how long the caller waits for dispatched work. Non-positive | |
| 39 | + | // durations fall back to DefaultSyncDispatchTimeout. | |
| 40 | + | Timeout time.Duration | |
| 41 | + | // IsOwner returns whether the caller already owns the target main context. | |
| 42 | + | IsOwner func() bool | |
| 43 | + | // Dispatch schedules a callback on the target main context. | |
| 44 | + | Dispatch func(func()) | |
| 45 | + | // AllowLateStartAfterTimeout keeps queued work runnable when the timeout fires | |
| 46 | + | // before Dispatch starts the callback. Use this only for cleanup work where | |
| 47 | + | // canceling would leak resources; decision callbacks should leave this false. | |
| 48 | + | AllowLateStartAfterTimeout bool | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | // SyncDispatchResult is returned by RunSynchronousDispatch. | |
| 52 | + | type SyncDispatchResult struct { | |
| 53 | + | Label string | |
| 54 | + | Status SyncDispatchStatus | |
| 55 | + | Elapsed time.Duration | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // Completed reports whether the callback completed before returning. | |
| 59 | + | func (r SyncDispatchResult) Completed() bool { | |
| 60 | + | return r.Status == SyncDispatchInline || r.Status == SyncDispatchCompleted || r.Status == SyncDispatchCompletedAfterTimeout | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | // DefaultSyncDispatchTimeout is a conservative upper bound for synchronous | |
| 64 | + | // waits on a target event loop. It is long enough for normal idle dispatch but | |
| 65 | + | // short enough to avoid converting a stalled loop into an unbounded caller | |
| 66 | + | // deadlock. | |
| 67 | + | const DefaultSyncDispatchTimeout = 2 * time.Second | |
| 68 | + | ||
| 69 | + | const ( | |
| 70 | + | dispatchStateInit int32 = iota | |
| 71 | + | dispatchStateStarted | |
| 72 | + | dispatchStateCompleted | |
| 73 | + | dispatchStateTimedOut | |
| 74 | + | ) | |
| 75 | + | ||
| 76 | + | // RunSynchronousDispatch executes fn inline when the target main context is | |
| 77 | + | // already owned by the caller; otherwise it schedules fn with opts.Dispatch and | |
| 78 | + | // waits for completion up to opts.Timeout. | |
| 79 | + | // | |
| 80 | + | // If the timeout fires before the dispatched callback starts, the default | |
| 81 | + | // behavior is to skip the callback when the main loop eventually services it. If | |
| 82 | + | // AllowLateStartAfterTimeout is true, queued work is left runnable and the | |
| 83 | + | // result reports queued_after_timeout instead. If the callback has already | |
| 84 | + | // started, Go cannot preempt it safely; this helper waits for it to finish and | |
| 85 | + | // reports completed_after_timeout so callers do not fail closed while side | |
| 86 | + | // effects are still possible. | |
| 87 | + | func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchResult { | |
| 88 | + | start := time.Now() | |
| 89 | + | result := SyncDispatchResult{Label: opts.Label, Status: SyncDispatchSkipped} | |
| 90 | + | if fn == nil { | |
| 91 | + | result.Elapsed = time.Since(start) | |
| 92 | + | return result | |
| 93 | + | } | |
| 94 | + | if opts.IsOwner != nil && opts.IsOwner() { | |
| 95 | + | fn() | |
| 96 | + | result.Status = SyncDispatchInline | |
| 97 | + | result.Elapsed = time.Since(start) | |
| 98 | + | return result | |
| 99 | + | } | |
| 100 | + | if opts.Dispatch == nil { | |
| 101 | + | result.Elapsed = time.Since(start) | |
| 102 | + | return result | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | timeout := opts.Timeout | |
| 106 | + | if timeout <= 0 { | |
| 107 | + | timeout = DefaultSyncDispatchTimeout | |
| 108 | + | } | |
| 109 | + | ||
| 110 | + | deadline := start.Add(timeout) | |
| 111 | + | done := make(chan struct{}) | |
| 112 | + | state := dispatchStateInit | |
| 113 | + | var stateMu sync.Mutex | |
| 114 | + | loadState := func() int32 { | |
| 115 | + | stateMu.Lock() | |
| 116 | + | defer stateMu.Unlock() | |
| 117 | + | return state | |
| 118 | + | } | |
| 119 | + | markCallbackStarted := func() bool { | |
| 120 | + | stateMu.Lock() | |
| 121 | + | defer stateMu.Unlock() | |
| 122 | + | if state != dispatchStateInit { | |
| 123 | + | return false | |
| 124 | + | } | |
| 125 | + | // Check deadline and transition to dispatchStateStarted under one lock so | |
| 126 | + | // the timer cannot mark dispatchStateTimedOut after this callback is allowed | |
| 127 | + | // to start. Late-start cleanup opts out through AllowLateStartAfterTimeout. | |
| 128 | + | if !opts.AllowLateStartAfterTimeout && time.Now().After(deadline) { | |
| 129 | + | state = dispatchStateTimedOut | |
| 130 | + | return false | |
| 131 | + | } | |
| 132 | + | state = dispatchStateStarted | |
| 133 | + | return true | |
| 134 | + | } | |
| 135 | + | markCallbackCompleted := func() { | |
| 136 | + | stateMu.Lock() | |
| 137 | + | defer stateMu.Unlock() | |
| 138 | + | state = dispatchStateCompleted | |
| 139 | + | } | |
| 140 | + | go func() { | |
| 141 | + | opts.Dispatch(func() { | |
| 142 | + | if !markCallbackStarted() { | |
| 143 | + | close(done) | |
| 144 | + | return | |
| 145 | + | } | |
| 146 | + | defer close(done) | |
| 147 | + | fn() | |
| 148 | + | markCallbackCompleted() | |
| 149 | + | }) | |
| 150 | + | }() | |
| 151 | + | ||
| 152 | + | timer := time.NewTimer(timeout) | |
| 153 | + | defer timer.Stop() | |
| 154 | + | select { | |
| 155 | + | case <-done: | |
| 156 | + | if loadState() == dispatchStateTimedOut { | |
| 157 | + | result.Status = SyncDispatchTimedOut | |
| 158 | + | } else { | |
| 159 | + | result.Status = SyncDispatchCompleted | |
| 160 | + | } | |
| 161 | + | case <-timer.C: | |
| 162 | + | stateMu.Lock() | |
| 163 | + | currentState := state | |
| 164 | + | if opts.AllowLateStartAfterTimeout && currentState == dispatchStateInit { | |
| 165 | + | // Cleanup work may remain queued after timer expiry; decision callbacks | |
| 166 | + | // instead fall through and mark dispatchStateTimedOut while still unstarted. | |
| 167 | + | stateMu.Unlock() | |
| 168 | + | result.Status = SyncDispatchQueuedAfterTimeout | |
| 169 | + | result.Elapsed = time.Since(start) | |
| 170 | + | return result | |
| 171 | + | } | |
| 172 | + | if currentState == dispatchStateInit { | |
| 173 | + | state = dispatchStateTimedOut | |
| 174 | + | stateMu.Unlock() | |
| 175 | + | result.Status = SyncDispatchTimedOut | |
| 176 | + | result.Elapsed = time.Since(start) | |
| 177 | + | return result | |
| 178 | + | } | |
| 179 | + | stateMu.Unlock() | |
| 180 | + | // dispatchStateStarted means the worker already owns fn side effects; wait on | |
| 181 | + | // done so the caller reports completed-after-timeout only after completion. | |
| 182 | + | <-done | |
| 183 | + | if loadState() == dispatchStateTimedOut { | |
| 184 | + | result.Status = SyncDispatchTimedOut | |
| 185 | + | } else { | |
| 186 | + | result.Status = SyncDispatchCompletedAfterTimeout | |
| 187 | + | } | |
| 188 | + | } | |
| 189 | + | result.Elapsed = time.Since(start) | |
| 190 | + | return result | |
| 191 | + | } | |
| @@ -0,0 +1,191 @@ | |||||
| 1 | + | package syncdispatch | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync" | |||
| 5 | + | "time" | |||
| 6 | + | ) | |||
| 7 | + | ||||
| 8 | + | // SyncDispatchStatus describes how a synchronous event-loop dispatch finished. | |||
| 9 | + | type SyncDispatchStatus string | |||
| 10 | + | ||||
| 11 | + | const ( | |||
| 12 | + | // SyncDispatchInline means the callback ran immediately because the caller | |||
| 13 | + | // already owned the target main context. | |||
| 14 | + | SyncDispatchInline SyncDispatchStatus = "inline" | |||
| 15 | + | // SyncDispatchCompleted means the callback was dispatched and completed before | |||
| 16 | + | // the configured timeout elapsed. | |||
| 17 | + | SyncDispatchCompleted SyncDispatchStatus = "completed" | |||
| 18 | + | // SyncDispatchTimedOut means the callback did not start before the timeout and | |||
| 19 | + | // was canceled before running side effects. | |||
| 20 | + | SyncDispatchTimedOut SyncDispatchStatus = "timed_out" | |||
| 21 | + | // SyncDispatchCompletedAfterTimeout means the callback started before the | |||
| 22 | + | // timeout and completed after the timeout elapsed. Callers must treat it as | |||
| 23 | + | // completed because side effects have run. | |||
| 24 | + | SyncDispatchCompletedAfterTimeout SyncDispatchStatus = "completed_after_timeout" | |||
| 25 | + | // SyncDispatchQueuedAfterTimeout means the callback did not start before the | |||
| 26 | + | // timeout, but the dispatch was intentionally left queued so it may run later. | |||
| 27 | + | SyncDispatchQueuedAfterTimeout SyncDispatchStatus = "queued_after_timeout" | |||
| 28 | + | // SyncDispatchSkipped means required inputs were missing, so no callback ran. | |||
| 29 | + | SyncDispatchSkipped SyncDispatchStatus = "skipped" | |||
| 30 | + | ) | |||
| 31 | + | ||||
| 32 | + | // SyncDispatchOptions configures RunSynchronousDispatch. | |||
| 33 | + | type SyncDispatchOptions struct { | |||
| 34 | + | // Label identifies the caller in diagnostics. The helper does not log by | |||
| 35 | + | // itself, but callers can include this label when reporting the returned | |||
| 36 | + | // result. | |||
| 37 | + | Label string | |||
| 38 | + | // Timeout bounds how long the caller waits for dispatched work. Non-positive | |||
| 39 | + | // durations fall back to DefaultSyncDispatchTimeout. | |||
| 40 | + | Timeout time.Duration | |||
| 41 | + | // IsOwner returns whether the caller already owns the target main context. | |||
| 42 | + | IsOwner func() bool | |||
| 43 | + | // Dispatch schedules a callback on the target main context. | |||
| 44 | + | Dispatch func(func()) | |||
| 45 | + | // AllowLateStartAfterTimeout keeps queued work runnable when the timeout fires | |||
| 46 | + | // before Dispatch starts the callback. Use this only for cleanup work where | |||
| 47 | + | // canceling would leak resources; decision callbacks should leave this false. | |||
| 48 | + | AllowLateStartAfterTimeout bool | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | // SyncDispatchResult is returned by RunSynchronousDispatch. | |||
| 52 | + | type SyncDispatchResult struct { | |||
| 53 | + | Label string | |||
| 54 | + | Status SyncDispatchStatus | |||
| 55 | + | Elapsed time.Duration | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | // Completed reports whether the callback completed before returning. | |||
| 59 | + | func (r SyncDispatchResult) Completed() bool { | |||
| 60 | + | return r.Status == SyncDispatchInline || r.Status == SyncDispatchCompleted || r.Status == SyncDispatchCompletedAfterTimeout | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | // DefaultSyncDispatchTimeout is a conservative upper bound for synchronous | |||
| 64 | + | // waits on a target event loop. It is long enough for normal idle dispatch but | |||
| 65 | + | // short enough to avoid converting a stalled loop into an unbounded caller | |||
| 66 | + | // deadlock. | |||
| 67 | + | const DefaultSyncDispatchTimeout = 2 * time.Second | |||
| 68 | + | ||||
| 69 | + | const ( | |||
| 70 | + | dispatchStateInit int32 = iota | |||
| 71 | + | dispatchStateStarted | |||
| 72 | + | dispatchStateCompleted | |||
| 73 | + | dispatchStateTimedOut | |||
| 74 | + | ) | |||
| 75 | + | ||||
| 76 | + | // RunSynchronousDispatch executes fn inline when the target main context is | |||
| 77 | + | // already owned by the caller; otherwise it schedules fn with opts.Dispatch and | |||
| 78 | + | // waits for completion up to opts.Timeout. | |||
| 79 | + | // | |||
| 80 | + | // If the timeout fires before the dispatched callback starts, the default | |||
| 81 | + | // behavior is to skip the callback when the main loop eventually services it. If | |||
| 82 | + | // AllowLateStartAfterTimeout is true, queued work is left runnable and the | |||
| 83 | + | // result reports queued_after_timeout instead. If the callback has already | |||
| 84 | + | // started, Go cannot preempt it safely; this helper waits for it to finish and | |||
| 85 | + | // reports completed_after_timeout so callers do not fail closed while side | |||
| 86 | + | // effects are still possible. | |||
| 87 | + | func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchResult { | |||
| 88 | + | start := time.Now() | |||
| 89 | + | result := SyncDispatchResult{Label: opts.Label, Status: SyncDispatchSkipped} | |||
| 90 | + | if fn == nil { | |||
| 91 | + | result.Elapsed = time.Since(start) | |||
| 92 | + | return result | |||
| 93 | + | } | |||
| 94 | + | if opts.IsOwner != nil && opts.IsOwner() { | |||
| 95 | + | fn() | |||
| 96 | + | result.Status = SyncDispatchInline | |||
| 97 | + | result.Elapsed = time.Since(start) | |||
| 98 | + | return result | |||
| 99 | + | } | |||
| 100 | + | if opts.Dispatch == nil { | |||
| 101 | + | result.Elapsed = time.Since(start) | |||
| 102 | + | return result | |||
| 103 | + | } | |||
| 104 | + | ||||
| 105 | + | timeout := opts.Timeout | |||
| 106 | + | if timeout <= 0 { | |||
| 107 | + | timeout = DefaultSyncDispatchTimeout | |||
| 108 | + | } | |||
| 109 | + | ||||
| 110 | + | deadline := start.Add(timeout) | |||
| 111 | + | done := make(chan struct{}) | |||
| 112 | + | state := dispatchStateInit | |||
| 113 | + | var stateMu sync.Mutex | |||
| 114 | + | loadState := func() int32 { | |||
| 115 | + | stateMu.Lock() | |||
| 116 | + | defer stateMu.Unlock() | |||
| 117 | + | return state | |||
| 118 | + | } | |||
| 119 | + | markCallbackStarted := func() bool { | |||
| 120 | + | stateMu.Lock() | |||
| 121 | + | defer stateMu.Unlock() | |||
| 122 | + | if state != dispatchStateInit { | |||
| 123 | + | return false | |||
| 124 | + | } | |||
| 125 | + | // Check deadline and transition to dispatchStateStarted under one lock so | |||
| 126 | + | // the timer cannot mark dispatchStateTimedOut after this callback is allowed | |||
| 127 | + | // to start. Late-start cleanup opts out through AllowLateStartAfterTimeout. | |||
| 128 | + | if !opts.AllowLateStartAfterTimeout && time.Now().After(deadline) { | |||
| 129 | + | state = dispatchStateTimedOut | |||
| 130 | + | return false | |||
| 131 | + | } | |||
| 132 | + | state = dispatchStateStarted | |||
| 133 | + | return true | |||
| 134 | + | } | |||
| 135 | + | markCallbackCompleted := func() { | |||
| 136 | + | stateMu.Lock() | |||
| 137 | + | defer stateMu.Unlock() | |||
| 138 | + | state = dispatchStateCompleted | |||
| 139 | + | } | |||
| 140 | + | go func() { | |||
| 141 | + | opts.Dispatch(func() { | |||
| 142 | + | if !markCallbackStarted() { | |||
| 143 | + | close(done) | |||
| 144 | + | return | |||
| 145 | + | } | |||
| 146 | + | defer close(done) | |||
| 147 | + | fn() | |||
| 148 | + | markCallbackCompleted() | |||
| 149 | + | }) | |||
| 150 | + | }() | |||
| 151 | + | ||||
| 152 | + | timer := time.NewTimer(timeout) | |||
| 153 | + | defer timer.Stop() | |||
| 154 | + | select { | |||
| 155 | + | case <-done: | |||
| 156 | + | if loadState() == dispatchStateTimedOut { | |||
| 157 | + | result.Status = SyncDispatchTimedOut | |||
| 158 | + | } else { | |||
| 159 | + | result.Status = SyncDispatchCompleted | |||
| 160 | + | } | |||
| 161 | + | case <-timer.C: | |||
| 162 | + | stateMu.Lock() | |||
| 163 | + | currentState := state | |||
| 164 | + | if opts.AllowLateStartAfterTimeout && currentState == dispatchStateInit { | |||
| 165 | + | // Cleanup work may remain queued after timer expiry; decision callbacks | |||
| 166 | + | // instead fall through and mark dispatchStateTimedOut while still unstarted. | |||
| 167 | + | stateMu.Unlock() | |||
| 168 | + | result.Status = SyncDispatchQueuedAfterTimeout | |||
| 169 | + | result.Elapsed = time.Since(start) | |||
| 170 | + | return result | |||
| 171 | + | } | |||
| 172 | + | if currentState == dispatchStateInit { | |||
| 173 | + | state = dispatchStateTimedOut | |||
| 174 | + | stateMu.Unlock() | |||
| 175 | + | result.Status = SyncDispatchTimedOut | |||
| 176 | + | result.Elapsed = time.Since(start) | |||
| 177 | + | return result | |||
| 178 | + | } | |||
| 179 | + | stateMu.Unlock() | |||
| 180 | + | // dispatchStateStarted means the worker already owns fn side effects; wait on | |||
| 181 | + | // done so the caller reports completed-after-timeout only after completion. | |||
| 182 | + | <-done | |||
| 183 | + | if loadState() == dispatchStateTimedOut { | |||
| 184 | + | result.Status = SyncDispatchTimedOut | |||
| 185 | + | } else { | |||
| 186 | + | result.Status = SyncDispatchCompletedAfterTimeout | |||
| 187 | + | } | |||
| 188 | + | } | |||
| 189 | + | result.Elapsed = time.Since(start) | |||
| 190 | + | return result | |||
| 191 | + | } | |||
A internal/shared/syncdispatch/sync_dispatch_test.go +294 -0
| @@ -0,0 +1,294 @@ | |||
| 1 | + | package syncdispatch | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync/atomic" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestRunSynchronousDispatchRunsInlineWhenOwner(t *testing.T) { | |
| 10 | + | var dispatched atomic.Bool | |
| 11 | + | var ran atomic.Bool | |
| 12 | + | ||
| 13 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 14 | + | Label: "test.inline", | |
| 15 | + | Timeout: time.Second, | |
| 16 | + | IsOwner: func() bool { return true }, | |
| 17 | + | Dispatch: func(func()) { | |
| 18 | + | dispatched.Store(true) | |
| 19 | + | }, | |
| 20 | + | }, func() { | |
| 21 | + | ran.Store(true) | |
| 22 | + | }) | |
| 23 | + | ||
| 24 | + | if result.Status != SyncDispatchInline { | |
| 25 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchInline) | |
| 26 | + | } | |
| 27 | + | if !ran.Load() { | |
| 28 | + | t.Fatal("callback did not run inline") | |
| 29 | + | } | |
| 30 | + | if dispatched.Load() { | |
| 31 | + | t.Fatal("callback was dispatched despite owner context") | |
| 32 | + | } | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | func TestRunSynchronousDispatchCompletesWhenDispatchedCallbackRuns(t *testing.T) { | |
| 36 | + | var ran atomic.Bool | |
| 37 | + | ||
| 38 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 39 | + | Label: "test.dispatch", | |
| 40 | + | Timeout: time.Second, | |
| 41 | + | IsOwner: func() bool { return false }, | |
| 42 | + | Dispatch: func(fn func()) { | |
| 43 | + | go fn() | |
| 44 | + | }, | |
| 45 | + | }, func() { | |
| 46 | + | ran.Store(true) | |
| 47 | + | }) | |
| 48 | + | ||
| 49 | + | if result.Status != SyncDispatchCompleted { | |
| 50 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchCompleted) | |
| 51 | + | } | |
| 52 | + | if !result.Completed() { | |
| 53 | + | t.Fatal("result.Completed() = false, want true") | |
| 54 | + | } | |
| 55 | + | if !ran.Load() { | |
| 56 | + | t.Fatal("callback did not run") | |
| 57 | + | } | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | func TestRunSynchronousDispatchTimesOutWhenDispatchBlocks(t *testing.T) { | |
| 61 | + | releaseDispatch := make(chan struct{}) | |
| 62 | + | var ran atomic.Bool | |
| 63 | + | ||
| 64 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 65 | + | Label: "test.blocking_dispatch", | |
| 66 | + | Timeout: 5 * time.Millisecond, | |
| 67 | + | IsOwner: func() bool { return false }, | |
| 68 | + | Dispatch: func(fn func()) { | |
| 69 | + | <-releaseDispatch | |
| 70 | + | fn() | |
| 71 | + | }, | |
| 72 | + | }, func() { | |
| 73 | + | ran.Store(true) | |
| 74 | + | }) | |
| 75 | + | ||
| 76 | + | if result.Status != SyncDispatchTimedOut { | |
| 77 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |
| 78 | + | } | |
| 79 | + | if ran.Load() { | |
| 80 | + | t.Fatal("callback ran while dispatch was blocked") | |
| 81 | + | } | |
| 82 | + | close(releaseDispatch) | |
| 83 | + | time.Sleep(20 * time.Millisecond) | |
| 84 | + | if ran.Load() { | |
| 85 | + | t.Fatal("callback ran after a blocking dispatch timed out") | |
| 86 | + | } | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func TestRunSynchronousDispatchTimesOutWhenCallbackNeverStarts(t *testing.T) { | |
| 90 | + | var ran atomic.Bool | |
| 91 | + | ||
| 92 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 93 | + | Label: "test.timeout", | |
| 94 | + | Timeout: 5 * time.Millisecond, | |
| 95 | + | IsOwner: func() bool { return false }, | |
| 96 | + | Dispatch: func(func()) { | |
| 97 | + | // Simulate an event loop that never services the queued callback. | |
| 98 | + | }, | |
| 99 | + | }, func() { | |
| 100 | + | ran.Store(true) | |
| 101 | + | }) | |
| 102 | + | ||
| 103 | + | if result.Status != SyncDispatchTimedOut { | |
| 104 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |
| 105 | + | } | |
| 106 | + | if result.Completed() { | |
| 107 | + | t.Fatal("result.Completed() = true, want false") | |
| 108 | + | } | |
| 109 | + | if ran.Load() { | |
| 110 | + | t.Fatal("callback ran despite timeout before dispatch start") | |
| 111 | + | } | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | func TestRunSynchronousDispatchWaitsForStartedCallbackPastTimeout(t *testing.T) { | |
| 115 | + | started := make(chan struct{}) | |
| 116 | + | release := make(chan struct{}) | |
| 117 | + | resultCh := make(chan SyncDispatchResult, 1) | |
| 118 | + | var ran atomic.Bool | |
| 119 | + | ||
| 120 | + | go func() { | |
| 121 | + | resultCh <- RunSynchronousDispatch(SyncDispatchOptions{ | |
| 122 | + | Label: "test.started_late_finish", | |
| 123 | + | Timeout: 50 * time.Millisecond, | |
| 124 | + | IsOwner: func() bool { return false }, | |
| 125 | + | Dispatch: func(fn func()) { | |
| 126 | + | go fn() | |
| 127 | + | }, | |
| 128 | + | }, func() { | |
| 129 | + | close(started) | |
| 130 | + | <-release | |
| 131 | + | ran.Store(true) | |
| 132 | + | }) | |
| 133 | + | }() | |
| 134 | + | ||
| 135 | + | select { | |
| 136 | + | case <-started: | |
| 137 | + | case <-time.After(time.Second): | |
| 138 | + | t.Fatal("callback did not start") | |
| 139 | + | } | |
| 140 | + | time.Sleep(100 * time.Millisecond) | |
| 141 | + | if ran.Load() { | |
| 142 | + | t.Fatal("callback ran before release") | |
| 143 | + | } | |
| 144 | + | close(release) | |
| 145 | + | ||
| 146 | + | select { | |
| 147 | + | case result := <-resultCh: | |
| 148 | + | if result.Status != SyncDispatchCompletedAfterTimeout { | |
| 149 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchCompletedAfterTimeout) | |
| 150 | + | } | |
| 151 | + | if !result.Completed() { | |
| 152 | + | t.Fatal("result.Completed() = false, want true") | |
| 153 | + | } | |
| 154 | + | case <-time.After(time.Second): | |
| 155 | + | t.Fatal("RunSynchronousDispatch did not return after started callback completed") | |
| 156 | + | } | |
| 157 | + | if !ran.Load() { | |
| 158 | + | t.Fatal("callback did not run") | |
| 159 | + | } | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | func TestRunSynchronousDispatchCancelsCallbackThatAttemptsToStartAfterDeadline(t *testing.T) { | |
| 163 | + | callbackAttempted := make(chan struct{}, 1) | |
| 164 | + | var ran atomic.Bool | |
| 165 | + | ||
| 166 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 167 | + | Label: "test.after_deadline", | |
| 168 | + | Timeout: 5 * time.Millisecond, | |
| 169 | + | IsOwner: func() bool { return false }, | |
| 170 | + | Dispatch: func(fn func()) { | |
| 171 | + | go func() { | |
| 172 | + | time.Sleep(20 * time.Millisecond) | |
| 173 | + | fn() | |
| 174 | + | callbackAttempted <- struct{}{} | |
| 175 | + | }() | |
| 176 | + | }, | |
| 177 | + | }, func() { | |
| 178 | + | ran.Store(true) | |
| 179 | + | }) | |
| 180 | + | ||
| 181 | + | if result.Status != SyncDispatchTimedOut { | |
| 182 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |
| 183 | + | } | |
| 184 | + | select { | |
| 185 | + | case <-callbackAttempted: | |
| 186 | + | case <-time.After(time.Second): | |
| 187 | + | t.Fatal("callback did not attempt late start") | |
| 188 | + | } | |
| 189 | + | if ran.Load() { | |
| 190 | + | t.Fatal("callback ran after deadline") | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | ||
| 194 | + | func TestRunSynchronousDispatchLeavesCallbackQueuedWhenBlockingDispatchTimesOutAndLateStartAllowed(t *testing.T) { | |
| 195 | + | releaseDispatch := make(chan struct{}) | |
| 196 | + | var ran atomic.Bool | |
| 197 | + | ||
| 198 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 199 | + | Label: "test.blocking_cleanup_dispatch", | |
| 200 | + | Timeout: 5 * time.Millisecond, | |
| 201 | + | IsOwner: func() bool { return false }, | |
| 202 | + | AllowLateStartAfterTimeout: true, | |
| 203 | + | Dispatch: func(fn func()) { | |
| 204 | + | <-releaseDispatch | |
| 205 | + | fn() | |
| 206 | + | }, | |
| 207 | + | }, func() { | |
| 208 | + | ran.Store(true) | |
| 209 | + | }) | |
| 210 | + | ||
| 211 | + | if result.Status != SyncDispatchQueuedAfterTimeout { | |
| 212 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchQueuedAfterTimeout) | |
| 213 | + | } | |
| 214 | + | if ran.Load() { | |
| 215 | + | t.Fatal("callback ran before dispatch was released") | |
| 216 | + | } | |
| 217 | + | close(releaseDispatch) | |
| 218 | + | deadline := time.Now().Add(time.Second) | |
| 219 | + | for !ran.Load() && time.Now().Before(deadline) { | |
| 220 | + | time.Sleep(time.Millisecond) | |
| 221 | + | } | |
| 222 | + | if !ran.Load() { | |
| 223 | + | t.Fatal("queued cleanup callback did not run") | |
| 224 | + | } | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | func TestRunSynchronousDispatchLeavesCallbackQueuedAfterTimeoutWhenConfigured(t *testing.T) { | |
| 228 | + | delayed := make(chan func(), 1) | |
| 229 | + | var ran atomic.Bool | |
| 230 | + | ||
| 231 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 232 | + | Label: "test.late_cleanup", | |
| 233 | + | Timeout: 5 * time.Millisecond, | |
| 234 | + | IsOwner: func() bool { return false }, | |
| 235 | + | AllowLateStartAfterTimeout: true, | |
| 236 | + | Dispatch: func(fn func()) { | |
| 237 | + | delayed <- fn | |
| 238 | + | }, | |
| 239 | + | }, func() { | |
| 240 | + | ran.Store(true) | |
| 241 | + | }) | |
| 242 | + | ||
| 243 | + | if result.Status != SyncDispatchQueuedAfterTimeout { | |
| 244 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchQueuedAfterTimeout) | |
| 245 | + | } | |
| 246 | + | if result.Completed() { | |
| 247 | + | t.Fatal("result.Completed() = true, want false") | |
| 248 | + | } | |
| 249 | + | if ran.Load() { | |
| 250 | + | t.Fatal("callback ran before late dispatch") | |
| 251 | + | } | |
| 252 | + | ||
| 253 | + | select { | |
| 254 | + | case fn := <-delayed: | |
| 255 | + | fn() | |
| 256 | + | case <-time.After(time.Second): | |
| 257 | + | t.Fatal("dispatch callback was not captured") | |
| 258 | + | } | |
| 259 | + | ||
| 260 | + | if !ran.Load() { | |
| 261 | + | t.Fatal("queued callback did not run after timeout") | |
| 262 | + | } | |
| 263 | + | } | |
| 264 | + | ||
| 265 | + | func TestRunSynchronousDispatchCancelsCallbackThatStartsAfterTimeout(t *testing.T) { | |
| 266 | + | delayed := make(chan func(), 1) | |
| 267 | + | var ran atomic.Bool | |
| 268 | + | ||
| 269 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |
| 270 | + | Label: "test.late", | |
| 271 | + | Timeout: 5 * time.Millisecond, | |
| 272 | + | IsOwner: func() bool { return false }, | |
| 273 | + | Dispatch: func(fn func()) { | |
| 274 | + | delayed <- fn | |
| 275 | + | }, | |
| 276 | + | }, func() { | |
| 277 | + | ran.Store(true) | |
| 278 | + | }) | |
| 279 | + | ||
| 280 | + | if result.Status != SyncDispatchTimedOut { | |
| 281 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |
| 282 | + | } | |
| 283 | + | ||
| 284 | + | select { | |
| 285 | + | case fn := <-delayed: | |
| 286 | + | fn() | |
| 287 | + | case <-time.After(time.Second): | |
| 288 | + | t.Fatal("dispatch callback was not captured") | |
| 289 | + | } | |
| 290 | + | ||
| 291 | + | if ran.Load() { | |
| 292 | + | t.Fatal("late callback ran after timeout") | |
| 293 | + | } | |
| 294 | + | } | |
| @@ -0,0 +1,294 @@ | |||||
| 1 | + | package syncdispatch | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync/atomic" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestRunSynchronousDispatchRunsInlineWhenOwner(t *testing.T) { | |||
| 10 | + | var dispatched atomic.Bool | |||
| 11 | + | var ran atomic.Bool | |||
| 12 | + | ||||
| 13 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 14 | + | Label: "test.inline", | |||
| 15 | + | Timeout: time.Second, | |||
| 16 | + | IsOwner: func() bool { return true }, | |||
| 17 | + | Dispatch: func(func()) { | |||
| 18 | + | dispatched.Store(true) | |||
| 19 | + | }, | |||
| 20 | + | }, func() { | |||
| 21 | + | ran.Store(true) | |||
| 22 | + | }) | |||
| 23 | + | ||||
| 24 | + | if result.Status != SyncDispatchInline { | |||
| 25 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchInline) | |||
| 26 | + | } | |||
| 27 | + | if !ran.Load() { | |||
| 28 | + | t.Fatal("callback did not run inline") | |||
| 29 | + | } | |||
| 30 | + | if dispatched.Load() { | |||
| 31 | + | t.Fatal("callback was dispatched despite owner context") | |||
| 32 | + | } | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | func TestRunSynchronousDispatchCompletesWhenDispatchedCallbackRuns(t *testing.T) { | |||
| 36 | + | var ran atomic.Bool | |||
| 37 | + | ||||
| 38 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 39 | + | Label: "test.dispatch", | |||
| 40 | + | Timeout: time.Second, | |||
| 41 | + | IsOwner: func() bool { return false }, | |||
| 42 | + | Dispatch: func(fn func()) { | |||
| 43 | + | go fn() | |||
| 44 | + | }, | |||
| 45 | + | }, func() { | |||
| 46 | + | ran.Store(true) | |||
| 47 | + | }) | |||
| 48 | + | ||||
| 49 | + | if result.Status != SyncDispatchCompleted { | |||
| 50 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchCompleted) | |||
| 51 | + | } | |||
| 52 | + | if !result.Completed() { | |||
| 53 | + | t.Fatal("result.Completed() = false, want true") | |||
| 54 | + | } | |||
| 55 | + | if !ran.Load() { | |||
| 56 | + | t.Fatal("callback did not run") | |||
| 57 | + | } | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | func TestRunSynchronousDispatchTimesOutWhenDispatchBlocks(t *testing.T) { | |||
| 61 | + | releaseDispatch := make(chan struct{}) | |||
| 62 | + | var ran atomic.Bool | |||
| 63 | + | ||||
| 64 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 65 | + | Label: "test.blocking_dispatch", | |||
| 66 | + | Timeout: 5 * time.Millisecond, | |||
| 67 | + | IsOwner: func() bool { return false }, | |||
| 68 | + | Dispatch: func(fn func()) { | |||
| 69 | + | <-releaseDispatch | |||
| 70 | + | fn() | |||
| 71 | + | }, | |||
| 72 | + | }, func() { | |||
| 73 | + | ran.Store(true) | |||
| 74 | + | }) | |||
| 75 | + | ||||
| 76 | + | if result.Status != SyncDispatchTimedOut { | |||
| 77 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |||
| 78 | + | } | |||
| 79 | + | if ran.Load() { | |||
| 80 | + | t.Fatal("callback ran while dispatch was blocked") | |||
| 81 | + | } | |||
| 82 | + | close(releaseDispatch) | |||
| 83 | + | time.Sleep(20 * time.Millisecond) | |||
| 84 | + | if ran.Load() { | |||
| 85 | + | t.Fatal("callback ran after a blocking dispatch timed out") | |||
| 86 | + | } | |||
| 87 | + | } | |||
| 88 | + | ||||
| 89 | + | func TestRunSynchronousDispatchTimesOutWhenCallbackNeverStarts(t *testing.T) { | |||
| 90 | + | var ran atomic.Bool | |||
| 91 | + | ||||
| 92 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 93 | + | Label: "test.timeout", | |||
| 94 | + | Timeout: 5 * time.Millisecond, | |||
| 95 | + | IsOwner: func() bool { return false }, | |||
| 96 | + | Dispatch: func(func()) { | |||
| 97 | + | // Simulate an event loop that never services the queued callback. | |||
| 98 | + | }, | |||
| 99 | + | }, func() { | |||
| 100 | + | ran.Store(true) | |||
| 101 | + | }) | |||
| 102 | + | ||||
| 103 | + | if result.Status != SyncDispatchTimedOut { | |||
| 104 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |||
| 105 | + | } | |||
| 106 | + | if result.Completed() { | |||
| 107 | + | t.Fatal("result.Completed() = true, want false") | |||
| 108 | + | } | |||
| 109 | + | if ran.Load() { | |||
| 110 | + | t.Fatal("callback ran despite timeout before dispatch start") | |||
| 111 | + | } | |||
| 112 | + | } | |||
| 113 | + | ||||
| 114 | + | func TestRunSynchronousDispatchWaitsForStartedCallbackPastTimeout(t *testing.T) { | |||
| 115 | + | started := make(chan struct{}) | |||
| 116 | + | release := make(chan struct{}) | |||
| 117 | + | resultCh := make(chan SyncDispatchResult, 1) | |||
| 118 | + | var ran atomic.Bool | |||
| 119 | + | ||||
| 120 | + | go func() { | |||
| 121 | + | resultCh <- RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 122 | + | Label: "test.started_late_finish", | |||
| 123 | + | Timeout: 50 * time.Millisecond, | |||
| 124 | + | IsOwner: func() bool { return false }, | |||
| 125 | + | Dispatch: func(fn func()) { | |||
| 126 | + | go fn() | |||
| 127 | + | }, | |||
| 128 | + | }, func() { | |||
| 129 | + | close(started) | |||
| 130 | + | <-release | |||
| 131 | + | ran.Store(true) | |||
| 132 | + | }) | |||
| 133 | + | }() | |||
| 134 | + | ||||
| 135 | + | select { | |||
| 136 | + | case <-started: | |||
| 137 | + | case <-time.After(time.Second): | |||
| 138 | + | t.Fatal("callback did not start") | |||
| 139 | + | } | |||
| 140 | + | time.Sleep(100 * time.Millisecond) | |||
| 141 | + | if ran.Load() { | |||
| 142 | + | t.Fatal("callback ran before release") | |||
| 143 | + | } | |||
| 144 | + | close(release) | |||
| 145 | + | ||||
| 146 | + | select { | |||
| 147 | + | case result := <-resultCh: | |||
| 148 | + | if result.Status != SyncDispatchCompletedAfterTimeout { | |||
| 149 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchCompletedAfterTimeout) | |||
| 150 | + | } | |||
| 151 | + | if !result.Completed() { | |||
| 152 | + | t.Fatal("result.Completed() = false, want true") | |||
| 153 | + | } | |||
| 154 | + | case <-time.After(time.Second): | |||
| 155 | + | t.Fatal("RunSynchronousDispatch did not return after started callback completed") | |||
| 156 | + | } | |||
| 157 | + | if !ran.Load() { | |||
| 158 | + | t.Fatal("callback did not run") | |||
| 159 | + | } | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | func TestRunSynchronousDispatchCancelsCallbackThatAttemptsToStartAfterDeadline(t *testing.T) { | |||
| 163 | + | callbackAttempted := make(chan struct{}, 1) | |||
| 164 | + | var ran atomic.Bool | |||
| 165 | + | ||||
| 166 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 167 | + | Label: "test.after_deadline", | |||
| 168 | + | Timeout: 5 * time.Millisecond, | |||
| 169 | + | IsOwner: func() bool { return false }, | |||
| 170 | + | Dispatch: func(fn func()) { | |||
| 171 | + | go func() { | |||
| 172 | + | time.Sleep(20 * time.Millisecond) | |||
| 173 | + | fn() | |||
| 174 | + | callbackAttempted <- struct{}{} | |||
| 175 | + | }() | |||
| 176 | + | }, | |||
| 177 | + | }, func() { | |||
| 178 | + | ran.Store(true) | |||
| 179 | + | }) | |||
| 180 | + | ||||
| 181 | + | if result.Status != SyncDispatchTimedOut { | |||
| 182 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |||
| 183 | + | } | |||
| 184 | + | select { | |||
| 185 | + | case <-callbackAttempted: | |||
| 186 | + | case <-time.After(time.Second): | |||
| 187 | + | t.Fatal("callback did not attempt late start") | |||
| 188 | + | } | |||
| 189 | + | if ran.Load() { | |||
| 190 | + | t.Fatal("callback ran after deadline") | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | ||||
| 194 | + | func TestRunSynchronousDispatchLeavesCallbackQueuedWhenBlockingDispatchTimesOutAndLateStartAllowed(t *testing.T) { | |||
| 195 | + | releaseDispatch := make(chan struct{}) | |||
| 196 | + | var ran atomic.Bool | |||
| 197 | + | ||||
| 198 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 199 | + | Label: "test.blocking_cleanup_dispatch", | |||
| 200 | + | Timeout: 5 * time.Millisecond, | |||
| 201 | + | IsOwner: func() bool { return false }, | |||
| 202 | + | AllowLateStartAfterTimeout: true, | |||
| 203 | + | Dispatch: func(fn func()) { | |||
| 204 | + | <-releaseDispatch | |||
| 205 | + | fn() | |||
| 206 | + | }, | |||
| 207 | + | }, func() { | |||
| 208 | + | ran.Store(true) | |||
| 209 | + | }) | |||
| 210 | + | ||||
| 211 | + | if result.Status != SyncDispatchQueuedAfterTimeout { | |||
| 212 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchQueuedAfterTimeout) | |||
| 213 | + | } | |||
| 214 | + | if ran.Load() { | |||
| 215 | + | t.Fatal("callback ran before dispatch was released") | |||
| 216 | + | } | |||
| 217 | + | close(releaseDispatch) | |||
| 218 | + | deadline := time.Now().Add(time.Second) | |||
| 219 | + | for !ran.Load() && time.Now().Before(deadline) { | |||
| 220 | + | time.Sleep(time.Millisecond) | |||
| 221 | + | } | |||
| 222 | + | if !ran.Load() { | |||
| 223 | + | t.Fatal("queued cleanup callback did not run") | |||
| 224 | + | } | |||
| 225 | + | } | |||
| 226 | + | ||||
| 227 | + | func TestRunSynchronousDispatchLeavesCallbackQueuedAfterTimeoutWhenConfigured(t *testing.T) { | |||
| 228 | + | delayed := make(chan func(), 1) | |||
| 229 | + | var ran atomic.Bool | |||
| 230 | + | ||||
| 231 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 232 | + | Label: "test.late_cleanup", | |||
| 233 | + | Timeout: 5 * time.Millisecond, | |||
| 234 | + | IsOwner: func() bool { return false }, | |||
| 235 | + | AllowLateStartAfterTimeout: true, | |||
| 236 | + | Dispatch: func(fn func()) { | |||
| 237 | + | delayed <- fn | |||
| 238 | + | }, | |||
| 239 | + | }, func() { | |||
| 240 | + | ran.Store(true) | |||
| 241 | + | }) | |||
| 242 | + | ||||
| 243 | + | if result.Status != SyncDispatchQueuedAfterTimeout { | |||
| 244 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchQueuedAfterTimeout) | |||
| 245 | + | } | |||
| 246 | + | if result.Completed() { | |||
| 247 | + | t.Fatal("result.Completed() = true, want false") | |||
| 248 | + | } | |||
| 249 | + | if ran.Load() { | |||
| 250 | + | t.Fatal("callback ran before late dispatch") | |||
| 251 | + | } | |||
| 252 | + | ||||
| 253 | + | select { | |||
| 254 | + | case fn := <-delayed: | |||
| 255 | + | fn() | |||
| 256 | + | case <-time.After(time.Second): | |||
| 257 | + | t.Fatal("dispatch callback was not captured") | |||
| 258 | + | } | |||
| 259 | + | ||||
| 260 | + | if !ran.Load() { | |||
| 261 | + | t.Fatal("queued callback did not run after timeout") | |||
| 262 | + | } | |||
| 263 | + | } | |||
| 264 | + | ||||
| 265 | + | func TestRunSynchronousDispatchCancelsCallbackThatStartsAfterTimeout(t *testing.T) { | |||
| 266 | + | delayed := make(chan func(), 1) | |||
| 267 | + | var ran atomic.Bool | |||
| 268 | + | ||||
| 269 | + | result := RunSynchronousDispatch(SyncDispatchOptions{ | |||
| 270 | + | Label: "test.late", | |||
| 271 | + | Timeout: 5 * time.Millisecond, | |||
| 272 | + | IsOwner: func() bool { return false }, | |||
| 273 | + | Dispatch: func(fn func()) { | |||
| 274 | + | delayed <- fn | |||
| 275 | + | }, | |||
| 276 | + | }, func() { | |||
| 277 | + | ran.Store(true) | |||
| 278 | + | }) | |||
| 279 | + | ||||
| 280 | + | if result.Status != SyncDispatchTimedOut { | |||
| 281 | + | t.Fatalf("status = %s, want %s", result.Status, SyncDispatchTimedOut) | |||
| 282 | + | } | |||
| 283 | + | ||||
| 284 | + | select { | |||
| 285 | + | case fn := <-delayed: | |||
| 286 | + | fn() | |||
| 287 | + | case <-time.After(time.Second): | |||
| 288 | + | t.Fatal("dispatch callback was not captured") | |||
| 289 | + | } | |||
| 290 | + | ||||
| 291 | + | if ran.Load() { | |||
| 292 | + | t.Fatal("late callback ran after timeout") | |||
| 293 | + | } | |||
| 294 | + | } | |||
M internal/ui/app.go +171 -84
| @@ -10,12 +10,14 @@ import ( | |||
| 10 | 10 | "sort" | |
| 11 | 11 | "strings" | |
| 12 | 12 | "sync" | |
| 13 | + | "time" | |
| 13 | 14 | "unsafe" | |
| 14 | 15 | ||
| 15 | 16 | "github.com/bnema/dumber/internal/application/port" | |
| 16 | 17 | "github.com/bnema/dumber/internal/application/usecase" | |
| 17 | 18 | "github.com/bnema/dumber/internal/domain/entity" | |
| 18 | 19 | urlutil "github.com/bnema/dumber/internal/domain/url" | |
| 20 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 19 | 21 | ||
| 20 | 22 | "github.com/bnema/dumber/internal/logging" | |
| 21 | 23 | "github.com/bnema/dumber/internal/ui/adapter" | |
| @@ -81,7 +83,7 @@ type App struct { | |||
| 81 | 83 | lastFocusedWindowID string | |
| 82 | 84 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | |
| 83 | 85 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | |
| 84 | - | dispatchOnMainThread func(func()) | |
| 86 | + | dispatchOnMainThread func(string, func()) syncdispatch.SyncDispatchResult | |
| 85 | 87 | ||
| 86 | 88 | // State | |
| 87 | 89 | tabs *entity.TabList | |
| @@ -179,17 +181,22 @@ func New(deps *Dependencies) (*App, error) { | |||
| 179 | 181 | ctx, cancel := context.WithCancelCause(deps.Ctx) | |
| 180 | 182 | ||
| 181 | 183 | app := &App{ | |
| 182 | - | deps: deps, | |
| 183 | - | tabs: entity.NewTabList(), | |
| 184 | - | tabsUC: deps.TabsUC, | |
| 185 | - | panesUC: deps.PanesUC, | |
| 186 | - | workspaceViews: make(map[entity.TabID]*component.WorkspaceView), | |
| 187 | - | windowForTab: make(map[entity.TabID]*browserWindow), | |
| 188 | - | floatingSessions: make(map[floatingSessionKey]*floatingWorkspaceSession), | |
| 189 | - | browserWindows: make(map[string]*browserWindow), | |
| 190 | - | dispatchOnMainThread: func(fn func()) { fn() }, | |
| 191 | - | engine: deps.Engine, | |
| 192 | - | cancel: cancel, | |
| 184 | + | deps: deps, | |
| 185 | + | tabs: entity.NewTabList(), | |
| 186 | + | tabsUC: deps.TabsUC, | |
| 187 | + | panesUC: deps.PanesUC, | |
| 188 | + | workspaceViews: make(map[entity.TabID]*component.WorkspaceView), | |
| 189 | + | windowForTab: make(map[entity.TabID]*browserWindow), | |
| 190 | + | floatingSessions: make(map[floatingSessionKey]*floatingWorkspaceSession), | |
| 191 | + | browserWindows: make(map[string]*browserWindow), | |
| 192 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 193 | + | if fn != nil { | |
| 194 | + | fn() | |
| 195 | + | } | |
| 196 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 197 | + | }, | |
| 198 | + | engine: deps.Engine, | |
| 199 | + | cancel: cancel, | |
| 193 | 200 | } | |
| 194 | 201 | var autoCopyConfig port.AutoCopyConfig | |
| 195 | 202 | var clipboardOrchestrator port.ClipboardTextOrchestrator | |
| @@ -1911,82 +1918,102 @@ func (a *App) GetSessionID() entity.SessionID { | |||
| 1911 | 1918 | return a.deps.CurrentSessionID | |
| 1912 | 1919 | } | |
| 1913 | 1920 | ||
| 1921 | + | type windowSnapshotCapture struct { | |
| 1922 | + | browserTabs map[string]*entity.TabList | |
| 1923 | + | windowOrder []string | |
| 1924 | + | globalTabs *entity.TabList | |
| 1925 | + | tabOwners map[entity.TabID]string | |
| 1926 | + | lastFocusedWindowID string | |
| 1927 | + | } | |
| 1928 | + | ||
| 1914 | 1929 | // GetWindowSnapshotState implements port.WindowStateProvider. | |
| 1915 | 1930 | // Returns per-window tab lists and active window index from one main-thread capture. | |
| 1916 | 1931 | func (a *App) GetWindowSnapshotState() ([]entity.WindowTabListState, int) { | |
| 1917 | - | var ( | |
| 1918 | - | browserTabs map[string]*entity.TabList | |
| 1919 | - | windowOrder []string | |
| 1920 | - | globalTabs *entity.TabList | |
| 1921 | - | tabOwners map[entity.TabID]string | |
| 1922 | - | lastFocusedWindowID string | |
| 1923 | - | ) | |
| 1932 | + | capture, ok := a.captureWindowSnapshotState() | |
| 1933 | + | if !ok { | |
| 1934 | + | return nil, -1 | |
| 1935 | + | } | |
| 1936 | + | return buildWindowSnapshotState(capture) | |
| 1937 | + | } | |
| 1924 | 1938 | ||
| 1925 | - | capture := func() { | |
| 1926 | - | globalTabs = entity.NewTabList() | |
| 1927 | - | if a.tabs != nil { | |
| 1928 | - | globalTabs = a.tabs.Snapshot() | |
| 1929 | - | } | |
| 1939 | + | func (a *App) captureWindowSnapshotState() (windowSnapshotCapture, bool) { | |
| 1940 | + | var capture windowSnapshotCapture | |
| 1941 | + | captureFn := func() { | |
| 1942 | + | capture = a.captureWindowSnapshotStateOnMainThread() | |
| 1943 | + | } | |
| 1944 | + | if a.dispatchOnMainThread == nil { | |
| 1945 | + | // nil dispatchOnMainThread is for single-threaded tests only; | |
| 1946 | + | // production should marshal capture to GTK main thread. | |
| 1947 | + | captureFn() | |
| 1948 | + | return capture, true | |
| 1949 | + | } | |
| 1930 | 1950 | ||
| 1931 | - | browserTabs = make(map[string]*entity.TabList, len(a.browserWindows)) | |
| 1932 | - | for id, bw := range a.browserWindows { | |
| 1933 | - | if bw == nil { | |
| 1934 | - | continue | |
| 1935 | - | } | |
| 1936 | - | if bw.tabs != nil { | |
| 1937 | - | browserTabs[id] = bw.tabs.Snapshot() | |
| 1938 | - | } else { | |
| 1939 | - | browserTabs[id] = nil | |
| 1940 | - | } | |
| 1941 | - | } | |
| 1951 | + | result := a.dispatchOnMainThread("ui.snapshot_window_state", captureFn) | |
| 1952 | + | if result.Completed() { | |
| 1953 | + | return capture, true | |
| 1954 | + | } | |
| 1942 | 1955 | ||
| 1943 | - | windowOrder = append([]string(nil), a.browserWindowOrder...) | |
| 1944 | - | tabOwners = make(map[entity.TabID]string, len(a.windowForTab)) | |
| 1945 | - | for tabID, bw := range a.windowForTab { | |
| 1946 | - | if bw != nil { | |
| 1947 | - | tabOwners[tabID] = bw.id | |
| 1948 | - | } | |
| 1956 | + | ctx := context.Background() | |
| 1957 | + | if a.deps != nil && a.deps.Ctx != nil { | |
| 1958 | + | ctx = a.deps.Ctx | |
| 1959 | + | } | |
| 1960 | + | logging.FromContext(ctx).Warn(). | |
| 1961 | + | Dur("elapsed", result.Elapsed). | |
| 1962 | + | Str("dispatch_status", string(result.Status)). | |
| 1963 | + | Msg("ui: window state snapshot unavailable after main-thread dispatch did not complete") | |
| 1964 | + | return windowSnapshotCapture{}, false | |
| 1965 | + | } | |
| 1966 | + | ||
| 1967 | + | func (a *App) captureWindowSnapshotStateOnMainThread() windowSnapshotCapture { | |
| 1968 | + | capture := windowSnapshotCapture{ | |
| 1969 | + | globalTabs: entity.NewTabList(), | |
| 1970 | + | } | |
| 1971 | + | if a.tabs != nil { | |
| 1972 | + | capture.globalTabs = a.tabs.Snapshot() | |
| 1973 | + | } | |
| 1974 | + | ||
| 1975 | + | capture.browserTabs = make(map[string]*entity.TabList, len(a.browserWindows)) | |
| 1976 | + | for id, bw := range a.browserWindows { | |
| 1977 | + | if bw == nil { | |
| 1978 | + | continue | |
| 1979 | + | } | |
| 1980 | + | if bw.tabs != nil { | |
| 1981 | + | capture.browserTabs[id] = bw.tabs.Snapshot() | |
| 1982 | + | } else { | |
| 1983 | + | capture.browserTabs[id] = nil | |
| 1949 | 1984 | } | |
| 1950 | - | lastFocusedWindowID = a.lastFocusedWindowID | |
| 1951 | 1985 | } | |
| 1952 | - | if a.dispatchOnMainThread != nil { | |
| 1953 | - | a.dispatchOnMainThread(capture) | |
| 1954 | - | } else { | |
| 1955 | - | // nil dispatchOnMainThread is for single-threaded tests only; | |
| 1956 | - | // production should marshal capture to GTK main thread. | |
| 1957 | - | capture() | |
| 1986 | + | ||
| 1987 | + | capture.windowOrder = append([]string(nil), a.browserWindowOrder...) | |
| 1988 | + | capture.tabOwners = make(map[entity.TabID]string, len(a.windowForTab)) | |
| 1989 | + | for tabID, bw := range a.windowForTab { | |
| 1990 | + | if bw != nil { | |
| 1991 | + | capture.tabOwners[tabID] = bw.id | |
| 1992 | + | } | |
| 1958 | 1993 | } | |
| 1994 | + | capture.lastFocusedWindowID = a.lastFocusedWindowID | |
| 1995 | + | return capture | |
| 1996 | + | } | |
| 1959 | 1997 | ||
| 1960 | - | if len(browserTabs) == 0 { | |
| 1998 | + | func buildWindowSnapshotState(capture windowSnapshotCapture) ([]entity.WindowTabListState, int) { | |
| 1999 | + | if len(capture.browserTabs) == 0 { | |
| 1961 | 2000 | return []entity.WindowTabListState{ | |
| 1962 | - | {WindowID: "", Tabs: globalTabs}, | |
| 2001 | + | {WindowID: "", Tabs: capture.globalTabs}, | |
| 1963 | 2002 | }, 0 | |
| 1964 | 2003 | } | |
| 1965 | 2004 | ||
| 1966 | - | windowIDs := windowOrderFrom(browserTabs, windowOrder) | |
| 2005 | + | windowIDs := windowOrderFrom(capture.browserTabs, capture.windowOrder) | |
| 1967 | 2006 | result := make([]entity.WindowTabListState, 0, len(windowIDs)) | |
| 1968 | 2007 | activeWindowIndex := 0 | |
| 1969 | 2008 | anyOwnedWindow := false | |
| 1970 | 2009 | for i, wid := range windowIDs { | |
| 1971 | - | if wid == lastFocusedWindowID { | |
| 2010 | + | if wid == capture.lastFocusedWindowID { | |
| 1972 | 2011 | activeWindowIndex = i | |
| 1973 | 2012 | } | |
| 1974 | 2013 | ||
| 1975 | - | tabs := browserTabs[wid] | |
| 2014 | + | tabs := capture.browserTabs[wid] | |
| 1976 | 2015 | if tabs == nil { | |
| 1977 | - | tabs = entity.NewTabList() | |
| 1978 | - | for _, tab := range globalTabs.Tabs { | |
| 1979 | - | if tabOwners[tab.ID] == wid { | |
| 1980 | - | tabs.Add(cloneTabForGlobalList(tab)) | |
| 1981 | - | } | |
| 1982 | - | } | |
| 1983 | - | ||
| 1984 | - | // Derive active tab from the global list when no per-window TabList | |
| 1985 | - | // exists only for snapshot/export compatibility with older flat snapshot state. | |
| 1986 | - | globalActive := globalTabs.ActiveTabID | |
| 1987 | - | if globalActive != "" && tabs.Find(globalActive) != nil { | |
| 1988 | - | tabs.SetActive(globalActive) | |
| 1989 | - | } | |
| 2016 | + | tabs = tabsForWindowFromGlobalList(capture.globalTabs, capture.tabOwners, wid) | |
| 1990 | 2017 | } | |
| 1991 | 2018 | if tabs.Count() > 0 { | |
| 1992 | 2019 | anyOwnedWindow = true | |
| @@ -1997,15 +2024,35 @@ func (a *App) GetWindowSnapshotState() ([]entity.WindowTabListState, int) { | |||
| 1997 | 2024 | Tabs: tabs, | |
| 1998 | 2025 | }) | |
| 1999 | 2026 | } | |
| 2000 | - | if !anyOwnedWindow && globalTabs.Count() > 0 { | |
| 2027 | + | if !anyOwnedWindow && capture.globalTabs.Count() > 0 { | |
| 2001 | 2028 | // No window ownership populated; return the flat global list for | |
| 2002 | 2029 | // snapshot/export compatibility rather than snapshotting empty windows. | |
| 2003 | - | return []entity.WindowTabListState{{WindowID: "", Tabs: globalTabs}}, 0 | |
| 2030 | + | return []entity.WindowTabListState{{WindowID: "", Tabs: capture.globalTabs}}, 0 | |
| 2004 | 2031 | } | |
| 2005 | 2032 | ||
| 2006 | 2033 | return result, activeWindowIndex | |
| 2007 | 2034 | } | |
| 2008 | 2035 | ||
| 2036 | + | func tabsForWindowFromGlobalList(globalTabs *entity.TabList, tabOwners map[entity.TabID]string, windowID string) *entity.TabList { | |
| 2037 | + | tabs := entity.NewTabList() | |
| 2038 | + | if globalTabs == nil { | |
| 2039 | + | return tabs | |
| 2040 | + | } | |
| 2041 | + | for _, tab := range globalTabs.Tabs { | |
| 2042 | + | if tabOwners[tab.ID] == windowID { | |
| 2043 | + | tabs.Add(cloneTabForGlobalList(tab)) | |
| 2044 | + | } | |
| 2045 | + | } | |
| 2046 | + | ||
| 2047 | + | // Derive active tab from the global list when no per-window TabList exists | |
| 2048 | + | // only for snapshot/export compatibility with older flat snapshot state. | |
| 2049 | + | globalActive := globalTabs.ActiveTabID | |
| 2050 | + | if globalActive != "" && tabs.Find(globalActive) != nil { | |
| 2051 | + | tabs.SetActive(globalActive) | |
| 2052 | + | } | |
| 2053 | + | return tabs | |
| 2054 | + | } | |
| 2055 | + | ||
| 2009 | 2056 | // GetWindowTabLists implements port.WindowStateProvider. | |
| 2010 | 2057 | // Returns per-window tab lists in browser window registration order. | |
| 2011 | 2058 | // Falls back to a single window with the shared tab list when no browser windows exist. | |
| @@ -3470,25 +3517,65 @@ func (a *App) generateWindowID() string { | |||
| 3470 | 3517 | return fmt.Sprintf("w%d", a.windowIDCounter) | |
| 3471 | 3518 | } | |
| 3472 | 3519 | ||
| 3520 | + | const ( | |
| 3521 | + | uiMainThreadDispatchTimeout = 2 * time.Second | |
| 3522 | + | uiMainThreadDispatchSlowThreshold = 250 * time.Millisecond | |
| 3523 | + | ) | |
| 3524 | + | ||
| 3473 | 3525 | // runOnMainThread executes fn inline when already on the GTK main context; | |
| 3474 | - | // otherwise it dispatches fn to the GTK main loop and waits for completion. | |
| 3475 | - | func (a *App) runOnMainThread(fn func()) { | |
| 3476 | - | if fn == nil { | |
| 3477 | - | return | |
| 3526 | + | // otherwise it dispatches fn to the GTK main loop and waits for bounded | |
| 3527 | + | // completion so IPC and CEF callback paths do not wait forever on a wedged UI. | |
| 3528 | + | func (a *App) runOnMainThread(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 3529 | + | result := syncdispatch.RunSynchronousDispatch(syncdispatch.SyncDispatchOptions{ | |
| 3530 | + | Label: label, | |
| 3531 | + | Timeout: uiMainThreadDispatchTimeout, | |
| 3532 | + | IsOwner: func() bool { | |
| 3533 | + | glibCtx := glib.MainContextDefault() | |
| 3534 | + | return glibCtx != nil && glibCtx.IsOwner() | |
| 3535 | + | }, | |
| 3536 | + | Dispatch: func(cb func()) { | |
| 3537 | + | wrapped := new(glib.SourceOnceFunc) | |
| 3538 | + | *wrapped = func(_ uintptr) { cb() } | |
| 3539 | + | glib.IdleAddOnce(wrapped, 0) | |
| 3540 | + | }, | |
| 3541 | + | }, fn) | |
| 3542 | + | a.logMainThreadDispatchResult(result) | |
| 3543 | + | return result | |
| 3544 | + | } | |
| 3545 | + | ||
| 3546 | + | func (a *App) logMainThreadDispatchResult(result syncdispatch.SyncDispatchResult) { | |
| 3547 | + | ctx := context.Background() | |
| 3548 | + | if a != nil && a.deps != nil && a.deps.Ctx != nil { | |
| 3549 | + | ctx = a.deps.Ctx | |
| 3478 | 3550 | } | |
| 3479 | - | glibCtx := glib.MainContextDefault() | |
| 3480 | - | if glibCtx != nil && glibCtx.IsOwner() { | |
| 3481 | - | fn() | |
| 3482 | - | return | |
| 3551 | + | logger := logging.FromContext(ctx) | |
| 3552 | + | switch result.Status { | |
| 3553 | + | case syncdispatch.SyncDispatchTimedOut: | |
| 3554 | + | logger.Warn(). | |
| 3555 | + | Str("dispatch_label", result.Label). | |
| 3556 | + | Dur("elapsed", result.Elapsed). | |
| 3557 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |
| 3558 | + | Msg("ui: main-thread dispatch timed out before callback started") | |
| 3559 | + | case syncdispatch.SyncDispatchCompletedAfterTimeout: | |
| 3560 | + | logger.Warn(). | |
| 3561 | + | Str("dispatch_label", result.Label). | |
| 3562 | + | Dur("elapsed", result.Elapsed). | |
| 3563 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |
| 3564 | + | Msg("ui: main-thread dispatch completed after timeout") | |
| 3565 | + | case syncdispatch.SyncDispatchQueuedAfterTimeout: | |
| 3566 | + | logger.Warn(). | |
| 3567 | + | Str("dispatch_label", result.Label). | |
| 3568 | + | Dur("elapsed", result.Elapsed). | |
| 3569 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |
| 3570 | + | Msg("ui: main-thread dispatch left queued after timeout") | |
| 3571 | + | case syncdispatch.SyncDispatchCompleted: | |
| 3572 | + | if result.Elapsed >= uiMainThreadDispatchSlowThreshold { | |
| 3573 | + | logger.Debug(). | |
| 3574 | + | Str("dispatch_label", result.Label). | |
| 3575 | + | Dur("elapsed", result.Elapsed). | |
| 3576 | + | Msg("ui: main-thread dispatch completed slowly") | |
| 3577 | + | } | |
| 3483 | 3578 | } | |
| 3484 | - | ||
| 3485 | - | done := make(chan struct{}) | |
| 3486 | - | cb := glib.SourceOnceFunc(func(_ uintptr) { | |
| 3487 | - | fn() | |
| 3488 | - | close(done) | |
| 3489 | - | }) | |
| 3490 | - | glib.IdleAddOnce(&cb, 0) | |
| 3491 | - | <-done | |
| 3492 | 3579 | } | |
| 3493 | 3580 | ||
| 3494 | 3581 | // generateID generates a unique ID for tabs and panes. | |
| @@ -10,12 +10,14 @@ import ( | |||||
| 10 | "sort" | 10 | "sort" | ||
| 11 | "strings" | 11 | "strings" | ||
| 12 | "sync" | 12 | "sync" | ||
| 13 | + | "time" | |||
| 13 | "unsafe" | 14 | "unsafe" | ||
| 14 | 15 | ||||
| 15 | "github.com/bnema/dumber/internal/application/port" | 16 | "github.com/bnema/dumber/internal/application/port" | ||
| 16 | "github.com/bnema/dumber/internal/application/usecase" | 17 | "github.com/bnema/dumber/internal/application/usecase" | ||
| 17 | "github.com/bnema/dumber/internal/domain/entity" | 18 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 18 | urlutil "github.com/bnema/dumber/internal/domain/url" | 19 | urlutil "github.com/bnema/dumber/internal/domain/url" | ||
| 20 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 19 | 21 | ||||
| 20 | "github.com/bnema/dumber/internal/logging" | 22 | "github.com/bnema/dumber/internal/logging" | ||
| 21 | "github.com/bnema/dumber/internal/ui/adapter" | 23 | "github.com/bnema/dumber/internal/ui/adapter" | ||
| @@ -81,7 +83,7 @@ type App struct { | |||||
| 81 | lastFocusedWindowID string | 83 | lastFocusedWindowID string | ||
| 82 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | 84 | nativePopupWindows map[port.WebViewID]*nativePopupWindow | ||
| 83 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | 85 | browserWindowFactory func(context.Context, string) (*browserWindow, error) | ||
| 84 | - | dispatchOnMainThread func(func()) | 86 | + | dispatchOnMainThread func(string, func()) syncdispatch.SyncDispatchResult |
| 85 | 87 | ||||
| 86 | // State | 88 | // State | ||
| 87 | tabs *entity.TabList | 89 | tabs *entity.TabList | ||
| @@ -179,17 +181,22 @@ func New(deps *Dependencies) (*App, error) { | |||||
| 179 | ctx, cancel := context.WithCancelCause(deps.Ctx) | 181 | ctx, cancel := context.WithCancelCause(deps.Ctx) | ||
| 180 | 182 | ||||
| 181 | app := &App{ | 183 | app := &App{ | ||
| 182 | - | deps: deps, | 184 | + | deps: deps, |
| 183 | - | tabs: entity.NewTabList(), | 185 | + | tabs: entity.NewTabList(), |
| 184 | - | tabsUC: deps.TabsUC, | 186 | + | tabsUC: deps.TabsUC, |
| 185 | - | panesUC: deps.PanesUC, | 187 | + | panesUC: deps.PanesUC, |
| 186 | - | workspaceViews: make(map[entity.TabID]*component.WorkspaceView), | 188 | + | workspaceViews: make(map[entity.TabID]*component.WorkspaceView), |
| 187 | - | windowForTab: make(map[entity.TabID]*browserWindow), | 189 | + | windowForTab: make(map[entity.TabID]*browserWindow), |
| 188 | - | floatingSessions: make(map[floatingSessionKey]*floatingWorkspaceSession), | 190 | + | floatingSessions: make(map[floatingSessionKey]*floatingWorkspaceSession), |
| 189 | - | browserWindows: make(map[string]*browserWindow), | 191 | + | browserWindows: make(map[string]*browserWindow), |
| 190 | - | dispatchOnMainThread: func(fn func()) { fn() }, | 192 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { |
| 191 | - | engine: deps.Engine, | 193 | + | if fn != nil { |
| 192 | - | cancel: cancel, | 194 | + | fn() |
| 195 | + | } | |||
| 196 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 197 | + | }, | |||
| 198 | + | engine: deps.Engine, | |||
| 199 | + | cancel: cancel, | |||
| 193 | } | 200 | } | ||
| 194 | var autoCopyConfig port.AutoCopyConfig | 201 | var autoCopyConfig port.AutoCopyConfig | ||
| 195 | var clipboardOrchestrator port.ClipboardTextOrchestrator | 202 | var clipboardOrchestrator port.ClipboardTextOrchestrator | ||
| @@ -1911,82 +1918,102 @@ func (a *App) GetSessionID() entity.SessionID { | |||||
| 1911 | return a.deps.CurrentSessionID | 1918 | return a.deps.CurrentSessionID | ||
| 1912 | } | 1919 | } | ||
| 1913 | 1920 | ||||
| 1921 | + | type windowSnapshotCapture struct { | |||
| 1922 | + | browserTabs map[string]*entity.TabList | |||
| 1923 | + | windowOrder []string | |||
| 1924 | + | globalTabs *entity.TabList | |||
| 1925 | + | tabOwners map[entity.TabID]string | |||
| 1926 | + | lastFocusedWindowID string | |||
| 1927 | + | } | |||
| 1928 | + | ||||
| 1914 | // GetWindowSnapshotState implements port.WindowStateProvider. | 1929 | // GetWindowSnapshotState implements port.WindowStateProvider. | ||
| 1915 | // Returns per-window tab lists and active window index from one main-thread capture. | 1930 | // Returns per-window tab lists and active window index from one main-thread capture. | ||
| 1916 | func (a *App) GetWindowSnapshotState() ([]entity.WindowTabListState, int) { | 1931 | func (a *App) GetWindowSnapshotState() ([]entity.WindowTabListState, int) { | ||
| 1917 | - | var ( | 1932 | + | capture, ok := a.captureWindowSnapshotState() |
| 1918 | - | browserTabs map[string]*entity.TabList | 1933 | + | if !ok { |
| 1919 | - | windowOrder []string | 1934 | + | return nil, -1 |
| 1920 | - | globalTabs *entity.TabList | 1935 | + | } |
| 1921 | - | tabOwners map[entity.TabID]string | 1936 | + | return buildWindowSnapshotState(capture) |
| 1922 | - | lastFocusedWindowID string | 1937 | + | } |
| 1923 | - | ) | |||
| 1924 | 1938 | ||||
| 1925 | - | capture := func() { | 1939 | + | func (a *App) captureWindowSnapshotState() (windowSnapshotCapture, bool) { |
| 1926 | - | globalTabs = entity.NewTabList() | 1940 | + | var capture windowSnapshotCapture |
| 1927 | - | if a.tabs != nil { | 1941 | + | captureFn := func() { |
| 1928 | - | globalTabs = a.tabs.Snapshot() | 1942 | + | capture = a.captureWindowSnapshotStateOnMainThread() |
| 1929 | - | } | 1943 | + | } |
| 1944 | + | if a.dispatchOnMainThread == nil { | |||
| 1945 | + | // nil dispatchOnMainThread is for single-threaded tests only; | |||
| 1946 | + | // production should marshal capture to GTK main thread. | |||
| 1947 | + | captureFn() | |||
| 1948 | + | return capture, true | |||
| 1949 | + | } | |||
| 1930 | 1950 | ||||
| 1931 | - | browserTabs = make(map[string]*entity.TabList, len(a.browserWindows)) | 1951 | + | result := a.dispatchOnMainThread("ui.snapshot_window_state", captureFn) |
| 1932 | - | for id, bw := range a.browserWindows { | 1952 | + | if result.Completed() { |
| 1933 | - | if bw == nil { | 1953 | + | return capture, true |
| 1934 | - | continue | 1954 | + | } |
| 1935 | - | } | |||
| 1936 | - | if bw.tabs != nil { | |||
| 1937 | - | browserTabs[id] = bw.tabs.Snapshot() | |||
| 1938 | - | } else { | |||
| 1939 | - | browserTabs[id] = nil | |||
| 1940 | - | } | |||
| 1941 | - | } | |||
| 1942 | 1955 | ||||
| 1943 | - | windowOrder = append([]string(nil), a.browserWindowOrder...) | 1956 | + | ctx := context.Background() |
| 1944 | - | tabOwners = make(map[entity.TabID]string, len(a.windowForTab)) | 1957 | + | if a.deps != nil && a.deps.Ctx != nil { |
| 1945 | - | for tabID, bw := range a.windowForTab { | 1958 | + | ctx = a.deps.Ctx |
| 1946 | - | if bw != nil { | 1959 | + | } |
| 1947 | - | tabOwners[tabID] = bw.id | 1960 | + | logging.FromContext(ctx).Warn(). |
| 1948 | - | } | 1961 | + | Dur("elapsed", result.Elapsed). |
| 1962 | + | Str("dispatch_status", string(result.Status)). | |||
| 1963 | + | Msg("ui: window state snapshot unavailable after main-thread dispatch did not complete") | |||
| 1964 | + | return windowSnapshotCapture{}, false | |||
| 1965 | + | } | |||
| 1966 | + | ||||
| 1967 | + | func (a *App) captureWindowSnapshotStateOnMainThread() windowSnapshotCapture { | |||
| 1968 | + | capture := windowSnapshotCapture{ | |||
| 1969 | + | globalTabs: entity.NewTabList(), | |||
| 1970 | + | } | |||
| 1971 | + | if a.tabs != nil { | |||
| 1972 | + | capture.globalTabs = a.tabs.Snapshot() | |||
| 1973 | + | } | |||
| 1974 | + | ||||
| 1975 | + | capture.browserTabs = make(map[string]*entity.TabList, len(a.browserWindows)) | |||
| 1976 | + | for id, bw := range a.browserWindows { | |||
| 1977 | + | if bw == nil { | |||
| 1978 | + | continue | |||
| 1979 | + | } | |||
| 1980 | + | if bw.tabs != nil { | |||
| 1981 | + | capture.browserTabs[id] = bw.tabs.Snapshot() | |||
| 1982 | + | } else { | |||
| 1983 | + | capture.browserTabs[id] = nil | |||
| 1949 | } | 1984 | } | ||
| 1950 | - | lastFocusedWindowID = a.lastFocusedWindowID | |||
| 1951 | } | 1985 | } | ||
| 1952 | - | if a.dispatchOnMainThread != nil { | 1986 | + | |
| 1953 | - | a.dispatchOnMainThread(capture) | 1987 | + | capture.windowOrder = append([]string(nil), a.browserWindowOrder...) |
| 1954 | - | } else { | 1988 | + | capture.tabOwners = make(map[entity.TabID]string, len(a.windowForTab)) |
| 1955 | - | // nil dispatchOnMainThread is for single-threaded tests only; | 1989 | + | for tabID, bw := range a.windowForTab { |
| 1956 | - | // production should marshal capture to GTK main thread. | 1990 | + | if bw != nil { |
| 1957 | - | capture() | 1991 | + | capture.tabOwners[tabID] = bw.id |
| 1992 | + | } | |||
| 1958 | } | 1993 | } | ||
| 1994 | + | capture.lastFocusedWindowID = a.lastFocusedWindowID | |||
| 1995 | + | return capture | |||
| 1996 | + | } | |||
| 1959 | 1997 | ||||
| 1960 | - | if len(browserTabs) == 0 { | 1998 | + | func buildWindowSnapshotState(capture windowSnapshotCapture) ([]entity.WindowTabListState, int) { |
| 1999 | + | if len(capture.browserTabs) == 0 { | |||
| 1961 | return []entity.WindowTabListState{ | 2000 | return []entity.WindowTabListState{ | ||
| 1962 | - | {WindowID: "", Tabs: globalTabs}, | 2001 | + | {WindowID: "", Tabs: capture.globalTabs}, |
| 1963 | }, 0 | 2002 | }, 0 | ||
| 1964 | } | 2003 | } | ||
| 1965 | 2004 | ||||
| 1966 | - | windowIDs := windowOrderFrom(browserTabs, windowOrder) | 2005 | + | windowIDs := windowOrderFrom(capture.browserTabs, capture.windowOrder) |
| 1967 | result := make([]entity.WindowTabListState, 0, len(windowIDs)) | 2006 | result := make([]entity.WindowTabListState, 0, len(windowIDs)) | ||
| 1968 | activeWindowIndex := 0 | 2007 | activeWindowIndex := 0 | ||
| 1969 | anyOwnedWindow := false | 2008 | anyOwnedWindow := false | ||
| 1970 | for i, wid := range windowIDs { | 2009 | for i, wid := range windowIDs { | ||
| 1971 | - | if wid == lastFocusedWindowID { | 2010 | + | if wid == capture.lastFocusedWindowID { |
| 1972 | activeWindowIndex = i | 2011 | activeWindowIndex = i | ||
| 1973 | } | 2012 | } | ||
| 1974 | 2013 | ||||
| 1975 | - | tabs := browserTabs[wid] | 2014 | + | tabs := capture.browserTabs[wid] |
| 1976 | if tabs == nil { | 2015 | if tabs == nil { | ||
| 1977 | - | tabs = entity.NewTabList() | 2016 | + | tabs = tabsForWindowFromGlobalList(capture.globalTabs, capture.tabOwners, wid) |
| 1978 | - | for _, tab := range globalTabs.Tabs { | |||
| 1979 | - | if tabOwners[tab.ID] == wid { | |||
| 1980 | - | tabs.Add(cloneTabForGlobalList(tab)) | |||
| 1981 | - | } | |||
| 1982 | - | } | |||
| 1983 | - | ||||
| 1984 | - | // Derive active tab from the global list when no per-window TabList | |||
| 1985 | - | // exists only for snapshot/export compatibility with older flat snapshot state. | |||
| 1986 | - | globalActive := globalTabs.ActiveTabID | |||
| 1987 | - | if globalActive != "" && tabs.Find(globalActive) != nil { | |||
| 1988 | - | tabs.SetActive(globalActive) | |||
| 1989 | - | } | |||
| 1990 | } | 2017 | } | ||
| 1991 | if tabs.Count() > 0 { | 2018 | if tabs.Count() > 0 { | ||
| 1992 | anyOwnedWindow = true | 2019 | anyOwnedWindow = true | ||
| @@ -1997,15 +2024,35 @@ func (a *App) GetWindowSnapshotState() ([]entity.WindowTabListState, int) { | |||||
| 1997 | Tabs: tabs, | 2024 | Tabs: tabs, | ||
| 1998 | }) | 2025 | }) | ||
| 1999 | } | 2026 | } | ||
| 2000 | - | if !anyOwnedWindow && globalTabs.Count() > 0 { | 2027 | + | if !anyOwnedWindow && capture.globalTabs.Count() > 0 { |
| 2001 | // No window ownership populated; return the flat global list for | 2028 | // No window ownership populated; return the flat global list for | ||
| 2002 | // snapshot/export compatibility rather than snapshotting empty windows. | 2029 | // snapshot/export compatibility rather than snapshotting empty windows. | ||
| 2003 | - | return []entity.WindowTabListState{{WindowID: "", Tabs: globalTabs}}, 0 | 2030 | + | return []entity.WindowTabListState{{WindowID: "", Tabs: capture.globalTabs}}, 0 |
| 2004 | } | 2031 | } | ||
| 2005 | 2032 | ||||
| 2006 | return result, activeWindowIndex | 2033 | return result, activeWindowIndex | ||
| 2007 | } | 2034 | } | ||
| 2008 | 2035 | ||||
| 2036 | + | func tabsForWindowFromGlobalList(globalTabs *entity.TabList, tabOwners map[entity.TabID]string, windowID string) *entity.TabList { | |||
| 2037 | + | tabs := entity.NewTabList() | |||
| 2038 | + | if globalTabs == nil { | |||
| 2039 | + | return tabs | |||
| 2040 | + | } | |||
| 2041 | + | for _, tab := range globalTabs.Tabs { | |||
| 2042 | + | if tabOwners[tab.ID] == windowID { | |||
| 2043 | + | tabs.Add(cloneTabForGlobalList(tab)) | |||
| 2044 | + | } | |||
| 2045 | + | } | |||
| 2046 | + | ||||
| 2047 | + | // Derive active tab from the global list when no per-window TabList exists | |||
| 2048 | + | // only for snapshot/export compatibility with older flat snapshot state. | |||
| 2049 | + | globalActive := globalTabs.ActiveTabID | |||
| 2050 | + | if globalActive != "" && tabs.Find(globalActive) != nil { | |||
| 2051 | + | tabs.SetActive(globalActive) | |||
| 2052 | + | } | |||
| 2053 | + | return tabs | |||
| 2054 | + | } | |||
| 2055 | + | ||||
| 2009 | // GetWindowTabLists implements port.WindowStateProvider. | 2056 | // GetWindowTabLists implements port.WindowStateProvider. | ||
| 2010 | // Returns per-window tab lists in browser window registration order. | 2057 | // Returns per-window tab lists in browser window registration order. | ||
| 2011 | // Falls back to a single window with the shared tab list when no browser windows exist. | 2058 | // Falls back to a single window with the shared tab list when no browser windows exist. | ||
| @@ -3470,25 +3517,65 @@ func (a *App) generateWindowID() string { | |||||
| 3470 | return fmt.Sprintf("w%d", a.windowIDCounter) | 3517 | return fmt.Sprintf("w%d", a.windowIDCounter) | ||
| 3471 | } | 3518 | } | ||
| 3472 | 3519 | ||||
| 3520 | + | const ( | |||
| 3521 | + | uiMainThreadDispatchTimeout = 2 * time.Second | |||
| 3522 | + | uiMainThreadDispatchSlowThreshold = 250 * time.Millisecond | |||
| 3523 | + | ) | |||
| 3524 | + | ||||
| 3473 | // runOnMainThread executes fn inline when already on the GTK main context; | 3525 | // runOnMainThread executes fn inline when already on the GTK main context; | ||
| 3474 | - | // otherwise it dispatches fn to the GTK main loop and waits for completion. | 3526 | + | // otherwise it dispatches fn to the GTK main loop and waits for bounded |
| 3475 | - | func (a *App) runOnMainThread(fn func()) { | 3527 | + | // completion so IPC and CEF callback paths do not wait forever on a wedged UI. |
| 3476 | - | if fn == nil { | 3528 | + | func (a *App) runOnMainThread(label string, fn func()) syncdispatch.SyncDispatchResult { |
| 3477 | - | return | 3529 | + | result := syncdispatch.RunSynchronousDispatch(syncdispatch.SyncDispatchOptions{ |
| 3530 | + | Label: label, | |||
| 3531 | + | Timeout: uiMainThreadDispatchTimeout, | |||
| 3532 | + | IsOwner: func() bool { | |||
| 3533 | + | glibCtx := glib.MainContextDefault() | |||
| 3534 | + | return glibCtx != nil && glibCtx.IsOwner() | |||
| 3535 | + | }, | |||
| 3536 | + | Dispatch: func(cb func()) { | |||
| 3537 | + | wrapped := new(glib.SourceOnceFunc) | |||
| 3538 | + | *wrapped = func(_ uintptr) { cb() } | |||
| 3539 | + | glib.IdleAddOnce(wrapped, 0) | |||
| 3540 | + | }, | |||
| 3541 | + | }, fn) | |||
| 3542 | + | a.logMainThreadDispatchResult(result) | |||
| 3543 | + | return result | |||
| 3544 | + | } | |||
| 3545 | + | ||||
| 3546 | + | func (a *App) logMainThreadDispatchResult(result syncdispatch.SyncDispatchResult) { | |||
| 3547 | + | ctx := context.Background() | |||
| 3548 | + | if a != nil && a.deps != nil && a.deps.Ctx != nil { | |||
| 3549 | + | ctx = a.deps.Ctx | |||
| 3478 | } | 3550 | } | ||
| 3479 | - | glibCtx := glib.MainContextDefault() | 3551 | + | logger := logging.FromContext(ctx) |
| 3480 | - | if glibCtx != nil && glibCtx.IsOwner() { | 3552 | + | switch result.Status { |
| 3481 | - | fn() | 3553 | + | case syncdispatch.SyncDispatchTimedOut: |
| 3482 | - | return | 3554 | + | logger.Warn(). |
| 3555 | + | Str("dispatch_label", result.Label). | |||
| 3556 | + | Dur("elapsed", result.Elapsed). | |||
| 3557 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |||
| 3558 | + | Msg("ui: main-thread dispatch timed out before callback started") | |||
| 3559 | + | case syncdispatch.SyncDispatchCompletedAfterTimeout: | |||
| 3560 | + | logger.Warn(). | |||
| 3561 | + | Str("dispatch_label", result.Label). | |||
| 3562 | + | Dur("elapsed", result.Elapsed). | |||
| 3563 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |||
| 3564 | + | Msg("ui: main-thread dispatch completed after timeout") | |||
| 3565 | + | case syncdispatch.SyncDispatchQueuedAfterTimeout: | |||
| 3566 | + | logger.Warn(). | |||
| 3567 | + | Str("dispatch_label", result.Label). | |||
| 3568 | + | Dur("elapsed", result.Elapsed). | |||
| 3569 | + | Dur("timeout", uiMainThreadDispatchTimeout). | |||
| 3570 | + | Msg("ui: main-thread dispatch left queued after timeout") | |||
| 3571 | + | case syncdispatch.SyncDispatchCompleted: | |||
| 3572 | + | if result.Elapsed >= uiMainThreadDispatchSlowThreshold { | |||
| 3573 | + | logger.Debug(). | |||
| 3574 | + | Str("dispatch_label", result.Label). | |||
| 3575 | + | Dur("elapsed", result.Elapsed). | |||
| 3576 | + | Msg("ui: main-thread dispatch completed slowly") | |||
| 3577 | + | } | |||
| 3483 | } | 3578 | } | ||
| 3484 | - | ||||
| 3485 | - | done := make(chan struct{}) | |||
| 3486 | - | cb := glib.SourceOnceFunc(func(_ uintptr) { | |||
| 3487 | - | fn() | |||
| 3488 | - | close(done) | |||
| 3489 | - | }) | |||
| 3490 | - | glib.IdleAddOnce(&cb, 0) | |||
| 3491 | - | <-done | |||
| 3492 | } | 3579 | } | ||
| 3493 | 3580 | ||||
| 3494 | // generateID generates a unique ID for tabs and panes. | 3581 | // generateID generates a unique ID for tabs and panes. | ||
M internal/ui/app_browser_launch_test.go +45 -0
| @@ -20,6 +20,7 @@ import ( | |||
| 20 | 20 | "github.com/bnema/dumber/internal/domain/entity" | |
| 21 | 21 | "github.com/bnema/dumber/internal/domain/repository" | |
| 22 | 22 | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 23 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 23 | 24 | "github.com/bnema/dumber/internal/ui/component" | |
| 24 | 25 | "github.com/bnema/dumber/internal/ui/coordinator" | |
| 25 | 26 | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| @@ -31,6 +32,7 @@ import ( | |||
| 31 | 32 | "github.com/bnema/puregotk/v4/gdk" | |
| 32 | 33 | "github.com/bnema/puregotk/v4/gio" | |
| 33 | 34 | "github.com/bnema/puregotk/v4/gtk" | |
| 35 | + | "github.com/stretchr/testify/require" | |
| 34 | 36 | ) | |
| 35 | 37 | ||
| 36 | 38 | type testBrowserLaunchRelay struct { | |
| @@ -400,6 +402,49 @@ func TestApp_FinalizeActivationStartsBrowserLaunchRelayOnceAndClosesOnShutdown(t | |||
| 400 | 402 | } | |
| 401 | 403 | } | |
| 402 | 404 | ||
| 405 | + | func TestApp_GetWindowSnapshotStateReturnsUnavailableWhenMainThreadDispatchTimesOut(t *testing.T) { | |
| 406 | + | app := &App{ | |
| 407 | + | tabs: entity.NewTabList(), | |
| 408 | + | browserWindows: map[string]*browserWindow{}, | |
| 409 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 410 | + | if label != "ui.snapshot_window_state" { | |
| 411 | + | t.Fatalf("dispatch label = %q, want ui.snapshot_window_state", label) | |
| 412 | + | } | |
| 413 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |
| 414 | + | }, | |
| 415 | + | } | |
| 416 | + | ||
| 417 | + | windows, activeWindowIndex := app.GetWindowSnapshotState() | |
| 418 | + | ||
| 419 | + | require.Nil(t, windows) | |
| 420 | + | require.Equal(t, -1, activeWindowIndex) | |
| 421 | + | } | |
| 422 | + | ||
| 423 | + | func TestApp_OpenFreshWindowReportsMainThreadDispatchTimeout(t *testing.T) { | |
| 424 | + | var factoryCalls int | |
| 425 | + | app := &App{ | |
| 426 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 427 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |
| 428 | + | }, | |
| 429 | + | browserWindowFactory: func(context.Context, string) (*browserWindow, error) { | |
| 430 | + | factoryCalls++ | |
| 431 | + | return &browserWindow{id: "window-timeout", tabs: entity.NewTabList()}, nil | |
| 432 | + | }, | |
| 433 | + | } | |
| 434 | + | ||
| 435 | + | err := app.OpenFreshWindow(context.Background(), "https://example.com") | |
| 436 | + | ||
| 437 | + | if err == nil { | |
| 438 | + | t.Fatal("OpenFreshWindow returned nil error, want dispatch timeout") | |
| 439 | + | } | |
| 440 | + | if !strings.Contains(err.Error(), "main thread dispatch did not complete") { | |
| 441 | + | t.Fatalf("OpenFreshWindow error = %q, want dispatch timeout", err.Error()) | |
| 442 | + | } | |
| 443 | + | if factoryCalls != 0 { | |
| 444 | + | t.Fatalf("browserWindowFactory calls = %d, want 0", factoryCalls) | |
| 445 | + | } | |
| 446 | + | } | |
| 447 | + | ||
| 403 | 448 | func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) { | |
| 404 | 449 | existingTab := entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))) | |
| 405 | 450 | existingTabs := entity.NewTabList() | |
| @@ -20,6 +20,7 @@ import ( | |||||
| 20 | "github.com/bnema/dumber/internal/domain/entity" | 20 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 21 | "github.com/bnema/dumber/internal/domain/repository" | 21 | "github.com/bnema/dumber/internal/domain/repository" | ||
| 22 | "github.com/bnema/dumber/internal/infrastructure/config" | 22 | "github.com/bnema/dumber/internal/infrastructure/config" | ||
| 23 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 23 | "github.com/bnema/dumber/internal/ui/component" | 24 | "github.com/bnema/dumber/internal/ui/component" | ||
| 24 | "github.com/bnema/dumber/internal/ui/coordinator" | 25 | "github.com/bnema/dumber/internal/ui/coordinator" | ||
| 25 | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | 26 | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | ||
| @@ -31,6 +32,7 @@ import ( | |||||
| 31 | "github.com/bnema/puregotk/v4/gdk" | 32 | "github.com/bnema/puregotk/v4/gdk" | ||
| 32 | "github.com/bnema/puregotk/v4/gio" | 33 | "github.com/bnema/puregotk/v4/gio" | ||
| 33 | "github.com/bnema/puregotk/v4/gtk" | 34 | "github.com/bnema/puregotk/v4/gtk" | ||
| 35 | + | "github.com/stretchr/testify/require" | |||
| 34 | ) | 36 | ) | ||
| 35 | 37 | ||||
| 36 | type testBrowserLaunchRelay struct { | 38 | type testBrowserLaunchRelay struct { | ||
| @@ -400,6 +402,49 @@ func TestApp_FinalizeActivationStartsBrowserLaunchRelayOnceAndClosesOnShutdown(t | |||||
| 400 | } | 402 | } | ||
| 401 | } | 403 | } | ||
| 402 | 404 | ||||
| 405 | + | func TestApp_GetWindowSnapshotStateReturnsUnavailableWhenMainThreadDispatchTimesOut(t *testing.T) { | |||
| 406 | + | app := &App{ | |||
| 407 | + | tabs: entity.NewTabList(), | |||
| 408 | + | browserWindows: map[string]*browserWindow{}, | |||
| 409 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 410 | + | if label != "ui.snapshot_window_state" { | |||
| 411 | + | t.Fatalf("dispatch label = %q, want ui.snapshot_window_state", label) | |||
| 412 | + | } | |||
| 413 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |||
| 414 | + | }, | |||
| 415 | + | } | |||
| 416 | + | ||||
| 417 | + | windows, activeWindowIndex := app.GetWindowSnapshotState() | |||
| 418 | + | ||||
| 419 | + | require.Nil(t, windows) | |||
| 420 | + | require.Equal(t, -1, activeWindowIndex) | |||
| 421 | + | } | |||
| 422 | + | ||||
| 423 | + | func TestApp_OpenFreshWindowReportsMainThreadDispatchTimeout(t *testing.T) { | |||
| 424 | + | var factoryCalls int | |||
| 425 | + | app := &App{ | |||
| 426 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 427 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |||
| 428 | + | }, | |||
| 429 | + | browserWindowFactory: func(context.Context, string) (*browserWindow, error) { | |||
| 430 | + | factoryCalls++ | |||
| 431 | + | return &browserWindow{id: "window-timeout", tabs: entity.NewTabList()}, nil | |||
| 432 | + | }, | |||
| 433 | + | } | |||
| 434 | + | ||||
| 435 | + | err := app.OpenFreshWindow(context.Background(), "https://example.com") | |||
| 436 | + | ||||
| 437 | + | if err == nil { | |||
| 438 | + | t.Fatal("OpenFreshWindow returned nil error, want dispatch timeout") | |||
| 439 | + | } | |||
| 440 | + | if !strings.Contains(err.Error(), "main thread dispatch did not complete") { | |||
| 441 | + | t.Fatalf("OpenFreshWindow error = %q, want dispatch timeout", err.Error()) | |||
| 442 | + | } | |||
| 443 | + | if factoryCalls != 0 { | |||
| 444 | + | t.Fatalf("browserWindowFactory calls = %d, want 0", factoryCalls) | |||
| 445 | + | } | |||
| 446 | + | } | |||
| 447 | + | ||||
| 403 | func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) { | 448 | func TestApp_OpenFreshWindowRecordsTabOwnership(t *testing.T) { | ||
| 404 | existingTab := entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))) | 449 | existingTab := entity.NewTab(entity.TabID("existing-tab"), entity.WorkspaceID("existing-workspace"), entity.NewPane(entity.PaneID("existing-pane"))) | ||
| 405 | existingTabs := entity.NewTabList() | 450 | existingTabs := entity.NewTabList() | ||
M internal/ui/browser_window.go +17 -2
| @@ -2,12 +2,14 @@ package ui | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "fmt" | |
| 5 | 6 | "sort" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/dumber/internal/application/port" | |
| 8 | 9 | "github.com/bnema/dumber/internal/application/usecase" | |
| 9 | 10 | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | 11 | "github.com/bnema/dumber/internal/logging" | |
| 12 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 11 | 13 | "github.com/bnema/dumber/internal/ui/component" | |
| 12 | 14 | "github.com/bnema/dumber/internal/ui/coordinator" | |
| 13 | 15 | "github.com/bnema/dumber/internal/ui/focus" | |
| @@ -343,13 +345,26 @@ func (a *App) deterministicBrowserWindowFallback() *browserWindow { | |||
| 343 | 345 | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | |
| 344 | 346 | dispatch := a.dispatchOnMainThread | |
| 345 | 347 | if dispatch == nil { | |
| 346 | - | dispatch = func(fn func()) { fn() } | |
| 348 | + | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 349 | + | if fn != nil { | |
| 350 | + | fn() | |
| 351 | + | } | |
| 352 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 353 | + | } | |
| 347 | 354 | } | |
| 348 | 355 | ||
| 349 | 356 | var openErr error | |
| 350 | - | dispatch(func() { | |
| 357 | + | result := dispatch("ui.open_fresh_window", func() { | |
| 351 | 358 | openErr = a.openFreshWindow(ctx, url) | |
| 352 | 359 | }) | |
| 360 | + | if !result.Completed() { | |
| 361 | + | logging.FromContext(ctx).Warn(). | |
| 362 | + | Str("url", logging.TruncateURL(url, logging.PermissionLogURLMaxLen)). | |
| 363 | + | Dur("elapsed", result.Elapsed). | |
| 364 | + | Str("dispatch_status", string(result.Status)). | |
| 365 | + | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |
| 366 | + | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |
| 367 | + | } | |
| 353 | 368 | ||
| 354 | 369 | return openErr | |
| 355 | 370 | } | |
| @@ -2,12 +2,14 @@ package ui | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "fmt" | |||
| 5 | "sort" | 6 | "sort" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/dumber/internal/application/port" | 8 | "github.com/bnema/dumber/internal/application/port" | ||
| 8 | "github.com/bnema/dumber/internal/application/usecase" | 9 | "github.com/bnema/dumber/internal/application/usecase" | ||
| 9 | "github.com/bnema/dumber/internal/domain/entity" | 10 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 10 | "github.com/bnema/dumber/internal/logging" | 11 | "github.com/bnema/dumber/internal/logging" | ||
| 12 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 11 | "github.com/bnema/dumber/internal/ui/component" | 13 | "github.com/bnema/dumber/internal/ui/component" | ||
| 12 | "github.com/bnema/dumber/internal/ui/coordinator" | 14 | "github.com/bnema/dumber/internal/ui/coordinator" | ||
| 13 | "github.com/bnema/dumber/internal/ui/focus" | 15 | "github.com/bnema/dumber/internal/ui/focus" | ||
| @@ -343,13 +345,26 @@ func (a *App) deterministicBrowserWindowFallback() *browserWindow { | |||||
| 343 | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | 345 | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | ||
| 344 | dispatch := a.dispatchOnMainThread | 346 | dispatch := a.dispatchOnMainThread | ||
| 345 | if dispatch == nil { | 347 | if dispatch == nil { | ||
| 346 | - | dispatch = func(fn func()) { fn() } | 348 | + | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { |
| 349 | + | if fn != nil { | |||
| 350 | + | fn() | |||
| 351 | + | } | |||
| 352 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 353 | + | } | |||
| 347 | } | 354 | } | ||
| 348 | 355 | ||||
| 349 | var openErr error | 356 | var openErr error | ||
| 350 | - | dispatch(func() { | 357 | + | result := dispatch("ui.open_fresh_window", func() { |
| 351 | openErr = a.openFreshWindow(ctx, url) | 358 | openErr = a.openFreshWindow(ctx, url) | ||
| 352 | }) | 359 | }) | ||
| 360 | + | if !result.Completed() { | |||
| 361 | + | logging.FromContext(ctx).Warn(). | |||
| 362 | + | Str("url", logging.TruncateURL(url, logging.PermissionLogURLMaxLen)). | |||
| 363 | + | Dur("elapsed", result.Elapsed). | |||
| 364 | + | Str("dispatch_status", string(result.Status)). | |||
| 365 | + | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |||
| 366 | + | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |||
| 367 | + | } | |||
| 353 | 368 | ||||
| 354 | return openErr | 369 | return openErr | ||
| 355 | } | 370 | } | ||
M internal/ui/browser_window_test.go +10 -2
| @@ -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 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 12 | 13 | "github.com/bnema/dumber/internal/ui/component" | |
| 13 | 14 | "github.com/bnema/dumber/internal/ui/focus" | |
| 14 | 15 | "github.com/bnema/dumber/internal/ui/layout" | |
| @@ -215,9 +216,13 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||
| 215 | 216 | ||
| 216 | 217 | dispatched := false | |
| 217 | 218 | createdURL := "" | |
| 218 | - | app.dispatchOnMainThread = func(fn func()) { | |
| 219 | + | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 219 | 220 | dispatched = true | |
| 221 | + | if label != "ui.open_fresh_window" { | |
| 222 | + | t.Fatalf("dispatch label = %q, want ui.open_fresh_window", label) | |
| 223 | + | } | |
| 220 | 224 | fn() | |
| 225 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |
| 221 | 226 | } | |
| 222 | 227 | app.browserWindowFactory = func(ctx context.Context, url string) (*browserWindow, error) { | |
| 223 | 228 | createdURL = url | |
| @@ -246,7 +251,10 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||
| 246 | 251 | ||
| 247 | 252 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | |
| 248 | 253 | app := &App{browserWindows: make(map[string]*browserWindow)} | |
| 249 | - | app.dispatchOnMainThread = func(fn func()) { fn() } | |
| 254 | + | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 255 | + | fn() | |
| 256 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |
| 257 | + | } | |
| 250 | 258 | wantErr := errors.New("factory error") | |
| 251 | 259 | app.browserWindowFactory = func(context.Context, string) (*browserWindow, error) { | |
| 252 | 260 | return nil, wantErr | |
| @@ -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 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 12 | "github.com/bnema/dumber/internal/ui/component" | 13 | "github.com/bnema/dumber/internal/ui/component" | ||
| 13 | "github.com/bnema/dumber/internal/ui/focus" | 14 | "github.com/bnema/dumber/internal/ui/focus" | ||
| 14 | "github.com/bnema/dumber/internal/ui/layout" | 15 | "github.com/bnema/dumber/internal/ui/layout" | ||
| @@ -215,9 +216,13 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||||
| 215 | 216 | ||||
| 216 | dispatched := false | 217 | dispatched := false | ||
| 217 | createdURL := "" | 218 | createdURL := "" | ||
| 218 | - | app.dispatchOnMainThread = func(fn func()) { | 219 | + | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { |
| 219 | dispatched = true | 220 | dispatched = true | ||
| 221 | + | if label != "ui.open_fresh_window" { | |||
| 222 | + | t.Fatalf("dispatch label = %q, want ui.open_fresh_window", label) | |||
| 223 | + | } | |||
| 220 | fn() | 224 | fn() | ||
| 225 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |||
| 221 | } | 226 | } | ||
| 222 | app.browserWindowFactory = func(ctx context.Context, url string) (*browserWindow, error) { | 227 | app.browserWindowFactory = func(ctx context.Context, url string) (*browserWindow, error) { | ||
| 223 | createdURL = url | 228 | createdURL = url | ||
| @@ -246,7 +251,10 @@ func TestOpenFreshWindow_DispatchesAndTracksBrowserWindow(t *testing.T) { | |||||
| 246 | 251 | ||||
| 247 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | 252 | func TestOpenFreshWindow_PropagatesFactoryError(t *testing.T) { | ||
| 248 | app := &App{browserWindows: make(map[string]*browserWindow)} | 253 | app := &App{browserWindows: make(map[string]*browserWindow)} | ||
| 249 | - | app.dispatchOnMainThread = func(fn func()) { fn() } | 254 | + | app.dispatchOnMainThread = func(label string, fn func()) syncdispatch.SyncDispatchResult { |
| 255 | + | fn() | |||
| 256 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |||
| 257 | + | } | |||
| 250 | wantErr := errors.New("factory error") | 258 | wantErr := errors.New("factory error") | ||
| 251 | app.browserWindowFactory = func(context.Context, string) (*browserWindow, error) { | 259 | app.browserWindowFactory = func(context.Context, string) (*browserWindow, error) { | ||
| 252 | return nil, wantErr | 260 | return nil, wantErr | ||
M internal/ui/native_popup_window.go +28 -4
| @@ -110,19 +110,19 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||
| 110 | 110 | ||
| 111 | 111 | if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok { | |
| 112 | 112 | aborter.SetNativePopupHostAbort(func() { | |
| 113 | - | a.dispatchOnMainThread(func() { | |
| 113 | + | a.dispatchNativePopupLifecycle("ui.native_popup.abort", popupID, func() { | |
| 114 | 114 | a.releaseNativePopupWindow(popupID, true, false) | |
| 115 | 115 | }) | |
| 116 | 116 | }) | |
| 117 | 117 | } | |
| 118 | 118 | if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok { | |
| 119 | 119 | lifecycle.SetOnReadyToShow(func() { | |
| 120 | - | a.dispatchOnMainThread(func() { | |
| 120 | + | a.dispatchNativePopupLifecycle("ui.native_popup.ready_to_show", popupID, func() { | |
| 121 | 121 | a.showNativePopupWindow(popupID) | |
| 122 | 122 | }) | |
| 123 | 123 | }) | |
| 124 | 124 | lifecycle.SetOnClose(func() { | |
| 125 | - | a.dispatchOnMainThread(func() { | |
| 125 | + | a.dispatchNativePopupLifecycle("ui.native_popup.close", popupID, func() { | |
| 126 | 126 | a.releaseNativePopupWindow(popupID, true, false) | |
| 127 | 127 | }) | |
| 128 | 128 | }) | |
| @@ -131,7 +131,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||
| 131 | 131 | } | |
| 132 | 132 | if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok { | |
| 133 | 133 | oauthWV.AddCloseCallback(func() { | |
| 134 | - | a.dispatchOnMainThread(func() { | |
| 134 | + | a.dispatchNativePopupLifecycle("ui.native_popup.oauth_close", popupID, func() { | |
| 135 | 135 | a.releaseNativePopupWindow(popupID, true, false) | |
| 136 | 136 | }) | |
| 137 | 137 | }) | |
| @@ -148,6 +148,30 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||
| 148 | 148 | return nil | |
| 149 | 149 | } | |
| 150 | 150 | ||
| 151 | + | func (a *App) dispatchNativePopupLifecycle(label string, popupID port.WebViewID, fn func()) { | |
| 152 | + | if a == nil || fn == nil { | |
| 153 | + | return | |
| 154 | + | } | |
| 155 | + | if a.dispatchOnMainThread == nil { | |
| 156 | + | fn() | |
| 157 | + | return | |
| 158 | + | } | |
| 159 | + | result := a.dispatchOnMainThread(label, fn) | |
| 160 | + | if result.Completed() { | |
| 161 | + | return | |
| 162 | + | } | |
| 163 | + | ctx := context.Background() | |
| 164 | + | if a.deps != nil && a.deps.Ctx != nil { | |
| 165 | + | ctx = a.deps.Ctx | |
| 166 | + | } | |
| 167 | + | logging.FromContext(ctx).Warn(). | |
| 168 | + | Uint64("popup_id", uint64(popupID)). | |
| 169 | + | Str("dispatch_label", result.Label). | |
| 170 | + | Dur("elapsed", result.Elapsed). | |
| 171 | + | Str("dispatch_status", string(result.Status)). | |
| 172 | + | Msg("native popup lifecycle dispatch did not complete") | |
| 173 | + | } | |
| 174 | + | ||
| 151 | 175 | func (a *App) showNativePopupWindow(popupID port.WebViewID) { | |
| 152 | 176 | if a == nil || a.nativePopupWindows == nil { | |
| 153 | 177 | return | |
| @@ -110,19 +110,19 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||||
| 110 | 110 | ||||
| 111 | if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok { | 111 | if aborter, ok := input.PopupWebView.(port.NativePopupHostAbortCapable); ok { | ||
| 112 | aborter.SetNativePopupHostAbort(func() { | 112 | aborter.SetNativePopupHostAbort(func() { | ||
| 113 | - | a.dispatchOnMainThread(func() { | 113 | + | a.dispatchNativePopupLifecycle("ui.native_popup.abort", popupID, func() { |
| 114 | a.releaseNativePopupWindow(popupID, true, false) | 114 | a.releaseNativePopupWindow(popupID, true, false) | ||
| 115 | }) | 115 | }) | ||
| 116 | }) | 116 | }) | ||
| 117 | } | 117 | } | ||
| 118 | if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok { | 118 | if lifecycle, ok := input.PopupWebView.(port.PopupLifecycleCapable); ok { | ||
| 119 | lifecycle.SetOnReadyToShow(func() { | 119 | lifecycle.SetOnReadyToShow(func() { | ||
| 120 | - | a.dispatchOnMainThread(func() { | 120 | + | a.dispatchNativePopupLifecycle("ui.native_popup.ready_to_show", popupID, func() { |
| 121 | a.showNativePopupWindow(popupID) | 121 | a.showNativePopupWindow(popupID) | ||
| 122 | }) | 122 | }) | ||
| 123 | }) | 123 | }) | ||
| 124 | lifecycle.SetOnClose(func() { | 124 | lifecycle.SetOnClose(func() { | ||
| 125 | - | a.dispatchOnMainThread(func() { | 125 | + | a.dispatchNativePopupLifecycle("ui.native_popup.close", popupID, func() { |
| 126 | a.releaseNativePopupWindow(popupID, true, false) | 126 | a.releaseNativePopupWindow(popupID, true, false) | ||
| 127 | }) | 127 | }) | ||
| 128 | }) | 128 | }) | ||
| @@ -131,7 +131,7 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||||
| 131 | } | 131 | } | ||
| 132 | if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok { | 132 | if oauthWV, ok := input.PopupWebView.(port.OAuthCallbackCapable); ok { | ||
| 133 | oauthWV.AddCloseCallback(func() { | 133 | oauthWV.AddCloseCallback(func() { | ||
| 134 | - | a.dispatchOnMainThread(func() { | 134 | + | a.dispatchNativePopupLifecycle("ui.native_popup.oauth_close", popupID, func() { |
| 135 | a.releaseNativePopupWindow(popupID, true, false) | 135 | a.releaseNativePopupWindow(popupID, true, false) | ||
| 136 | }) | 136 | }) | ||
| 137 | }) | 137 | }) | ||
| @@ -148,6 +148,30 @@ func (a *App) openNativePopupWindow(ctx context.Context, input content.NativePop | |||||
| 148 | return nil | 148 | return nil | ||
| 149 | } | 149 | } | ||
| 150 | 150 | ||||
| 151 | + | func (a *App) dispatchNativePopupLifecycle(label string, popupID port.WebViewID, fn func()) { | |||
| 152 | + | if a == nil || fn == nil { | |||
| 153 | + | return | |||
| 154 | + | } | |||
| 155 | + | if a.dispatchOnMainThread == nil { | |||
| 156 | + | fn() | |||
| 157 | + | return | |||
| 158 | + | } | |||
| 159 | + | result := a.dispatchOnMainThread(label, fn) | |||
| 160 | + | if result.Completed() { | |||
| 161 | + | return | |||
| 162 | + | } | |||
| 163 | + | ctx := context.Background() | |||
| 164 | + | if a.deps != nil && a.deps.Ctx != nil { | |||
| 165 | + | ctx = a.deps.Ctx | |||
| 166 | + | } | |||
| 167 | + | logging.FromContext(ctx).Warn(). | |||
| 168 | + | Uint64("popup_id", uint64(popupID)). | |||
| 169 | + | Str("dispatch_label", result.Label). | |||
| 170 | + | Dur("elapsed", result.Elapsed). | |||
| 171 | + | Str("dispatch_status", string(result.Status)). | |||
| 172 | + | Msg("native popup lifecycle dispatch did not complete") | |||
| 173 | + | } | |||
| 174 | + | ||||
| 151 | func (a *App) showNativePopupWindow(popupID port.WebViewID) { | 175 | func (a *App) showNativePopupWindow(popupID port.WebViewID) { | ||
| 152 | if a == nil || a.nativePopupWindows == nil { | 176 | if a == nil || a.nativePopupWindows == nil { | ||
| 153 | return | 177 | return | ||
M internal/ui/native_popup_window_test.go +29 -0
| @@ -4,9 +4,11 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "testing" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/dumber/internal/application/port" | |
| 9 | 10 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 10 | 12 | "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 11 | 13 | layoutmocks "github.com/bnema/dumber/internal/ui/layout/mocks" | |
| 12 | 14 | "github.com/bnema/dumber/internal/ui/window" | |
| @@ -75,6 +77,33 @@ func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) { | |||
| 75 | 77 | assert.True(t, shell.destroyed) | |
| 76 | 78 | } | |
| 77 | 79 | ||
| 80 | + | func TestDispatchNativePopupLifecycleSkipsWorkWhenDispatchTimesOutBeforeStart(t *testing.T) { | |
| 81 | + | var ran bool | |
| 82 | + | app := &App{ | |
| 83 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 84 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |
| 85 | + | }, | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | app.dispatchNativePopupLifecycle("ui.native_popup.close", port.WebViewID(99), func() { ran = true }) | |
| 89 | + | ||
| 90 | + | assert.False(t, ran) | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | func TestDispatchNativePopupLifecycleTreatsCompletedAfterTimeoutAsCompleted(t *testing.T) { | |
| 94 | + | var ran bool | |
| 95 | + | app := &App{ | |
| 96 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 97 | + | fn() | |
| 98 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompletedAfterTimeout, Elapsed: 10 * time.Millisecond} | |
| 99 | + | }, | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | app.dispatchNativePopupLifecycle("ui.native_popup.close", port.WebViewID(99), func() { ran = true }) | |
| 103 | + | ||
| 104 | + | assert.True(t, ran) | |
| 105 | + | } | |
| 106 | + | ||
| 78 | 107 | func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) { | |
| 79 | 108 | wv := portmocks.NewMockWebView(t) | |
| 80 | 109 | wv.EXPECT().IsDestroyed().Return(false).Once() | |
| @@ -4,9 +4,11 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | "github.com/bnema/dumber/internal/application/port" | 9 | "github.com/bnema/dumber/internal/application/port" | ||
| 9 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | 10 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 11 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |||
| 10 | "github.com/bnema/dumber/internal/ui/coordinator/content" | 12 | "github.com/bnema/dumber/internal/ui/coordinator/content" | ||
| 11 | layoutmocks "github.com/bnema/dumber/internal/ui/layout/mocks" | 13 | layoutmocks "github.com/bnema/dumber/internal/ui/layout/mocks" | ||
| 12 | "github.com/bnema/dumber/internal/ui/window" | 14 | "github.com/bnema/dumber/internal/ui/window" | ||
| @@ -75,6 +77,33 @@ func TestDestroyFailedNativePopupSetup_DestroysWebViewAndShell(t *testing.T) { | |||||
| 75 | assert.True(t, shell.destroyed) | 77 | assert.True(t, shell.destroyed) | ||
| 76 | } | 78 | } | ||
| 77 | 79 | ||||
| 80 | + | func TestDispatchNativePopupLifecycleSkipsWorkWhenDispatchTimesOutBeforeStart(t *testing.T) { | |||
| 81 | + | var ran bool | |||
| 82 | + | app := &App{ | |||
| 83 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 84 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchTimedOut, Elapsed: 5 * time.Millisecond} | |||
| 85 | + | }, | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | app.dispatchNativePopupLifecycle("ui.native_popup.close", port.WebViewID(99), func() { ran = true }) | |||
| 89 | + | ||||
| 90 | + | assert.False(t, ran) | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | func TestDispatchNativePopupLifecycleTreatsCompletedAfterTimeoutAsCompleted(t *testing.T) { | |||
| 94 | + | var ran bool | |||
| 95 | + | app := &App{ | |||
| 96 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 97 | + | fn() | |||
| 98 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompletedAfterTimeout, Elapsed: 10 * time.Millisecond} | |||
| 99 | + | }, | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | app.dispatchNativePopupLifecycle("ui.native_popup.close", port.WebViewID(99), func() { ran = true }) | |||
| 103 | + | ||||
| 104 | + | assert.True(t, ran) | |||
| 105 | + | } | |||
| 106 | + | ||||
| 78 | func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) { | 107 | func TestReleaseNativePopupWindow_DestroysWebViewAndRemovesState(t *testing.T) { | ||
| 79 | wv := portmocks.NewMockWebView(t) | 108 | wv := portmocks.NewMockWebView(t) | ||
| 80 | wv.EXPECT().IsDestroyed().Return(false).Once() | 109 | wv.EXPECT().IsDestroyed().Return(false).Once() | ||