Merge pull request #246 from bnema/fix/cef-resize-hardening
Harden CEF viewport sync and live resize repaint
12 files · +918 -66
M internal/application/port/webview.go +10 -0
| @@ -336,6 +336,16 @@ type PopupOpenerCapable interface { | |||
| 336 | 336 | HasActivePopupOpenerBridge() bool | |
| 337 | 337 | } | |
| 338 | 338 | ||
| 339 | + | // ViewportSyncCapable is implemented by WebViews that can explicitly resync | |
| 340 | + | // their internal viewport state after UI lifecycle events such as reparenting, | |
| 341 | + | // visibility toggles, or layout promotion that may not emit a size change. | |
| 342 | + | // | |
| 343 | + | // Implementations should make this cheap, safe to call often, and resilient to | |
| 344 | + | // duplicate requests. | |
| 345 | + | type ViewportSyncCapable interface { | |
| 346 | + | SyncViewport(ctx context.Context, reason string) | |
| 347 | + | } | |
| 348 | + | ||
| 339 | 349 | // OAuthCallbackCapable is implemented by WebViews that support OAuth auto-close callbacks. | |
| 340 | 350 | // It is an optional capability; callers should type-assert port.WebView to this interface | |
| 341 | 351 | // before use and degrade gracefully when it is absent. | |
| @@ -336,6 +336,16 @@ type PopupOpenerCapable interface { | |||||
| 336 | HasActivePopupOpenerBridge() bool | 336 | HasActivePopupOpenerBridge() bool | ||
| 337 | } | 337 | } | ||
| 338 | 338 | ||||
| 339 | + | // ViewportSyncCapable is implemented by WebViews that can explicitly resync | |||
| 340 | + | // their internal viewport state after UI lifecycle events such as reparenting, | |||
| 341 | + | // visibility toggles, or layout promotion that may not emit a size change. | |||
| 342 | + | // | |||
| 343 | + | // Implementations should make this cheap, safe to call often, and resilient to | |||
| 344 | + | // duplicate requests. | |||
| 345 | + | type ViewportSyncCapable interface { | |||
| 346 | + | SyncViewport(ctx context.Context, reason string) | |||
| 347 | + | } | |||
| 348 | + | ||||
| 339 | // OAuthCallbackCapable is implemented by WebViews that support OAuth auto-close callbacks. | 349 | // OAuthCallbackCapable is implemented by WebViews that support OAuth auto-close callbacks. | ||
| 340 | // It is an optional capability; callers should type-assert port.WebView to this interface | 350 | // It is an optional capability; callers should type-assert port.WebView to this interface | ||
| 341 | // before use and degrade gracefully when it is absent. | 351 | // before use and degrade gracefully when it is absent. | ||
M internal/infrastructure/cef/factory.go +5 -17
| @@ -113,6 +113,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||
| 113 | 113 | id: id, | |
| 114 | 114 | ctx: ctx, | |
| 115 | 115 | engine: f.engine, | |
| 116 | + | factory: f, | |
| 116 | 117 | viewBridge: viewBridge, | |
| 117 | 118 | audioOutputFactory: f.audioOutputFactory, | |
| 118 | 119 | windowlessFrameRate: f.windowlessFrameRate, | |
| @@ -131,6 +132,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||
| 131 | 132 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | |
| 132 | 133 | } | |
| 133 | 134 | } | |
| 135 | + | wv.installViewportSyncHooks() | |
| 134 | 136 | wv.startRenderStallWatchdog() | |
| 135 | 137 | return wv, nil | |
| 136 | 138 | } | |
| @@ -175,27 +177,13 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||
| 175 | 177 | return | |
| 176 | 178 | } | |
| 177 | 179 | ||
| 178 | - | wv.mu.RLock() | |
| 179 | - | host := wv.host | |
| 180 | - | wv.mu.RUnlock() | |
| 181 | - | if host == nil { | |
| 182 | - | logging.FromContext(ctx).Debug(). | |
| 183 | - | Uint64("webview_id", uint64(wv.id)). | |
| 184 | - | Int32("resize_width", w). | |
| 185 | - | Int32("resize_height", h). | |
| 186 | - | Bool("host_nil", true). | |
| 187 | - | Bool("browser_ready", false). | |
| 188 | - | Msg("cef: resize observed before browser host existed") | |
| 189 | - | return | |
| 190 | - | } | |
| 191 | - | notifyBrowserResize(host) | |
| 180 | + | synced := wv.syncResizeViewportOnGTK(ctx, "gtk-size-observer") | |
| 192 | 181 | logging.FromContext(ctx).Debug(). | |
| 193 | 182 | Uint64("webview_id", uint64(wv.id)). | |
| 194 | 183 | Int32("resize_width", w). | |
| 195 | 184 | Int32("resize_height", h). | |
| 196 | - | Bool("host_nil", false). | |
| 197 | - | Bool("browser_ready", true). | |
| 198 | - | Msg("cef: browser host WasResized invoked") | |
| 185 | + | Bool("browser_ready", synced). | |
| 186 | + | Msg("cef: viewport sync handled after GTK size change") | |
| 199 | 187 | }) | |
| 200 | 188 | }) | |
| 201 | 189 | } | |
| @@ -113,6 +113,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||||
| 113 | id: id, | 113 | id: id, | ||
| 114 | ctx: ctx, | 114 | ctx: ctx, | ||
| 115 | engine: f.engine, | 115 | engine: f.engine, | ||
| 116 | + | factory: f, | |||
| 116 | viewBridge: viewBridge, | 117 | viewBridge: viewBridge, | ||
| 117 | audioOutputFactory: f.audioOutputFactory, | 118 | audioOutputFactory: f.audioOutputFactory, | ||
| 118 | windowlessFrameRate: f.windowlessFrameRate, | 119 | windowlessFrameRate: f.windowlessFrameRate, | ||
| @@ -131,6 +132,7 @@ func (f *WebViewFactory) newWebView(ctx context.Context) (*WebView, error) { | |||||
| 131 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | 132 | logging.FromContext(ctx).Warn().Err(err).Uint64("webview_id", uint64(id)).Msg("cef2gtk: failed to enable profiling") | ||
| 132 | } | 133 | } | ||
| 133 | } | 134 | } | ||
| 135 | + | wv.installViewportSyncHooks() | |||
| 134 | wv.startRenderStallWatchdog() | 136 | wv.startRenderStallWatchdog() | ||
| 135 | return wv, nil | 137 | return wv, nil | ||
| 136 | } | 138 | } | ||
| @@ -175,27 +177,13 @@ func (f *WebViewFactory) configureInitialBrowserCreation( | |||||
| 175 | return | 177 | return | ||
| 176 | } | 178 | } | ||
| 177 | 179 | ||||
| 178 | - | wv.mu.RLock() | 180 | + | synced := wv.syncResizeViewportOnGTK(ctx, "gtk-size-observer") |
| 179 | - | host := wv.host | |||
| 180 | - | wv.mu.RUnlock() | |||
| 181 | - | if host == nil { | |||
| 182 | - | logging.FromContext(ctx).Debug(). | |||
| 183 | - | Uint64("webview_id", uint64(wv.id)). | |||
| 184 | - | Int32("resize_width", w). | |||
| 185 | - | Int32("resize_height", h). | |||
| 186 | - | Bool("host_nil", true). | |||
| 187 | - | Bool("browser_ready", false). | |||
| 188 | - | Msg("cef: resize observed before browser host existed") | |||
| 189 | - | return | |||
| 190 | - | } | |||
| 191 | - | notifyBrowserResize(host) | |||
| 192 | logging.FromContext(ctx).Debug(). | 181 | logging.FromContext(ctx).Debug(). | ||
| 193 | Uint64("webview_id", uint64(wv.id)). | 182 | Uint64("webview_id", uint64(wv.id)). | ||
| 194 | Int32("resize_width", w). | 183 | Int32("resize_width", w). | ||
| 195 | Int32("resize_height", h). | 184 | Int32("resize_height", h). | ||
| 196 | - | Bool("host_nil", false). | 185 | + | Bool("browser_ready", synced). |
| 197 | - | Bool("browser_ready", true). | 186 | + | Msg("cef: viewport sync handled after GTK size change") |
| 198 | - | Msg("cef: browser host WasResized invoked") | |||
| 199 | }) | 187 | }) | ||
| 200 | }) | 188 | }) | ||
| 201 | } | 189 | } | ||
M internal/infrastructure/cef/webview.go +15 -7
| @@ -29,6 +29,7 @@ var ( | |||
| 29 | 29 | _ port.DevToolsOpener = (*WebView)(nil) | |
| 30 | 30 | _ port.PopupLifecycleCapable = (*WebView)(nil) | |
| 31 | 31 | _ port.PopupOpenerCapable = (*WebView)(nil) | |
| 32 | + | _ port.ViewportSyncCapable = (*WebView)(nil) | |
| 32 | 33 | _ port.OAuthCallbackCapable = (*WebView)(nil) | |
| 33 | 34 | ) | |
| 34 | 35 | ||
| @@ -72,6 +73,7 @@ type WebView struct { | |||
| 72 | 73 | id port.WebViewID | |
| 73 | 74 | ctx context.Context | |
| 74 | 75 | engine *Engine | |
| 76 | + | factory *WebViewFactory | |
| 75 | 77 | browser purecef.Browser | |
| 76 | 78 | host purecef.BrowserHost | |
| 77 | 79 | client purecef.RawClient // prevent GC from collecting the client before CEF AddRef's it | |
| @@ -88,7 +90,17 @@ type WebView struct { | |||
| 88 | 90 | previousProfileSnapshot cef2gtk.ProfileSnapshot | |
| 89 | 91 | latestProfileSnapshotAt time.Time | |
| 90 | 92 | ||
| 91 | - | removeSizeObserver func() | |
| 93 | + | removeSizeObserver func() | |
| 94 | + | viewportSyncMu sync.Mutex | |
| 95 | + | viewportSyncPending bool | |
| 96 | + | viewportSyncReason string | |
| 97 | + | viewportMapFunc func(gtk.Widget) | |
| 98 | + | viewportShowFunc func(gtk.Widget) | |
| 99 | + | viewportRealizeFunc func(gtk.Widget) | |
| 100 | + | viewportMapSignalID uint | |
| 101 | + | viewportShowSignalID uint | |
| 102 | + | viewportRealizeSignalID uint | |
| 103 | + | viewportResizePulseSeq atomic.Uint64 | |
| 92 | 104 | ||
| 93 | 105 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | |
| 94 | 106 | // widget is visible. Access is guarded by mu. | |
| @@ -541,12 +553,7 @@ func (wv *WebView) Show() { | |||
| 541 | 553 | if wv == nil || wv.destroyed.Load() { | |
| 542 | 554 | return | |
| 543 | 555 | } | |
| 544 | - | wv.mu.RLock() | |
| 545 | - | host := wv.host | |
| 546 | - | wv.mu.RUnlock() | |
| 547 | - | if host != nil { | |
| 548 | - | host.WasHidden(0) | |
| 549 | - | } | |
| 556 | + | wv.SyncViewport(wv.ctx, "popup-show") | |
| 550 | 557 | } | |
| 551 | 558 | ||
| 552 | 559 | // PrimePopupNavigation implements port.PopupLifecycleCapable. | |
| @@ -1081,6 +1088,7 @@ func (wv *WebView) destroyViewBridgeOnGTKThread() { | |||
| 1081 | 1088 | wv.removeSizeObserver() | |
| 1082 | 1089 | wv.removeSizeObserver = nil | |
| 1083 | 1090 | } | |
| 1091 | + | wv.disconnectViewportSyncHooksOnGTKThread() | |
| 1084 | 1092 | _ = wv.viewBridge.DetachInput() | |
| 1085 | 1093 | _ = wv.viewBridge.Destroy() | |
| 1086 | 1094 | wv.viewBridge = nil | |
| @@ -29,6 +29,7 @@ var ( | |||||
| 29 | _ port.DevToolsOpener = (*WebView)(nil) | 29 | _ port.DevToolsOpener = (*WebView)(nil) | ||
| 30 | _ port.PopupLifecycleCapable = (*WebView)(nil) | 30 | _ port.PopupLifecycleCapable = (*WebView)(nil) | ||
| 31 | _ port.PopupOpenerCapable = (*WebView)(nil) | 31 | _ port.PopupOpenerCapable = (*WebView)(nil) | ||
| 32 | + | _ port.ViewportSyncCapable = (*WebView)(nil) | |||
| 32 | _ port.OAuthCallbackCapable = (*WebView)(nil) | 33 | _ port.OAuthCallbackCapable = (*WebView)(nil) | ||
| 33 | ) | 34 | ) | ||
| 34 | 35 | ||||
| @@ -72,6 +73,7 @@ type WebView struct { | |||||
| 72 | id port.WebViewID | 73 | id port.WebViewID | ||
| 73 | ctx context.Context | 74 | ctx context.Context | ||
| 74 | engine *Engine | 75 | engine *Engine | ||
| 76 | + | factory *WebViewFactory | |||
| 75 | browser purecef.Browser | 77 | browser purecef.Browser | ||
| 76 | host purecef.BrowserHost | 78 | host purecef.BrowserHost | ||
| 77 | client purecef.RawClient // prevent GC from collecting the client before CEF AddRef's it | 79 | client purecef.RawClient // prevent GC from collecting the client before CEF AddRef's it | ||
| @@ -88,7 +90,17 @@ type WebView struct { | |||||
| 88 | previousProfileSnapshot cef2gtk.ProfileSnapshot | 90 | previousProfileSnapshot cef2gtk.ProfileSnapshot | ||
| 89 | latestProfileSnapshotAt time.Time | 91 | latestProfileSnapshotAt time.Time | ||
| 90 | 92 | ||||
| 91 | - | removeSizeObserver func() | 93 | + | removeSizeObserver func() |
| 94 | + | viewportSyncMu sync.Mutex | |||
| 95 | + | viewportSyncPending bool | |||
| 96 | + | viewportSyncReason string | |||
| 97 | + | viewportMapFunc func(gtk.Widget) | |||
| 98 | + | viewportShowFunc func(gtk.Widget) | |||
| 99 | + | viewportRealizeFunc func(gtk.Widget) | |||
| 100 | + | viewportMapSignalID uint | |||
| 101 | + | viewportShowSignalID uint | |||
| 102 | + | viewportRealizeSignalID uint | |||
| 103 | + | viewportResizePulseSeq atomic.Uint64 | |||
| 92 | 104 | ||||
| 93 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | 105 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | ||
| 94 | // widget is visible. Access is guarded by mu. | 106 | // widget is visible. Access is guarded by mu. | ||
| @@ -541,12 +553,7 @@ func (wv *WebView) Show() { | |||||
| 541 | if wv == nil || wv.destroyed.Load() { | 553 | if wv == nil || wv.destroyed.Load() { | ||
| 542 | return | 554 | return | ||
| 543 | } | 555 | } | ||
| 544 | - | wv.mu.RLock() | 556 | + | wv.SyncViewport(wv.ctx, "popup-show") |
| 545 | - | host := wv.host | |||
| 546 | - | wv.mu.RUnlock() | |||
| 547 | - | if host != nil { | |||
| 548 | - | host.WasHidden(0) | |||
| 549 | - | } | |||
| 550 | } | 557 | } | ||
| 551 | 558 | ||||
| 552 | // PrimePopupNavigation implements port.PopupLifecycleCapable. | 559 | // PrimePopupNavigation implements port.PopupLifecycleCapable. | ||
| @@ -1081,6 +1088,7 @@ func (wv *WebView) destroyViewBridgeOnGTKThread() { | |||||
| 1081 | wv.removeSizeObserver() | 1088 | wv.removeSizeObserver() | ||
| 1082 | wv.removeSizeObserver = nil | 1089 | wv.removeSizeObserver = nil | ||
| 1083 | } | 1090 | } | ||
| 1091 | + | wv.disconnectViewportSyncHooksOnGTKThread() | |||
| 1084 | _ = wv.viewBridge.DetachInput() | 1092 | _ = wv.viewBridge.DetachInput() | ||
| 1085 | _ = wv.viewBridge.Destroy() | 1093 | _ = wv.viewBridge.Destroy() | ||
| 1086 | wv.viewBridge = nil | 1094 | wv.viewBridge = nil | ||
A internal/infrastructure/cef/webview_viewport.go +276 -0
| @@ -0,0 +1,276 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "strings" | |
| 6 | + | ||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 8 | + | "github.com/bnema/puregotk/v4/gobject" | |
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/dumber/internal/logging" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | type viewportSyncBrowserHost interface { | |
| 15 | + | resizeNotifiableBrowserHost | |
| 16 | + | NotifyScreenInfoChanged() | |
| 17 | + | WasHidden(int32) | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | // SyncViewport requests an explicit viewport resync for CEF OSR browsers. | |
| 21 | + | // | |
| 22 | + | // This is a hardening path for UI lifecycle transitions that may not emit a | |
| 23 | + | // fresh size change (reparenting, stacked visibility toggles, floating pane | |
| 24 | + | // updates, popup show, sibling promotion after close, etc.). Calls are | |
| 25 | + | // coalesced and executed on the GTK thread before notifying the CEF host. | |
| 26 | + | func (wv *WebView) SyncViewport(ctx context.Context, reason string) { | |
| 27 | + | if wv == nil || wv.destroyed.Load() { | |
| 28 | + | return | |
| 29 | + | } | |
| 30 | + | reason = strings.TrimSpace(reason) | |
| 31 | + | if reason == "" { | |
| 32 | + | reason = "manual" | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | wv.viewportSyncMu.Lock() | |
| 36 | + | wv.viewportSyncReason = reason | |
| 37 | + | if wv.viewportSyncPending { | |
| 38 | + | wv.viewportSyncMu.Unlock() | |
| 39 | + | return | |
| 40 | + | } | |
| 41 | + | wv.viewportSyncPending = true | |
| 42 | + | wv.viewportSyncMu.Unlock() | |
| 43 | + | ||
| 44 | + | wv.runOnGTK(func() { | |
| 45 | + | wv.syncViewportOnGTK(wv.viewportSyncContext(ctx)) | |
| 46 | + | }) | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | func (wv *WebView) viewportSyncContext(ctx context.Context) context.Context { | |
| 50 | + | if ctx != nil { | |
| 51 | + | return ctx | |
| 52 | + | } | |
| 53 | + | if wv != nil && wv.ctx != nil { | |
| 54 | + | return wv.ctx | |
| 55 | + | } | |
| 56 | + | return context.Background() | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | func (wv *WebView) takeViewportSyncReason() string { | |
| 60 | + | wv.viewportSyncMu.Lock() | |
| 61 | + | defer wv.viewportSyncMu.Unlock() | |
| 62 | + | reason := wv.viewportSyncReason | |
| 63 | + | wv.viewportSyncReason = "" | |
| 64 | + | wv.viewportSyncPending = false | |
| 65 | + | return reason | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | func (wv *WebView) syncViewportOnGTK(ctx context.Context) { | |
| 69 | + | if wv == nil { | |
| 70 | + | return | |
| 71 | + | } | |
| 72 | + | wv.syncViewportNowOnGTK(ctx, wv.takeViewportSyncReason()) | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool { | |
| 76 | + | if wv == nil { | |
| 77 | + | return false | |
| 78 | + | } | |
| 79 | + | reason = strings.TrimSpace(reason) | |
| 80 | + | if reason == "" { | |
| 81 | + | reason = "manual" | |
| 82 | + | } | |
| 83 | + | if wv.destroyed.Load() || wv.viewBridge == nil { | |
| 84 | + | return false | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | if wv.tryStartPendingBrowserCreateOnGTKThread(ctx, reason) { | |
| 88 | + | return false | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | wv.mu.RLock() | |
| 92 | + | host := wv.host | |
| 93 | + | wv.mu.RUnlock() | |
| 94 | + | if host == nil { | |
| 95 | + | logging.FromContext(ctx).Debug(). | |
| 96 | + | Uint64("webview_id", uint64(wv.id)). | |
| 97 | + | Str("reason", reason). | |
| 98 | + | Msg("cef: viewport sync skipped; browser host not ready") | |
| 99 | + | return false | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | widget := wv.viewBridge.Widget() | |
| 103 | + | visible := widget != nil && widget.IsVisible() | |
| 104 | + | notifyBrowserViewportSync(host, visible) | |
| 105 | + | logging.FromContext(ctx).Debug(). | |
| 106 | + | Uint64("webview_id", uint64(wv.id)). | |
| 107 | + | Bool("visible", visible). | |
| 108 | + | Str("reason", reason). | |
| 109 | + | Msg("cef: viewport sync requested") | |
| 110 | + | return true | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | func (wv *WebView) syncResizeViewportOnGTK(ctx context.Context, reason string) bool { | |
| 114 | + | if !wv.syncViewportNowOnGTK(ctx, reason) { | |
| 115 | + | return false | |
| 116 | + | } | |
| 117 | + | wv.scheduleResizeRepaintPulse(ctx, reason) | |
| 118 | + | return true | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | func (wv *WebView) scheduleResizeRepaintPulse(ctx context.Context, reason string) { | |
| 122 | + | if wv == nil || wv.destroyed.Load() { | |
| 123 | + | return | |
| 124 | + | } | |
| 125 | + | seq := wv.viewportResizePulseSeq.Add(1) | |
| 126 | + | for _, delayMs := range [...]int64{16, 48} { | |
| 127 | + | task := cefNewTask(cefTaskFunc(func() { | |
| 128 | + | if wv == nil || wv.destroyed.Load() || wv.viewportResizePulseSeq.Load() != seq { | |
| 129 | + | return | |
| 130 | + | } | |
| 131 | + | wv.mu.RLock() | |
| 132 | + | host := wv.host | |
| 133 | + | wv.mu.RUnlock() | |
| 134 | + | if host == nil { | |
| 135 | + | return | |
| 136 | + | } | |
| 137 | + | host.Invalidate(purecef.PaintElementTypePetView) | |
| 138 | + | logging.FromContext(ctx).Debug(). | |
| 139 | + | Uint64("webview_id", uint64(wv.id)). | |
| 140 | + | Int64("delay_ms", delayMs). | |
| 141 | + | Str("reason", reason). | |
| 142 | + | Msg("cef: delayed resize repaint pulse") | |
| 143 | + | })) | |
| 144 | + | if task == nil { | |
| 145 | + | continue | |
| 146 | + | } | |
| 147 | + | cefPostDelayedTask(purecef.ThreadIDTidUi, task, delayMs) | |
| 148 | + | } | |
| 149 | + | } | |
| 150 | + | ||
| 151 | + | func (wv *WebView) tryStartPendingBrowserCreateOnGTKThread(ctx context.Context, reason string) bool { | |
| 152 | + | if wv == nil || wv.destroyed.Load() || wv.factory == nil || wv.viewBridge == nil { | |
| 153 | + | return false | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | wv.mu.RLock() | |
| 157 | + | hasHost := wv.host != nil | |
| 158 | + | pending := wv.pendingCreate != nil | |
| 159 | + | wv.mu.RUnlock() | |
| 160 | + | if hasHost || !pending { | |
| 161 | + | return false | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | widget := wv.viewBridge.Widget() | |
| 165 | + | if widget == nil { | |
| 166 | + | return false | |
| 167 | + | } | |
| 168 | + | width := int32(widget.GetAllocatedWidth()) | |
| 169 | + | height := int32(widget.GetAllocatedHeight()) | |
| 170 | + | if width <= 0 { | |
| 171 | + | width = int32(widget.GetWidth()) | |
| 172 | + | } | |
| 173 | + | if height <= 0 { | |
| 174 | + | height = int32(widget.GetHeight()) | |
| 175 | + | } | |
| 176 | + | if width <= 0 || height <= 0 { | |
| 177 | + | logging.FromContext(ctx).Debug(). | |
| 178 | + | Uint64("webview_id", uint64(wv.id)). | |
| 179 | + | Int32("width", width). | |
| 180 | + | Int32("height", height). | |
| 181 | + | Str("reason", reason). | |
| 182 | + | Msg("cef: viewport sync skipped pending browser creation; widget has no allocation yet") | |
| 183 | + | return false | |
| 184 | + | } | |
| 185 | + | if err := wv.viewBridge.PrepareOnGTKThread(); err != nil { | |
| 186 | + | logging.FromContext(ctx).Debug(). | |
| 187 | + | Err(err). | |
| 188 | + | Uint64("webview_id", uint64(wv.id)). | |
| 189 | + | Int32("width", width). | |
| 190 | + | Int32("height", height). | |
| 191 | + | Str("reason", reason). | |
| 192 | + | Msg("cef: viewport sync could not prepare bridge for pending browser creation") | |
| 193 | + | return false | |
| 194 | + | } | |
| 195 | + | ||
| 196 | + | wv.factory.postPendingBrowserCreate(ctx, wv, width, height) | |
| 197 | + | logging.FromContext(ctx).Debug(). | |
| 198 | + | Uint64("webview_id", uint64(wv.id)). | |
| 199 | + | Int32("width", width). | |
| 200 | + | Int32("height", height). | |
| 201 | + | Str("reason", reason). | |
| 202 | + | Msg("cef: viewport sync nudged pending browser creation") | |
| 203 | + | return true | |
| 204 | + | } | |
| 205 | + | ||
| 206 | + | func (wv *WebView) installViewportSyncHooks() { | |
| 207 | + | if wv == nil || wv.viewBridge == nil { | |
| 208 | + | return | |
| 209 | + | } | |
| 210 | + | wv.runOnGTKSync(func() { | |
| 211 | + | wv.installViewportSyncHooksOnGTKThread() | |
| 212 | + | }) | |
| 213 | + | } | |
| 214 | + | ||
| 215 | + | func (wv *WebView) installViewportSyncHooksOnGTKThread() { | |
| 216 | + | if wv == nil || wv.viewBridge == nil || wv.viewportMapFunc != nil { | |
| 217 | + | return | |
| 218 | + | } | |
| 219 | + | widget := wv.viewBridge.Widget() | |
| 220 | + | if widget == nil { | |
| 221 | + | return | |
| 222 | + | } | |
| 223 | + | ||
| 224 | + | wv.viewportMapFunc = func(gtk.Widget) { | |
| 225 | + | wv.SyncViewport(wv.ctx, "gtk-map") | |
| 226 | + | } | |
| 227 | + | wv.viewportShowFunc = func(gtk.Widget) { | |
| 228 | + | wv.SyncViewport(wv.ctx, "gtk-show") | |
| 229 | + | } | |
| 230 | + | wv.viewportRealizeFunc = func(gtk.Widget) { | |
| 231 | + | wv.SyncViewport(wv.ctx, "gtk-realize") | |
| 232 | + | } | |
| 233 | + | ||
| 234 | + | wv.viewportMapSignalID = widget.ConnectMap(&wv.viewportMapFunc) | |
| 235 | + | wv.viewportShowSignalID = widget.ConnectShow(&wv.viewportShowFunc) | |
| 236 | + | wv.viewportRealizeSignalID = widget.ConnectRealize(&wv.viewportRealizeFunc) | |
| 237 | + | } | |
| 238 | + | ||
| 239 | + | func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |
| 240 | + | if wv == nil || wv.viewBridge == nil { | |
| 241 | + | return | |
| 242 | + | } | |
| 243 | + | widget := wv.viewBridge.Widget() | |
| 244 | + | if widget != nil { | |
| 245 | + | widgetPtr := widget.GoPointer() | |
| 246 | + | if widgetPtr != 0 { | |
| 247 | + | obj := gobject.ObjectNewFromInternalPtr(widgetPtr) | |
| 248 | + | if wv.viewportMapSignalID != 0 { | |
| 249 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportMapSignalID) | |
| 250 | + | wv.viewportMapSignalID = 0 | |
| 251 | + | } | |
| 252 | + | if wv.viewportShowSignalID != 0 { | |
| 253 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportShowSignalID) | |
| 254 | + | wv.viewportShowSignalID = 0 | |
| 255 | + | } | |
| 256 | + | if wv.viewportRealizeSignalID != 0 { | |
| 257 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportRealizeSignalID) | |
| 258 | + | wv.viewportRealizeSignalID = 0 | |
| 259 | + | } | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | wv.viewportMapFunc = nil | |
| 263 | + | wv.viewportShowFunc = nil | |
| 264 | + | wv.viewportRealizeFunc = nil | |
| 265 | + | } | |
| 266 | + | ||
| 267 | + | func notifyBrowserViewportSync(host viewportSyncBrowserHost, visible bool) { | |
| 268 | + | if host == nil { | |
| 269 | + | return | |
| 270 | + | } | |
| 271 | + | if visible { | |
| 272 | + | host.WasHidden(0) | |
| 273 | + | } | |
| 274 | + | host.NotifyScreenInfoChanged() | |
| 275 | + | notifyBrowserResize(host) | |
| 276 | + | } | |
| @@ -0,0 +1,276 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "strings" | |||
| 6 | + | ||||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 8 | + | "github.com/bnema/puregotk/v4/gobject" | |||
| 9 | + | "github.com/bnema/puregotk/v4/gtk" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/dumber/internal/logging" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | type viewportSyncBrowserHost interface { | |||
| 15 | + | resizeNotifiableBrowserHost | |||
| 16 | + | NotifyScreenInfoChanged() | |||
| 17 | + | WasHidden(int32) | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | // SyncViewport requests an explicit viewport resync for CEF OSR browsers. | |||
| 21 | + | // | |||
| 22 | + | // This is a hardening path for UI lifecycle transitions that may not emit a | |||
| 23 | + | // fresh size change (reparenting, stacked visibility toggles, floating pane | |||
| 24 | + | // updates, popup show, sibling promotion after close, etc.). Calls are | |||
| 25 | + | // coalesced and executed on the GTK thread before notifying the CEF host. | |||
| 26 | + | func (wv *WebView) SyncViewport(ctx context.Context, reason string) { | |||
| 27 | + | if wv == nil || wv.destroyed.Load() { | |||
| 28 | + | return | |||
| 29 | + | } | |||
| 30 | + | reason = strings.TrimSpace(reason) | |||
| 31 | + | if reason == "" { | |||
| 32 | + | reason = "manual" | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | wv.viewportSyncMu.Lock() | |||
| 36 | + | wv.viewportSyncReason = reason | |||
| 37 | + | if wv.viewportSyncPending { | |||
| 38 | + | wv.viewportSyncMu.Unlock() | |||
| 39 | + | return | |||
| 40 | + | } | |||
| 41 | + | wv.viewportSyncPending = true | |||
| 42 | + | wv.viewportSyncMu.Unlock() | |||
| 43 | + | ||||
| 44 | + | wv.runOnGTK(func() { | |||
| 45 | + | wv.syncViewportOnGTK(wv.viewportSyncContext(ctx)) | |||
| 46 | + | }) | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | func (wv *WebView) viewportSyncContext(ctx context.Context) context.Context { | |||
| 50 | + | if ctx != nil { | |||
| 51 | + | return ctx | |||
| 52 | + | } | |||
| 53 | + | if wv != nil && wv.ctx != nil { | |||
| 54 | + | return wv.ctx | |||
| 55 | + | } | |||
| 56 | + | return context.Background() | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | func (wv *WebView) takeViewportSyncReason() string { | |||
| 60 | + | wv.viewportSyncMu.Lock() | |||
| 61 | + | defer wv.viewportSyncMu.Unlock() | |||
| 62 | + | reason := wv.viewportSyncReason | |||
| 63 | + | wv.viewportSyncReason = "" | |||
| 64 | + | wv.viewportSyncPending = false | |||
| 65 | + | return reason | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | func (wv *WebView) syncViewportOnGTK(ctx context.Context) { | |||
| 69 | + | if wv == nil { | |||
| 70 | + | return | |||
| 71 | + | } | |||
| 72 | + | wv.syncViewportNowOnGTK(ctx, wv.takeViewportSyncReason()) | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func (wv *WebView) syncViewportNowOnGTK(ctx context.Context, reason string) bool { | |||
| 76 | + | if wv == nil { | |||
| 77 | + | return false | |||
| 78 | + | } | |||
| 79 | + | reason = strings.TrimSpace(reason) | |||
| 80 | + | if reason == "" { | |||
| 81 | + | reason = "manual" | |||
| 82 | + | } | |||
| 83 | + | if wv.destroyed.Load() || wv.viewBridge == nil { | |||
| 84 | + | return false | |||
| 85 | + | } | |||
| 86 | + | ||||
| 87 | + | if wv.tryStartPendingBrowserCreateOnGTKThread(ctx, reason) { | |||
| 88 | + | return false | |||
| 89 | + | } | |||
| 90 | + | ||||
| 91 | + | wv.mu.RLock() | |||
| 92 | + | host := wv.host | |||
| 93 | + | wv.mu.RUnlock() | |||
| 94 | + | if host == nil { | |||
| 95 | + | logging.FromContext(ctx).Debug(). | |||
| 96 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 97 | + | Str("reason", reason). | |||
| 98 | + | Msg("cef: viewport sync skipped; browser host not ready") | |||
| 99 | + | return false | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | widget := wv.viewBridge.Widget() | |||
| 103 | + | visible := widget != nil && widget.IsVisible() | |||
| 104 | + | notifyBrowserViewportSync(host, visible) | |||
| 105 | + | logging.FromContext(ctx).Debug(). | |||
| 106 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 107 | + | Bool("visible", visible). | |||
| 108 | + | Str("reason", reason). | |||
| 109 | + | Msg("cef: viewport sync requested") | |||
| 110 | + | return true | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | func (wv *WebView) syncResizeViewportOnGTK(ctx context.Context, reason string) bool { | |||
| 114 | + | if !wv.syncViewportNowOnGTK(ctx, reason) { | |||
| 115 | + | return false | |||
| 116 | + | } | |||
| 117 | + | wv.scheduleResizeRepaintPulse(ctx, reason) | |||
| 118 | + | return true | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | func (wv *WebView) scheduleResizeRepaintPulse(ctx context.Context, reason string) { | |||
| 122 | + | if wv == nil || wv.destroyed.Load() { | |||
| 123 | + | return | |||
| 124 | + | } | |||
| 125 | + | seq := wv.viewportResizePulseSeq.Add(1) | |||
| 126 | + | for _, delayMs := range [...]int64{16, 48} { | |||
| 127 | + | task := cefNewTask(cefTaskFunc(func() { | |||
| 128 | + | if wv == nil || wv.destroyed.Load() || wv.viewportResizePulseSeq.Load() != seq { | |||
| 129 | + | return | |||
| 130 | + | } | |||
| 131 | + | wv.mu.RLock() | |||
| 132 | + | host := wv.host | |||
| 133 | + | wv.mu.RUnlock() | |||
| 134 | + | if host == nil { | |||
| 135 | + | return | |||
| 136 | + | } | |||
| 137 | + | host.Invalidate(purecef.PaintElementTypePetView) | |||
| 138 | + | logging.FromContext(ctx).Debug(). | |||
| 139 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 140 | + | Int64("delay_ms", delayMs). | |||
| 141 | + | Str("reason", reason). | |||
| 142 | + | Msg("cef: delayed resize repaint pulse") | |||
| 143 | + | })) | |||
| 144 | + | if task == nil { | |||
| 145 | + | continue | |||
| 146 | + | } | |||
| 147 | + | cefPostDelayedTask(purecef.ThreadIDTidUi, task, delayMs) | |||
| 148 | + | } | |||
| 149 | + | } | |||
| 150 | + | ||||
| 151 | + | func (wv *WebView) tryStartPendingBrowserCreateOnGTKThread(ctx context.Context, reason string) bool { | |||
| 152 | + | if wv == nil || wv.destroyed.Load() || wv.factory == nil || wv.viewBridge == nil { | |||
| 153 | + | return false | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | wv.mu.RLock() | |||
| 157 | + | hasHost := wv.host != nil | |||
| 158 | + | pending := wv.pendingCreate != nil | |||
| 159 | + | wv.mu.RUnlock() | |||
| 160 | + | if hasHost || !pending { | |||
| 161 | + | return false | |||
| 162 | + | } | |||
| 163 | + | ||||
| 164 | + | widget := wv.viewBridge.Widget() | |||
| 165 | + | if widget == nil { | |||
| 166 | + | return false | |||
| 167 | + | } | |||
| 168 | + | width := int32(widget.GetAllocatedWidth()) | |||
| 169 | + | height := int32(widget.GetAllocatedHeight()) | |||
| 170 | + | if width <= 0 { | |||
| 171 | + | width = int32(widget.GetWidth()) | |||
| 172 | + | } | |||
| 173 | + | if height <= 0 { | |||
| 174 | + | height = int32(widget.GetHeight()) | |||
| 175 | + | } | |||
| 176 | + | if width <= 0 || height <= 0 { | |||
| 177 | + | logging.FromContext(ctx).Debug(). | |||
| 178 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 179 | + | Int32("width", width). | |||
| 180 | + | Int32("height", height). | |||
| 181 | + | Str("reason", reason). | |||
| 182 | + | Msg("cef: viewport sync skipped pending browser creation; widget has no allocation yet") | |||
| 183 | + | return false | |||
| 184 | + | } | |||
| 185 | + | if err := wv.viewBridge.PrepareOnGTKThread(); err != nil { | |||
| 186 | + | logging.FromContext(ctx).Debug(). | |||
| 187 | + | Err(err). | |||
| 188 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 189 | + | Int32("width", width). | |||
| 190 | + | Int32("height", height). | |||
| 191 | + | Str("reason", reason). | |||
| 192 | + | Msg("cef: viewport sync could not prepare bridge for pending browser creation") | |||
| 193 | + | return false | |||
| 194 | + | } | |||
| 195 | + | ||||
| 196 | + | wv.factory.postPendingBrowserCreate(ctx, wv, width, height) | |||
| 197 | + | logging.FromContext(ctx).Debug(). | |||
| 198 | + | Uint64("webview_id", uint64(wv.id)). | |||
| 199 | + | Int32("width", width). | |||
| 200 | + | Int32("height", height). | |||
| 201 | + | Str("reason", reason). | |||
| 202 | + | Msg("cef: viewport sync nudged pending browser creation") | |||
| 203 | + | return true | |||
| 204 | + | } | |||
| 205 | + | ||||
| 206 | + | func (wv *WebView) installViewportSyncHooks() { | |||
| 207 | + | if wv == nil || wv.viewBridge == nil { | |||
| 208 | + | return | |||
| 209 | + | } | |||
| 210 | + | wv.runOnGTKSync(func() { | |||
| 211 | + | wv.installViewportSyncHooksOnGTKThread() | |||
| 212 | + | }) | |||
| 213 | + | } | |||
| 214 | + | ||||
| 215 | + | func (wv *WebView) installViewportSyncHooksOnGTKThread() { | |||
| 216 | + | if wv == nil || wv.viewBridge == nil || wv.viewportMapFunc != nil { | |||
| 217 | + | return | |||
| 218 | + | } | |||
| 219 | + | widget := wv.viewBridge.Widget() | |||
| 220 | + | if widget == nil { | |||
| 221 | + | return | |||
| 222 | + | } | |||
| 223 | + | ||||
| 224 | + | wv.viewportMapFunc = func(gtk.Widget) { | |||
| 225 | + | wv.SyncViewport(wv.ctx, "gtk-map") | |||
| 226 | + | } | |||
| 227 | + | wv.viewportShowFunc = func(gtk.Widget) { | |||
| 228 | + | wv.SyncViewport(wv.ctx, "gtk-show") | |||
| 229 | + | } | |||
| 230 | + | wv.viewportRealizeFunc = func(gtk.Widget) { | |||
| 231 | + | wv.SyncViewport(wv.ctx, "gtk-realize") | |||
| 232 | + | } | |||
| 233 | + | ||||
| 234 | + | wv.viewportMapSignalID = widget.ConnectMap(&wv.viewportMapFunc) | |||
| 235 | + | wv.viewportShowSignalID = widget.ConnectShow(&wv.viewportShowFunc) | |||
| 236 | + | wv.viewportRealizeSignalID = widget.ConnectRealize(&wv.viewportRealizeFunc) | |||
| 237 | + | } | |||
| 238 | + | ||||
| 239 | + | func (wv *WebView) disconnectViewportSyncHooksOnGTKThread() { | |||
| 240 | + | if wv == nil || wv.viewBridge == nil { | |||
| 241 | + | return | |||
| 242 | + | } | |||
| 243 | + | widget := wv.viewBridge.Widget() | |||
| 244 | + | if widget != nil { | |||
| 245 | + | widgetPtr := widget.GoPointer() | |||
| 246 | + | if widgetPtr != 0 { | |||
| 247 | + | obj := gobject.ObjectNewFromInternalPtr(widgetPtr) | |||
| 248 | + | if wv.viewportMapSignalID != 0 { | |||
| 249 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportMapSignalID) | |||
| 250 | + | wv.viewportMapSignalID = 0 | |||
| 251 | + | } | |||
| 252 | + | if wv.viewportShowSignalID != 0 { | |||
| 253 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportShowSignalID) | |||
| 254 | + | wv.viewportShowSignalID = 0 | |||
| 255 | + | } | |||
| 256 | + | if wv.viewportRealizeSignalID != 0 { | |||
| 257 | + | gobject.SignalHandlerDisconnect(obj, wv.viewportRealizeSignalID) | |||
| 258 | + | wv.viewportRealizeSignalID = 0 | |||
| 259 | + | } | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | wv.viewportMapFunc = nil | |||
| 263 | + | wv.viewportShowFunc = nil | |||
| 264 | + | wv.viewportRealizeFunc = nil | |||
| 265 | + | } | |||
| 266 | + | ||||
| 267 | + | func notifyBrowserViewportSync(host viewportSyncBrowserHost, visible bool) { | |||
| 268 | + | if host == nil { | |||
| 269 | + | return | |||
| 270 | + | } | |||
| 271 | + | if visible { | |||
| 272 | + | host.WasHidden(0) | |||
| 273 | + | } | |||
| 274 | + | host.NotifyScreenInfoChanged() | |||
| 275 | + | notifyBrowserResize(host) | |||
| 276 | + | } | |||
A internal/infrastructure/cef/webview_viewport_test.go +87 -0
| @@ -0,0 +1,87 @@ | |||
| 1 | + | package cef | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | type viewportSyncOrderHost struct { | |
| 12 | + | purecef.BrowserHost | |
| 13 | + | calls []string | |
| 14 | + | } | |
| 15 | + | ||
| 16 | + | func (h *viewportSyncOrderHost) WasHidden(state int32) { | |
| 17 | + | h.calls = append(h.calls, "WasHidden") | |
| 18 | + | if state != 0 { | |
| 19 | + | h.calls = append(h.calls, "WasHidden(nonzero)") | |
| 20 | + | } | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func (h *viewportSyncOrderHost) NotifyScreenInfoChanged() { | |
| 24 | + | h.calls = append(h.calls, "NotifyScreenInfoChanged") | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | func (h *viewportSyncOrderHost) WasResized() { | |
| 28 | + | h.calls = append(h.calls, "WasResized") | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | func (h *viewportSyncOrderHost) Invalidate(_ purecef.PaintElementType) { | |
| 32 | + | h.calls = append(h.calls, "Invalidate") | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | func TestNotifyBrowserViewportSync_VisibleCallsFullSequence(t *testing.T) { | |
| 36 | + | host := &viewportSyncOrderHost{} | |
| 37 | + | ||
| 38 | + | notifyBrowserViewportSync(host, true) | |
| 39 | + | ||
| 40 | + | require.Equal(t, []string{"WasHidden", "NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | |
| 41 | + | } | |
| 42 | + | ||
| 43 | + | func TestNotifyBrowserViewportSync_HiddenSkipsWasHidden(t *testing.T) { | |
| 44 | + | host := &viewportSyncOrderHost{} | |
| 45 | + | ||
| 46 | + | notifyBrowserViewportSync(host, false) | |
| 47 | + | ||
| 48 | + | require.Equal(t, []string{"NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | func TestScheduleResizeRepaintPulse_CoalescesToLatestSequence(t *testing.T) { | |
| 52 | + | oldNewTask := cefNewTask | |
| 53 | + | oldPostDelayedTask := cefPostDelayedTask | |
| 54 | + | defer func() { | |
| 55 | + | cefNewTask = oldNewTask | |
| 56 | + | cefPostDelayedTask = oldPostDelayedTask | |
| 57 | + | }() | |
| 58 | + | ||
| 59 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 60 | + | ||
| 61 | + | var scheduled []purecef.Task | |
| 62 | + | var delays []int64 | |
| 63 | + | cefPostDelayedTask = func(threadID purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |
| 64 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |
| 65 | + | require.NotNil(t, task) | |
| 66 | + | scheduled = append(scheduled, task) | |
| 67 | + | delays = append(delays, delayMs) | |
| 68 | + | return 1 | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | host := &viewportSyncOrderHost{} | |
| 72 | + | wv := &WebView{ctx: context.Background(), host: host} | |
| 73 | + | ||
| 74 | + | wv.scheduleResizeRepaintPulse(context.Background(), "first") | |
| 75 | + | wv.scheduleResizeRepaintPulse(context.Background(), "second") | |
| 76 | + | ||
| 77 | + | require.Equal(t, []int64{16, 48, 16, 48}, delays) | |
| 78 | + | require.Len(t, scheduled, 4) | |
| 79 | + | ||
| 80 | + | scheduled[0].Execute() | |
| 81 | + | scheduled[1].Execute() | |
| 82 | + | require.Empty(t, host.calls) | |
| 83 | + | ||
| 84 | + | scheduled[2].Execute() | |
| 85 | + | scheduled[3].Execute() | |
| 86 | + | require.Equal(t, []string{"Invalidate", "Invalidate"}, host.calls) | |
| 87 | + | } | |
| @@ -0,0 +1,87 @@ | |||||
| 1 | + | package cef | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | purecef "github.com/bnema/purego-cef/cef" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | type viewportSyncOrderHost struct { | |||
| 12 | + | purecef.BrowserHost | |||
| 13 | + | calls []string | |||
| 14 | + | } | |||
| 15 | + | ||||
| 16 | + | func (h *viewportSyncOrderHost) WasHidden(state int32) { | |||
| 17 | + | h.calls = append(h.calls, "WasHidden") | |||
| 18 | + | if state != 0 { | |||
| 19 | + | h.calls = append(h.calls, "WasHidden(nonzero)") | |||
| 20 | + | } | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func (h *viewportSyncOrderHost) NotifyScreenInfoChanged() { | |||
| 24 | + | h.calls = append(h.calls, "NotifyScreenInfoChanged") | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | func (h *viewportSyncOrderHost) WasResized() { | |||
| 28 | + | h.calls = append(h.calls, "WasResized") | |||
| 29 | + | } | |||
| 30 | + | ||||
| 31 | + | func (h *viewportSyncOrderHost) Invalidate(_ purecef.PaintElementType) { | |||
| 32 | + | h.calls = append(h.calls, "Invalidate") | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | func TestNotifyBrowserViewportSync_VisibleCallsFullSequence(t *testing.T) { | |||
| 36 | + | host := &viewportSyncOrderHost{} | |||
| 37 | + | ||||
| 38 | + | notifyBrowserViewportSync(host, true) | |||
| 39 | + | ||||
| 40 | + | require.Equal(t, []string{"WasHidden", "NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | |||
| 41 | + | } | |||
| 42 | + | ||||
| 43 | + | func TestNotifyBrowserViewportSync_HiddenSkipsWasHidden(t *testing.T) { | |||
| 44 | + | host := &viewportSyncOrderHost{} | |||
| 45 | + | ||||
| 46 | + | notifyBrowserViewportSync(host, false) | |||
| 47 | + | ||||
| 48 | + | require.Equal(t, []string{"NotifyScreenInfoChanged", "WasResized", "Invalidate"}, host.calls) | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | func TestScheduleResizeRepaintPulse_CoalescesToLatestSequence(t *testing.T) { | |||
| 52 | + | oldNewTask := cefNewTask | |||
| 53 | + | oldPostDelayedTask := cefPostDelayedTask | |||
| 54 | + | defer func() { | |||
| 55 | + | cefNewTask = oldNewTask | |||
| 56 | + | cefPostDelayedTask = oldPostDelayedTask | |||
| 57 | + | }() | |||
| 58 | + | ||||
| 59 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 60 | + | ||||
| 61 | + | var scheduled []purecef.Task | |||
| 62 | + | var delays []int64 | |||
| 63 | + | cefPostDelayedTask = func(threadID purecef.ThreadID, task purecef.Task, delayMs int64) int32 { | |||
| 64 | + | require.Equal(t, purecef.ThreadIDTidUi, threadID) | |||
| 65 | + | require.NotNil(t, task) | |||
| 66 | + | scheduled = append(scheduled, task) | |||
| 67 | + | delays = append(delays, delayMs) | |||
| 68 | + | return 1 | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | host := &viewportSyncOrderHost{} | |||
| 72 | + | wv := &WebView{ctx: context.Background(), host: host} | |||
| 73 | + | ||||
| 74 | + | wv.scheduleResizeRepaintPulse(context.Background(), "first") | |||
| 75 | + | wv.scheduleResizeRepaintPulse(context.Background(), "second") | |||
| 76 | + | ||||
| 77 | + | require.Equal(t, []int64{16, 48, 16, 48}, delays) | |||
| 78 | + | require.Len(t, scheduled, 4) | |||
| 79 | + | ||||
| 80 | + | scheduled[0].Execute() | |||
| 81 | + | scheduled[1].Execute() | |||
| 82 | + | require.Empty(t, host.calls) | |||
| 83 | + | ||||
| 84 | + | scheduled[2].Execute() | |||
| 85 | + | scheduled[3].Execute() | |||
| 86 | + | require.Equal(t, []string{"Invalidate", "Invalidate"}, host.calls) | |||
| 87 | + | } | |||
M internal/ui/app.go +53 -14
| @@ -3686,6 +3686,18 @@ func (a *App) createWorkspaceViewWithoutAttach(ctx context.Context, tab *entity. | |||
| 3686 | 3686 | return | |
| 3687 | 3687 | } | |
| 3688 | 3688 | a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget()) | |
| 3689 | + | if a.contentCoord != nil { | |
| 3690 | + | syncCtx := context.Background() | |
| 3691 | + | if a.deps != nil && a.deps.Ctx != nil { | |
| 3692 | + | syncCtx = a.deps.Ctx | |
| 3693 | + | } | |
| 3694 | + | wsView.SetOnWebViewAttached(func(paneID entity.PaneID) { | |
| 3695 | + | a.contentCoord.SyncWebViewViewport(syncCtx, paneID, "workspace-widget-attached") | |
| 3696 | + | }) | |
| 3697 | + | wsView.SetOnActivePaneChanged(func(paneID entity.PaneID) { | |
| 3698 | + | a.contentCoord.SyncWebViewViewport(syncCtx, paneID, "workspace-pane-activated") | |
| 3699 | + | }) | |
| 3700 | + | } | |
| 3689 | 3701 | ||
| 3690 | 3702 | // Set the workspace | |
| 3691 | 3703 | if err := wsView.SetWorkspace(ctx, tab.Workspace); err != nil { | |
| @@ -3792,28 +3804,48 @@ func (a *App) getActiveWebViewTarget() port.TextInputTarget { | |||
| 3792 | 3804 | // This is called when a popup uses tabbed behavior. | |
| 3793 | 3805 | func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) { | |
| 3794 | 3806 | log := logging.FromContext(ctx) | |
| 3807 | + | cleanupUnattachedPopup := func() { | |
| 3808 | + | if wv != nil { | |
| 3809 | + | wv.Destroy() | |
| 3810 | + | } | |
| 3811 | + | } | |
| 3795 | 3812 | ||
| 3796 | 3813 | wsView := a.workspaceViews[tabID] | |
| 3797 | 3814 | if wsView == nil { | |
| 3815 | + | cleanupUnattachedPopup() | |
| 3798 | 3816 | log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab") | |
| 3799 | 3817 | return | |
| 3800 | 3818 | } | |
| 3819 | + | if pane == nil { | |
| 3820 | + | cleanupUnattachedPopup() | |
| 3821 | + | log.Warn().Str("tab_id", string(tabID)).Msg("popup pane is nil") | |
| 3822 | + | return | |
| 3823 | + | } | |
| 3824 | + | if wsView.GetPaneView(pane.ID) == nil { | |
| 3825 | + | cleanupUnattachedPopup() | |
| 3826 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | |
| 3827 | + | return | |
| 3828 | + | } | |
| 3829 | + | if a.contentCoord == nil { | |
| 3830 | + | cleanupUnattachedPopup() | |
| 3831 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("content coordinator not configured for popup") | |
| 3832 | + | return | |
| 3833 | + | } | |
| 3801 | 3834 | ||
| 3802 | - | // Register WebView with content coordinator | |
| 3803 | - | if a.contentCoord != nil { | |
| 3804 | - | // Track the WebView | |
| 3805 | - | a.contentCoord.RegisterPopupWebView(pane.ID, wv) | |
| 3835 | + | // Register WebView with content coordinator. | |
| 3836 | + | a.contentCoord.RegisterPopupWebView(pane.ID, wv) | |
| 3806 | 3837 | ||
| 3807 | - | // Wrap and attach widget | |
| 3808 | - | widget := a.contentCoord.WrapWidget(ctx, wv) | |
| 3809 | - | if widget != nil { | |
| 3810 | - | paneView := wsView.GetPaneView(pane.ID) | |
| 3811 | - | if paneView != nil { | |
| 3812 | - | paneView.SetWebViewWidget(widget) | |
| 3813 | - | } else { | |
| 3814 | - | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | |
| 3815 | - | } | |
| 3816 | - | } | |
| 3838 | + | // Wrap and attach widget. | |
| 3839 | + | widget := a.contentCoord.WrapWidget(ctx, wv) | |
| 3840 | + | if widget == nil { | |
| 3841 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) | |
| 3842 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("failed to wrap popup webview") | |
| 3843 | + | return | |
| 3844 | + | } | |
| 3845 | + | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | |
| 3846 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) | |
| 3847 | + | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | |
| 3848 | + | return | |
| 3817 | 3849 | } | |
| 3818 | 3850 | log.Debug(). | |
| 3819 | 3851 | Str("tab_id", string(tabID)). | |
| @@ -4393,6 +4425,13 @@ func (a *App) resizeFloatingWidget(session *floatingWorkspaceSession) { | |||
| 4393 | 4425 | session.widget.SetSizeRequest(width, height) | |
| 4394 | 4426 | session.appliedWidth = width | |
| 4395 | 4427 | session.appliedHeight = height | |
| 4428 | + | if a.contentCoord != nil { | |
| 4429 | + | syncCtx := context.Background() | |
| 4430 | + | if a.deps != nil && a.deps.Ctx != nil { | |
| 4431 | + | syncCtx = a.deps.Ctx | |
| 4432 | + | } | |
| 4433 | + | a.contentCoord.SyncWebViewViewport(syncCtx, session.paneID, "floating-resize") | |
| 4434 | + | } | |
| 4396 | 4435 | } | |
| 4397 | 4436 | ||
| 4398 | 4437 | func (a *App) handleFloatingViewportTick(session *floatingWorkspaceSession) bool { | |
| @@ -3686,6 +3686,18 @@ func (a *App) createWorkspaceViewWithoutAttach(ctx context.Context, tab *entity. | |||||
| 3686 | return | 3686 | return | ||
| 3687 | } | 3687 | } | ||
| 3688 | a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget()) | 3688 | a.installFloatingOverlayPositioning(tab.ID, wsView.WorkspaceOverlayWidget()) | ||
| 3689 | + | if a.contentCoord != nil { | |||
| 3690 | + | syncCtx := context.Background() | |||
| 3691 | + | if a.deps != nil && a.deps.Ctx != nil { | |||
| 3692 | + | syncCtx = a.deps.Ctx | |||
| 3693 | + | } | |||
| 3694 | + | wsView.SetOnWebViewAttached(func(paneID entity.PaneID) { | |||
| 3695 | + | a.contentCoord.SyncWebViewViewport(syncCtx, paneID, "workspace-widget-attached") | |||
| 3696 | + | }) | |||
| 3697 | + | wsView.SetOnActivePaneChanged(func(paneID entity.PaneID) { | |||
| 3698 | + | a.contentCoord.SyncWebViewViewport(syncCtx, paneID, "workspace-pane-activated") | |||
| 3699 | + | }) | |||
| 3700 | + | } | |||
| 3689 | 3701 | ||||
| 3690 | // Set the workspace | 3702 | // Set the workspace | ||
| 3691 | if err := wsView.SetWorkspace(ctx, tab.Workspace); err != nil { | 3703 | if err := wsView.SetWorkspace(ctx, tab.Workspace); err != nil { | ||
| @@ -3792,28 +3804,48 @@ func (a *App) getActiveWebViewTarget() port.TextInputTarget { | |||||
| 3792 | // This is called when a popup uses tabbed behavior. | 3804 | // This is called when a popup uses tabbed behavior. | ||
| 3793 | func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) { | 3805 | func (a *App) attachPopupToTab(ctx context.Context, tabID entity.TabID, pane *entity.Pane, wv port.WebView) { | ||
| 3794 | log := logging.FromContext(ctx) | 3806 | log := logging.FromContext(ctx) | ||
| 3807 | + | cleanupUnattachedPopup := func() { | |||
| 3808 | + | if wv != nil { | |||
| 3809 | + | wv.Destroy() | |||
| 3810 | + | } | |||
| 3811 | + | } | |||
| 3795 | 3812 | ||||
| 3796 | wsView := a.workspaceViews[tabID] | 3813 | wsView := a.workspaceViews[tabID] | ||
| 3797 | if wsView == nil { | 3814 | if wsView == nil { | ||
| 3815 | + | cleanupUnattachedPopup() | |||
| 3798 | log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab") | 3816 | log.Warn().Str("tab_id", string(tabID)).Msg("workspace view not found for popup tab") | ||
| 3799 | return | 3817 | return | ||
| 3800 | } | 3818 | } | ||
| 3819 | + | if pane == nil { | |||
| 3820 | + | cleanupUnattachedPopup() | |||
| 3821 | + | log.Warn().Str("tab_id", string(tabID)).Msg("popup pane is nil") | |||
| 3822 | + | return | |||
| 3823 | + | } | |||
| 3824 | + | if wsView.GetPaneView(pane.ID) == nil { | |||
| 3825 | + | cleanupUnattachedPopup() | |||
| 3826 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | |||
| 3827 | + | return | |||
| 3828 | + | } | |||
| 3829 | + | if a.contentCoord == nil { | |||
| 3830 | + | cleanupUnattachedPopup() | |||
| 3831 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("content coordinator not configured for popup") | |||
| 3832 | + | return | |||
| 3833 | + | } | |||
| 3801 | 3834 | ||||
| 3802 | - | // Register WebView with content coordinator | 3835 | + | // Register WebView with content coordinator. |
| 3803 | - | if a.contentCoord != nil { | 3836 | + | a.contentCoord.RegisterPopupWebView(pane.ID, wv) |
| 3804 | - | // Track the WebView | |||
| 3805 | - | a.contentCoord.RegisterPopupWebView(pane.ID, wv) | |||
| 3806 | 3837 | ||||
| 3807 | - | // Wrap and attach widget | 3838 | + | // Wrap and attach widget. |
| 3808 | - | widget := a.contentCoord.WrapWidget(ctx, wv) | 3839 | + | widget := a.contentCoord.WrapWidget(ctx, wv) |
| 3809 | - | if widget != nil { | 3840 | + | if widget == nil { |
| 3810 | - | paneView := wsView.GetPaneView(pane.ID) | 3841 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) |
| 3811 | - | if paneView != nil { | 3842 | + | log.Warn().Str("pane_id", string(pane.ID)).Msg("failed to wrap popup webview") |
| 3812 | - | paneView.SetWebViewWidget(widget) | 3843 | + | return |
| 3813 | - | } else { | 3844 | + | } |
| 3814 | - | log.Warn().Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") | 3845 | + | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { |
| 3815 | - | } | 3846 | + | a.contentCoord.ReleaseWebView(ctx, pane.ID) |
| 3816 | - | } | 3847 | + | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("pane view not found for popup") |
| 3848 | + | return | |||
| 3817 | } | 3849 | } | ||
| 3818 | log.Debug(). | 3850 | log.Debug(). | ||
| 3819 | Str("tab_id", string(tabID)). | 3851 | Str("tab_id", string(tabID)). | ||
| @@ -4393,6 +4425,13 @@ func (a *App) resizeFloatingWidget(session *floatingWorkspaceSession) { | |||||
| 4393 | session.widget.SetSizeRequest(width, height) | 4425 | session.widget.SetSizeRequest(width, height) | ||
| 4394 | session.appliedWidth = width | 4426 | session.appliedWidth = width | ||
| 4395 | session.appliedHeight = height | 4427 | session.appliedHeight = height | ||
| 4428 | + | if a.contentCoord != nil { | |||
| 4429 | + | syncCtx := context.Background() | |||
| 4430 | + | if a.deps != nil && a.deps.Ctx != nil { | |||
| 4431 | + | syncCtx = a.deps.Ctx | |||
| 4432 | + | } | |||
| 4433 | + | a.contentCoord.SyncWebViewViewport(syncCtx, session.paneID, "floating-resize") | |||
| 4434 | + | } | |||
| 4396 | } | 4435 | } | ||
| 4397 | 4436 | ||||
| 4398 | func (a *App) handleFloatingViewportTick(session *floatingWorkspaceSession) bool { | 4437 | func (a *App) handleFloatingViewportTick(session *floatingWorkspaceSession) bool { | ||
M internal/ui/app_browser_launch_test.go +92 -2
| @@ -1763,6 +1763,7 @@ type fakeRecordingWebView struct { | |||
| 1763 | 1763 | setZoomLevelLastLevel float64 | |
| 1764 | 1764 | openDevToolsCalls int | |
| 1765 | 1765 | printPageCalls int | |
| 1766 | + | destroyCalls int | |
| 1766 | 1767 | } | |
| 1767 | 1768 | ||
| 1768 | 1769 | func (f *fakeRecordingWebView) ID() port.WebViewID { return f.id } | |
| @@ -1832,8 +1833,97 @@ func (f *fakeRecordingWebView) Favicon() port.Texture { retu | |||
| 1832 | 1833 | func (f *fakeRecordingWebView) Generation() uint64 { return 0 } | |
| 1833 | 1834 | func (f *fakeRecordingWebView) IsFullscreen() bool { return false } | |
| 1834 | 1835 | func (f *fakeRecordingWebView) IsPlayingAudio() bool { return false } | |
| 1835 | - | func (f *fakeRecordingWebView) IsDestroyed() bool { return false } | |
| 1836 | - | func (f *fakeRecordingWebView) Destroy() {} | |
| 1836 | + | func (f *fakeRecordingWebView) IsDestroyed() bool { return f.destroyCalls > 0 } | |
| 1837 | + | func (f *fakeRecordingWebView) Destroy() { f.destroyCalls++ } | |
| 1838 | + | ||
| 1839 | + | func TestApp_AttachPopupToTabDestroysPopupWhenWorkspaceViewMissing(t *testing.T) { | |
| 1840 | + | ctx := context.Background() | |
| 1841 | + | popupWV := &fakeRecordingWebView{id: 1} | |
| 1842 | + | app := &App{workspaceViews: map[entity.TabID]*component.WorkspaceView{}} | |
| 1843 | + | ||
| 1844 | + | app.attachPopupToTab(ctx, entity.TabID("missing-tab"), entity.NewPane(entity.PaneID("popup-pane")), popupWV) | |
| 1845 | + | ||
| 1846 | + | if popupWV.destroyCalls != 1 { | |
| 1847 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |
| 1848 | + | } | |
| 1849 | + | } | |
| 1850 | + | ||
| 1851 | + | func TestApp_AttachPopupToTabDestroysPopupWhenPaneNil(t *testing.T) { | |
| 1852 | + | ctx := context.Background() | |
| 1853 | + | popupWV := &fakeRecordingWebView{id: 1} | |
| 1854 | + | tabID := entity.TabID("tab-1") | |
| 1855 | + | app := &App{workspaceViews: map[entity.TabID]*component.WorkspaceView{tabID: &component.WorkspaceView{}}} | |
| 1856 | + | ||
| 1857 | + | app.attachPopupToTab(ctx, tabID, nil, popupWV) | |
| 1858 | + | ||
| 1859 | + | if popupWV.destroyCalls != 1 { | |
| 1860 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |
| 1861 | + | } | |
| 1862 | + | } | |
| 1863 | + | ||
| 1864 | + | func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T) { | |
| 1865 | + | ctx := context.Background() | |
| 1866 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |
| 1867 | + | tabID := entity.TabID("tab-1") | |
| 1868 | + | pane := entity.NewPane(entity.PaneID("missing-pane")) | |
| 1869 | + | app := &App{ | |
| 1870 | + | contentCoord: contentCoord, | |
| 1871 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ | |
| 1872 | + | tabID: &component.WorkspaceView{}, | |
| 1873 | + | }, | |
| 1874 | + | } | |
| 1875 | + | ||
| 1876 | + | popupWV := &fakeRecordingWebView{id: 1} | |
| 1877 | + | app.attachPopupToTab(ctx, tabID, pane, popupWV) | |
| 1878 | + | ||
| 1879 | + | if got := contentCoord.GetWebView(pane.ID); got != nil { | |
| 1880 | + | t.Fatalf("popup webview was registered for missing pane view: %v", got) | |
| 1881 | + | } | |
| 1882 | + | if popupWV.destroyCalls != 1 { | |
| 1883 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |
| 1884 | + | } | |
| 1885 | + | } | |
| 1886 | + | ||
| 1887 | + | func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) { | |
| 1888 | + | ctx := context.Background() | |
| 1889 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |
| 1890 | + | tabID := entity.TabID("tab-1") | |
| 1891 | + | pane := entity.NewPane(entity.PaneID("popup-pane")) | |
| 1892 | + | ||
| 1893 | + | factory := layoutmocks.NewMockWidgetFactory(t) | |
| 1894 | + | container := layoutmocks.NewMockBoxWidget(t) | |
| 1895 | + | overlay := layoutmocks.NewMockOverlayWidget(t) | |
| 1896 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(container).Once() | |
| 1897 | + | container.EXPECT().SetHexpand(true).Once() | |
| 1898 | + | container.EXPECT().SetVexpand(true).Once() | |
| 1899 | + | container.EXPECT().SetVisible(true).Once() | |
| 1900 | + | factory.EXPECT().NewOverlay().Return(overlay).Once() | |
| 1901 | + | overlay.EXPECT().SetHexpand(true).Once() | |
| 1902 | + | overlay.EXPECT().SetVexpand(true).Once() | |
| 1903 | + | overlay.EXPECT().SetChild(container).Once() | |
| 1904 | + | overlay.EXPECT().SetVisible(true).Once() | |
| 1905 | + | ||
| 1906 | + | wsView := component.NewWorkspaceView(ctx, factory) | |
| 1907 | + | container.EXPECT().AddCssClass("single-pane").Once() | |
| 1908 | + | wsView.RegisterPaneView(pane.ID, &component.PaneView{}) | |
| 1909 | + | ||
| 1910 | + | app := &App{ | |
| 1911 | + | contentCoord: contentCoord, | |
| 1912 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ | |
| 1913 | + | tabID: wsView, | |
| 1914 | + | }, | |
| 1915 | + | } | |
| 1916 | + | popupWV := &fakeRecordingWebView{id: 1} | |
| 1917 | + | ||
| 1918 | + | app.attachPopupToTab(ctx, tabID, pane, popupWV) | |
| 1919 | + | ||
| 1920 | + | if got := contentCoord.GetWebView(pane.ID); got != nil { | |
| 1921 | + | t.Fatalf("popup webview registration remained after wrap failure: %v", got) | |
| 1922 | + | } | |
| 1923 | + | if popupWV.destroyCalls != 1 { | |
| 1924 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |
| 1925 | + | } | |
| 1926 | + | } | |
| 1837 | 1927 | ||
| 1838 | 1928 | type fakeZoomRepo struct{} | |
| 1839 | 1929 | ||
| @@ -1763,6 +1763,7 @@ type fakeRecordingWebView struct { | |||||
| 1763 | setZoomLevelLastLevel float64 | 1763 | setZoomLevelLastLevel float64 | ||
| 1764 | openDevToolsCalls int | 1764 | openDevToolsCalls int | ||
| 1765 | printPageCalls int | 1765 | printPageCalls int | ||
| 1766 | + | destroyCalls int | |||
| 1766 | } | 1767 | } | ||
| 1767 | 1768 | ||||
| 1768 | func (f *fakeRecordingWebView) ID() port.WebViewID { return f.id } | 1769 | func (f *fakeRecordingWebView) ID() port.WebViewID { return f.id } | ||
| @@ -1832,8 +1833,97 @@ func (f *fakeRecordingWebView) Favicon() port.Texture { retu | |||||
| 1832 | func (f *fakeRecordingWebView) Generation() uint64 { return 0 } | 1833 | func (f *fakeRecordingWebView) Generation() uint64 { return 0 } | ||
| 1833 | func (f *fakeRecordingWebView) IsFullscreen() bool { return false } | 1834 | func (f *fakeRecordingWebView) IsFullscreen() bool { return false } | ||
| 1834 | func (f *fakeRecordingWebView) IsPlayingAudio() bool { return false } | 1835 | func (f *fakeRecordingWebView) IsPlayingAudio() bool { return false } | ||
| 1835 | - | func (f *fakeRecordingWebView) IsDestroyed() bool { return false } | 1836 | + | func (f *fakeRecordingWebView) IsDestroyed() bool { return f.destroyCalls > 0 } |
| 1836 | - | func (f *fakeRecordingWebView) Destroy() {} | 1837 | + | func (f *fakeRecordingWebView) Destroy() { f.destroyCalls++ } |
| 1838 | + | ||||
| 1839 | + | func TestApp_AttachPopupToTabDestroysPopupWhenWorkspaceViewMissing(t *testing.T) { | |||
| 1840 | + | ctx := context.Background() | |||
| 1841 | + | popupWV := &fakeRecordingWebView{id: 1} | |||
| 1842 | + | app := &App{workspaceViews: map[entity.TabID]*component.WorkspaceView{}} | |||
| 1843 | + | ||||
| 1844 | + | app.attachPopupToTab(ctx, entity.TabID("missing-tab"), entity.NewPane(entity.PaneID("popup-pane")), popupWV) | |||
| 1845 | + | ||||
| 1846 | + | if popupWV.destroyCalls != 1 { | |||
| 1847 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |||
| 1848 | + | } | |||
| 1849 | + | } | |||
| 1850 | + | ||||
| 1851 | + | func TestApp_AttachPopupToTabDestroysPopupWhenPaneNil(t *testing.T) { | |||
| 1852 | + | ctx := context.Background() | |||
| 1853 | + | popupWV := &fakeRecordingWebView{id: 1} | |||
| 1854 | + | tabID := entity.TabID("tab-1") | |||
| 1855 | + | app := &App{workspaceViews: map[entity.TabID]*component.WorkspaceView{tabID: &component.WorkspaceView{}}} | |||
| 1856 | + | ||||
| 1857 | + | app.attachPopupToTab(ctx, tabID, nil, popupWV) | |||
| 1858 | + | ||||
| 1859 | + | if popupWV.destroyCalls != 1 { | |||
| 1860 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |||
| 1861 | + | } | |||
| 1862 | + | } | |||
| 1863 | + | ||||
| 1864 | + | func TestApp_AttachPopupToTabSkipsRegistrationWhenPaneViewMissing(t *testing.T) { | |||
| 1865 | + | ctx := context.Background() | |||
| 1866 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |||
| 1867 | + | tabID := entity.TabID("tab-1") | |||
| 1868 | + | pane := entity.NewPane(entity.PaneID("missing-pane")) | |||
| 1869 | + | app := &App{ | |||
| 1870 | + | contentCoord: contentCoord, | |||
| 1871 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ | |||
| 1872 | + | tabID: &component.WorkspaceView{}, | |||
| 1873 | + | }, | |||
| 1874 | + | } | |||
| 1875 | + | ||||
| 1876 | + | popupWV := &fakeRecordingWebView{id: 1} | |||
| 1877 | + | app.attachPopupToTab(ctx, tabID, pane, popupWV) | |||
| 1878 | + | ||||
| 1879 | + | if got := contentCoord.GetWebView(pane.ID); got != nil { | |||
| 1880 | + | t.Fatalf("popup webview was registered for missing pane view: %v", got) | |||
| 1881 | + | } | |||
| 1882 | + | if popupWV.destroyCalls != 1 { | |||
| 1883 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |||
| 1884 | + | } | |||
| 1885 | + | } | |||
| 1886 | + | ||||
| 1887 | + | func TestApp_AttachPopupToTabReleasesRegistrationWhenWrapFails(t *testing.T) { | |||
| 1888 | + | ctx := context.Background() | |||
| 1889 | + | contentCoord := contentcoord.NewCoordinator(ctx, nil, nil, nil, nil, nil, nil, nil) | |||
| 1890 | + | tabID := entity.TabID("tab-1") | |||
| 1891 | + | pane := entity.NewPane(entity.PaneID("popup-pane")) | |||
| 1892 | + | ||||
| 1893 | + | factory := layoutmocks.NewMockWidgetFactory(t) | |||
| 1894 | + | container := layoutmocks.NewMockBoxWidget(t) | |||
| 1895 | + | overlay := layoutmocks.NewMockOverlayWidget(t) | |||
| 1896 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(container).Once() | |||
| 1897 | + | container.EXPECT().SetHexpand(true).Once() | |||
| 1898 | + | container.EXPECT().SetVexpand(true).Once() | |||
| 1899 | + | container.EXPECT().SetVisible(true).Once() | |||
| 1900 | + | factory.EXPECT().NewOverlay().Return(overlay).Once() | |||
| 1901 | + | overlay.EXPECT().SetHexpand(true).Once() | |||
| 1902 | + | overlay.EXPECT().SetVexpand(true).Once() | |||
| 1903 | + | overlay.EXPECT().SetChild(container).Once() | |||
| 1904 | + | overlay.EXPECT().SetVisible(true).Once() | |||
| 1905 | + | ||||
| 1906 | + | wsView := component.NewWorkspaceView(ctx, factory) | |||
| 1907 | + | container.EXPECT().AddCssClass("single-pane").Once() | |||
| 1908 | + | wsView.RegisterPaneView(pane.ID, &component.PaneView{}) | |||
| 1909 | + | ||||
| 1910 | + | app := &App{ | |||
| 1911 | + | contentCoord: contentCoord, | |||
| 1912 | + | workspaceViews: map[entity.TabID]*component.WorkspaceView{ | |||
| 1913 | + | tabID: wsView, | |||
| 1914 | + | }, | |||
| 1915 | + | } | |||
| 1916 | + | popupWV := &fakeRecordingWebView{id: 1} | |||
| 1917 | + | ||||
| 1918 | + | app.attachPopupToTab(ctx, tabID, pane, popupWV) | |||
| 1919 | + | ||||
| 1920 | + | if got := contentCoord.GetWebView(pane.ID); got != nil { | |||
| 1921 | + | t.Fatalf("popup webview registration remained after wrap failure: %v", got) | |||
| 1922 | + | } | |||
| 1923 | + | if popupWV.destroyCalls != 1 { | |||
| 1924 | + | t.Fatalf("popup webview destroy calls = %d, want 1", popupWV.destroyCalls) | |||
| 1925 | + | } | |||
| 1926 | + | } | |||
| 1837 | 1927 | ||||
| 1838 | type fakeZoomRepo struct{} | 1928 | type fakeZoomRepo struct{} | ||
| 1839 | 1929 | ||||
M internal/ui/component/workspace_view.go +91 -17
| @@ -4,7 +4,6 @@ package component | |||
| 4 | 4 | import ( | |
| 5 | 5 | "context" | |
| 6 | 6 | "errors" | |
| 7 | - | "fmt" | |
| 8 | 7 | "sync" | |
| 9 | 8 | "sync/atomic" | |
| 10 | 9 | "time" | |
| @@ -51,6 +50,8 @@ type WorkspaceView struct { | |||
| 51 | 50 | paneViews map[entity.PaneID]*PaneView | |
| 52 | 51 | ||
| 53 | 52 | onPaneFocused func(paneID entity.PaneID) | |
| 53 | + | onActivePaneChanged func(paneID entity.PaneID) | |
| 54 | + | onWebViewAttached func(paneID entity.PaneID) | |
| 54 | 55 | onSplitRatioDragged func(nodeID string, ratio float64) | |
| 55 | 56 | ||
| 56 | 57 | // Hover suppression for keyboard navigation (Issue #89) | |
| @@ -215,12 +216,10 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||
| 215 | 216 | } | |
| 216 | 217 | } | |
| 217 | 218 | ||
| 218 | - | // Set initial active pane | |
| 219 | - | if ws.ActivePaneID != "" { | |
| 220 | - | if err := wv.setActivePaneIDInternal(ws.ActivePaneID); err != nil { | |
| 221 | - | return fmt.Errorf("set initial active pane: %w", err) | |
| 222 | - | } | |
| 223 | - | } | |
| 219 | + | // Set initial active pane. A persisted workspace can contain a stale active | |
| 220 | + | // pane ID after restore/rebuild; choose a valid fallback instead of leaving | |
| 221 | + | // later active-pane logic pointing at a missing pane. | |
| 222 | + | wv.restoreActivePaneInternal() | |
| 224 | 223 | ||
| 225 | 224 | // Update single-pane mode based on pane count | |
| 226 | 225 | wv.updateSinglePaneModeInternal() | |
| @@ -231,9 +230,63 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||
| 231 | 230 | // SetActivePaneID updates which pane is visually marked as active. | |
| 232 | 231 | func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error { | |
| 233 | 232 | wv.mu.Lock() | |
| 234 | - | defer wv.mu.Unlock() | |
| 233 | + | currentActiveID := wv.getActivePaneIDInternal() | |
| 234 | + | err := wv.setActivePaneIDInternal(paneID) | |
| 235 | + | callback := wv.onActivePaneChanged | |
| 236 | + | wv.mu.Unlock() | |
| 237 | + | if err != nil { | |
| 238 | + | return err | |
| 239 | + | } | |
| 240 | + | if currentActiveID != paneID && callback != nil { | |
| 241 | + | callback(paneID) | |
| 242 | + | } | |
| 243 | + | return nil | |
| 244 | + | } | |
| 235 | 245 | ||
| 236 | - | return wv.setActivePaneIDInternal(paneID) | |
| 246 | + | // restoreActivePaneInternal restores the active pane after building paneViews. | |
| 247 | + | // Must be called with the lock held. | |
| 248 | + | func (wv *WorkspaceView) restoreActivePaneInternal() { | |
| 249 | + | if wv.workspace == nil { | |
| 250 | + | return | |
| 251 | + | } | |
| 252 | + | ||
| 253 | + | paneID := wv.workspace.ActivePaneID | |
| 254 | + | if paneID != "" { | |
| 255 | + | if pv, ok := wv.paneViews[paneID]; ok && pv != nil { | |
| 256 | + | _ = wv.setActivePaneIDInternal(paneID) | |
| 257 | + | return | |
| 258 | + | } | |
| 259 | + | wv.logger.Debug().Str("pane_id", string(paneID)).Msg("stale active pane ignored") | |
| 260 | + | wv.workspace.ActivePaneID = "" | |
| 261 | + | } | |
| 262 | + | ||
| 263 | + | if fallbackID, ok := wv.firstPaneIDInTreeInternal(); ok { | |
| 264 | + | _ = wv.setActivePaneIDInternal(fallbackID) | |
| 265 | + | } | |
| 266 | + | } | |
| 267 | + | ||
| 268 | + | func (wv *WorkspaceView) firstPaneIDInTreeInternal() (entity.PaneID, bool) { | |
| 269 | + | if wv.workspace == nil { | |
| 270 | + | return "", false | |
| 271 | + | } | |
| 272 | + | return wv.firstPaneIDInNodeInternal(wv.workspace.Root) | |
| 273 | + | } | |
| 274 | + | ||
| 275 | + | func (wv *WorkspaceView) firstPaneIDInNodeInternal(node *entity.PaneNode) (entity.PaneID, bool) { | |
| 276 | + | if node == nil { | |
| 277 | + | return "", false | |
| 278 | + | } | |
| 279 | + | if node.Pane != nil { | |
| 280 | + | paneID := node.Pane.ID | |
| 281 | + | pv, ok := wv.paneViews[paneID] | |
| 282 | + | return paneID, ok && pv != nil | |
| 283 | + | } | |
| 284 | + | for _, child := range node.Children { | |
| 285 | + | if paneID, ok := wv.firstPaneIDInNodeInternal(child); ok { | |
| 286 | + | return paneID, true | |
| 287 | + | } | |
| 288 | + | } | |
| 289 | + | return "", false | |
| 237 | 290 | } | |
| 238 | 291 | ||
| 239 | 292 | // setActivePaneIDInternal updates active pane without locking. | |
| @@ -246,6 +299,13 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error { | |||
| 246 | 299 | Str("to_pane", string(paneID)). | |
| 247 | 300 | Msg("active pane changing") | |
| 248 | 301 | ||
| 302 | + | // Validate the target pane before mutating UI state. Invalid/stale IDs | |
| 303 | + | // should leave the current active pane styling and overlays untouched. | |
| 304 | + | newPV, ok := wv.paneViews[paneID] | |
| 305 | + | if !ok || newPV == nil { | |
| 306 | + | return ErrPaneNotFound | |
| 307 | + | } | |
| 308 | + | ||
| 249 | 309 | // Destroy find bar if active pane is changing | |
| 250 | 310 | if currentActiveID != paneID && wv.findBar != nil { | |
| 251 | 311 | wv.hideFindBarInternal() | |
| @@ -258,17 +318,11 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error { | |||
| 258 | 318 | ||
| 259 | 319 | // Deactivate current active pane | |
| 260 | 320 | if currentActiveID != "" { | |
| 261 | - | if oldPV, ok := wv.paneViews[currentActiveID]; ok { | |
| 321 | + | if oldPV, ok := wv.paneViews[currentActiveID]; ok && oldPV != nil { | |
| 262 | 322 | oldPV.SetActive(false) | |
| 263 | 323 | } | |
| 264 | 324 | } | |
| 265 | 325 | ||
| 266 | - | // Activate new pane | |
| 267 | - | newPV, ok := wv.paneViews[paneID] | |
| 268 | - | if !ok { | |
| 269 | - | return ErrPaneNotFound | |
| 270 | - | } | |
| 271 | - | ||
| 272 | 326 | newPV.SetActive(true) | |
| 273 | 327 | ||
| 274 | 328 | // Update domain model - single source of truth | |
| @@ -357,6 +411,22 @@ func (wv *WorkspaceView) SetOnPaneFocused(fn func(paneID entity.PaneID)) { | |||
| 357 | 411 | wv.onPaneFocused = fn | |
| 358 | 412 | } | |
| 359 | 413 | ||
| 414 | + | // SetOnActivePaneChanged sets the callback invoked after the active pane changes. | |
| 415 | + | func (wv *WorkspaceView) SetOnActivePaneChanged(fn func(paneID entity.PaneID)) { | |
| 416 | + | wv.mu.Lock() | |
| 417 | + | defer wv.mu.Unlock() | |
| 418 | + | ||
| 419 | + | wv.onActivePaneChanged = fn | |
| 420 | + | } | |
| 421 | + | ||
| 422 | + | // SetOnWebViewAttached sets the callback invoked after a pane receives a WebView widget. | |
| 423 | + | func (wv *WorkspaceView) SetOnWebViewAttached(fn func(paneID entity.PaneID)) { | |
| 424 | + | wv.mu.Lock() | |
| 425 | + | defer wv.mu.Unlock() | |
| 426 | + | ||
| 427 | + | wv.onWebViewAttached = fn | |
| 428 | + | } | |
| 429 | + | ||
| 360 | 430 | func (wv *WorkspaceView) SetOnSplitRatioDragged(fn func(nodeID string, ratio float64)) { | |
| 361 | 431 | wv.mu.Lock() | |
| 362 | 432 | defer wv.mu.Unlock() | |
| @@ -408,13 +478,17 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||
| 408 | 478 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | |
| 409 | 479 | wv.mu.RLock() | |
| 410 | 480 | pv, ok := wv.paneViews[paneID] | |
| 481 | + | callback := wv.onWebViewAttached | |
| 411 | 482 | wv.mu.RUnlock() | |
| 412 | 483 | ||
| 413 | - | if !ok { | |
| 484 | + | if !ok || pv == nil { | |
| 414 | 485 | return ErrPaneNotFound | |
| 415 | 486 | } | |
| 416 | 487 | ||
| 417 | 488 | pv.SetWebViewWidget(widget) | |
| 489 | + | if widget != nil && callback != nil { | |
| 490 | + | callback(paneID) | |
| 491 | + | } | |
| 418 | 492 | return nil | |
| 419 | 493 | } | |
| 420 | 494 | ||
| @@ -4,7 +4,6 @@ package component | |||||
| 4 | import ( | 4 | import ( | ||
| 5 | "context" | 5 | "context" | ||
| 6 | "errors" | 6 | "errors" | ||
| 7 | - | "fmt" | |||
| 8 | "sync" | 7 | "sync" | ||
| 9 | "sync/atomic" | 8 | "sync/atomic" | ||
| 10 | "time" | 9 | "time" | ||
| @@ -51,6 +50,8 @@ type WorkspaceView struct { | |||||
| 51 | paneViews map[entity.PaneID]*PaneView | 50 | paneViews map[entity.PaneID]*PaneView | ||
| 52 | 51 | ||||
| 53 | onPaneFocused func(paneID entity.PaneID) | 52 | onPaneFocused func(paneID entity.PaneID) | ||
| 53 | + | onActivePaneChanged func(paneID entity.PaneID) | |||
| 54 | + | onWebViewAttached func(paneID entity.PaneID) | |||
| 54 | onSplitRatioDragged func(nodeID string, ratio float64) | 55 | onSplitRatioDragged func(nodeID string, ratio float64) | ||
| 55 | 56 | ||||
| 56 | // Hover suppression for keyboard navigation (Issue #89) | 57 | // Hover suppression for keyboard navigation (Issue #89) | ||
| @@ -215,12 +216,10 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||||
| 215 | } | 216 | } | ||
| 216 | } | 217 | } | ||
| 217 | 218 | ||||
| 218 | - | // Set initial active pane | 219 | + | // Set initial active pane. A persisted workspace can contain a stale active |
| 219 | - | if ws.ActivePaneID != "" { | 220 | + | // pane ID after restore/rebuild; choose a valid fallback instead of leaving |
| 220 | - | if err := wv.setActivePaneIDInternal(ws.ActivePaneID); err != nil { | 221 | + | // later active-pane logic pointing at a missing pane. |
| 221 | - | return fmt.Errorf("set initial active pane: %w", err) | 222 | + | wv.restoreActivePaneInternal() |
| 222 | - | } | |||
| 223 | - | } | |||
| 224 | 223 | ||||
| 225 | // Update single-pane mode based on pane count | 224 | // Update single-pane mode based on pane count | ||
| 226 | wv.updateSinglePaneModeInternal() | 225 | wv.updateSinglePaneModeInternal() | ||
| @@ -231,9 +230,63 @@ func (wv *WorkspaceView) SetWorkspace(ctx context.Context, ws *entity.Workspace) | |||||
| 231 | // SetActivePaneID updates which pane is visually marked as active. | 230 | // SetActivePaneID updates which pane is visually marked as active. | ||
| 232 | func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error { | 231 | func (wv *WorkspaceView) SetActivePaneID(paneID entity.PaneID) error { | ||
| 233 | wv.mu.Lock() | 232 | wv.mu.Lock() | ||
| 234 | - | defer wv.mu.Unlock() | 233 | + | currentActiveID := wv.getActivePaneIDInternal() |
| 234 | + | err := wv.setActivePaneIDInternal(paneID) | |||
| 235 | + | callback := wv.onActivePaneChanged | |||
| 236 | + | wv.mu.Unlock() | |||
| 237 | + | if err != nil { | |||
| 238 | + | return err | |||
| 239 | + | } | |||
| 240 | + | if currentActiveID != paneID && callback != nil { | |||
| 241 | + | callback(paneID) | |||
| 242 | + | } | |||
| 243 | + | return nil | |||
| 244 | + | } | |||
| 235 | 245 | ||||
| 236 | - | return wv.setActivePaneIDInternal(paneID) | 246 | + | // restoreActivePaneInternal restores the active pane after building paneViews. |
| 247 | + | // Must be called with the lock held. | |||
| 248 | + | func (wv *WorkspaceView) restoreActivePaneInternal() { | |||
| 249 | + | if wv.workspace == nil { | |||
| 250 | + | return | |||
| 251 | + | } | |||
| 252 | + | ||||
| 253 | + | paneID := wv.workspace.ActivePaneID | |||
| 254 | + | if paneID != "" { | |||
| 255 | + | if pv, ok := wv.paneViews[paneID]; ok && pv != nil { | |||
| 256 | + | _ = wv.setActivePaneIDInternal(paneID) | |||
| 257 | + | return | |||
| 258 | + | } | |||
| 259 | + | wv.logger.Debug().Str("pane_id", string(paneID)).Msg("stale active pane ignored") | |||
| 260 | + | wv.workspace.ActivePaneID = "" | |||
| 261 | + | } | |||
| 262 | + | ||||
| 263 | + | if fallbackID, ok := wv.firstPaneIDInTreeInternal(); ok { | |||
| 264 | + | _ = wv.setActivePaneIDInternal(fallbackID) | |||
| 265 | + | } | |||
| 266 | + | } | |||
| 267 | + | ||||
| 268 | + | func (wv *WorkspaceView) firstPaneIDInTreeInternal() (entity.PaneID, bool) { | |||
| 269 | + | if wv.workspace == nil { | |||
| 270 | + | return "", false | |||
| 271 | + | } | |||
| 272 | + | return wv.firstPaneIDInNodeInternal(wv.workspace.Root) | |||
| 273 | + | } | |||
| 274 | + | ||||
| 275 | + | func (wv *WorkspaceView) firstPaneIDInNodeInternal(node *entity.PaneNode) (entity.PaneID, bool) { | |||
| 276 | + | if node == nil { | |||
| 277 | + | return "", false | |||
| 278 | + | } | |||
| 279 | + | if node.Pane != nil { | |||
| 280 | + | paneID := node.Pane.ID | |||
| 281 | + | pv, ok := wv.paneViews[paneID] | |||
| 282 | + | return paneID, ok && pv != nil | |||
| 283 | + | } | |||
| 284 | + | for _, child := range node.Children { | |||
| 285 | + | if paneID, ok := wv.firstPaneIDInNodeInternal(child); ok { | |||
| 286 | + | return paneID, true | |||
| 287 | + | } | |||
| 288 | + | } | |||
| 289 | + | return "", false | |||
| 237 | } | 290 | } | ||
| 238 | 291 | ||||
| 239 | // setActivePaneIDInternal updates active pane without locking. | 292 | // setActivePaneIDInternal updates active pane without locking. | ||
| @@ -246,6 +299,13 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error { | |||||
| 246 | Str("to_pane", string(paneID)). | 299 | Str("to_pane", string(paneID)). | ||
| 247 | Msg("active pane changing") | 300 | Msg("active pane changing") | ||
| 248 | 301 | ||||
| 302 | + | // Validate the target pane before mutating UI state. Invalid/stale IDs | |||
| 303 | + | // should leave the current active pane styling and overlays untouched. | |||
| 304 | + | newPV, ok := wv.paneViews[paneID] | |||
| 305 | + | if !ok || newPV == nil { | |||
| 306 | + | return ErrPaneNotFound | |||
| 307 | + | } | |||
| 308 | + | ||||
| 249 | // Destroy find bar if active pane is changing | 309 | // Destroy find bar if active pane is changing | ||
| 250 | if currentActiveID != paneID && wv.findBar != nil { | 310 | if currentActiveID != paneID && wv.findBar != nil { | ||
| 251 | wv.hideFindBarInternal() | 311 | wv.hideFindBarInternal() | ||
| @@ -258,17 +318,11 @@ func (wv *WorkspaceView) setActivePaneIDInternal(paneID entity.PaneID) error { | |||||
| 258 | 318 | ||||
| 259 | // Deactivate current active pane | 319 | // Deactivate current active pane | ||
| 260 | if currentActiveID != "" { | 320 | if currentActiveID != "" { | ||
| 261 | - | if oldPV, ok := wv.paneViews[currentActiveID]; ok { | 321 | + | if oldPV, ok := wv.paneViews[currentActiveID]; ok && oldPV != nil { |
| 262 | oldPV.SetActive(false) | 322 | oldPV.SetActive(false) | ||
| 263 | } | 323 | } | ||
| 264 | } | 324 | } | ||
| 265 | 325 | ||||
| 266 | - | // Activate new pane | |||
| 267 | - | newPV, ok := wv.paneViews[paneID] | |||
| 268 | - | if !ok { | |||
| 269 | - | return ErrPaneNotFound | |||
| 270 | - | } | |||
| 271 | - | ||||
| 272 | newPV.SetActive(true) | 326 | newPV.SetActive(true) | ||
| 273 | 327 | ||||
| 274 | // Update domain model - single source of truth | 328 | // Update domain model - single source of truth | ||
| @@ -357,6 +411,22 @@ func (wv *WorkspaceView) SetOnPaneFocused(fn func(paneID entity.PaneID)) { | |||||
| 357 | wv.onPaneFocused = fn | 411 | wv.onPaneFocused = fn | ||
| 358 | } | 412 | } | ||
| 359 | 413 | ||||
| 414 | + | // SetOnActivePaneChanged sets the callback invoked after the active pane changes. | |||
| 415 | + | func (wv *WorkspaceView) SetOnActivePaneChanged(fn func(paneID entity.PaneID)) { | |||
| 416 | + | wv.mu.Lock() | |||
| 417 | + | defer wv.mu.Unlock() | |||
| 418 | + | ||||
| 419 | + | wv.onActivePaneChanged = fn | |||
| 420 | + | } | |||
| 421 | + | ||||
| 422 | + | // SetOnWebViewAttached sets the callback invoked after a pane receives a WebView widget. | |||
| 423 | + | func (wv *WorkspaceView) SetOnWebViewAttached(fn func(paneID entity.PaneID)) { | |||
| 424 | + | wv.mu.Lock() | |||
| 425 | + | defer wv.mu.Unlock() | |||
| 426 | + | ||||
| 427 | + | wv.onWebViewAttached = fn | |||
| 428 | + | } | |||
| 429 | + | ||||
| 360 | func (wv *WorkspaceView) SetOnSplitRatioDragged(fn func(nodeID string, ratio float64)) { | 430 | func (wv *WorkspaceView) SetOnSplitRatioDragged(fn func(nodeID string, ratio float64)) { | ||
| 361 | wv.mu.Lock() | 431 | wv.mu.Lock() | ||
| 362 | defer wv.mu.Unlock() | 432 | defer wv.mu.Unlock() | ||
| @@ -408,13 +478,17 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||||
| 408 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | 478 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | ||
| 409 | wv.mu.RLock() | 479 | wv.mu.RLock() | ||
| 410 | pv, ok := wv.paneViews[paneID] | 480 | pv, ok := wv.paneViews[paneID] | ||
| 481 | + | callback := wv.onWebViewAttached | |||
| 411 | wv.mu.RUnlock() | 482 | wv.mu.RUnlock() | ||
| 412 | 483 | ||||
| 413 | - | if !ok { | 484 | + | if !ok || pv == nil { |
| 414 | return ErrPaneNotFound | 485 | return ErrPaneNotFound | ||
| 415 | } | 486 | } | ||
| 416 | 487 | ||||
| 417 | pv.SetWebViewWidget(widget) | 488 | pv.SetWebViewWidget(widget) | ||
| 489 | + | if widget != nil && callback != nil { | |||
| 490 | + | callback(paneID) | |||
| 491 | + | } | |||
| 418 | return nil | 492 | return nil | ||
| 419 | } | 493 | } | ||
| 420 | 494 | ||||
M internal/ui/component/workspace_view_test.go +215 -1
| @@ -128,7 +128,7 @@ func setupWorkspacePaneViewMocks(t *testing.T, mockFactory *mocks.MockWidgetFact | |||
| 128 | 128 | mockOverlay.EXPECT().SetMeasureOverlay(mockBorderBox, false).Once() | |
| 129 | 129 | ||
| 130 | 130 | // GtkWidget is called when attaching hover handler - return nil for tests | |
| 131 | - | mockOverlay.EXPECT().GtkWidget().Return(nil).Once() | |
| 131 | + | mockOverlay.EXPECT().GtkWidget().Return(nil).Maybe() | |
| 132 | 132 | ||
| 133 | 133 | return mockOverlay, mockBorderBox | |
| 134 | 134 | } | |
| @@ -259,6 +259,93 @@ func TestSetWorkspace_SinglePane_CreatesPaneView(t *testing.T) { | |||
| 259 | 259 | assert.NotNil(t, wv.GetPaneView(pane.ID)) | |
| 260 | 260 | } | |
| 261 | 261 | ||
| 262 | + | func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T) { | |
| 263 | + | // Arrange | |
| 264 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 265 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 266 | + | ||
| 267 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 268 | + | ||
| 269 | + | mockOverlay, mockBorderBox := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 270 | + | mockStackBox := setupStackedLeafMocks(t, mockFactory, mockOverlay) | |
| 271 | + | ||
| 272 | + | mockStackBox.EXPECT().SetVisible(true).Once() | |
| 273 | + | mockBox.EXPECT().Append(mockStackBox).Once() | |
| 274 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |
| 275 | + | mockBorderBox.EXPECT().AddCssClass("pane-active").Once() | |
| 276 | + | ||
| 277 | + | pane := entity.NewPane(entity.PaneID("pane-1")) | |
| 278 | + | node := &entity.PaneNode{ID: "node-1", Pane: pane} | |
| 279 | + | ws := &entity.Workspace{ | |
| 280 | + | ID: "ws-1", | |
| 281 | + | Root: node, | |
| 282 | + | ActivePaneID: entity.PaneID("stale-pane"), | |
| 283 | + | } | |
| 284 | + | ||
| 285 | + | // Act | |
| 286 | + | err := wv.SetWorkspace(ctx, ws) | |
| 287 | + | ||
| 288 | + | // Assert | |
| 289 | + | require.NoError(t, err) | |
| 290 | + | assert.Equal(t, 1, wv.PaneCount()) | |
| 291 | + | assert.Equal(t, pane.ID, wv.GetActivePaneID()) | |
| 292 | + | assert.True(t, wv.GetPaneView(pane.ID).IsActive()) | |
| 293 | + | } | |
| 294 | + | ||
| 295 | + | func TestSetWorkspace_StaleActivePaneID_UsesFirstPaneInTree(t *testing.T) { | |
| 296 | + | // Arrange | |
| 297 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 298 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 299 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 300 | + | ||
| 301 | + | pane1 := entity.NewPane(entity.PaneID("pane-1")) | |
| 302 | + | pane2 := entity.NewPane(entity.PaneID("pane-2")) | |
| 303 | + | node1 := &entity.PaneNode{ID: "node-1", Pane: pane1} | |
| 304 | + | node2 := &entity.PaneNode{ID: "node-2", Pane: pane2} | |
| 305 | + | ||
| 306 | + | mockPaned := mocks.NewMockPanedWidget(t) | |
| 307 | + | mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 308 | + | mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1) | |
| 309 | + | mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 310 | + | mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2) | |
| 311 | + | ||
| 312 | + | mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once() | |
| 313 | + | mockPaned.EXPECT().SetResizeStartChild(true).Once() | |
| 314 | + | mockPaned.EXPECT().SetResizeEndChild(true).Once() | |
| 315 | + | mockPaned.EXPECT().SetVisible(true).Twice() | |
| 316 | + | mockStackBox1.EXPECT().SetVisible(true).Once() | |
| 317 | + | mockStackBox2.EXPECT().SetVisible(true).Once() | |
| 318 | + | mockPaned.EXPECT().SetStartChild(mockStackBox1).Once() | |
| 319 | + | mockPaned.EXPECT().SetEndChild(mockStackBox2).Once() | |
| 320 | + | mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |
| 321 | + | mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once() | |
| 322 | + | mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |
| 323 | + | mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once() | |
| 324 | + | mockBox.EXPECT().Append(mockPaned).Once() | |
| 325 | + | mockBox.EXPECT().RemoveCssClass("single-pane").Once() | |
| 326 | + | mockBorderBox1.EXPECT().AddCssClass("pane-active").Once() | |
| 327 | + | ||
| 328 | + | ws := &entity.Workspace{ | |
| 329 | + | ID: "ws-1", | |
| 330 | + | Root: &entity.PaneNode{ | |
| 331 | + | ID: "split", | |
| 332 | + | SplitDir: entity.SplitHorizontal, | |
| 333 | + | SplitRatio: 0.5, | |
| 334 | + | Children: []*entity.PaneNode{node1, node2}, | |
| 335 | + | }, | |
| 336 | + | ActivePaneID: entity.PaneID("stale-pane"), | |
| 337 | + | } | |
| 338 | + | ||
| 339 | + | // Act | |
| 340 | + | err := wv.SetWorkspace(ctx, ws) | |
| 341 | + | ||
| 342 | + | // Assert | |
| 343 | + | require.NoError(t, err) | |
| 344 | + | assert.Equal(t, pane1.ID, wv.GetActivePaneID()) | |
| 345 | + | assert.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |
| 346 | + | assert.False(t, wv.GetPaneView(pane2.ID).IsActive()) | |
| 347 | + | } | |
| 348 | + | ||
| 262 | 349 | func TestWorkspaceView_HoverFocusLockState(t *testing.T) { | |
| 263 | 350 | // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should | |
| 264 | 351 | // be safe with its language-level zero value before constructor wiring. | |
| @@ -415,6 +502,10 @@ func TestSetActivePaneID_UpdatesStyling(t *testing.T) { | |||
| 415 | 502 | // Now change active pane | |
| 416 | 503 | mockBorderBox1.EXPECT().RemoveCssClass("pane-active").Once() | |
| 417 | 504 | mockBorderBox2.EXPECT().AddCssClass("pane-active").Once() | |
| 505 | + | activatedPane := entity.PaneID("") | |
| 506 | + | wv.SetOnActivePaneChanged(func(paneID entity.PaneID) { | |
| 507 | + | activatedPane = paneID | |
| 508 | + | }) | |
| 418 | 509 | ||
| 419 | 510 | // Act | |
| 420 | 511 | err = wv.SetActivePaneID(pane2.ID) | |
| @@ -422,6 +513,7 @@ func TestSetActivePaneID_UpdatesStyling(t *testing.T) { | |||
| 422 | 513 | // Assert | |
| 423 | 514 | require.NoError(t, err) | |
| 424 | 515 | assert.Equal(t, pane2.ID, wv.GetActivePaneID()) | |
| 516 | + | assert.Equal(t, pane2.ID, activatedPane) | |
| 425 | 517 | } | |
| 426 | 518 | ||
| 427 | 519 | func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) { | |
| @@ -437,3 +529,125 @@ func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) { | |||
| 437 | 529 | // Assert | |
| 438 | 530 | assert.ErrorIs(t, err, component.ErrPaneNotFound) | |
| 439 | 531 | } | |
| 532 | + | ||
| 533 | + | func TestSetActivePaneID_NilRegisteredPaneView_ReturnsError(t *testing.T) { | |
| 534 | + | // Arrange | |
| 535 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 536 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 537 | + | ||
| 538 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 539 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |
| 540 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), nil) | |
| 541 | + | ||
| 542 | + | // Act | |
| 543 | + | err := wv.SetActivePaneID(entity.PaneID("pane-1")) | |
| 544 | + | ||
| 545 | + | // Assert | |
| 546 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |
| 547 | + | } | |
| 548 | + | ||
| 549 | + | func TestSetWebViewWidget_NilRegisteredPaneView_ReturnsError(t *testing.T) { | |
| 550 | + | // Arrange | |
| 551 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 552 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 553 | + | ||
| 554 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 555 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |
| 556 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), nil) | |
| 557 | + | ||
| 558 | + | // Act | |
| 559 | + | err := wv.SetWebViewWidget(entity.PaneID("pane-1"), mocks.NewMockWidget(t)) | |
| 560 | + | ||
| 561 | + | // Assert | |
| 562 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |
| 563 | + | } | |
| 564 | + | ||
| 565 | + | func TestSetActivePaneID_InvalidID_PreservesCurrentActivePane(t *testing.T) { | |
| 566 | + | // Arrange | |
| 567 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 568 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 569 | + | ||
| 570 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 571 | + | ||
| 572 | + | pane1 := entity.NewPane(entity.PaneID("pane-1")) | |
| 573 | + | pane2 := entity.NewPane(entity.PaneID("pane-2")) | |
| 574 | + | node1 := &entity.PaneNode{ID: "node-1", Pane: pane1} | |
| 575 | + | node2 := &entity.PaneNode{ID: "node-2", Pane: pane2} | |
| 576 | + | ||
| 577 | + | mockPaned := mocks.NewMockPanedWidget(t) | |
| 578 | + | mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 579 | + | mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1) | |
| 580 | + | mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 581 | + | mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2) | |
| 582 | + | ||
| 583 | + | mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once() | |
| 584 | + | mockPaned.EXPECT().SetResizeStartChild(true).Once() | |
| 585 | + | mockPaned.EXPECT().SetResizeEndChild(true).Once() | |
| 586 | + | mockPaned.EXPECT().SetVisible(true).Twice() | |
| 587 | + | mockStackBox1.EXPECT().SetVisible(true).Once() | |
| 588 | + | mockStackBox2.EXPECT().SetVisible(true).Once() | |
| 589 | + | mockPaned.EXPECT().SetStartChild(mockStackBox1).Once() | |
| 590 | + | mockPaned.EXPECT().SetEndChild(mockStackBox2).Once() | |
| 591 | + | mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |
| 592 | + | mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once() | |
| 593 | + | mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |
| 594 | + | mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once() | |
| 595 | + | ||
| 596 | + | mockBox.EXPECT().Append(mockPaned).Once() | |
| 597 | + | mockBox.EXPECT().RemoveCssClass("single-pane").Once() | |
| 598 | + | mockBorderBox1.EXPECT().AddCssClass("pane-active").Once() | |
| 599 | + | ||
| 600 | + | splitNode := &entity.PaneNode{ | |
| 601 | + | ID: "split", | |
| 602 | + | SplitDir: entity.SplitHorizontal, | |
| 603 | + | SplitRatio: 0.5, | |
| 604 | + | Children: []*entity.PaneNode{node1, node2}, | |
| 605 | + | } | |
| 606 | + | ws := &entity.Workspace{ | |
| 607 | + | ID: "ws-1", | |
| 608 | + | Root: splitNode, | |
| 609 | + | ActivePaneID: pane1.ID, | |
| 610 | + | } | |
| 611 | + | ||
| 612 | + | err := wv.SetWorkspace(ctx, ws) | |
| 613 | + | require.NoError(t, err) | |
| 614 | + | require.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |
| 615 | + | ||
| 616 | + | // Act | |
| 617 | + | err = wv.SetActivePaneID(entity.PaneID("non-existent")) | |
| 618 | + | ||
| 619 | + | // Assert | |
| 620 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |
| 621 | + | assert.Equal(t, pane1.ID, wv.GetActivePaneID()) | |
| 622 | + | assert.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |
| 623 | + | } | |
| 624 | + | ||
| 625 | + | func TestSetWebViewWidget_InvokesAttachedCallback(t *testing.T) { | |
| 626 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |
| 627 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |
| 628 | + | ||
| 629 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |
| 630 | + | mockOverlay, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |
| 631 | + | pv := component.NewPaneView(ctx, mockFactory, entity.PaneID("pane-1"), nil) | |
| 632 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |
| 633 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), pv) | |
| 634 | + | ||
| 635 | + | attachedPane := entity.PaneID("") | |
| 636 | + | wv.SetOnWebViewAttached(func(paneID entity.PaneID) { | |
| 637 | + | attachedPane = paneID | |
| 638 | + | }) | |
| 639 | + | ||
| 640 | + | mockWebViewWidget := mocks.NewMockWidget(t) | |
| 641 | + | mockOverlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 642 | + | mockOverlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 643 | + | mockWebViewWidget.EXPECT().GetParent().Return(nil).Once() | |
| 644 | + | mockWebViewWidget.EXPECT().IsVisible().Return(false).Once() | |
| 645 | + | mockWebViewWidget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 646 | + | mockWebViewWidget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 647 | + | mockOverlay.EXPECT().SetChild(mockWebViewWidget).Once() | |
| 648 | + | ||
| 649 | + | err := wv.SetWebViewWidget(entity.PaneID("pane-1"), mockWebViewWidget) | |
| 650 | + | ||
| 651 | + | require.NoError(t, err) | |
| 652 | + | assert.Equal(t, entity.PaneID("pane-1"), attachedPane) | |
| 653 | + | } | |
| @@ -128,7 +128,7 @@ func setupWorkspacePaneViewMocks(t *testing.T, mockFactory *mocks.MockWidgetFact | |||||
| 128 | mockOverlay.EXPECT().SetMeasureOverlay(mockBorderBox, false).Once() | 128 | mockOverlay.EXPECT().SetMeasureOverlay(mockBorderBox, false).Once() | ||
| 129 | 129 | ||||
| 130 | // GtkWidget is called when attaching hover handler - return nil for tests | 130 | // GtkWidget is called when attaching hover handler - return nil for tests | ||
| 131 | - | mockOverlay.EXPECT().GtkWidget().Return(nil).Once() | 131 | + | mockOverlay.EXPECT().GtkWidget().Return(nil).Maybe() |
| 132 | 132 | ||||
| 133 | return mockOverlay, mockBorderBox | 133 | return mockOverlay, mockBorderBox | ||
| 134 | } | 134 | } | ||
| @@ -259,6 +259,93 @@ func TestSetWorkspace_SinglePane_CreatesPaneView(t *testing.T) { | |||||
| 259 | assert.NotNil(t, wv.GetPaneView(pane.ID)) | 259 | assert.NotNil(t, wv.GetPaneView(pane.ID)) | ||
| 260 | } | 260 | } | ||
| 261 | 261 | ||||
| 262 | + | func TestSetWorkspace_StaleActivePaneID_DoesNotFailInitialization(t *testing.T) { | |||
| 263 | + | // Arrange | |||
| 264 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 265 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 266 | + | ||||
| 267 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 268 | + | ||||
| 269 | + | mockOverlay, mockBorderBox := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 270 | + | mockStackBox := setupStackedLeafMocks(t, mockFactory, mockOverlay) | |||
| 271 | + | ||||
| 272 | + | mockStackBox.EXPECT().SetVisible(true).Once() | |||
| 273 | + | mockBox.EXPECT().Append(mockStackBox).Once() | |||
| 274 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |||
| 275 | + | mockBorderBox.EXPECT().AddCssClass("pane-active").Once() | |||
| 276 | + | ||||
| 277 | + | pane := entity.NewPane(entity.PaneID("pane-1")) | |||
| 278 | + | node := &entity.PaneNode{ID: "node-1", Pane: pane} | |||
| 279 | + | ws := &entity.Workspace{ | |||
| 280 | + | ID: "ws-1", | |||
| 281 | + | Root: node, | |||
| 282 | + | ActivePaneID: entity.PaneID("stale-pane"), | |||
| 283 | + | } | |||
| 284 | + | ||||
| 285 | + | // Act | |||
| 286 | + | err := wv.SetWorkspace(ctx, ws) | |||
| 287 | + | ||||
| 288 | + | // Assert | |||
| 289 | + | require.NoError(t, err) | |||
| 290 | + | assert.Equal(t, 1, wv.PaneCount()) | |||
| 291 | + | assert.Equal(t, pane.ID, wv.GetActivePaneID()) | |||
| 292 | + | assert.True(t, wv.GetPaneView(pane.ID).IsActive()) | |||
| 293 | + | } | |||
| 294 | + | ||||
| 295 | + | func TestSetWorkspace_StaleActivePaneID_UsesFirstPaneInTree(t *testing.T) { | |||
| 296 | + | // Arrange | |||
| 297 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 298 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 299 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 300 | + | ||||
| 301 | + | pane1 := entity.NewPane(entity.PaneID("pane-1")) | |||
| 302 | + | pane2 := entity.NewPane(entity.PaneID("pane-2")) | |||
| 303 | + | node1 := &entity.PaneNode{ID: "node-1", Pane: pane1} | |||
| 304 | + | node2 := &entity.PaneNode{ID: "node-2", Pane: pane2} | |||
| 305 | + | ||||
| 306 | + | mockPaned := mocks.NewMockPanedWidget(t) | |||
| 307 | + | mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 308 | + | mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1) | |||
| 309 | + | mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 310 | + | mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2) | |||
| 311 | + | ||||
| 312 | + | mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once() | |||
| 313 | + | mockPaned.EXPECT().SetResizeStartChild(true).Once() | |||
| 314 | + | mockPaned.EXPECT().SetResizeEndChild(true).Once() | |||
| 315 | + | mockPaned.EXPECT().SetVisible(true).Twice() | |||
| 316 | + | mockStackBox1.EXPECT().SetVisible(true).Once() | |||
| 317 | + | mockStackBox2.EXPECT().SetVisible(true).Once() | |||
| 318 | + | mockPaned.EXPECT().SetStartChild(mockStackBox1).Once() | |||
| 319 | + | mockPaned.EXPECT().SetEndChild(mockStackBox2).Once() | |||
| 320 | + | mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |||
| 321 | + | mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once() | |||
| 322 | + | mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |||
| 323 | + | mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once() | |||
| 324 | + | mockBox.EXPECT().Append(mockPaned).Once() | |||
| 325 | + | mockBox.EXPECT().RemoveCssClass("single-pane").Once() | |||
| 326 | + | mockBorderBox1.EXPECT().AddCssClass("pane-active").Once() | |||
| 327 | + | ||||
| 328 | + | ws := &entity.Workspace{ | |||
| 329 | + | ID: "ws-1", | |||
| 330 | + | Root: &entity.PaneNode{ | |||
| 331 | + | ID: "split", | |||
| 332 | + | SplitDir: entity.SplitHorizontal, | |||
| 333 | + | SplitRatio: 0.5, | |||
| 334 | + | Children: []*entity.PaneNode{node1, node2}, | |||
| 335 | + | }, | |||
| 336 | + | ActivePaneID: entity.PaneID("stale-pane"), | |||
| 337 | + | } | |||
| 338 | + | ||||
| 339 | + | // Act | |||
| 340 | + | err := wv.SetWorkspace(ctx, ws) | |||
| 341 | + | ||||
| 342 | + | // Assert | |||
| 343 | + | require.NoError(t, err) | |||
| 344 | + | assert.Equal(t, pane1.ID, wv.GetActivePaneID()) | |||
| 345 | + | assert.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |||
| 346 | + | assert.False(t, wv.GetPaneView(pane2.ID).IsActive()) | |||
| 347 | + | } | |||
| 348 | + | ||||
| 262 | func TestWorkspaceView_HoverFocusLockState(t *testing.T) { | 349 | func TestWorkspaceView_HoverFocusLockState(t *testing.T) { | ||
| 263 | // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should | 350 | // Intentional zero-value check: hoverFocusLock is an atomic.Bool and should | ||
| 264 | // be safe with its language-level zero value before constructor wiring. | 351 | // be safe with its language-level zero value before constructor wiring. | ||
| @@ -415,6 +502,10 @@ func TestSetActivePaneID_UpdatesStyling(t *testing.T) { | |||||
| 415 | // Now change active pane | 502 | // Now change active pane | ||
| 416 | mockBorderBox1.EXPECT().RemoveCssClass("pane-active").Once() | 503 | mockBorderBox1.EXPECT().RemoveCssClass("pane-active").Once() | ||
| 417 | mockBorderBox2.EXPECT().AddCssClass("pane-active").Once() | 504 | mockBorderBox2.EXPECT().AddCssClass("pane-active").Once() | ||
| 505 | + | activatedPane := entity.PaneID("") | |||
| 506 | + | wv.SetOnActivePaneChanged(func(paneID entity.PaneID) { | |||
| 507 | + | activatedPane = paneID | |||
| 508 | + | }) | |||
| 418 | 509 | ||||
| 419 | // Act | 510 | // Act | ||
| 420 | err = wv.SetActivePaneID(pane2.ID) | 511 | err = wv.SetActivePaneID(pane2.ID) | ||
| @@ -422,6 +513,7 @@ func TestSetActivePaneID_UpdatesStyling(t *testing.T) { | |||||
| 422 | // Assert | 513 | // Assert | ||
| 423 | require.NoError(t, err) | 514 | require.NoError(t, err) | ||
| 424 | assert.Equal(t, pane2.ID, wv.GetActivePaneID()) | 515 | assert.Equal(t, pane2.ID, wv.GetActivePaneID()) | ||
| 516 | + | assert.Equal(t, pane2.ID, activatedPane) | |||
| 425 | } | 517 | } | ||
| 426 | 518 | ||||
| 427 | func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) { | 519 | func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) { | ||
| @@ -437,3 +529,125 @@ func TestSetActivePaneID_InvalidID_ReturnsError(t *testing.T) { | |||||
| 437 | // Assert | 529 | // Assert | ||
| 438 | assert.ErrorIs(t, err, component.ErrPaneNotFound) | 530 | assert.ErrorIs(t, err, component.ErrPaneNotFound) | ||
| 439 | } | 531 | } | ||
| 532 | + | ||||
| 533 | + | func TestSetActivePaneID_NilRegisteredPaneView_ReturnsError(t *testing.T) { | |||
| 534 | + | // Arrange | |||
| 535 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 536 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 537 | + | ||||
| 538 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 539 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |||
| 540 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), nil) | |||
| 541 | + | ||||
| 542 | + | // Act | |||
| 543 | + | err := wv.SetActivePaneID(entity.PaneID("pane-1")) | |||
| 544 | + | ||||
| 545 | + | // Assert | |||
| 546 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |||
| 547 | + | } | |||
| 548 | + | ||||
| 549 | + | func TestSetWebViewWidget_NilRegisteredPaneView_ReturnsError(t *testing.T) { | |||
| 550 | + | // Arrange | |||
| 551 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 552 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 553 | + | ||||
| 554 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 555 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |||
| 556 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), nil) | |||
| 557 | + | ||||
| 558 | + | // Act | |||
| 559 | + | err := wv.SetWebViewWidget(entity.PaneID("pane-1"), mocks.NewMockWidget(t)) | |||
| 560 | + | ||||
| 561 | + | // Assert | |||
| 562 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |||
| 563 | + | } | |||
| 564 | + | ||||
| 565 | + | func TestSetActivePaneID_InvalidID_PreservesCurrentActivePane(t *testing.T) { | |||
| 566 | + | // Arrange | |||
| 567 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 568 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 569 | + | ||||
| 570 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 571 | + | ||||
| 572 | + | pane1 := entity.NewPane(entity.PaneID("pane-1")) | |||
| 573 | + | pane2 := entity.NewPane(entity.PaneID("pane-2")) | |||
| 574 | + | node1 := &entity.PaneNode{ID: "node-1", Pane: pane1} | |||
| 575 | + | node2 := &entity.PaneNode{ID: "node-2", Pane: pane2} | |||
| 576 | + | ||||
| 577 | + | mockPaned := mocks.NewMockPanedWidget(t) | |||
| 578 | + | mockOverlay1, mockBorderBox1 := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 579 | + | mockStackBox1 := setupStackedLeafMocks(t, mockFactory, mockOverlay1) | |||
| 580 | + | mockOverlay2, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 581 | + | mockStackBox2 := setupStackedLeafMocks(t, mockFactory, mockOverlay2) | |||
| 582 | + | ||||
| 583 | + | mockFactory.EXPECT().NewPaned(layout.OrientationHorizontal).Return(mockPaned).Once() | |||
| 584 | + | mockPaned.EXPECT().SetResizeStartChild(true).Once() | |||
| 585 | + | mockPaned.EXPECT().SetResizeEndChild(true).Once() | |||
| 586 | + | mockPaned.EXPECT().SetVisible(true).Twice() | |||
| 587 | + | mockStackBox1.EXPECT().SetVisible(true).Once() | |||
| 588 | + | mockStackBox2.EXPECT().SetVisible(true).Once() | |||
| 589 | + | mockPaned.EXPECT().SetStartChild(mockStackBox1).Once() | |||
| 590 | + | mockPaned.EXPECT().SetEndChild(mockStackBox2).Once() | |||
| 591 | + | mockPaned.EXPECT().ConnectNotifyPosition(mock.Anything).Return(uint(0)).Once() | |||
| 592 | + | mockPaned.EXPECT().GetAllocatedWidth().Return(0).Once() | |||
| 593 | + | mockPaned.EXPECT().ConnectMap(mock.Anything).Return(uint(0)).Once() | |||
| 594 | + | mockPaned.EXPECT().AddTickCallback(mock.Anything).Return(uint(0)).Once() | |||
| 595 | + | ||||
| 596 | + | mockBox.EXPECT().Append(mockPaned).Once() | |||
| 597 | + | mockBox.EXPECT().RemoveCssClass("single-pane").Once() | |||
| 598 | + | mockBorderBox1.EXPECT().AddCssClass("pane-active").Once() | |||
| 599 | + | ||||
| 600 | + | splitNode := &entity.PaneNode{ | |||
| 601 | + | ID: "split", | |||
| 602 | + | SplitDir: entity.SplitHorizontal, | |||
| 603 | + | SplitRatio: 0.5, | |||
| 604 | + | Children: []*entity.PaneNode{node1, node2}, | |||
| 605 | + | } | |||
| 606 | + | ws := &entity.Workspace{ | |||
| 607 | + | ID: "ws-1", | |||
| 608 | + | Root: splitNode, | |||
| 609 | + | ActivePaneID: pane1.ID, | |||
| 610 | + | } | |||
| 611 | + | ||||
| 612 | + | err := wv.SetWorkspace(ctx, ws) | |||
| 613 | + | require.NoError(t, err) | |||
| 614 | + | require.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |||
| 615 | + | ||||
| 616 | + | // Act | |||
| 617 | + | err = wv.SetActivePaneID(entity.PaneID("non-existent")) | |||
| 618 | + | ||||
| 619 | + | // Assert | |||
| 620 | + | require.ErrorIs(t, err, component.ErrPaneNotFound) | |||
| 621 | + | assert.Equal(t, pane1.ID, wv.GetActivePaneID()) | |||
| 622 | + | assert.True(t, wv.GetPaneView(pane1.ID).IsActive()) | |||
| 623 | + | } | |||
| 624 | + | ||||
| 625 | + | func TestSetWebViewWidget_InvokesAttachedCallback(t *testing.T) { | |||
| 626 | + | mockFactory := mocks.NewMockWidgetFactory(t) | |||
| 627 | + | ctx, mockBox := setupWorkspaceViewBase(t, mockFactory) | |||
| 628 | + | ||||
| 629 | + | wv := component.NewWorkspaceView(ctx, mockFactory) | |||
| 630 | + | mockOverlay, _ := setupWorkspacePaneViewMocks(t, mockFactory) | |||
| 631 | + | pv := component.NewPaneView(ctx, mockFactory, entity.PaneID("pane-1"), nil) | |||
| 632 | + | mockBox.EXPECT().AddCssClass("single-pane").Once() | |||
| 633 | + | wv.RegisterPaneView(entity.PaneID("pane-1"), pv) | |||
| 634 | + | ||||
| 635 | + | attachedPane := entity.PaneID("") | |||
| 636 | + | wv.SetOnWebViewAttached(func(paneID entity.PaneID) { | |||
| 637 | + | attachedPane = paneID | |||
| 638 | + | }) | |||
| 639 | + | ||||
| 640 | + | mockWebViewWidget := mocks.NewMockWidget(t) | |||
| 641 | + | mockOverlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 642 | + | mockOverlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 643 | + | mockWebViewWidget.EXPECT().GetParent().Return(nil).Once() | |||
| 644 | + | mockWebViewWidget.EXPECT().IsVisible().Return(false).Once() | |||
| 645 | + | mockWebViewWidget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 646 | + | mockWebViewWidget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 647 | + | mockOverlay.EXPECT().SetChild(mockWebViewWidget).Once() | |||
| 648 | + | ||||
| 649 | + | err := wv.SetWebViewWidget(entity.PaneID("pane-1"), mockWebViewWidget) | |||
| 650 | + | ||||
| 651 | + | require.NoError(t, err) | |||
| 652 | + | assert.Equal(t, entity.PaneID("pane-1"), attachedPane) | |||
| 653 | + | } | |||
M internal/ui/coordinator/content/lifecycle.go +19 -0
| @@ -137,6 +137,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||
| 137 | 137 | ||
| 138 | 138 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | |
| 139 | 139 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | |
| 140 | + | continue | |
| 140 | 141 | } | |
| 141 | 142 | logging.Trace().Mark("webview_attached") | |
| 142 | 143 | } | |
| @@ -229,3 +230,21 @@ func (c *Coordinator) RegisterPopupWebView(paneID entity.PaneID, wv port.WebView | |||
| 229 | 230 | c.setWebViewLocked(paneID, wv) | |
| 230 | 231 | } | |
| 231 | 232 | } | |
| 233 | + | ||
| 234 | + | // SyncWebViewViewport requests an explicit viewport resync when the engine | |
| 235 | + | // supports it. This is useful after widget attachment, reparenting, visibility | |
| 236 | + | // changes, or layout promotion paths that may not emit a size change. | |
| 237 | + | func (c *Coordinator) SyncWebViewViewport(ctx context.Context, paneID entity.PaneID, reason string) { | |
| 238 | + | if c == nil || paneID == "" { | |
| 239 | + | return | |
| 240 | + | } | |
| 241 | + | wv := c.getWebViewLocked(paneID) | |
| 242 | + | if wv == nil { | |
| 243 | + | return | |
| 244 | + | } | |
| 245 | + | syncer, ok := wv.(port.ViewportSyncCapable) | |
| 246 | + | if !ok { | |
| 247 | + | return | |
| 248 | + | } | |
| 249 | + | syncer.SyncViewport(ctx, reason) | |
| 250 | + | } | |
| @@ -137,6 +137,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||||
| 137 | 137 | ||||
| 138 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | 138 | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | ||
| 139 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | 139 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | ||
| 140 | + | continue | |||
| 140 | } | 141 | } | ||
| 141 | logging.Trace().Mark("webview_attached") | 142 | logging.Trace().Mark("webview_attached") | ||
| 142 | } | 143 | } | ||
| @@ -229,3 +230,21 @@ func (c *Coordinator) RegisterPopupWebView(paneID entity.PaneID, wv port.WebView | |||||
| 229 | c.setWebViewLocked(paneID, wv) | 230 | c.setWebViewLocked(paneID, wv) | ||
| 230 | } | 231 | } | ||
| 231 | } | 232 | } | ||
| 233 | + | ||||
| 234 | + | // SyncWebViewViewport requests an explicit viewport resync when the engine | |||
| 235 | + | // supports it. This is useful after widget attachment, reparenting, visibility | |||
| 236 | + | // changes, or layout promotion paths that may not emit a size change. | |||
| 237 | + | func (c *Coordinator) SyncWebViewViewport(ctx context.Context, paneID entity.PaneID, reason string) { | |||
| 238 | + | if c == nil || paneID == "" { | |||
| 239 | + | return | |||
| 240 | + | } | |||
| 241 | + | wv := c.getWebViewLocked(paneID) | |||
| 242 | + | if wv == nil { | |||
| 243 | + | return | |||
| 244 | + | } | |||
| 245 | + | syncer, ok := wv.(port.ViewportSyncCapable) | |||
| 246 | + | if !ok { | |||
| 247 | + | return | |||
| 248 | + | } | |||
| 249 | + | syncer.SyncViewport(ctx, reason) | |||
| 250 | + | } | |||
M internal/ui/coordinator/content/lifecycle_test.go +41 -0
| @@ -258,3 +258,44 @@ func TestLifecycle_RegisterPopupWebView_IgnoresNil(t *testing.T) { | |||
| 258 | 258 | ||
| 259 | 259 | assert.Nil(t, c.GetWebView("popup-1")) | |
| 260 | 260 | } | |
| 261 | + | ||
| 262 | + | type syncViewportContextKey struct{} | |
| 263 | + | ||
| 264 | + | type syncViewportCapableStub struct { | |
| 265 | + | port.WebView | |
| 266 | + | called bool | |
| 267 | + | ctx context.Context | |
| 268 | + | reason string | |
| 269 | + | } | |
| 270 | + | ||
| 271 | + | func (s *syncViewportCapableStub) SyncViewport(ctx context.Context, reason string) { | |
| 272 | + | s.called = true | |
| 273 | + | s.ctx = ctx | |
| 274 | + | s.reason = reason | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | func TestLifecycle_SyncWebViewViewport_DelegatesWhenCapabilityPresent(t *testing.T) { | |
| 278 | + | t.Parallel() | |
| 279 | + | ||
| 280 | + | base := mocks.NewMockWebView(t) | |
| 281 | + | wv := &syncViewportCapableStub{WebView: base} | |
| 282 | + | c := newMinimalCoordinator() | |
| 283 | + | c.webViews[entity.PaneID("pane-1")] = wv | |
| 284 | + | ||
| 285 | + | ctx := context.WithValue(context.Background(), syncViewportContextKey{}, "marker") | |
| 286 | + | c.SyncWebViewViewport(ctx, "pane-1", "unit-test") | |
| 287 | + | ||
| 288 | + | require.True(t, wv.called) | |
| 289 | + | require.Equal(t, "unit-test", wv.reason) | |
| 290 | + | require.Same(t, ctx, wv.ctx) | |
| 291 | + | } | |
| 292 | + | ||
| 293 | + | func TestLifecycle_SyncWebViewViewport_NoopWithoutCapability(t *testing.T) { | |
| 294 | + | t.Parallel() | |
| 295 | + | ||
| 296 | + | wv := mocks.NewMockWebView(t) | |
| 297 | + | c := newMinimalCoordinator() | |
| 298 | + | c.webViews[entity.PaneID("pane-1")] = wv | |
| 299 | + | ||
| 300 | + | c.SyncWebViewViewport(context.Background(), "pane-1", "unit-test") | |
| 301 | + | } | |
| @@ -258,3 +258,44 @@ func TestLifecycle_RegisterPopupWebView_IgnoresNil(t *testing.T) { | |||||
| 258 | 258 | ||||
| 259 | assert.Nil(t, c.GetWebView("popup-1")) | 259 | assert.Nil(t, c.GetWebView("popup-1")) | ||
| 260 | } | 260 | } | ||
| 261 | + | ||||
| 262 | + | type syncViewportContextKey struct{} | |||
| 263 | + | ||||
| 264 | + | type syncViewportCapableStub struct { | |||
| 265 | + | port.WebView | |||
| 266 | + | called bool | |||
| 267 | + | ctx context.Context | |||
| 268 | + | reason string | |||
| 269 | + | } | |||
| 270 | + | ||||
| 271 | + | func (s *syncViewportCapableStub) SyncViewport(ctx context.Context, reason string) { | |||
| 272 | + | s.called = true | |||
| 273 | + | s.ctx = ctx | |||
| 274 | + | s.reason = reason | |||
| 275 | + | } | |||
| 276 | + | ||||
| 277 | + | func TestLifecycle_SyncWebViewViewport_DelegatesWhenCapabilityPresent(t *testing.T) { | |||
| 278 | + | t.Parallel() | |||
| 279 | + | ||||
| 280 | + | base := mocks.NewMockWebView(t) | |||
| 281 | + | wv := &syncViewportCapableStub{WebView: base} | |||
| 282 | + | c := newMinimalCoordinator() | |||
| 283 | + | c.webViews[entity.PaneID("pane-1")] = wv | |||
| 284 | + | ||||
| 285 | + | ctx := context.WithValue(context.Background(), syncViewportContextKey{}, "marker") | |||
| 286 | + | c.SyncWebViewViewport(ctx, "pane-1", "unit-test") | |||
| 287 | + | ||||
| 288 | + | require.True(t, wv.called) | |||
| 289 | + | require.Equal(t, "unit-test", wv.reason) | |||
| 290 | + | require.Same(t, ctx, wv.ctx) | |||
| 291 | + | } | |||
| 292 | + | ||||
| 293 | + | func TestLifecycle_SyncWebViewViewport_NoopWithoutCapability(t *testing.T) { | |||
| 294 | + | t.Parallel() | |||
| 295 | + | ||||
| 296 | + | wv := mocks.NewMockWebView(t) | |||
| 297 | + | c := newMinimalCoordinator() | |||
| 298 | + | c.webViews[entity.PaneID("pane-1")] = wv | |||
| 299 | + | ||||
| 300 | + | c.SyncWebViewViewport(context.Background(), "pane-1", "unit-test") | |||
| 301 | + | } | |||
M internal/ui/coordinator/workspace.go +14 -8
| @@ -605,7 +605,9 @@ func (c *WorkspaceCoordinator) doIncrementalStackSplit( | |||
| 605 | 605 | ||
| 606 | 606 | widget := c.contentCoord.WrapWidget(ctx, wv) | |
| 607 | 607 | if widget != nil { | |
| 608 | - | newPaneView.SetWebViewWidget(widget) | |
| 608 | + | if err := wsView.SetWebViewWidget(output.NewPaneNode.Pane.ID, widget); err != nil { | |
| 609 | + | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to attach webview widget for stack split") | |
| 610 | + | } | |
| 609 | 611 | } | |
| 610 | 612 | ||
| 611 | 613 | log.Debug().Msg("incremental stack split completed successfully") | |
| @@ -743,7 +745,9 @@ func (c *WorkspaceCoordinator) doIncrementalSplit( | |||
| 743 | 745 | ||
| 744 | 746 | widget := c.contentCoord.WrapWidget(ctx, wv) | |
| 745 | 747 | if widget != nil { | |
| 746 | - | newPaneView.SetWebViewWidget(widget) | |
| 748 | + | if err := wsView.SetWebViewWidget(output.NewPaneNode.Pane.ID, widget); err != nil { | |
| 749 | + | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to attach webview widget for split") | |
| 750 | + | } | |
| 747 | 751 | } | |
| 748 | 752 | ||
| 749 | 753 | log.Debug().Msg("incremental split completed successfully") | |
| @@ -1622,7 +1626,9 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||
| 1622 | 1626 | } else { | |
| 1623 | 1627 | widget := c.contentCoord.WrapWidget(ctx, wv) | |
| 1624 | 1628 | if widget != nil { | |
| 1625 | - | newPaneView.SetWebViewWidget(widget) | |
| 1629 | + | if err := stackCtx.wsView.SetWebViewWidget(newPaneID, widget); err != nil { | |
| 1630 | + | log.Warn().Err(err).Str("pane_id", string(newPaneID)).Msg("failed to attach webview widget for stacked pane") | |
| 1631 | + | } | |
| 1626 | 1632 | } | |
| 1627 | 1633 | // Load initial page for the new pane | |
| 1628 | 1634 | if err := wv.LoadURI(ctx, newPane.URI); err != nil { | |
| @@ -2089,11 +2095,9 @@ func (c *WorkspaceCoordinator) attachPopupWebView( | |||
| 2089 | 2095 | return | |
| 2090 | 2096 | } | |
| 2091 | 2097 | ||
| 2092 | - | paneView := wsView.GetPaneView(input.PopupPane.ID) | |
| 2093 | - | if paneView == nil { | |
| 2094 | - | return | |
| 2098 | + | if err := wsView.SetWebViewWidget(input.PopupPane.ID, widget); err != nil { | |
| 2099 | + | logging.FromContext(ctx).Warn().Err(err).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to attach popup webview widget") | |
| 2095 | 2100 | } | |
| 2096 | - | paneView.SetWebViewWidget(widget) | |
| 2097 | 2101 | } | |
| 2098 | 2102 | ||
| 2099 | 2103 | // insertPopupStacked inserts a popup as a stacked pane on top of the parent. | |
| @@ -2259,7 +2263,9 @@ func (c *WorkspaceCoordinator) attachPopupPaneView( | |||
| 2259 | 2263 | if input.WebView != nil { | |
| 2260 | 2264 | widget := c.contentCoord.WrapWidget(ctx, input.WebView) | |
| 2261 | 2265 | if widget != nil { | |
| 2262 | - | newPaneView.SetWebViewWidget(widget) | |
| 2266 | + | if err := wsView.SetWebViewWidget(input.PopupPane.ID, widget); err != nil { | |
| 2267 | + | log.Warn().Err(err).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to attach stacked popup webview widget") | |
| 2268 | + | } | |
| 2263 | 2269 | } | |
| 2264 | 2270 | } | |
| 2265 | 2271 | ||
| @@ -605,7 +605,9 @@ func (c *WorkspaceCoordinator) doIncrementalStackSplit( | |||||
| 605 | 605 | ||||
| 606 | widget := c.contentCoord.WrapWidget(ctx, wv) | 606 | widget := c.contentCoord.WrapWidget(ctx, wv) | ||
| 607 | if widget != nil { | 607 | if widget != nil { | ||
| 608 | - | newPaneView.SetWebViewWidget(widget) | 608 | + | if err := wsView.SetWebViewWidget(output.NewPaneNode.Pane.ID, widget); err != nil { |
| 609 | + | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to attach webview widget for stack split") | |||
| 610 | + | } | |||
| 609 | } | 611 | } | ||
| 610 | 612 | ||||
| 611 | log.Debug().Msg("incremental stack split completed successfully") | 613 | log.Debug().Msg("incremental stack split completed successfully") | ||
| @@ -743,7 +745,9 @@ func (c *WorkspaceCoordinator) doIncrementalSplit( | |||||
| 743 | 745 | ||||
| 744 | widget := c.contentCoord.WrapWidget(ctx, wv) | 746 | widget := c.contentCoord.WrapWidget(ctx, wv) | ||
| 745 | if widget != nil { | 747 | if widget != nil { | ||
| 746 | - | newPaneView.SetWebViewWidget(widget) | 748 | + | if err := wsView.SetWebViewWidget(output.NewPaneNode.Pane.ID, widget); err != nil { |
| 749 | + | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to attach webview widget for split") | |||
| 750 | + | } | |||
| 747 | } | 751 | } | ||
| 748 | 752 | ||||
| 749 | log.Debug().Msg("incremental split completed successfully") | 753 | log.Debug().Msg("incremental split completed successfully") | ||
| @@ -1622,7 +1626,9 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||||
| 1622 | } else { | 1626 | } else { | ||
| 1623 | widget := c.contentCoord.WrapWidget(ctx, wv) | 1627 | widget := c.contentCoord.WrapWidget(ctx, wv) | ||
| 1624 | if widget != nil { | 1628 | if widget != nil { | ||
| 1625 | - | newPaneView.SetWebViewWidget(widget) | 1629 | + | if err := stackCtx.wsView.SetWebViewWidget(newPaneID, widget); err != nil { |
| 1630 | + | log.Warn().Err(err).Str("pane_id", string(newPaneID)).Msg("failed to attach webview widget for stacked pane") | |||
| 1631 | + | } | |||
| 1626 | } | 1632 | } | ||
| 1627 | // Load initial page for the new pane | 1633 | // Load initial page for the new pane | ||
| 1628 | if err := wv.LoadURI(ctx, newPane.URI); err != nil { | 1634 | if err := wv.LoadURI(ctx, newPane.URI); err != nil { | ||
| @@ -2089,11 +2095,9 @@ func (c *WorkspaceCoordinator) attachPopupWebView( | |||||
| 2089 | return | 2095 | return | ||
| 2090 | } | 2096 | } | ||
| 2091 | 2097 | ||||
| 2092 | - | paneView := wsView.GetPaneView(input.PopupPane.ID) | 2098 | + | if err := wsView.SetWebViewWidget(input.PopupPane.ID, widget); err != nil { |
| 2093 | - | if paneView == nil { | 2099 | + | logging.FromContext(ctx).Warn().Err(err).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to attach popup webview widget") |
| 2094 | - | return | |||
| 2095 | } | 2100 | } | ||
| 2096 | - | paneView.SetWebViewWidget(widget) | |||
| 2097 | } | 2101 | } | ||
| 2098 | 2102 | ||||
| 2099 | // insertPopupStacked inserts a popup as a stacked pane on top of the parent. | 2103 | // insertPopupStacked inserts a popup as a stacked pane on top of the parent. | ||
| @@ -2259,7 +2263,9 @@ func (c *WorkspaceCoordinator) attachPopupPaneView( | |||||
| 2259 | if input.WebView != nil { | 2263 | if input.WebView != nil { | ||
| 2260 | widget := c.contentCoord.WrapWidget(ctx, input.WebView) | 2264 | widget := c.contentCoord.WrapWidget(ctx, input.WebView) | ||
| 2261 | if widget != nil { | 2265 | if widget != nil { | ||
| 2262 | - | newPaneView.SetWebViewWidget(widget) | 2266 | + | if err := wsView.SetWebViewWidget(input.PopupPane.ID, widget); err != nil { |
| 2267 | + | log.Warn().Err(err).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to attach stacked popup webview widget") | |||
| 2268 | + | } | |||
| 2263 | } | 2269 | } | ||
| 2264 | } | 2270 | } | ||
| 2265 | 2271 | ||||