Merge pull request #351 from bnema/fix/reused-webview-skeleton
fix(ui): preserve revealed webviews across rebuilds
10 files · +664 -268
M internal/ui/component/pane_view.go +54 -39
| @@ -135,58 +135,73 @@ func (pv *PaneView) WebViewWidget() layout.Widget { | |||
| 135 | 135 | return pv.webViewWidget | |
| 136 | 136 | } | |
| 137 | 137 | ||
| 138 | - | // SetWebViewWidget replaces the WebView widget. | |
| 138 | + | // SetWebViewWidget replaces the WebView widget as not-yet-revealed content. | |
| 139 | + | // Call AttachWebViewWidget when the content coordinator knows the WebView has | |
| 140 | + | // already painted. | |
| 139 | 141 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | |
| 142 | + | pv.AttachWebViewWidget(widget, false) | |
| 143 | + | } | |
| 144 | + | ||
| 145 | + | // AttachWebViewWidget replaces the WebView widget and applies its explicit | |
| 146 | + | // content reveal state. GTK parentage is used only to satisfy GTK's one-parent | |
| 147 | + | // ownership rule; it must not be used to infer whether CEF has painted. | |
| 148 | + | func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) { | |
| 140 | 149 | pv.mu.Lock() | |
| 141 | 150 | defer pv.mu.Unlock() | |
| 142 | 151 | ||
| 143 | - | // Remove old widget from this overlay | |
| 152 | + | // Remove old widget from this overlay. | |
| 144 | 153 | if pv.webViewWidget != nil { | |
| 145 | 154 | pv.overlay.SetChild(nil) | |
| 146 | 155 | } | |
| 147 | 156 | ||
| 148 | 157 | pv.webViewWidget = widget | |
| 158 | + | if widget == nil { | |
| 159 | + | return | |
| 160 | + | } | |
| 149 | 161 | ||
| 150 | - | if widget != nil { | |
| 151 | - | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | |
| 152 | - | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | |
| 153 | - | widgetAllocWidthBefore := widget.GetAllocatedWidth() | |
| 154 | - | widgetAllocHeightBefore := widget.GetAllocatedHeight() | |
| 155 | - | hadParent := widget.GetParent() != nil | |
| 156 | - | wasVisible := widget.IsVisible() | |
| 157 | - | ||
| 158 | - | // If this widget already had a parent, we're reparenting an existing WebView | |
| 159 | - | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | |
| 160 | - | // already painted, so the loading skeleton should be hidden immediately. | |
| 161 | - | ||
| 162 | - | // Unparent widget from any previous parent (critical for rebuild scenarios) | |
| 163 | - | // In GTK4, a widget can only have one parent at a time | |
| 164 | - | if hadParent { | |
| 165 | - | widget.Unparent() | |
| 166 | - | } | |
| 167 | - | pv.overlay.SetChild(widget) | |
| 162 | + | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | |
| 163 | + | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | |
| 164 | + | widgetAllocWidthBefore := widget.GetAllocatedWidth() | |
| 165 | + | widgetAllocHeightBefore := widget.GetAllocatedHeight() | |
| 166 | + | hadParent := widget.GetParent() != nil | |
| 167 | + | wasVisible := widget.IsVisible() | |
| 168 | + | ||
| 169 | + | // GTK4 widgets may have only one parent. Cleanup during a WorkspaceView | |
| 170 | + | // rebuild leaves a reusable native widget unparented, which says nothing | |
| 171 | + | // about whether its content was previously revealed. | |
| 172 | + | if hadParent { | |
| 173 | + | widget.Unparent() | |
| 174 | + | } | |
| 175 | + | pv.overlay.SetChild(widget) | |
| 168 | 176 | ||
| 169 | - | if hadParent && wasVisible { | |
| 170 | - | widget.SetVisible(true) | |
| 171 | - | } | |
| 172 | - | if hadParent && pv.loading != nil { | |
| 173 | - | pv.loading.SetVisible(false) | |
| 174 | - | } | |
| 177 | + | if hadParent && wasVisible { | |
| 178 | + | widget.SetVisible(true) | |
| 179 | + | } | |
| 180 | + | // A replacement can follow an already-revealed WebView in the same pane. | |
| 181 | + | // Apply both states explicitly: an unrevealed current WebView must restart | |
| 182 | + | // the skeleton rather than inheriting the previous view's hidden overlay. | |
| 183 | + | if pv.loading != nil { | |
| 184 | + | pv.loading.SetVisible(!revealed) | |
| 185 | + | } | |
| 175 | 186 | ||
| 176 | - | logging.FromContext(pv.ctx).Debug(). | |
| 177 | - | Str("pane_id", string(pv.paneID)). | |
| 178 | - | Bool("had_parent", hadParent). | |
| 179 | - | Bool("was_visible", wasVisible). | |
| 180 | - | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |
| 181 | - | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |
| 182 | - | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |
| 183 | - | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |
| 184 | - | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |
| 185 | - | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |
| 186 | - | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |
| 187 | - | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |
| 188 | - | Msg("pane view webview widget attached") | |
| 187 | + | ctx := pv.ctx | |
| 188 | + | if ctx == nil { | |
| 189 | + | ctx = context.Background() | |
| 189 | 190 | } | |
| 191 | + | logging.FromContext(ctx).Debug(). | |
| 192 | + | Str("pane_id", string(pv.paneID)). | |
| 193 | + | Bool("had_parent", hadParent). | |
| 194 | + | Bool("was_visible", wasVisible). | |
| 195 | + | Bool("revealed", revealed). | |
| 196 | + | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |
| 197 | + | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |
| 198 | + | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |
| 199 | + | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |
| 200 | + | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |
| 201 | + | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |
| 202 | + | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |
| 203 | + | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |
| 204 | + | Msg("pane view webview widget attached") | |
| 190 | 205 | } | |
| 191 | 206 | ||
| 192 | 207 | // GrabFocus attempts to focus the WebView. | |
| @@ -135,58 +135,73 @@ func (pv *PaneView) WebViewWidget() layout.Widget { | |||||
| 135 | return pv.webViewWidget | 135 | return pv.webViewWidget | ||
| 136 | } | 136 | } | ||
| 137 | 137 | ||||
| 138 | - | // SetWebViewWidget replaces the WebView widget. | 138 | + | // SetWebViewWidget replaces the WebView widget as not-yet-revealed content. |
| 139 | + | // Call AttachWebViewWidget when the content coordinator knows the WebView has | |||
| 140 | + | // already painted. | |||
| 139 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | 141 | func (pv *PaneView) SetWebViewWidget(widget layout.Widget) { | ||
| 142 | + | pv.AttachWebViewWidget(widget, false) | |||
| 143 | + | } | |||
| 144 | + | ||||
| 145 | + | // AttachWebViewWidget replaces the WebView widget and applies its explicit | |||
| 146 | + | // content reveal state. GTK parentage is used only to satisfy GTK's one-parent | |||
| 147 | + | // ownership rule; it must not be used to infer whether CEF has painted. | |||
| 148 | + | func (pv *PaneView) AttachWebViewWidget(widget layout.Widget, revealed bool) { | |||
| 140 | pv.mu.Lock() | 149 | pv.mu.Lock() | ||
| 141 | defer pv.mu.Unlock() | 150 | defer pv.mu.Unlock() | ||
| 142 | 151 | ||||
| 143 | - | // Remove old widget from this overlay | 152 | + | // Remove old widget from this overlay. |
| 144 | if pv.webViewWidget != nil { | 153 | if pv.webViewWidget != nil { | ||
| 145 | pv.overlay.SetChild(nil) | 154 | pv.overlay.SetChild(nil) | ||
| 146 | } | 155 | } | ||
| 147 | 156 | ||||
| 148 | pv.webViewWidget = widget | 157 | pv.webViewWidget = widget | ||
| 158 | + | if widget == nil { | |||
| 159 | + | return | |||
| 160 | + | } | |||
| 149 | 161 | ||||
| 150 | - | if widget != nil { | 162 | + | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() |
| 151 | - | overlayAllocWidthBefore := pv.overlay.GetAllocatedWidth() | 163 | + | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() |
| 152 | - | overlayAllocHeightBefore := pv.overlay.GetAllocatedHeight() | 164 | + | widgetAllocWidthBefore := widget.GetAllocatedWidth() |
| 153 | - | widgetAllocWidthBefore := widget.GetAllocatedWidth() | 165 | + | widgetAllocHeightBefore := widget.GetAllocatedHeight() |
| 154 | - | widgetAllocHeightBefore := widget.GetAllocatedHeight() | 166 | + | hadParent := widget.GetParent() != nil |
| 155 | - | hadParent := widget.GetParent() != nil | 167 | + | wasVisible := widget.IsVisible() |
| 156 | - | wasVisible := widget.IsVisible() | 168 | + | |
| 157 | - | 169 | + | // GTK4 widgets may have only one parent. Cleanup during a WorkspaceView | |
| 158 | - | // If this widget already had a parent, we're reparenting an existing WebView | 170 | + | // rebuild leaves a reusable native widget unparented, which says nothing |
| 159 | - | // (e.g. after rebuilding/moving panes). In that case the WebView has likely | 171 | + | // about whether its content was previously revealed. |
| 160 | - | // already painted, so the loading skeleton should be hidden immediately. | 172 | + | if hadParent { |
| 161 | - | 173 | + | widget.Unparent() | |
| 162 | - | // Unparent widget from any previous parent (critical for rebuild scenarios) | 174 | + | } |
| 163 | - | // In GTK4, a widget can only have one parent at a time | 175 | + | pv.overlay.SetChild(widget) |
| 164 | - | if hadParent { | |||
| 165 | - | widget.Unparent() | |||
| 166 | - | } | |||
| 167 | - | pv.overlay.SetChild(widget) | |||
| 168 | 176 | ||||
| 169 | - | if hadParent && wasVisible { | 177 | + | if hadParent && wasVisible { |
| 170 | - | widget.SetVisible(true) | 178 | + | widget.SetVisible(true) |
| 171 | - | } | 179 | + | } |
| 172 | - | if hadParent && pv.loading != nil { | 180 | + | // A replacement can follow an already-revealed WebView in the same pane. |
| 173 | - | pv.loading.SetVisible(false) | 181 | + | // Apply both states explicitly: an unrevealed current WebView must restart |
| 174 | - | } | 182 | + | // the skeleton rather than inheriting the previous view's hidden overlay. |
| 183 | + | if pv.loading != nil { | |||
| 184 | + | pv.loading.SetVisible(!revealed) | |||
| 185 | + | } | |||
| 175 | 186 | ||||
| 176 | - | logging.FromContext(pv.ctx).Debug(). | 187 | + | ctx := pv.ctx |
| 177 | - | Str("pane_id", string(pv.paneID)). | 188 | + | if ctx == nil { |
| 178 | - | Bool("had_parent", hadParent). | 189 | + | ctx = context.Background() |
| 179 | - | Bool("was_visible", wasVisible). | |||
| 180 | - | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |||
| 181 | - | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |||
| 182 | - | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |||
| 183 | - | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |||
| 184 | - | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |||
| 185 | - | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |||
| 186 | - | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |||
| 187 | - | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |||
| 188 | - | Msg("pane view webview widget attached") | |||
| 189 | } | 190 | } | ||
| 191 | + | logging.FromContext(ctx).Debug(). | |||
| 192 | + | Str("pane_id", string(pv.paneID)). | |||
| 193 | + | Bool("had_parent", hadParent). | |||
| 194 | + | Bool("was_visible", wasVisible). | |||
| 195 | + | Bool("revealed", revealed). | |||
| 196 | + | Int("overlay_alloc_width_before", overlayAllocWidthBefore). | |||
| 197 | + | Int("overlay_alloc_height_before", overlayAllocHeightBefore). | |||
| 198 | + | Int("widget_alloc_width_before", widgetAllocWidthBefore). | |||
| 199 | + | Int("widget_alloc_height_before", widgetAllocHeightBefore). | |||
| 200 | + | Int("overlay_alloc_width_after", pv.overlay.GetAllocatedWidth()). | |||
| 201 | + | Int("overlay_alloc_height_after", pv.overlay.GetAllocatedHeight()). | |||
| 202 | + | Int("widget_alloc_width_after", widget.GetAllocatedWidth()). | |||
| 203 | + | Int("widget_alloc_height_after", widget.GetAllocatedHeight()). | |||
| 204 | + | Msg("pane view webview widget attached") | |||
| 190 | } | 205 | } | ||
| 191 | 206 | ||||
| 192 | // GrabFocus attempts to focus the WebView. | 207 | // GrabFocus attempts to focus the WebView. | ||
M internal/ui/component/pane_view_internal_test.go +35 -0
| @@ -3,6 +3,7 @@ package component | |||
| 3 | 3 | import ( | |
| 4 | 4 | "testing" | |
| 5 | 5 | ||
| 6 | + | "github.com/stretchr/testify/assert" | |
| 6 | 7 | "github.com/stretchr/testify/mock" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/dumber/internal/ui/layout" | |
| @@ -80,3 +81,37 @@ func TestPaneView_HideLoadingSkeleton_NoOpWhenNil(t *testing.T) { | |||
| 80 | 81 | pv := &PaneView{} | |
| 81 | 82 | pv.HideLoadingSkeleton() | |
| 82 | 83 | } | |
| 84 | + | ||
| 85 | + | func TestPaneView_AttachWebViewWidget_RevealedThenUnrevealedReplacementRestartsSkeleton(t *testing.T) { | |
| 86 | + | overlay := mocks.NewMockOverlayWidget(t) | |
| 87 | + | revealedWidget := mocks.NewMockWidget(t) | |
| 88 | + | unrevealedWidget := mocks.NewMockWidget(t) | |
| 89 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |
| 90 | + | spinner := mocks.NewMockSpinnerWidget(t) | |
| 91 | + | ||
| 92 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Times(4) | |
| 93 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Times(4) | |
| 94 | + | for _, widget := range []*mocks.MockWidget{revealedWidget, unrevealedWidget} { | |
| 95 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 96 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 97 | + | widget.EXPECT().GetParent().Return(nil).Once() | |
| 98 | + | widget.EXPECT().IsVisible().Return(true).Once() | |
| 99 | + | } | |
| 100 | + | overlay.EXPECT().SetChild(revealedWidget).Once() | |
| 101 | + | loadingContainer.EXPECT().SetVisible(false).Once() | |
| 102 | + | spinner.EXPECT().Stop().Once() | |
| 103 | + | overlay.EXPECT().SetChild(nil).Once() | |
| 104 | + | overlay.EXPECT().SetChild(unrevealedWidget).Once() | |
| 105 | + | loadingContainer.EXPECT().SetVisible(true).Once() | |
| 106 | + | spinner.EXPECT().Start().Once() | |
| 107 | + | ||
| 108 | + | pv := &PaneView{ | |
| 109 | + | overlay: overlay, | |
| 110 | + | loading: &LoadingSkeleton{container: loadingContainer, spinner: spinner}, | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | pv.AttachWebViewWidget(revealedWidget, true) | |
| 114 | + | pv.AttachWebViewWidget(unrevealedWidget, false) | |
| 115 | + | ||
| 116 | + | assert.Same(t, unrevealedWidget, pv.WebViewWidget()) | |
| 117 | + | } | |
| @@ -3,6 +3,7 @@ package component | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "testing" | 4 | "testing" | ||
| 5 | 5 | ||||
| 6 | + | "github.com/stretchr/testify/assert" | |||
| 6 | "github.com/stretchr/testify/mock" | 7 | "github.com/stretchr/testify/mock" | ||
| 7 | 8 | ||||
| 8 | "github.com/bnema/dumber/internal/ui/layout" | 9 | "github.com/bnema/dumber/internal/ui/layout" | ||
| @@ -80,3 +81,37 @@ func TestPaneView_HideLoadingSkeleton_NoOpWhenNil(t *testing.T) { | |||||
| 80 | pv := &PaneView{} | 81 | pv := &PaneView{} | ||
| 81 | pv.HideLoadingSkeleton() | 82 | pv.HideLoadingSkeleton() | ||
| 82 | } | 83 | } | ||
| 84 | + | ||||
| 85 | + | func TestPaneView_AttachWebViewWidget_RevealedThenUnrevealedReplacementRestartsSkeleton(t *testing.T) { | |||
| 86 | + | overlay := mocks.NewMockOverlayWidget(t) | |||
| 87 | + | revealedWidget := mocks.NewMockWidget(t) | |||
| 88 | + | unrevealedWidget := mocks.NewMockWidget(t) | |||
| 89 | + | loadingContainer := mocks.NewMockBoxWidget(t) | |||
| 90 | + | spinner := mocks.NewMockSpinnerWidget(t) | |||
| 91 | + | ||||
| 92 | + | overlay.EXPECT().GetAllocatedWidth().Return(0).Times(4) | |||
| 93 | + | overlay.EXPECT().GetAllocatedHeight().Return(0).Times(4) | |||
| 94 | + | for _, widget := range []*mocks.MockWidget{revealedWidget, unrevealedWidget} { | |||
| 95 | + | widget.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 96 | + | widget.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 97 | + | widget.EXPECT().GetParent().Return(nil).Once() | |||
| 98 | + | widget.EXPECT().IsVisible().Return(true).Once() | |||
| 99 | + | } | |||
| 100 | + | overlay.EXPECT().SetChild(revealedWidget).Once() | |||
| 101 | + | loadingContainer.EXPECT().SetVisible(false).Once() | |||
| 102 | + | spinner.EXPECT().Stop().Once() | |||
| 103 | + | overlay.EXPECT().SetChild(nil).Once() | |||
| 104 | + | overlay.EXPECT().SetChild(unrevealedWidget).Once() | |||
| 105 | + | loadingContainer.EXPECT().SetVisible(true).Once() | |||
| 106 | + | spinner.EXPECT().Start().Once() | |||
| 107 | + | ||||
| 108 | + | pv := &PaneView{ | |||
| 109 | + | overlay: overlay, | |||
| 110 | + | loading: &LoadingSkeleton{container: loadingContainer, spinner: spinner}, | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | pv.AttachWebViewWidget(revealedWidget, true) | |||
| 114 | + | pv.AttachWebViewWidget(unrevealedWidget, false) | |||
| 115 | + | ||||
| 116 | + | assert.Same(t, unrevealedWidget, pv.WebViewWidget()) | |||
| 117 | + | } | |||
M internal/ui/component/workspace_view.go +10 -2
| @@ -482,8 +482,16 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||
| 482 | 482 | } | |
| 483 | 483 | } | |
| 484 | 484 | ||
| 485 | - | // SetWebViewWidget attaches a WebView widget to a specific pane. | |
| 485 | + | // SetWebViewWidget attaches a not-yet-revealed WebView widget to a specific pane. | |
| 486 | + | // Call AttachWebViewWidget when the content coordinator has explicit reveal state. | |
| 486 | 487 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | |
| 488 | + | return wv.AttachWebViewWidget(paneID, widget, false) | |
| 489 | + | } | |
| 490 | + | ||
| 491 | + | // AttachWebViewWidget attaches a WebView widget with the content coordinator's | |
| 492 | + | // explicit reveal state. A WorkspaceView rebuild can unparent a live widget, so | |
| 493 | + | // widget parentage cannot determine whether its loading skeleton is still needed. | |
| 494 | + | func (wv *WorkspaceView) AttachWebViewWidget(paneID entity.PaneID, widget layout.Widget, revealed bool) error { | |
| 487 | 495 | wv.mu.RLock() | |
| 488 | 496 | pv, ok := wv.paneViews[paneID] | |
| 489 | 497 | callback := wv.onWebViewAttached | |
| @@ -493,7 +501,7 @@ func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Wi | |||
| 493 | 501 | return ErrPaneNotFound | |
| 494 | 502 | } | |
| 495 | 503 | ||
| 496 | - | pv.SetWebViewWidget(widget) | |
| 504 | + | pv.AttachWebViewWidget(widget, revealed) | |
| 497 | 505 | if widget != nil && callback != nil { | |
| 498 | 506 | callback(paneID) | |
| 499 | 507 | } | |
| @@ -482,8 +482,16 @@ func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||||
| 482 | } | 482 | } | ||
| 483 | } | 483 | } | ||
| 484 | 484 | ||||
| 485 | - | // SetWebViewWidget attaches a WebView widget to a specific pane. | 485 | + | // SetWebViewWidget attaches a not-yet-revealed WebView widget to a specific pane. |
| 486 | + | // Call AttachWebViewWidget when the content coordinator has explicit reveal state. | |||
| 486 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | 487 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | ||
| 488 | + | return wv.AttachWebViewWidget(paneID, widget, false) | |||
| 489 | + | } | |||
| 490 | + | ||||
| 491 | + | // AttachWebViewWidget attaches a WebView widget with the content coordinator's | |||
| 492 | + | // explicit reveal state. A WorkspaceView rebuild can unparent a live widget, so | |||
| 493 | + | // widget parentage cannot determine whether its loading skeleton is still needed. | |||
| 494 | + | func (wv *WorkspaceView) AttachWebViewWidget(paneID entity.PaneID, widget layout.Widget, revealed bool) error { | |||
| 487 | wv.mu.RLock() | 495 | wv.mu.RLock() | ||
| 488 | pv, ok := wv.paneViews[paneID] | 496 | pv, ok := wv.paneViews[paneID] | ||
| 489 | callback := wv.onWebViewAttached | 497 | callback := wv.onWebViewAttached | ||
| @@ -493,7 +501,7 @@ func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Wi | |||||
| 493 | return ErrPaneNotFound | 501 | return ErrPaneNotFound | ||
| 494 | } | 502 | } | ||
| 495 | 503 | ||||
| 496 | - | pv.SetWebViewWidget(widget) | 504 | + | pv.AttachWebViewWidget(widget, revealed) |
| 497 | if widget != nil && callback != nil { | 505 | if widget != nil && callback != nil { | ||
| 498 | callback(paneID) | 506 | callback(paneID) | ||
| 499 | } | 507 | } | ||
M internal/ui/coordinator/content/callbacks.go +9 -4
| @@ -62,8 +62,13 @@ func buildCrashPageURI(originalURI string) string { | |||
| 62 | 62 | // setupWebViewCallbacks configures standard callbacks and popup handling. | |
| 63 | 63 | func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | |
| 64 | 64 | log := logging.FromContext(ctx) | |
| 65 | + | identity, ok := identityForWebView(wv) | |
| 66 | + | if !ok { | |
| 67 | + | return | |
| 68 | + | } | |
| 65 | 69 | ||
| 66 | - | // Build port callbacks for standard events | |
| 70 | + | // Capture the association at installation, rather than reading mutable | |
| 71 | + | // WebView identity from a callback that can outlive a pool reuse. | |
| 67 | 72 | callbacks := &port.WebViewCallbacks{ | |
| 68 | 73 | OnTitleChanged: func(title string) { | |
| 69 | 74 | c.onTitleChanged(ctx, paneID, title) | |
| @@ -73,13 +78,13 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||
| 73 | 78 | case port.LoadStarted: | |
| 74 | 79 | c.onLoadStarted(paneID) | |
| 75 | 80 | case port.LoadCommitted: | |
| 76 | - | c.onLoadCommitted(ctx, paneID, wv) | |
| 81 | + | c.onLoadCommitted(ctx, paneID, wv, identity) | |
| 77 | 82 | case port.LoadFinished: | |
| 78 | - | c.onLoadFinished(ctx, paneID, wv) | |
| 83 | + | c.onLoadFinished(ctx, paneID, wv, identity) | |
| 79 | 84 | } | |
| 80 | 85 | }, | |
| 81 | 86 | OnProgressChanged: func(progress float64) { | |
| 82 | - | c.onProgressChanged(paneID, progress) | |
| 87 | + | c.onProgressChanged(paneID, wv, identity, progress) | |
| 83 | 88 | }, | |
| 84 | 89 | OnURIChanged: func(uri string) { | |
| 85 | 90 | c.handleURIChanged(ctx, paneID, wv, uri) | |
| @@ -62,8 +62,13 @@ func buildCrashPageURI(originalURI string) string { | |||||
| 62 | // setupWebViewCallbacks configures standard callbacks and popup handling. | 62 | // setupWebViewCallbacks configures standard callbacks and popup handling. | ||
| 63 | func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | 63 | func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | ||
| 64 | log := logging.FromContext(ctx) | 64 | log := logging.FromContext(ctx) | ||
| 65 | + | identity, ok := identityForWebView(wv) | |||
| 66 | + | if !ok { | |||
| 67 | + | return | |||
| 68 | + | } | |||
| 65 | 69 | ||||
| 66 | - | // Build port callbacks for standard events | 70 | + | // Capture the association at installation, rather than reading mutable |
| 71 | + | // WebView identity from a callback that can outlive a pool reuse. | |||
| 67 | callbacks := &port.WebViewCallbacks{ | 72 | callbacks := &port.WebViewCallbacks{ | ||
| 68 | OnTitleChanged: func(title string) { | 73 | OnTitleChanged: func(title string) { | ||
| 69 | c.onTitleChanged(ctx, paneID, title) | 74 | c.onTitleChanged(ctx, paneID, title) | ||
| @@ -73,13 +78,13 @@ func (c *Coordinator) setupWebViewCallbacks(ctx context.Context, paneID entity.P | |||||
| 73 | case port.LoadStarted: | 78 | case port.LoadStarted: | ||
| 74 | c.onLoadStarted(paneID) | 79 | c.onLoadStarted(paneID) | ||
| 75 | case port.LoadCommitted: | 80 | case port.LoadCommitted: | ||
| 76 | - | c.onLoadCommitted(ctx, paneID, wv) | 81 | + | c.onLoadCommitted(ctx, paneID, wv, identity) |
| 77 | case port.LoadFinished: | 82 | case port.LoadFinished: | ||
| 78 | - | c.onLoadFinished(ctx, paneID, wv) | 83 | + | c.onLoadFinished(ctx, paneID, wv, identity) |
| 79 | } | 84 | } | ||
| 80 | }, | 85 | }, | ||
| 81 | OnProgressChanged: func(progress float64) { | 86 | OnProgressChanged: func(progress float64) { | ||
| 82 | - | c.onProgressChanged(paneID, progress) | 87 | + | c.onProgressChanged(paneID, wv, identity, progress) |
| 83 | }, | 88 | }, | ||
| 84 | OnURIChanged: func(uri string) { | 89 | OnURIChanged: func(uri string) { | ||
| 85 | c.handleURIChanged(ctx, paneID, wv, uri) | 90 | c.handleURIChanged(ctx, paneID, wv, uri) | ||
M internal/ui/coordinator/content/content_reveal_test.go +154 -171
| @@ -2,10 +2,10 @@ package content | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "sync" | |
| 6 | 5 | "testing" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/stretchr/testify/assert" | |
| 8 | + | "github.com/stretchr/testify/mock" | |
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| 11 | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | |
| @@ -13,201 +13,184 @@ import ( | |||
| 13 | 13 | "github.com/bnema/dumber/internal/ui/component" | |
| 14 | 14 | ) | |
| 15 | 15 | ||
| 16 | - | // TestPendingReveal tests the pending reveal state management. | |
| 17 | - | // These are internal package tests that verify the state transitions | |
| 18 | - | // without needing to mock WebKit dependencies. | |
| 19 | - | ||
| 20 | - | func TestMarkPendingReveal_SetsState(t *testing.T) { | |
| 21 | - | c := &Coordinator{ | |
| 22 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 16 | + | func newRevealTestCoordinator() *Coordinator { | |
| 17 | + | return &Coordinator{ | |
| 18 | + | webViews: make(map[entity.PaneID]port.WebView), | |
| 19 | + | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), | |
| 20 | + | paneTitles: make(map[entity.PaneID]string), | |
| 21 | + | navOrigins: make(map[entity.PaneID]string), | |
| 23 | 22 | } | |
| 24 | - | paneID := entity.PaneID("pane-1") | |
| 25 | - | ||
| 26 | - | c.markPendingReveal(paneID) | |
| 23 | + | } | |
| 27 | 24 | ||
| 28 | - | c.revealMu.Lock() | |
| 29 | - | defer c.revealMu.Unlock() | |
| 30 | - | assert.True(t, c.pendingReveal[paneID]) | |
| 25 | + | func revealTestWebView(t *testing.T, id port.WebViewID, generation uint64) *mocks.MockWebView { | |
| 26 | + | t.Helper() | |
| 27 | + | wv := mocks.NewMockWebView(t) | |
| 28 | + | wv.EXPECT().ID().Return(id).Maybe() | |
| 29 | + | wv.EXPECT().Generation().Return(generation).Maybe() | |
| 30 | + | return wv | |
| 31 | 31 | } | |
| 32 | 32 | ||
| 33 | - | func TestClearPendingReveal_RemovesState(t *testing.T) { | |
| 34 | - | c := &Coordinator{ | |
| 35 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 36 | - | } | |
| 33 | + | func TestRevealState_IsBoundToCurrentWebViewAcrossReplacementPaths(t *testing.T) { | |
| 37 | 34 | paneID := entity.PaneID("pane-1") | |
| 35 | + | first := revealTestWebView(t, 101, 1) | |
| 36 | + | // The pool may reuse the same native WebView ID; its reuse generation is | |
| 37 | + | // part of the presentation identity. | |
| 38 | + | directReplacement := revealTestWebView(t, 101, 2) | |
| 39 | + | popupReplacement := revealTestWebView(t, 103, 1) | |
| 40 | + | c := newRevealTestCoordinator() | |
| 41 | + | ||
| 42 | + | c.setWebViewLocked(paneID, first) | |
| 43 | + | firstIdentity, _ := identityForWebView(first) | |
| 44 | + | c.markWebViewRevealed(paneID, firstIdentity) | |
| 45 | + | assert.True(t, c.WebViewRevealed(paneID)) | |
| 46 | + | ||
| 47 | + | // A direct replacement for the same pane must show the fresh loading state. | |
| 48 | + | c.setWebViewLocked(paneID, directReplacement) | |
| 49 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 50 | + | ||
| 51 | + | directReplacementIdentity, _ := identityForWebView(directReplacement) | |
| 52 | + | c.markWebViewRevealed(paneID, directReplacementIdentity) | |
| 53 | + | c.RegisterPopupWebView(paneID, popupReplacement) | |
| 54 | + | assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") | |
| 55 | + | } | |
| 38 | 56 | ||
| 39 | - | // First mark as pending | |
| 40 | - | c.markPendingReveal(paneID) | |
| 41 | - | ||
| 42 | - | // Then clear | |
| 43 | - | c.clearPendingReveal(paneID) | |
| 57 | + | func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { | |
| 58 | + | paneID := entity.PaneID("pane-1") | |
| 59 | + | wv := revealTestWebView(t, 101, 1) | |
| 60 | + | c := newRevealTestCoordinator() | |
| 61 | + | c.setWebViewLocked(paneID, wv) | |
| 62 | + | identity, _ := identityForWebView(wv) | |
| 63 | + | c.markWebViewRevealed(paneID, identity) | |
| 64 | + | ||
| 65 | + | // Model an earlier partial cleanup that lost the WebView mapping. Release | |
| 66 | + | // remains responsible for clearing presentation state on its early return. | |
| 67 | + | c.webViewsMu.Lock() | |
| 68 | + | delete(c.webViews, paneID) | |
| 69 | + | c.webViewsMu.Unlock() | |
| 70 | + | c.ReleaseWebView(context.Background(), paneID) | |
| 44 | 71 | ||
| 45 | 72 | c.revealMu.Lock() | |
| 46 | - | defer c.revealMu.Unlock() | |
| 47 | - | assert.False(t, c.pendingReveal[paneID]) | |
| 73 | + | _, retained := c.revealedWebViews[paneID] | |
| 74 | + | c.revealMu.Unlock() | |
| 75 | + | assert.False(t, retained) | |
| 48 | 76 | } | |
| 49 | 77 | ||
| 50 | - | func TestRevealIfPending_NotPending_DoesNothing(t *testing.T) { | |
| 51 | - | callbackCalled := false | |
| 52 | - | c := &Coordinator{ | |
| 53 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 54 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 55 | - | onWebViewShown: func(paneID entity.PaneID) { | |
| 56 | - | callbackCalled = true | |
| 57 | - | }, | |
| 58 | - | } | |
| 78 | + | func TestRevealState_ReleaseMappedWebViewClearsRevealStateAndReturnsToPool(t *testing.T) { | |
| 59 | 79 | paneID := entity.PaneID("pane-1") | |
| 60 | - | ||
| 61 | - | // Call reveal without marking pending first | |
| 62 | - | c.revealIfPending(context.Background(), paneID, "http://example.com", "test") | |
| 63 | - | ||
| 64 | - | assert.False(t, callbackCalled, "callback should not be called when not pending") | |
| 80 | + | wv := revealTestWebView(t, 101, 1) | |
| 81 | + | pool := mocks.NewMockWebViewPool(t) | |
| 82 | + | pool.EXPECT().Release(wv).Once() | |
| 83 | + | c := newRevealTestCoordinator() | |
| 84 | + | c.pool = pool | |
| 85 | + | c.setWebViewLocked(paneID, wv) | |
| 86 | + | identity, _ := identityForWebView(wv) | |
| 87 | + | c.markWebViewRevealed(paneID, identity) | |
| 88 | + | ||
| 89 | + | c.ReleaseWebView(context.Background(), paneID) | |
| 90 | + | ||
| 91 | + | assert.Nil(t, c.GetWebView(paneID)) | |
| 92 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 65 | 93 | } | |
| 66 | 94 | ||
| 67 | - | func TestRevealIfPending_ClearsStateOnReveal(t *testing.T) { | |
| 68 | - | c := &Coordinator{ | |
| 69 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 70 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 71 | - | } | |
| 95 | + | func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | |
| 72 | 96 | paneID := entity.PaneID("pane-1") | |
| 73 | - | ||
| 74 | - | // Mark as pending | |
| 75 | - | c.markPendingReveal(paneID) | |
| 76 | - | ||
| 77 | - | // Reveal (will return early since webview is nil, but should still clear state) | |
| 78 | - | c.revealIfPending(context.Background(), paneID, "http://example.com", "test") | |
| 79 | - | ||
| 80 | - | c.revealMu.Lock() | |
| 81 | - | defer c.revealMu.Unlock() | |
| 82 | - | assert.False(t, c.pendingReveal[paneID], "pending state should be cleared after reveal attempt") | |
| 97 | + | oldWV := revealTestWebView(t, 101, 1) | |
| 98 | + | newWV := revealTestWebView(t, 102, 1) | |
| 99 | + | oldWV.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 100 | + | c := newRevealTestCoordinator() | |
| 101 | + | c.setWebViewLocked(paneID, oldWV) | |
| 102 | + | oldIdentity, _ := identityForWebView(oldWV) | |
| 103 | + | c.markPendingReveal(paneID, oldIdentity) | |
| 104 | + | c.setWebViewLocked(paneID, newWV) | |
| 105 | + | ||
| 106 | + | c.revealIfPending(context.Background(), paneID, oldWV, oldIdentity, "", "stale") | |
| 107 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 83 | 108 | } | |
| 84 | 109 | ||
| 85 | - | func TestRevealIfPending_OnlyRevealsOnce(t *testing.T) { | |
| 86 | - | revealCount := 0 | |
| 87 | - | c := &Coordinator{ | |
| 88 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 89 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 90 | - | onWebViewShown: func(paneID entity.PaneID) { | |
| 91 | - | revealCount++ | |
| 92 | - | }, | |
| 93 | - | } | |
| 110 | + | func TestRevealState_RevealedCurrentWebViewPersistsForWorkspaceRebuild(t *testing.T) { | |
| 94 | 111 | paneID := entity.PaneID("pane-1") | |
| 95 | - | ||
| 96 | - | // Mark as pending | |
| 97 | - | c.markPendingReveal(paneID) | |
| 98 | - | ||
| 99 | - | // Call reveal multiple times | |
| 100 | - | c.revealIfPending(context.Background(), paneID, "", "first") | |
| 101 | - | c.revealIfPending(context.Background(), paneID, "", "second") | |
| 102 | - | c.revealIfPending(context.Background(), paneID, "", "third") | |
| 103 | - | ||
| 104 | - | // Callback should not be called since webview is nil, but state should be cleared | |
| 105 | - | assert.Equal(t, 0, revealCount, "callback not called when webview is nil") | |
| 106 | - | ||
| 107 | - | // Verify state is cleared | |
| 108 | - | c.revealMu.Lock() | |
| 109 | - | defer c.revealMu.Unlock() | |
| 110 | - | assert.False(t, c.pendingReveal[paneID]) | |
| 112 | + | wv := revealTestWebView(t, 101, 1) | |
| 113 | + | c := newRevealTestCoordinator() | |
| 114 | + | c.setWebViewLocked(paneID, wv) | |
| 115 | + | identity, _ := identityForWebView(wv) | |
| 116 | + | c.markWebViewRevealed(paneID, identity) | |
| 117 | + | ||
| 118 | + | // AttachToWorkspace asks this identity-aware query when rebuilding a pane, | |
| 119 | + | // so the replacement PaneView hides its skeleton for this same WebView. | |
| 120 | + | assert.True(t, c.webViewRevealed(paneID, wv)) | |
| 121 | + | ||
| 122 | + | replacement := revealTestWebView(t, 102, 1) | |
| 123 | + | assert.False(t, c.webViewRevealed(paneID, replacement)) | |
| 111 | 124 | } | |
| 112 | 125 | ||
| 113 | - | func TestOnLoadCommitted_RevealsPendingWebViewForCommittedPage(t *testing.T) { | |
| 126 | + | func TestRevealCallback_CapturesGenerationBeforeSameObjectPoolReuse(t *testing.T) { | |
| 114 | 127 | paneID := entity.PaneID("pane-1") | |
| 115 | - | shownCount := 0 | |
| 116 | - | var shownPane entity.PaneID | |
| 117 | - | ||
| 118 | 128 | wv := mocks.NewMockWebView(t) | |
| 119 | - | wv.EXPECT().URI().Return("https://example.com") | |
| 120 | - | wv.EXPECT().ResetBackgroundToDefault() | |
| 121 | - | wv.EXPECT().Title().Return("") | |
| 129 | + | generation := uint64(1) | |
| 130 | + | wv.EXPECT().ID().Return(port.WebViewID(101)).Maybe() | |
| 131 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() | |
| 122 | 132 | wv.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 123 | - | ||
| 124 | - | c := &Coordinator{ | |
| 125 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 126 | - | webViews: map[entity.PaneID]port.WebView{ | |
| 127 | - | paneID: wv, | |
| 128 | - | }, | |
| 129 | - | paneTitles: make(map[entity.PaneID]string), | |
| 130 | - | navOrigins: make(map[entity.PaneID]string), | |
| 131 | - | getActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { return nil, nil }, | |
| 132 | - | onWebViewShown: func(id entity.PaneID) { | |
| 133 | - | shownCount++ | |
| 134 | - | shownPane = id | |
| 135 | - | }, | |
| 136 | - | } | |
| 137 | - | c.pendingReveal[paneID] = true | |
| 138 | - | ||
| 139 | - | c.onLoadCommitted(context.Background(), paneID, wv) | |
| 140 | - | ||
| 141 | - | assert.Equal(t, 1, shownCount) | |
| 142 | - | assert.Equal(t, paneID, shownPane) | |
| 143 | - | } | |
| 144 | - | ||
| 145 | - | func TestPendingReveal_ConcurrentAccess(t *testing.T) { | |
| 146 | - | c := &Coordinator{ | |
| 147 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 148 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 149 | - | } | |
| 150 | - | ||
| 151 | - | var wg sync.WaitGroup | |
| 152 | - | paneIDs := []entity.PaneID{"pane-1", "pane-2", "pane-3", "pane-4", "pane-5"} | |
| 153 | - | ||
| 154 | - | // Concurrently mark and clear panes | |
| 155 | - | for range 100 { | |
| 156 | - | for _, paneID := range paneIDs { | |
| 157 | - | wg.Add(3) | |
| 158 | - | ||
| 159 | - | go func(id entity.PaneID) { | |
| 160 | - | defer wg.Done() | |
| 161 | - | c.markPendingReveal(id) | |
| 162 | - | }(paneID) | |
| 163 | - | ||
| 164 | - | go func(id entity.PaneID) { | |
| 165 | - | defer wg.Done() | |
| 166 | - | c.clearPendingReveal(id) | |
| 167 | - | }(paneID) | |
| 168 | - | ||
| 169 | - | go func(id entity.PaneID) { | |
| 170 | - | defer wg.Done() | |
| 171 | - | c.revealIfPending(context.Background(), id, "", "concurrent") | |
| 172 | - | }(paneID) | |
| 173 | - | } | |
| 174 | - | } | |
| 175 | - | ||
| 176 | - | wg.Wait() | |
| 177 | - | // Test passes if no race conditions occur | |
| 133 | + | var callbacks *port.WebViewCallbacks | |
| 134 | + | wv.EXPECT().SetCallbacks(mock.Anything).RunAndReturn(func(value *port.WebViewCallbacks) { | |
| 135 | + | callbacks = value | |
| 136 | + | }).Once() | |
| 137 | + | ||
| 138 | + | c := newRevealTestCoordinator() | |
| 139 | + | c.getActiveWS = func() (*entity.Workspace, *component.WorkspaceView) { return nil, nil } | |
| 140 | + | c.setWebViewLocked(paneID, wv) | |
| 141 | + | installedIdentity, _ := identityForWebView(wv) | |
| 142 | + | c.markPendingReveal(paneID, installedIdentity) | |
| 143 | + | c.setupWebViewCallbacks(context.Background(), paneID, wv) | |
| 144 | + | revealed := false | |
| 145 | + | c.setWebViewVisible = func(port.WebView) { revealed = true } | |
| 146 | + | ||
| 147 | + | // The callback was installed at generation 1. A pool reuse can advance the | |
| 148 | + | // same Go WebView object before its old callback is delivered. | |
| 149 | + | generation = 2 | |
| 150 | + | callbacks.OnProgressChanged(1) | |
| 151 | + | ||
| 152 | + | assert.False(t, revealed, "the generation-1 callback must not reveal generation 2") | |
| 153 | + | assert.False(t, c.WebViewRevealed(paneID)) | |
| 178 | 154 | } | |
| 179 | 155 | ||
| 180 | - | func TestMarkPendingReveal_MultiplePanes(t *testing.T) { | |
| 181 | - | c := &Coordinator{ | |
| 182 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 156 | + | func TestRevealIfPending_SerializesReplacementUntilNativeVisibility(t *testing.T) { | |
| 157 | + | paneID := entity.PaneID("pane-1") | |
| 158 | + | oldWV := revealTestWebView(t, 101, 1) | |
| 159 | + | oldWV.EXPECT().IsDestroyed().Return(false).Once() | |
| 160 | + | newWV := revealTestWebView(t, 102, 1) | |
| 161 | + | c := newRevealTestCoordinator() | |
| 162 | + | c.setWebViewLocked(paneID, oldWV) | |
| 163 | + | identity, _ := identityForWebView(oldWV) | |
| 164 | + | c.markPendingReveal(paneID, identity) | |
| 165 | + | ||
| 166 | + | nativeStarted := make(chan struct{}) | |
| 167 | + | allowNativeReturn := make(chan struct{}) | |
| 168 | + | c.setWebViewVisible = func(wv port.WebView) { | |
| 169 | + | assert.Same(t, oldWV, wv) | |
| 170 | + | close(nativeStarted) | |
| 171 | + | <-allowNativeReturn | |
| 183 | 172 | } | |
| 184 | - | ||
| 185 | - | pane1 := entity.PaneID("pane-1") | |
| 186 | - | pane2 := entity.PaneID("pane-2") | |
| 187 | - | ||
| 188 | - | c.markPendingReveal(pane1) | |
| 189 | - | c.markPendingReveal(pane2) | |
| 190 | - | ||
| 191 | - | c.revealMu.Lock() | |
| 192 | - | defer c.revealMu.Unlock() | |
| 193 | - | assert.True(t, c.pendingReveal[pane1]) | |
| 194 | - | assert.True(t, c.pendingReveal[pane2]) | |
| 195 | - | } | |
| 196 | - | ||
| 197 | - | func TestClearPendingReveal_OnlyAffectsTargetPane(t *testing.T) { | |
| 198 | - | c := &Coordinator{ | |
| 199 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 173 | + | revealed := make(chan struct{}) | |
| 174 | + | go func() { | |
| 175 | + | c.revealIfPending(context.Background(), paneID, oldWV, identity, "", "test") | |
| 176 | + | close(revealed) | |
| 177 | + | }() | |
| 178 | + | <-nativeStarted | |
| 179 | + | ||
| 180 | + | replaced := make(chan struct{}) | |
| 181 | + | go func() { | |
| 182 | + | c.setWebViewLocked(paneID, newWV) | |
| 183 | + | close(replaced) | |
| 184 | + | }() | |
| 185 | + | select { | |
| 186 | + | case <-replaced: | |
| 187 | + | t.Fatal("replacement interleaved before native visibility completed") | |
| 188 | + | default: | |
| 200 | 189 | } | |
| 201 | 190 | ||
| 202 | - | pane1 := entity.PaneID("pane-1") | |
| 203 | - | pane2 := entity.PaneID("pane-2") | |
| 204 | - | ||
| 205 | - | c.markPendingReveal(pane1) | |
| 206 | - | c.markPendingReveal(pane2) | |
| 207 | - | c.clearPendingReveal(pane1) | |
| 208 | - | ||
| 209 | - | c.revealMu.Lock() | |
| 210 | - | defer c.revealMu.Unlock() | |
| 211 | - | assert.False(t, c.pendingReveal[pane1], "pane1 should be cleared") | |
| 212 | - | assert.True(t, c.pendingReveal[pane2], "pane2 should still be pending") | |
| 191 | + | close(allowNativeReturn) | |
| 192 | + | <-revealed | |
| 193 | + | <-replaced | |
| 194 | + | assert.Same(t, newWV, c.GetWebView(paneID)) | |
| 195 | + | assert.False(t, c.WebViewRevealed(paneID), "replacement must not inherit old reveal state") | |
| 213 | 196 | } | |
| @@ -2,10 +2,10 @@ package content | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "sync" | |||
| 6 | "testing" | 5 | "testing" | ||
| 7 | 6 | ||||
| 8 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| 8 | + | "github.com/stretchr/testify/mock" | |||
| 9 | 9 | ||||
| 10 | "github.com/bnema/dumber/internal/application/port" | 10 | "github.com/bnema/dumber/internal/application/port" | ||
| 11 | "github.com/bnema/dumber/internal/application/port/mocks" | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | ||
| @@ -13,201 +13,184 @@ import ( | |||||
| 13 | "github.com/bnema/dumber/internal/ui/component" | 13 | "github.com/bnema/dumber/internal/ui/component" | ||
| 14 | ) | 14 | ) | ||
| 15 | 15 | ||||
| 16 | - | // TestPendingReveal tests the pending reveal state management. | 16 | + | func newRevealTestCoordinator() *Coordinator { |
| 17 | - | // These are internal package tests that verify the state transitions | 17 | + | return &Coordinator{ |
| 18 | - | // without needing to mock WebKit dependencies. | 18 | + | webViews: make(map[entity.PaneID]port.WebView), |
| 19 | - | 19 | + | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), | |
| 20 | - | func TestMarkPendingReveal_SetsState(t *testing.T) { | 20 | + | paneTitles: make(map[entity.PaneID]string), |
| 21 | - | c := &Coordinator{ | 21 | + | navOrigins: make(map[entity.PaneID]string), |
| 22 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 23 | } | 22 | } | ||
| 24 | - | paneID := entity.PaneID("pane-1") | 23 | + | } |
| 25 | - | ||||
| 26 | - | c.markPendingReveal(paneID) | |||
| 27 | 24 | ||||
| 28 | - | c.revealMu.Lock() | 25 | + | func revealTestWebView(t *testing.T, id port.WebViewID, generation uint64) *mocks.MockWebView { |
| 29 | - | defer c.revealMu.Unlock() | 26 | + | t.Helper() |
| 30 | - | assert.True(t, c.pendingReveal[paneID]) | 27 | + | wv := mocks.NewMockWebView(t) |
| 28 | + | wv.EXPECT().ID().Return(id).Maybe() | |||
| 29 | + | wv.EXPECT().Generation().Return(generation).Maybe() | |||
| 30 | + | return wv | |||
| 31 | } | 31 | } | ||
| 32 | 32 | ||||
| 33 | - | func TestClearPendingReveal_RemovesState(t *testing.T) { | 33 | + | func TestRevealState_IsBoundToCurrentWebViewAcrossReplacementPaths(t *testing.T) { |
| 34 | - | c := &Coordinator{ | |||
| 35 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 36 | - | } | |||
| 37 | paneID := entity.PaneID("pane-1") | 34 | paneID := entity.PaneID("pane-1") | ||
| 35 | + | first := revealTestWebView(t, 101, 1) | |||
| 36 | + | // The pool may reuse the same native WebView ID; its reuse generation is | |||
| 37 | + | // part of the presentation identity. | |||
| 38 | + | directReplacement := revealTestWebView(t, 101, 2) | |||
| 39 | + | popupReplacement := revealTestWebView(t, 103, 1) | |||
| 40 | + | c := newRevealTestCoordinator() | |||
| 41 | + | ||||
| 42 | + | c.setWebViewLocked(paneID, first) | |||
| 43 | + | firstIdentity, _ := identityForWebView(first) | |||
| 44 | + | c.markWebViewRevealed(paneID, firstIdentity) | |||
| 45 | + | assert.True(t, c.WebViewRevealed(paneID)) | |||
| 46 | + | ||||
| 47 | + | // A direct replacement for the same pane must show the fresh loading state. | |||
| 48 | + | c.setWebViewLocked(paneID, directReplacement) | |||
| 49 | + | assert.False(t, c.WebViewRevealed(paneID)) | |||
| 50 | + | ||||
| 51 | + | directReplacementIdentity, _ := identityForWebView(directReplacement) | |||
| 52 | + | c.markWebViewRevealed(paneID, directReplacementIdentity) | |||
| 53 | + | c.RegisterPopupWebView(paneID, popupReplacement) | |||
| 54 | + | assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") | |||
| 55 | + | } | |||
| 38 | 56 | ||||
| 39 | - | // First mark as pending | 57 | + | func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { |
| 40 | - | c.markPendingReveal(paneID) | 58 | + | paneID := entity.PaneID("pane-1") |
| 41 | - | 59 | + | wv := revealTestWebView(t, 101, 1) | |
| 42 | - | // Then clear | 60 | + | c := newRevealTestCoordinator() |
| 43 | - | c.clearPendingReveal(paneID) | 61 | + | c.setWebViewLocked(paneID, wv) |
| 62 | + | identity, _ := identityForWebView(wv) | |||
| 63 | + | c.markWebViewRevealed(paneID, identity) | |||
| 64 | + | ||||
| 65 | + | // Model an earlier partial cleanup that lost the WebView mapping. Release | |||
| 66 | + | // remains responsible for clearing presentation state on its early return. | |||
| 67 | + | c.webViewsMu.Lock() | |||
| 68 | + | delete(c.webViews, paneID) | |||
| 69 | + | c.webViewsMu.Unlock() | |||
| 70 | + | c.ReleaseWebView(context.Background(), paneID) | |||
| 44 | 71 | ||||
| 45 | c.revealMu.Lock() | 72 | c.revealMu.Lock() | ||
| 46 | - | defer c.revealMu.Unlock() | 73 | + | _, retained := c.revealedWebViews[paneID] |
| 47 | - | assert.False(t, c.pendingReveal[paneID]) | 74 | + | c.revealMu.Unlock() |
| 75 | + | assert.False(t, retained) | |||
| 48 | } | 76 | } | ||
| 49 | 77 | ||||
| 50 | - | func TestRevealIfPending_NotPending_DoesNothing(t *testing.T) { | 78 | + | func TestRevealState_ReleaseMappedWebViewClearsRevealStateAndReturnsToPool(t *testing.T) { |
| 51 | - | callbackCalled := false | |||
| 52 | - | c := &Coordinator{ | |||
| 53 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 54 | - | webViews: make(map[entity.PaneID]port.WebView), | |||
| 55 | - | onWebViewShown: func(paneID entity.PaneID) { | |||
| 56 | - | callbackCalled = true | |||
| 57 | - | }, | |||
| 58 | - | } | |||
| 59 | paneID := entity.PaneID("pane-1") | 79 | paneID := entity.PaneID("pane-1") | ||
| 60 | - | 80 | + | wv := revealTestWebView(t, 101, 1) | |
| 61 | - | // Call reveal without marking pending first | 81 | + | pool := mocks.NewMockWebViewPool(t) |
| 62 | - | c.revealIfPending(context.Background(), paneID, "http://example.com", "test") | 82 | + | pool.EXPECT().Release(wv).Once() |
| 63 | - | 83 | + | c := newRevealTestCoordinator() | |
| 64 | - | assert.False(t, callbackCalled, "callback should not be called when not pending") | 84 | + | c.pool = pool |
| 85 | + | c.setWebViewLocked(paneID, wv) | |||
| 86 | + | identity, _ := identityForWebView(wv) | |||
| 87 | + | c.markWebViewRevealed(paneID, identity) | |||
| 88 | + | ||||
| 89 | + | c.ReleaseWebView(context.Background(), paneID) | |||
| 90 | + | ||||
| 91 | + | assert.Nil(t, c.GetWebView(paneID)) | |||
| 92 | + | assert.False(t, c.WebViewRevealed(paneID)) | |||
| 65 | } | 93 | } | ||
| 66 | 94 | ||||
| 67 | - | func TestRevealIfPending_ClearsStateOnReveal(t *testing.T) { | 95 | + | func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { |
| 68 | - | c := &Coordinator{ | |||
| 69 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 70 | - | webViews: make(map[entity.PaneID]port.WebView), | |||
| 71 | - | } | |||
| 72 | paneID := entity.PaneID("pane-1") | 96 | paneID := entity.PaneID("pane-1") | ||
| 73 | - | 97 | + | oldWV := revealTestWebView(t, 101, 1) | |
| 74 | - | // Mark as pending | 98 | + | newWV := revealTestWebView(t, 102, 1) |
| 75 | - | c.markPendingReveal(paneID) | 99 | + | oldWV.EXPECT().IsDestroyed().Return(false).Maybe() |
| 76 | - | 100 | + | c := newRevealTestCoordinator() | |
| 77 | - | // Reveal (will return early since webview is nil, but should still clear state) | 101 | + | c.setWebViewLocked(paneID, oldWV) |
| 78 | - | c.revealIfPending(context.Background(), paneID, "http://example.com", "test") | 102 | + | oldIdentity, _ := identityForWebView(oldWV) |
| 79 | - | 103 | + | c.markPendingReveal(paneID, oldIdentity) | |
| 80 | - | c.revealMu.Lock() | 104 | + | c.setWebViewLocked(paneID, newWV) |
| 81 | - | defer c.revealMu.Unlock() | 105 | + | |
| 82 | - | assert.False(t, c.pendingReveal[paneID], "pending state should be cleared after reveal attempt") | 106 | + | c.revealIfPending(context.Background(), paneID, oldWV, oldIdentity, "", "stale") |
| 107 | + | assert.False(t, c.WebViewRevealed(paneID)) | |||
| 83 | } | 108 | } | ||
| 84 | 109 | ||||
| 85 | - | func TestRevealIfPending_OnlyRevealsOnce(t *testing.T) { | 110 | + | func TestRevealState_RevealedCurrentWebViewPersistsForWorkspaceRebuild(t *testing.T) { |
| 86 | - | revealCount := 0 | |||
| 87 | - | c := &Coordinator{ | |||
| 88 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 89 | - | webViews: make(map[entity.PaneID]port.WebView), | |||
| 90 | - | onWebViewShown: func(paneID entity.PaneID) { | |||
| 91 | - | revealCount++ | |||
| 92 | - | }, | |||
| 93 | - | } | |||
| 94 | paneID := entity.PaneID("pane-1") | 111 | paneID := entity.PaneID("pane-1") | ||
| 95 | - | 112 | + | wv := revealTestWebView(t, 101, 1) | |
| 96 | - | // Mark as pending | 113 | + | c := newRevealTestCoordinator() |
| 97 | - | c.markPendingReveal(paneID) | 114 | + | c.setWebViewLocked(paneID, wv) |
| 98 | - | 115 | + | identity, _ := identityForWebView(wv) | |
| 99 | - | // Call reveal multiple times | 116 | + | c.markWebViewRevealed(paneID, identity) |
| 100 | - | c.revealIfPending(context.Background(), paneID, "", "first") | 117 | + | |
| 101 | - | c.revealIfPending(context.Background(), paneID, "", "second") | 118 | + | // AttachToWorkspace asks this identity-aware query when rebuilding a pane, |
| 102 | - | c.revealIfPending(context.Background(), paneID, "", "third") | 119 | + | // so the replacement PaneView hides its skeleton for this same WebView. |
| 103 | - | 120 | + | assert.True(t, c.webViewRevealed(paneID, wv)) | |
| 104 | - | // Callback should not be called since webview is nil, but state should be cleared | 121 | + | |
| 105 | - | assert.Equal(t, 0, revealCount, "callback not called when webview is nil") | 122 | + | replacement := revealTestWebView(t, 102, 1) |
| 106 | - | 123 | + | assert.False(t, c.webViewRevealed(paneID, replacement)) | |
| 107 | - | // Verify state is cleared | |||
| 108 | - | c.revealMu.Lock() | |||
| 109 | - | defer c.revealMu.Unlock() | |||
| 110 | - | assert.False(t, c.pendingReveal[paneID]) | |||
| 111 | } | 124 | } | ||
| 112 | 125 | ||||
| 113 | - | func TestOnLoadCommitted_RevealsPendingWebViewForCommittedPage(t *testing.T) { | 126 | + | func TestRevealCallback_CapturesGenerationBeforeSameObjectPoolReuse(t *testing.T) { |
| 114 | paneID := entity.PaneID("pane-1") | 127 | paneID := entity.PaneID("pane-1") | ||
| 115 | - | shownCount := 0 | |||
| 116 | - | var shownPane entity.PaneID | |||
| 117 | - | ||||
| 118 | wv := mocks.NewMockWebView(t) | 128 | wv := mocks.NewMockWebView(t) | ||
| 119 | - | wv.EXPECT().URI().Return("https://example.com") | 129 | + | generation := uint64(1) |
| 120 | - | wv.EXPECT().ResetBackgroundToDefault() | 130 | + | wv.EXPECT().ID().Return(port.WebViewID(101)).Maybe() |
| 121 | - | wv.EXPECT().Title().Return("") | 131 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() |
| 122 | wv.EXPECT().IsDestroyed().Return(false).Maybe() | 132 | wv.EXPECT().IsDestroyed().Return(false).Maybe() | ||
| 123 | - | 133 | + | var callbacks *port.WebViewCallbacks | |
| 124 | - | c := &Coordinator{ | 134 | + | wv.EXPECT().SetCallbacks(mock.Anything).RunAndReturn(func(value *port.WebViewCallbacks) { |
| 125 | - | pendingReveal: make(map[entity.PaneID]bool), | 135 | + | callbacks = value |
| 126 | - | webViews: map[entity.PaneID]port.WebView{ | 136 | + | }).Once() |
| 127 | - | paneID: wv, | 137 | + | |
| 128 | - | }, | 138 | + | c := newRevealTestCoordinator() |
| 129 | - | paneTitles: make(map[entity.PaneID]string), | 139 | + | c.getActiveWS = func() (*entity.Workspace, *component.WorkspaceView) { return nil, nil } |
| 130 | - | navOrigins: make(map[entity.PaneID]string), | 140 | + | c.setWebViewLocked(paneID, wv) |
| 131 | - | getActiveWS: func() (*entity.Workspace, *component.WorkspaceView) { return nil, nil }, | 141 | + | installedIdentity, _ := identityForWebView(wv) |
| 132 | - | onWebViewShown: func(id entity.PaneID) { | 142 | + | c.markPendingReveal(paneID, installedIdentity) |
| 133 | - | shownCount++ | 143 | + | c.setupWebViewCallbacks(context.Background(), paneID, wv) |
| 134 | - | shownPane = id | 144 | + | revealed := false |
| 135 | - | }, | 145 | + | c.setWebViewVisible = func(port.WebView) { revealed = true } |
| 136 | - | } | 146 | + | |
| 137 | - | c.pendingReveal[paneID] = true | 147 | + | // The callback was installed at generation 1. A pool reuse can advance the |
| 138 | - | 148 | + | // same Go WebView object before its old callback is delivered. | |
| 139 | - | c.onLoadCommitted(context.Background(), paneID, wv) | 149 | + | generation = 2 |
| 140 | - | 150 | + | callbacks.OnProgressChanged(1) | |
| 141 | - | assert.Equal(t, 1, shownCount) | 151 | + | |
| 142 | - | assert.Equal(t, paneID, shownPane) | 152 | + | assert.False(t, revealed, "the generation-1 callback must not reveal generation 2") |
| 143 | - | } | 153 | + | assert.False(t, c.WebViewRevealed(paneID)) |
| 144 | - | ||||
| 145 | - | func TestPendingReveal_ConcurrentAccess(t *testing.T) { | |||
| 146 | - | c := &Coordinator{ | |||
| 147 | - | pendingReveal: make(map[entity.PaneID]bool), | |||
| 148 | - | webViews: make(map[entity.PaneID]port.WebView), | |||
| 149 | - | } | |||
| 150 | - | ||||
| 151 | - | var wg sync.WaitGroup | |||
| 152 | - | paneIDs := []entity.PaneID{"pane-1", "pane-2", "pane-3", "pane-4", "pane-5"} | |||
| 153 | - | ||||
| 154 | - | // Concurrently mark and clear panes | |||
| 155 | - | for range 100 { | |||
| 156 | - | for _, paneID := range paneIDs { | |||
| 157 | - | wg.Add(3) | |||
| 158 | - | ||||
| 159 | - | go func(id entity.PaneID) { | |||
| 160 | - | defer wg.Done() | |||
| 161 | - | c.markPendingReveal(id) | |||
| 162 | - | }(paneID) | |||
| 163 | - | ||||
| 164 | - | go func(id entity.PaneID) { | |||
| 165 | - | defer wg.Done() | |||
| 166 | - | c.clearPendingReveal(id) | |||
| 167 | - | }(paneID) | |||
| 168 | - | ||||
| 169 | - | go func(id entity.PaneID) { | |||
| 170 | - | defer wg.Done() | |||
| 171 | - | c.revealIfPending(context.Background(), id, "", "concurrent") | |||
| 172 | - | }(paneID) | |||
| 173 | - | } | |||
| 174 | - | } | |||
| 175 | - | ||||
| 176 | - | wg.Wait() | |||
| 177 | - | // Test passes if no race conditions occur | |||
| 178 | } | 154 | } | ||
| 179 | 155 | ||||
| 180 | - | func TestMarkPendingReveal_MultiplePanes(t *testing.T) { | 156 | + | func TestRevealIfPending_SerializesReplacementUntilNativeVisibility(t *testing.T) { |
| 181 | - | c := &Coordinator{ | 157 | + | paneID := entity.PaneID("pane-1") |
| 182 | - | pendingReveal: make(map[entity.PaneID]bool), | 158 | + | oldWV := revealTestWebView(t, 101, 1) |
| 159 | + | oldWV.EXPECT().IsDestroyed().Return(false).Once() | |||
| 160 | + | newWV := revealTestWebView(t, 102, 1) | |||
| 161 | + | c := newRevealTestCoordinator() | |||
| 162 | + | c.setWebViewLocked(paneID, oldWV) | |||
| 163 | + | identity, _ := identityForWebView(oldWV) | |||
| 164 | + | c.markPendingReveal(paneID, identity) | |||
| 165 | + | ||||
| 166 | + | nativeStarted := make(chan struct{}) | |||
| 167 | + | allowNativeReturn := make(chan struct{}) | |||
| 168 | + | c.setWebViewVisible = func(wv port.WebView) { | |||
| 169 | + | assert.Same(t, oldWV, wv) | |||
| 170 | + | close(nativeStarted) | |||
| 171 | + | <-allowNativeReturn | |||
| 183 | } | 172 | } | ||
| 184 | - | 173 | + | revealed := make(chan struct{}) | |
| 185 | - | pane1 := entity.PaneID("pane-1") | 174 | + | go func() { |
| 186 | - | pane2 := entity.PaneID("pane-2") | 175 | + | c.revealIfPending(context.Background(), paneID, oldWV, identity, "", "test") |
| 187 | - | 176 | + | close(revealed) | |
| 188 | - | c.markPendingReveal(pane1) | 177 | + | }() |
| 189 | - | c.markPendingReveal(pane2) | 178 | + | <-nativeStarted |
| 190 | - | 179 | + | ||
| 191 | - | c.revealMu.Lock() | 180 | + | replaced := make(chan struct{}) |
| 192 | - | defer c.revealMu.Unlock() | 181 | + | go func() { |
| 193 | - | assert.True(t, c.pendingReveal[pane1]) | 182 | + | c.setWebViewLocked(paneID, newWV) |
| 194 | - | assert.True(t, c.pendingReveal[pane2]) | 183 | + | close(replaced) |
| 195 | - | } | 184 | + | }() |
| 196 | - | 185 | + | select { | |
| 197 | - | func TestClearPendingReveal_OnlyAffectsTargetPane(t *testing.T) { | 186 | + | case <-replaced: |
| 198 | - | c := &Coordinator{ | 187 | + | t.Fatal("replacement interleaved before native visibility completed") |
| 199 | - | pendingReveal: make(map[entity.PaneID]bool), | 188 | + | default: |
| 200 | } | 189 | } | ||
| 201 | 190 | ||||
| 202 | - | pane1 := entity.PaneID("pane-1") | 191 | + | close(allowNativeReturn) |
| 203 | - | pane2 := entity.PaneID("pane-2") | 192 | + | <-revealed |
| 204 | - | 193 | + | <-replaced | |
| 205 | - | c.markPendingReveal(pane1) | 194 | + | assert.Same(t, newWV, c.GetWebView(paneID)) |
| 206 | - | c.markPendingReveal(pane2) | 195 | + | assert.False(t, c.WebViewRevealed(paneID), "replacement must not inherit old reveal state") |
| 207 | - | c.clearPendingReveal(pane1) | |||
| 208 | - | ||||
| 209 | - | c.revealMu.Lock() | |||
| 210 | - | defer c.revealMu.Unlock() | |||
| 211 | - | assert.False(t, c.pendingReveal[pane1], "pane1 should be cleared") | |||
| 212 | - | assert.True(t, c.pendingReveal[pane2], "pane2 should still be pending") | |||
| 213 | } | 196 | } | ||
M internal/ui/coordinator/content/coordinator.go +84 -19
| @@ -70,8 +70,21 @@ type Coordinator struct { | |||
| 70 | 70 | // Callback when the WebView becomes visible (first real commit) | |
| 71 | 71 | onWebViewShown func(paneID entity.PaneID) | |
| 72 | 72 | ||
| 73 | - | revealMu sync.Mutex | |
| 74 | - | pendingReveal map[entity.PaneID]bool | |
| 73 | + | // setWebViewVisible is test-only injection for deterministic lifecycle | |
| 74 | + | // serialization tests. Production uses the native widget provider directly. | |
| 75 | + | setWebViewVisible func(port.WebView) | |
| 76 | + | ||
| 77 | + | // revealMutationMu serializes mapping replacement/release with the native | |
| 78 | + | // visibility transition. It is acquired before webViewsMu and revealMu. | |
| 79 | + | revealMutationMu sync.Mutex | |
| 80 | + | ||
| 81 | + | // revealMu is always acquired after webViewsMu. Reveal state is keyed by the | |
| 82 | + | // WebView identity (ID plus pool-reuse generation), never by pane alone. | |
| 83 | + | revealMu sync.Mutex | |
| 84 | + | pendingReveal map[entity.PaneID]bool | |
| 85 | + | revealedWebViews map[entity.PaneID]bool | |
| 86 | + | revealedWebViewIdentity map[entity.PaneID]webViewIdentity | |
| 87 | + | pendingRevealIdentity map[entity.PaneID]webViewIdentity | |
| 75 | 88 | ||
| 76 | 89 | appearanceMu sync.Mutex | |
| 77 | 90 | pendingScriptRefresh map[entity.PaneID]bool | |
| @@ -136,22 +149,25 @@ func NewCoordinator( | |||
| 136 | 149 | log.Debug().Msg("creating content coordinator") | |
| 137 | 150 | ||
| 138 | 151 | return &Coordinator{ | |
| 139 | - | logger: log.With().Str("component", "content-coordinator").Logger(), | |
| 140 | - | pool: pool, | |
| 141 | - | injector: injector, | |
| 142 | - | widgetFactory: widgetFactory, | |
| 143 | - | faviconAdapter: faviconAdapter, | |
| 144 | - | zoomUC: zoomUC, | |
| 145 | - | permissionUC: permissionUC, | |
| 146 | - | webViews: make(map[entity.PaneID]port.WebView), | |
| 147 | - | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), | |
| 148 | - | paneTitles: make(map[entity.PaneID]string), | |
| 149 | - | navOrigins: make(map[entity.PaneID]string), | |
| 150 | - | pendingReveal: make(map[entity.PaneID]bool), | |
| 151 | - | pendingScriptRefresh: make(map[entity.PaneID]bool), | |
| 152 | - | pendingThemePanes: make(map[entity.PaneID]bool), | |
| 153 | - | getActiveWS: getActiveWS, | |
| 154 | - | popups: newPopupManager(), | |
| 152 | + | logger: log.With().Str("component", "content-coordinator").Logger(), | |
| 153 | + | pool: pool, | |
| 154 | + | injector: injector, | |
| 155 | + | widgetFactory: widgetFactory, | |
| 156 | + | faviconAdapter: faviconAdapter, | |
| 157 | + | zoomUC: zoomUC, | |
| 158 | + | permissionUC: permissionUC, | |
| 159 | + | webViews: make(map[entity.PaneID]port.WebView), | |
| 160 | + | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), | |
| 161 | + | paneTitles: make(map[entity.PaneID]string), | |
| 162 | + | navOrigins: make(map[entity.PaneID]string), | |
| 163 | + | pendingReveal: make(map[entity.PaneID]bool), | |
| 164 | + | revealedWebViews: make(map[entity.PaneID]bool), | |
| 165 | + | revealedWebViewIdentity: make(map[entity.PaneID]webViewIdentity), | |
| 166 | + | pendingRevealIdentity: make(map[entity.PaneID]webViewIdentity), | |
| 167 | + | pendingScriptRefresh: make(map[entity.PaneID]bool), | |
| 168 | + | pendingThemePanes: make(map[entity.PaneID]bool), | |
| 169 | + | getActiveWS: getActiveWS, | |
| 170 | + | popups: newPopupManager(), | |
| 155 | 171 | } | |
| 156 | 172 | } | |
| 157 | 173 | ||
| @@ -310,8 +326,42 @@ func (c *Coordinator) getWebViewLocked(paneID entity.PaneID) port.WebView { | |||
| 310 | 326 | return c.webViews[paneID] | |
| 311 | 327 | } | |
| 312 | 328 | ||
| 329 | + | type webViewIdentity struct { | |
| 330 | + | id port.WebViewID | |
| 331 | + | generation uint64 | |
| 332 | + | } | |
| 333 | + | ||
| 334 | + | func identityForWebView(wv port.WebView) (webViewIdentity, bool) { | |
| 335 | + | if wv == nil { | |
| 336 | + | return webViewIdentity{}, false | |
| 337 | + | } | |
| 338 | + | return webViewIdentity{id: wv.ID(), generation: wv.Generation()}, true | |
| 339 | + | } | |
| 340 | + | ||
| 341 | + | func (c *Coordinator) ensureRevealMapsLocked() { | |
| 342 | + | if c.pendingReveal == nil { | |
| 343 | + | c.pendingReveal = make(map[entity.PaneID]bool) | |
| 344 | + | } | |
| 345 | + | if c.revealedWebViews == nil { | |
| 346 | + | c.revealedWebViews = make(map[entity.PaneID]bool) | |
| 347 | + | } | |
| 348 | + | if c.revealedWebViewIdentity == nil { | |
| 349 | + | c.revealedWebViewIdentity = make(map[entity.PaneID]webViewIdentity) | |
| 350 | + | } | |
| 351 | + | if c.pendingRevealIdentity == nil { | |
| 352 | + | c.pendingRevealIdentity = make(map[entity.PaneID]webViewIdentity) | |
| 353 | + | } | |
| 354 | + | } | |
| 355 | + | ||
| 356 | + | // setWebViewLocked replaces the pane mapping and resets all reveal state in | |
| 357 | + | // one critical section. It is used by normal acquisition and popup paths. | |
| 313 | 358 | func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |
| 359 | + | c.revealMutationMu.Lock() | |
| 360 | + | defer c.revealMutationMu.Unlock() | |
| 314 | 361 | c.webViewsMu.Lock() | |
| 362 | + | c.revealMu.Lock() | |
| 363 | + | defer c.revealMu.Unlock() | |
| 364 | + | defer c.webViewsMu.Unlock() | |
| 315 | 365 | if c.webViews == nil { | |
| 316 | 366 | c.webViews = make(map[entity.PaneID]port.WebView) | |
| 317 | 367 | } | |
| @@ -322,17 +372,32 @@ func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |||
| 322 | 372 | if wv != nil && c.webViewPaneIDs != nil { | |
| 323 | 373 | c.webViewPaneIDs[wv.ID()] = paneID | |
| 324 | 374 | } | |
| 325 | - | c.webViewsMu.Unlock() | |
| 375 | + | c.ensureRevealMapsLocked() | |
| 376 | + | delete(c.pendingReveal, paneID) | |
| 377 | + | delete(c.pendingRevealIdentity, paneID) | |
| 378 | + | delete(c.revealedWebViews, paneID) | |
| 379 | + | delete(c.revealedWebViewIdentity, paneID) | |
| 326 | 380 | } | |
| 327 | 381 | ||
| 382 | + | // deleteWebViewLocked removes both the mapping and any presentation state, | |
| 383 | + | // including callers which release after another path already removed the map. | |
| 328 | 384 | func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView { | |
| 385 | + | c.revealMutationMu.Lock() | |
| 386 | + | defer c.revealMutationMu.Unlock() | |
| 329 | 387 | c.webViewsMu.Lock() | |
| 388 | + | c.revealMu.Lock() | |
| 389 | + | defer c.revealMu.Unlock() | |
| 330 | 390 | defer c.webViewsMu.Unlock() | |
| 331 | 391 | wv := c.webViews[paneID] | |
| 332 | 392 | delete(c.webViews, paneID) | |
| 333 | 393 | if wv != nil && c.webViewPaneIDs != nil { | |
| 334 | 394 | delete(c.webViewPaneIDs, wv.ID()) | |
| 335 | 395 | } | |
| 396 | + | c.ensureRevealMapsLocked() | |
| 397 | + | delete(c.pendingReveal, paneID) | |
| 398 | + | delete(c.pendingRevealIdentity, paneID) | |
| 399 | + | delete(c.revealedWebViews, paneID) | |
| 400 | + | delete(c.revealedWebViewIdentity, paneID) | |
| 336 | 401 | return wv | |
| 337 | 402 | } | |
| 338 | 403 | ||
| @@ -70,8 +70,21 @@ type Coordinator struct { | |||||
| 70 | // Callback when the WebView becomes visible (first real commit) | 70 | // Callback when the WebView becomes visible (first real commit) | ||
| 71 | onWebViewShown func(paneID entity.PaneID) | 71 | onWebViewShown func(paneID entity.PaneID) | ||
| 72 | 72 | ||||
| 73 | - | revealMu sync.Mutex | 73 | + | // setWebViewVisible is test-only injection for deterministic lifecycle |
| 74 | - | pendingReveal map[entity.PaneID]bool | 74 | + | // serialization tests. Production uses the native widget provider directly. |
| 75 | + | setWebViewVisible func(port.WebView) | |||
| 76 | + | ||||
| 77 | + | // revealMutationMu serializes mapping replacement/release with the native | |||
| 78 | + | // visibility transition. It is acquired before webViewsMu and revealMu. | |||
| 79 | + | revealMutationMu sync.Mutex | |||
| 80 | + | ||||
| 81 | + | // revealMu is always acquired after webViewsMu. Reveal state is keyed by the | |||
| 82 | + | // WebView identity (ID plus pool-reuse generation), never by pane alone. | |||
| 83 | + | revealMu sync.Mutex | |||
| 84 | + | pendingReveal map[entity.PaneID]bool | |||
| 85 | + | revealedWebViews map[entity.PaneID]bool | |||
| 86 | + | revealedWebViewIdentity map[entity.PaneID]webViewIdentity | |||
| 87 | + | pendingRevealIdentity map[entity.PaneID]webViewIdentity | |||
| 75 | 88 | ||||
| 76 | appearanceMu sync.Mutex | 89 | appearanceMu sync.Mutex | ||
| 77 | pendingScriptRefresh map[entity.PaneID]bool | 90 | pendingScriptRefresh map[entity.PaneID]bool | ||
| @@ -136,22 +149,25 @@ func NewCoordinator( | |||||
| 136 | log.Debug().Msg("creating content coordinator") | 149 | log.Debug().Msg("creating content coordinator") | ||
| 137 | 150 | ||||
| 138 | return &Coordinator{ | 151 | return &Coordinator{ | ||
| 139 | - | logger: log.With().Str("component", "content-coordinator").Logger(), | 152 | + | logger: log.With().Str("component", "content-coordinator").Logger(), |
| 140 | - | pool: pool, | 153 | + | pool: pool, |
| 141 | - | injector: injector, | 154 | + | injector: injector, |
| 142 | - | widgetFactory: widgetFactory, | 155 | + | widgetFactory: widgetFactory, |
| 143 | - | faviconAdapter: faviconAdapter, | 156 | + | faviconAdapter: faviconAdapter, |
| 144 | - | zoomUC: zoomUC, | 157 | + | zoomUC: zoomUC, |
| 145 | - | permissionUC: permissionUC, | 158 | + | permissionUC: permissionUC, |
| 146 | - | webViews: make(map[entity.PaneID]port.WebView), | 159 | + | webViews: make(map[entity.PaneID]port.WebView), |
| 147 | - | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), | 160 | + | webViewPaneIDs: make(map[port.WebViewID]entity.PaneID), |
| 148 | - | paneTitles: make(map[entity.PaneID]string), | 161 | + | paneTitles: make(map[entity.PaneID]string), |
| 149 | - | navOrigins: make(map[entity.PaneID]string), | 162 | + | navOrigins: make(map[entity.PaneID]string), |
| 150 | - | pendingReveal: make(map[entity.PaneID]bool), | 163 | + | pendingReveal: make(map[entity.PaneID]bool), |
| 151 | - | pendingScriptRefresh: make(map[entity.PaneID]bool), | 164 | + | revealedWebViews: make(map[entity.PaneID]bool), |
| 152 | - | pendingThemePanes: make(map[entity.PaneID]bool), | 165 | + | revealedWebViewIdentity: make(map[entity.PaneID]webViewIdentity), |
| 153 | - | getActiveWS: getActiveWS, | 166 | + | pendingRevealIdentity: make(map[entity.PaneID]webViewIdentity), |
| 154 | - | popups: newPopupManager(), | 167 | + | pendingScriptRefresh: make(map[entity.PaneID]bool), |
| 168 | + | pendingThemePanes: make(map[entity.PaneID]bool), | |||
| 169 | + | getActiveWS: getActiveWS, | |||
| 170 | + | popups: newPopupManager(), | |||
| 155 | } | 171 | } | ||
| 156 | } | 172 | } | ||
| 157 | 173 | ||||
| @@ -310,8 +326,42 @@ func (c *Coordinator) getWebViewLocked(paneID entity.PaneID) port.WebView { | |||||
| 310 | return c.webViews[paneID] | 326 | return c.webViews[paneID] | ||
| 311 | } | 327 | } | ||
| 312 | 328 | ||||
| 329 | + | type webViewIdentity struct { | |||
| 330 | + | id port.WebViewID | |||
| 331 | + | generation uint64 | |||
| 332 | + | } | |||
| 333 | + | ||||
| 334 | + | func identityForWebView(wv port.WebView) (webViewIdentity, bool) { | |||
| 335 | + | if wv == nil { | |||
| 336 | + | return webViewIdentity{}, false | |||
| 337 | + | } | |||
| 338 | + | return webViewIdentity{id: wv.ID(), generation: wv.Generation()}, true | |||
| 339 | + | } | |||
| 340 | + | ||||
| 341 | + | func (c *Coordinator) ensureRevealMapsLocked() { | |||
| 342 | + | if c.pendingReveal == nil { | |||
| 343 | + | c.pendingReveal = make(map[entity.PaneID]bool) | |||
| 344 | + | } | |||
| 345 | + | if c.revealedWebViews == nil { | |||
| 346 | + | c.revealedWebViews = make(map[entity.PaneID]bool) | |||
| 347 | + | } | |||
| 348 | + | if c.revealedWebViewIdentity == nil { | |||
| 349 | + | c.revealedWebViewIdentity = make(map[entity.PaneID]webViewIdentity) | |||
| 350 | + | } | |||
| 351 | + | if c.pendingRevealIdentity == nil { | |||
| 352 | + | c.pendingRevealIdentity = make(map[entity.PaneID]webViewIdentity) | |||
| 353 | + | } | |||
| 354 | + | } | |||
| 355 | + | ||||
| 356 | + | // setWebViewLocked replaces the pane mapping and resets all reveal state in | |||
| 357 | + | // one critical section. It is used by normal acquisition and popup paths. | |||
| 313 | func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | 358 | func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | ||
| 359 | + | c.revealMutationMu.Lock() | |||
| 360 | + | defer c.revealMutationMu.Unlock() | |||
| 314 | c.webViewsMu.Lock() | 361 | c.webViewsMu.Lock() | ||
| 362 | + | c.revealMu.Lock() | |||
| 363 | + | defer c.revealMu.Unlock() | |||
| 364 | + | defer c.webViewsMu.Unlock() | |||
| 315 | if c.webViews == nil { | 365 | if c.webViews == nil { | ||
| 316 | c.webViews = make(map[entity.PaneID]port.WebView) | 366 | c.webViews = make(map[entity.PaneID]port.WebView) | ||
| 317 | } | 367 | } | ||
| @@ -322,17 +372,32 @@ func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |||||
| 322 | if wv != nil && c.webViewPaneIDs != nil { | 372 | if wv != nil && c.webViewPaneIDs != nil { | ||
| 323 | c.webViewPaneIDs[wv.ID()] = paneID | 373 | c.webViewPaneIDs[wv.ID()] = paneID | ||
| 324 | } | 374 | } | ||
| 325 | - | c.webViewsMu.Unlock() | 375 | + | c.ensureRevealMapsLocked() |
| 376 | + | delete(c.pendingReveal, paneID) | |||
| 377 | + | delete(c.pendingRevealIdentity, paneID) | |||
| 378 | + | delete(c.revealedWebViews, paneID) | |||
| 379 | + | delete(c.revealedWebViewIdentity, paneID) | |||
| 326 | } | 380 | } | ||
| 327 | 381 | ||||
| 382 | + | // deleteWebViewLocked removes both the mapping and any presentation state, | |||
| 383 | + | // including callers which release after another path already removed the map. | |||
| 328 | func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView { | 384 | func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView { | ||
| 385 | + | c.revealMutationMu.Lock() | |||
| 386 | + | defer c.revealMutationMu.Unlock() | |||
| 329 | c.webViewsMu.Lock() | 387 | c.webViewsMu.Lock() | ||
| 388 | + | c.revealMu.Lock() | |||
| 389 | + | defer c.revealMu.Unlock() | |||
| 330 | defer c.webViewsMu.Unlock() | 390 | defer c.webViewsMu.Unlock() | ||
| 331 | wv := c.webViews[paneID] | 391 | wv := c.webViews[paneID] | ||
| 332 | delete(c.webViews, paneID) | 392 | delete(c.webViews, paneID) | ||
| 333 | if wv != nil && c.webViewPaneIDs != nil { | 393 | if wv != nil && c.webViewPaneIDs != nil { | ||
| 334 | delete(c.webViewPaneIDs, wv.ID()) | 394 | delete(c.webViewPaneIDs, wv.ID()) | ||
| 335 | } | 395 | } | ||
| 396 | + | c.ensureRevealMapsLocked() | |||
| 397 | + | delete(c.pendingReveal, paneID) | |||
| 398 | + | delete(c.pendingRevealIdentity, paneID) | |||
| 399 | + | delete(c.revealedWebViews, paneID) | |||
| 400 | + | delete(c.revealedWebViewIdentity, paneID) | |||
| 336 | return wv | 401 | return wv | ||
| 337 | } | 402 | } | ||
| 338 | 403 | ||||
M internal/ui/coordinator/content/lifecycle.go +5 -1
| @@ -39,6 +39,8 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||
| 39 | 39 | } | |
| 40 | 40 | logging.Trace().Mark("webview_acquired") | |
| 41 | 41 | ||
| 42 | + | // setWebViewLocked atomically resets presentation state for the acquired | |
| 43 | + | // WebView, including a pooled instance previously revealed in another pane. | |
| 42 | 44 | c.setWebViewLocked(paneID, wv) | |
| 43 | 45 | c.setupWebViewCallbacks(ctx, paneID, wv) | |
| 44 | 46 | ||
| @@ -53,6 +55,8 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||
| 53 | 55 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | |
| 54 | 56 | log := logging.FromContext(ctx) | |
| 55 | 57 | ||
| 58 | + | // deleteWebViewLocked clears reveal state before returning, even when a | |
| 59 | + | // previous cleanup already removed this pane's mapping. | |
| 56 | 60 | wv := c.deleteWebViewLocked(paneID) | |
| 57 | 61 | if wv == nil { | |
| 58 | 62 | return | |
| @@ -136,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||
| 136 | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | |
| 137 | 141 | Msg("wrapped webview widget prepared for pane") | |
| 138 | 142 | ||
| 139 | - | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | |
| 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.webViewRevealed(pane.ID, wv)); err != nil { | |
| 140 | 144 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | |
| 141 | 145 | continue | |
| 142 | 146 | } | |
| @@ -39,6 +39,8 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||||
| 39 | } | 39 | } | ||
| 40 | logging.Trace().Mark("webview_acquired") | 40 | logging.Trace().Mark("webview_acquired") | ||
| 41 | 41 | ||||
| 42 | + | // setWebViewLocked atomically resets presentation state for the acquired | |||
| 43 | + | // WebView, including a pooled instance previously revealed in another pane. | |||
| 42 | c.setWebViewLocked(paneID, wv) | 44 | c.setWebViewLocked(paneID, wv) | ||
| 43 | c.setupWebViewCallbacks(ctx, paneID, wv) | 45 | c.setupWebViewCallbacks(ctx, paneID, wv) | ||
| 44 | 46 | ||||
| @@ -53,6 +55,8 @@ func (c *Coordinator) EnsureWebView(ctx context.Context, paneID entity.PaneID) ( | |||||
| 53 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | 55 | func (c *Coordinator) ReleaseWebView(ctx context.Context, paneID entity.PaneID) { | ||
| 54 | log := logging.FromContext(ctx) | 56 | log := logging.FromContext(ctx) | ||
| 55 | 57 | ||||
| 58 | + | // deleteWebViewLocked clears reveal state before returning, even when a | |||
| 59 | + | // previous cleanup already removed this pane's mapping. | |||
| 56 | wv := c.deleteWebViewLocked(paneID) | 60 | wv := c.deleteWebViewLocked(paneID) | ||
| 57 | if wv == nil { | 61 | if wv == nil { | ||
| 58 | return | 62 | return | ||
| @@ -136,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||||
| 136 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | ||
| 137 | Msg("wrapped webview widget prepared for pane") | 141 | Msg("wrapped webview widget prepared for pane") | ||
| 138 | 142 | ||||
| 139 | - | if err := wsView.SetWebViewWidget(pane.ID, widget); err != nil { | 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.webViewRevealed(pane.ID, wv)); err != nil { |
| 140 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | 144 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | ||
| 141 | continue | 145 | continue | ||
| 142 | } | 146 | } | ||
M internal/ui/coordinator/content/lifecycle_test.go +204 -2
| @@ -58,7 +58,7 @@ func TestLifecycle_EnsureWebView_AcquiresFromPoolWhenNoneExists(t *testing.T) { | |||
| 58 | 58 | newWV := mocks.NewMockWebView(t) | |
| 59 | 59 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 60 | 60 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | |
| 61 | - | newWV.EXPECT().ID().Return(port.WebViewID(102)).Once() | |
| 61 | + | newWV.EXPECT().ID().Return(port.WebViewID(102)).Maybe() | |
| 62 | 62 | ||
| 63 | 63 | pool := mocks.NewMockWebViewPool(t) | |
| 64 | 64 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | |
| @@ -82,7 +82,7 @@ func TestLifecycle_EnsureWebView_AcquiresFromPoolWhenExistingIsDestroyed(t *test | |||
| 82 | 82 | newWV := mocks.NewMockWebView(t) | |
| 83 | 83 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | |
| 84 | 84 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | |
| 85 | - | newWV.EXPECT().ID().Return(port.WebViewID(103)).Once() | |
| 85 | + | newWV.EXPECT().ID().Return(port.WebViewID(103)).Maybe() | |
| 86 | 86 | ||
| 87 | 87 | pool := mocks.NewMockWebViewPool(t) | |
| 88 | 88 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | |
| @@ -458,3 +458,205 @@ func TestLifecycle_SyncWebViewViewport_NoopWithoutCapability(t *testing.T) { | |||
| 458 | 458 | ||
| 459 | 459 | c.SyncWebViewViewport(context.Background(), "pane-1", "unit-test") | |
| 460 | 460 | } | |
| 461 | + | ||
| 462 | + | type nativeLifecycleWebView struct { | |
| 463 | + | port.WebView | |
| 464 | + | widget uintptr | |
| 465 | + | } | |
| 466 | + | ||
| 467 | + | func (w *nativeLifecycleWebView) NativeWidget() uintptr { return w.widget } | |
| 468 | + | ||
| 469 | + | func TestLifecycle_AttachToWorkspace_RebuildPropagatesRevealStateToFreshPaneView(t *testing.T) { | |
| 470 | + | for _, tc := range []struct { | |
| 471 | + | name string | |
| 472 | + | revealed bool | |
| 473 | + | }{ | |
| 474 | + | {name: "revealed current identity hides and stops skeleton", revealed: true}, | |
| 475 | + | {name: "unrevealed current identity shows and starts skeleton", revealed: false}, | |
| 476 | + | } { | |
| 477 | + | t.Run(tc.name, func(t *testing.T) { | |
| 478 | + | paneID := entity.PaneID("pane-1") | |
| 479 | + | ctx := context.Background() | |
| 480 | + | factory := layoutmocks.NewMockWidgetFactory(t) | |
| 481 | + | wsView, workspace, paneOverlay, loadingContainer, spinner := newLifecycleRebuildWorkspaceView(t, ctx, factory, paneID) | |
| 482 | + | wrappedWidget := layoutmocks.NewMockWidget(t) | |
| 483 | + | webViewMock := mocks.NewMockWebView(t) | |
| 484 | + | webViewMock.EXPECT().ID().Return(port.WebViewID(201)).Maybe() | |
| 485 | + | webViewMock.EXPECT().Generation().Return(uint64(3)).Maybe() | |
| 486 | + | webViewMock.EXPECT().IsDestroyed().Return(false).Once() | |
| 487 | + | webView := &nativeLifecycleWebView{WebView: webViewMock, widget: 0x351} | |
| 488 | + | ||
| 489 | + | factory.EXPECT().WrapNativeWidget(uintptr(0x351)).Return(wrappedWidget).Once() | |
| 490 | + | wrappedWidget.EXPECT().GtkWidget().Return(nil).Once() | |
| 491 | + | wrappedWidget.EXPECT().GetAllocatedWidth().Return(0).Times(3) | |
| 492 | + | wrappedWidget.EXPECT().GetAllocatedHeight().Return(0).Times(3) | |
| 493 | + | wrappedWidget.EXPECT().GetParent().Return(nil).Once() | |
| 494 | + | wrappedWidget.EXPECT().IsVisible().Return(true).Once() | |
| 495 | + | paneOverlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |
| 496 | + | paneOverlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |
| 497 | + | paneOverlay.EXPECT().SetChild(wrappedWidget).Once() | |
| 498 | + | loadingContainer.EXPECT().SetVisible(!tc.revealed).Once() | |
| 499 | + | if tc.revealed { | |
| 500 | + | spinner.EXPECT().Stop().Once() | |
| 501 | + | } else { | |
| 502 | + | spinner.EXPECT().Start().Once() | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | coordinator := newRevealTestCoordinator() | |
| 506 | + | coordinator.widgetFactory = factory | |
| 507 | + | coordinator.setWebViewLocked(paneID, webView) | |
| 508 | + | if tc.revealed { | |
| 509 | + | identity, ok := identityForWebView(webView) | |
| 510 | + | require.True(t, ok) | |
| 511 | + | coordinator.markWebViewRevealed(paneID, identity) | |
| 512 | + | } | |
| 513 | + | ||
| 514 | + | coordinator.AttachToWorkspace(ctx, workspace, wsView) | |
| 515 | + | ||
| 516 | + | assert.Same(t, wrappedWidget, wsView.GetPaneView(paneID).WebViewWidget()) | |
| 517 | + | }) | |
| 518 | + | } | |
| 519 | + | } | |
| 520 | + | ||
| 521 | + | func newLifecycleRebuildWorkspaceView( | |
| 522 | + | t *testing.T, | |
| 523 | + | ctx context.Context, | |
| 524 | + | factory *layoutmocks.MockWidgetFactory, | |
| 525 | + | paneID entity.PaneID, | |
| 526 | + | ) (*component.WorkspaceView, *entity.Workspace, *layoutmocks.MockOverlayWidget, *layoutmocks.MockBoxWidget, *layoutmocks.MockSpinnerWidget) { | |
| 527 | + | t.Helper() | |
| 528 | + | workspaceContainer := layoutmocks.NewMockBoxWidget(t) | |
| 529 | + | workspaceOverlay := layoutmocks.NewMockOverlayWidget(t) | |
| 530 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(workspaceContainer).Once() | |
| 531 | + | workspaceContainer.EXPECT().SetHexpand(true).Once() | |
| 532 | + | workspaceContainer.EXPECT().SetVexpand(true).Once() | |
| 533 | + | workspaceContainer.EXPECT().SetVisible(true).Once() | |
| 534 | + | factory.EXPECT().NewOverlay().Return(workspaceOverlay).Once() | |
| 535 | + | workspaceOverlay.EXPECT().SetHexpand(true).Once() | |
| 536 | + | workspaceOverlay.EXPECT().SetVexpand(true).Once() | |
| 537 | + | workspaceOverlay.EXPECT().SetChild(workspaceContainer).Once() | |
| 538 | + | workspaceOverlay.EXPECT().SetVisible(true).Once() | |
| 539 | + | wsView := component.NewWorkspaceView(ctx, factory) | |
| 540 | + | ||
| 541 | + | paneOverlay := layoutmocks.NewMockOverlayWidget(t) | |
| 542 | + | loadingContainer := layoutmocks.NewMockBoxWidget(t) | |
| 543 | + | loadingContent := layoutmocks.NewMockBoxWidget(t) | |
| 544 | + | spinner := layoutmocks.NewMockSpinnerWidget(t) | |
| 545 | + | logo := layoutmocks.NewMockImageWidget(t) | |
| 546 | + | version := layoutmocks.NewMockLabelWidget(t) | |
| 547 | + | border := layoutmocks.NewMockBoxWidget(t) | |
| 548 | + | ||
| 549 | + | factory.EXPECT().NewOverlay().Return(paneOverlay).Once() | |
| 550 | + | paneOverlay.EXPECT().SetHexpand(true).Once() | |
| 551 | + | paneOverlay.EXPECT().SetVexpand(true).Once() | |
| 552 | + | paneOverlay.EXPECT().SetVisible(true).Once() | |
| 553 | + | paneOverlay.EXPECT().AddCssClass("pane-overlay").Once() | |
| 554 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(loadingContainer).Once() | |
| 555 | + | loadingContainer.EXPECT().SetHexpand(true).Once() | |
| 556 | + | loadingContainer.EXPECT().SetVexpand(true).Once() | |
| 557 | + | loadingContainer.EXPECT().SetHalign(mock.Anything).Once() | |
| 558 | + | loadingContainer.EXPECT().SetValign(mock.Anything).Once() | |
| 559 | + | loadingContainer.EXPECT().SetCanFocus(false).Times(2) | |
| 560 | + | loadingContainer.EXPECT().SetCanTarget(false).Times(2) | |
| 561 | + | loadingContainer.EXPECT().AddCssClass("loading-skeleton").Once() | |
| 562 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 6).Return(loadingContent).Once() | |
| 563 | + | loadingContent.EXPECT().SetHalign(mock.Anything).Once() | |
| 564 | + | loadingContent.EXPECT().SetValign(mock.Anything).Once() | |
| 565 | + | loadingContent.EXPECT().SetCanFocus(false).Once() | |
| 566 | + | loadingContent.EXPECT().SetCanTarget(false).Once() | |
| 567 | + | loadingContent.EXPECT().AddCssClass("loading-skeleton-content").Once() | |
| 568 | + | factory.EXPECT().NewImage().Return(logo).Once() | |
| 569 | + | logo.EXPECT().SetHalign(mock.Anything).Once() | |
| 570 | + | logo.EXPECT().SetValign(mock.Anything).Once() | |
| 571 | + | logo.EXPECT().SetCanFocus(false).Once() | |
| 572 | + | logo.EXPECT().SetCanTarget(false).Once() | |
| 573 | + | logo.EXPECT().SetSizeRequest(256, 256).Once() | |
| 574 | + | logo.EXPECT().SetPixelSize(256).Once() | |
| 575 | + | logo.EXPECT().AddCssClass("loading-skeleton-logo").Once() | |
| 576 | + | logo.EXPECT().SetFromPaintable(mock.Anything).Maybe() | |
| 577 | + | factory.EXPECT().NewSpinner().Return(spinner).Once() | |
| 578 | + | spinner.EXPECT().SetHalign(mock.Anything).Once() | |
| 579 | + | spinner.EXPECT().SetValign(mock.Anything).Once() | |
| 580 | + | spinner.EXPECT().SetCanFocus(false).Once() | |
| 581 | + | spinner.EXPECT().SetCanTarget(false).Once() | |
| 582 | + | spinner.EXPECT().SetSizeRequest(32, 32).Once() | |
| 583 | + | spinner.EXPECT().AddCssClass("loading-skeleton-spinner").Once() | |
| 584 | + | factory.EXPECT().NewLabel(mock.Anything).Return(version).Once() | |
| 585 | + | version.EXPECT().SetHalign(mock.Anything).Once() | |
| 586 | + | version.EXPECT().SetValign(mock.Anything).Once() | |
| 587 | + | version.EXPECT().SetCanFocus(false).Once() | |
| 588 | + | version.EXPECT().SetCanTarget(false).Once() | |
| 589 | + | version.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |
| 590 | + | version.EXPECT().SetEllipsize(mock.Anything).Once() | |
| 591 | + | version.EXPECT().AddCssClass("loading-skeleton-version").Once() | |
| 592 | + | loadingContent.EXPECT().Append(logo).Once() | |
| 593 | + | loadingContent.EXPECT().Append(spinner).Once() | |
| 594 | + | loadingContent.EXPECT().Append(version).Once() | |
| 595 | + | loadingContainer.EXPECT().Append(loadingContent).Once() | |
| 596 | + | loadingContainer.EXPECT().SetVisible(true).Once() | |
| 597 | + | spinner.EXPECT().Start().Once() | |
| 598 | + | paneOverlay.EXPECT().AddOverlay(loadingContainer).Once() | |
| 599 | + | paneOverlay.EXPECT().SetClipOverlay(loadingContainer, false).Once() | |
| 600 | + | paneOverlay.EXPECT().SetMeasureOverlay(loadingContainer, false).Once() | |
| 601 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(border).Once() | |
| 602 | + | border.EXPECT().SetCanFocus(false).Once() | |
| 603 | + | border.EXPECT().SetCanTarget(false).Once() | |
| 604 | + | border.EXPECT().AddCssClass("pane-border").Once() | |
| 605 | + | border.EXPECT().SetHexpand(true).Once() | |
| 606 | + | border.EXPECT().SetVexpand(true).Once() | |
| 607 | + | paneOverlay.EXPECT().AddOverlay(border).Once() | |
| 608 | + | paneOverlay.EXPECT().SetClipOverlay(border, false).Once() | |
| 609 | + | paneOverlay.EXPECT().SetMeasureOverlay(border, false).Once() | |
| 610 | + | ||
| 611 | + | stackBox := layoutmocks.NewMockBoxWidget(t) | |
| 612 | + | titleBar := layoutmocks.NewMockBoxWidget(t) | |
| 613 | + | favicon := layoutmocks.NewMockImageWidget(t) | |
| 614 | + | title := layoutmocks.NewMockLabelWidget(t) | |
| 615 | + | closeButton := layoutmocks.NewMockButtonWidget(t) | |
| 616 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(stackBox).Once() | |
| 617 | + | stackBox.EXPECT().SetHexpand(true).Once() | |
| 618 | + | stackBox.EXPECT().SetVexpand(true).Once() | |
| 619 | + | factory.EXPECT().NewBox(layout.OrientationHorizontal, 4).Return(titleBar).Once() | |
| 620 | + | titleBar.EXPECT().AddCssClass("stacked-pane-titlebar").Once() | |
| 621 | + | titleBar.EXPECT().AddCssClass("stacked-pane-title-clickable").Once() | |
| 622 | + | titleBar.EXPECT().SetVexpand(false).Once() | |
| 623 | + | titleBar.EXPECT().SetHexpand(true).Once() | |
| 624 | + | factory.EXPECT().NewImage().Return(favicon).Once() | |
| 625 | + | favicon.EXPECT().SetFromIconName(mock.Anything).Once() | |
| 626 | + | favicon.EXPECT().SetPixelSize(16).Once() | |
| 627 | + | titleBar.EXPECT().Append(favicon).Once() | |
| 628 | + | factory.EXPECT().NewLabel(mock.Anything).Return(title).Once() | |
| 629 | + | title.EXPECT().SetEllipsize(layout.EllipsizeEnd).Once() | |
| 630 | + | title.EXPECT().SetMaxWidthChars(30).Once() | |
| 631 | + | title.EXPECT().SetHexpand(true).Once() | |
| 632 | + | title.EXPECT().SetXalign(float32(0)).Once() | |
| 633 | + | titleBar.EXPECT().Append(title).Once() | |
| 634 | + | factory.EXPECT().NewButton().Return(closeButton).Once() | |
| 635 | + | closeButton.EXPECT().SetIconName("window-close-symbolic").Once() | |
| 636 | + | closeButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |
| 637 | + | closeButton.EXPECT().SetFocusOnClick(false).Once() | |
| 638 | + | closeButton.EXPECT().SetVexpand(false).Once() | |
| 639 | + | closeButton.EXPECT().SetHexpand(false).Once() | |
| 640 | + | titleBar.EXPECT().Append(closeButton).Once() | |
| 641 | + | titleBar.EXPECT().AddController(mock.Anything).Once() | |
| 642 | + | closeButton.EXPECT().ConnectClicked(mock.Anything).Return(uint(2)).Once() | |
| 643 | + | closeButton.EXPECT().GtkWidget().Return(nil).Maybe() | |
| 644 | + | stackBox.EXPECT().Append(titleBar).Once() | |
| 645 | + | stackBox.EXPECT().Append(paneOverlay).Once() | |
| 646 | + | titleBar.EXPECT().SetVisible(false).Once() | |
| 647 | + | paneOverlay.EXPECT().SetVisible(true).Once() | |
| 648 | + | titleBar.EXPECT().AddCssClass("active").Once() | |
| 649 | + | paneOverlay.EXPECT().GtkWidget().Return(nil).Maybe() | |
| 650 | + | stackBox.EXPECT().SetVisible(true).Once() | |
| 651 | + | workspaceContainer.EXPECT().Append(stackBox).Once() | |
| 652 | + | workspaceContainer.EXPECT().AddCssClass("single-pane").Once() | |
| 653 | + | border.EXPECT().AddCssClass("pane-active").Once() | |
| 654 | + | ||
| 655 | + | workspace := &entity.Workspace{ | |
| 656 | + | ID: "workspace-1", | |
| 657 | + | Root: &entity.PaneNode{ID: "node-1", Pane: entity.NewPane(paneID)}, | |
| 658 | + | ActivePaneID: paneID, | |
| 659 | + | } | |
| 660 | + | require.NoError(t, wsView.SetWorkspace(ctx, workspace)) | |
| 661 | + | return wsView, workspace, paneOverlay, loadingContainer, spinner | |
| 662 | + | } | |
| @@ -58,7 +58,7 @@ func TestLifecycle_EnsureWebView_AcquiresFromPoolWhenNoneExists(t *testing.T) { | |||||
| 58 | newWV := mocks.NewMockWebView(t) | 58 | newWV := mocks.NewMockWebView(t) | ||
| 59 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | 59 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | ||
| 60 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | 60 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | ||
| 61 | - | newWV.EXPECT().ID().Return(port.WebViewID(102)).Once() | 61 | + | newWV.EXPECT().ID().Return(port.WebViewID(102)).Maybe() |
| 62 | 62 | ||||
| 63 | pool := mocks.NewMockWebViewPool(t) | 63 | pool := mocks.NewMockWebViewPool(t) | ||
| 64 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | 64 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | ||
| @@ -82,7 +82,7 @@ func TestLifecycle_EnsureWebView_AcquiresFromPoolWhenExistingIsDestroyed(t *test | |||||
| 82 | newWV := mocks.NewMockWebView(t) | 82 | newWV := mocks.NewMockWebView(t) | ||
| 83 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | 83 | newWV.EXPECT().SetCallbacks(mock.Anything).Maybe() | ||
| 84 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | 84 | newWV.EXPECT().Generation().Return(uint64(0)).Maybe() | ||
| 85 | - | newWV.EXPECT().ID().Return(port.WebViewID(103)).Once() | 85 | + | newWV.EXPECT().ID().Return(port.WebViewID(103)).Maybe() |
| 86 | 86 | ||||
| 87 | pool := mocks.NewMockWebViewPool(t) | 87 | pool := mocks.NewMockWebViewPool(t) | ||
| 88 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | 88 | pool.EXPECT().Acquire(mock.Anything).Return(newWV, nil) | ||
| @@ -458,3 +458,205 @@ func TestLifecycle_SyncWebViewViewport_NoopWithoutCapability(t *testing.T) { | |||||
| 458 | 458 | ||||
| 459 | c.SyncWebViewViewport(context.Background(), "pane-1", "unit-test") | 459 | c.SyncWebViewViewport(context.Background(), "pane-1", "unit-test") | ||
| 460 | } | 460 | } | ||
| 461 | + | ||||
| 462 | + | type nativeLifecycleWebView struct { | |||
| 463 | + | port.WebView | |||
| 464 | + | widget uintptr | |||
| 465 | + | } | |||
| 466 | + | ||||
| 467 | + | func (w *nativeLifecycleWebView) NativeWidget() uintptr { return w.widget } | |||
| 468 | + | ||||
| 469 | + | func TestLifecycle_AttachToWorkspace_RebuildPropagatesRevealStateToFreshPaneView(t *testing.T) { | |||
| 470 | + | for _, tc := range []struct { | |||
| 471 | + | name string | |||
| 472 | + | revealed bool | |||
| 473 | + | }{ | |||
| 474 | + | {name: "revealed current identity hides and stops skeleton", revealed: true}, | |||
| 475 | + | {name: "unrevealed current identity shows and starts skeleton", revealed: false}, | |||
| 476 | + | } { | |||
| 477 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 478 | + | paneID := entity.PaneID("pane-1") | |||
| 479 | + | ctx := context.Background() | |||
| 480 | + | factory := layoutmocks.NewMockWidgetFactory(t) | |||
| 481 | + | wsView, workspace, paneOverlay, loadingContainer, spinner := newLifecycleRebuildWorkspaceView(t, ctx, factory, paneID) | |||
| 482 | + | wrappedWidget := layoutmocks.NewMockWidget(t) | |||
| 483 | + | webViewMock := mocks.NewMockWebView(t) | |||
| 484 | + | webViewMock.EXPECT().ID().Return(port.WebViewID(201)).Maybe() | |||
| 485 | + | webViewMock.EXPECT().Generation().Return(uint64(3)).Maybe() | |||
| 486 | + | webViewMock.EXPECT().IsDestroyed().Return(false).Once() | |||
| 487 | + | webView := &nativeLifecycleWebView{WebView: webViewMock, widget: 0x351} | |||
| 488 | + | ||||
| 489 | + | factory.EXPECT().WrapNativeWidget(uintptr(0x351)).Return(wrappedWidget).Once() | |||
| 490 | + | wrappedWidget.EXPECT().GtkWidget().Return(nil).Once() | |||
| 491 | + | wrappedWidget.EXPECT().GetAllocatedWidth().Return(0).Times(3) | |||
| 492 | + | wrappedWidget.EXPECT().GetAllocatedHeight().Return(0).Times(3) | |||
| 493 | + | wrappedWidget.EXPECT().GetParent().Return(nil).Once() | |||
| 494 | + | wrappedWidget.EXPECT().IsVisible().Return(true).Once() | |||
| 495 | + | paneOverlay.EXPECT().GetAllocatedWidth().Return(0).Twice() | |||
| 496 | + | paneOverlay.EXPECT().GetAllocatedHeight().Return(0).Twice() | |||
| 497 | + | paneOverlay.EXPECT().SetChild(wrappedWidget).Once() | |||
| 498 | + | loadingContainer.EXPECT().SetVisible(!tc.revealed).Once() | |||
| 499 | + | if tc.revealed { | |||
| 500 | + | spinner.EXPECT().Stop().Once() | |||
| 501 | + | } else { | |||
| 502 | + | spinner.EXPECT().Start().Once() | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | coordinator := newRevealTestCoordinator() | |||
| 506 | + | coordinator.widgetFactory = factory | |||
| 507 | + | coordinator.setWebViewLocked(paneID, webView) | |||
| 508 | + | if tc.revealed { | |||
| 509 | + | identity, ok := identityForWebView(webView) | |||
| 510 | + | require.True(t, ok) | |||
| 511 | + | coordinator.markWebViewRevealed(paneID, identity) | |||
| 512 | + | } | |||
| 513 | + | ||||
| 514 | + | coordinator.AttachToWorkspace(ctx, workspace, wsView) | |||
| 515 | + | ||||
| 516 | + | assert.Same(t, wrappedWidget, wsView.GetPaneView(paneID).WebViewWidget()) | |||
| 517 | + | }) | |||
| 518 | + | } | |||
| 519 | + | } | |||
| 520 | + | ||||
| 521 | + | func newLifecycleRebuildWorkspaceView( | |||
| 522 | + | t *testing.T, | |||
| 523 | + | ctx context.Context, | |||
| 524 | + | factory *layoutmocks.MockWidgetFactory, | |||
| 525 | + | paneID entity.PaneID, | |||
| 526 | + | ) (*component.WorkspaceView, *entity.Workspace, *layoutmocks.MockOverlayWidget, *layoutmocks.MockBoxWidget, *layoutmocks.MockSpinnerWidget) { | |||
| 527 | + | t.Helper() | |||
| 528 | + | workspaceContainer := layoutmocks.NewMockBoxWidget(t) | |||
| 529 | + | workspaceOverlay := layoutmocks.NewMockOverlayWidget(t) | |||
| 530 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(workspaceContainer).Once() | |||
| 531 | + | workspaceContainer.EXPECT().SetHexpand(true).Once() | |||
| 532 | + | workspaceContainer.EXPECT().SetVexpand(true).Once() | |||
| 533 | + | workspaceContainer.EXPECT().SetVisible(true).Once() | |||
| 534 | + | factory.EXPECT().NewOverlay().Return(workspaceOverlay).Once() | |||
| 535 | + | workspaceOverlay.EXPECT().SetHexpand(true).Once() | |||
| 536 | + | workspaceOverlay.EXPECT().SetVexpand(true).Once() | |||
| 537 | + | workspaceOverlay.EXPECT().SetChild(workspaceContainer).Once() | |||
| 538 | + | workspaceOverlay.EXPECT().SetVisible(true).Once() | |||
| 539 | + | wsView := component.NewWorkspaceView(ctx, factory) | |||
| 540 | + | ||||
| 541 | + | paneOverlay := layoutmocks.NewMockOverlayWidget(t) | |||
| 542 | + | loadingContainer := layoutmocks.NewMockBoxWidget(t) | |||
| 543 | + | loadingContent := layoutmocks.NewMockBoxWidget(t) | |||
| 544 | + | spinner := layoutmocks.NewMockSpinnerWidget(t) | |||
| 545 | + | logo := layoutmocks.NewMockImageWidget(t) | |||
| 546 | + | version := layoutmocks.NewMockLabelWidget(t) | |||
| 547 | + | border := layoutmocks.NewMockBoxWidget(t) | |||
| 548 | + | ||||
| 549 | + | factory.EXPECT().NewOverlay().Return(paneOverlay).Once() | |||
| 550 | + | paneOverlay.EXPECT().SetHexpand(true).Once() | |||
| 551 | + | paneOverlay.EXPECT().SetVexpand(true).Once() | |||
| 552 | + | paneOverlay.EXPECT().SetVisible(true).Once() | |||
| 553 | + | paneOverlay.EXPECT().AddCssClass("pane-overlay").Once() | |||
| 554 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(loadingContainer).Once() | |||
| 555 | + | loadingContainer.EXPECT().SetHexpand(true).Once() | |||
| 556 | + | loadingContainer.EXPECT().SetVexpand(true).Once() | |||
| 557 | + | loadingContainer.EXPECT().SetHalign(mock.Anything).Once() | |||
| 558 | + | loadingContainer.EXPECT().SetValign(mock.Anything).Once() | |||
| 559 | + | loadingContainer.EXPECT().SetCanFocus(false).Times(2) | |||
| 560 | + | loadingContainer.EXPECT().SetCanTarget(false).Times(2) | |||
| 561 | + | loadingContainer.EXPECT().AddCssClass("loading-skeleton").Once() | |||
| 562 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 6).Return(loadingContent).Once() | |||
| 563 | + | loadingContent.EXPECT().SetHalign(mock.Anything).Once() | |||
| 564 | + | loadingContent.EXPECT().SetValign(mock.Anything).Once() | |||
| 565 | + | loadingContent.EXPECT().SetCanFocus(false).Once() | |||
| 566 | + | loadingContent.EXPECT().SetCanTarget(false).Once() | |||
| 567 | + | loadingContent.EXPECT().AddCssClass("loading-skeleton-content").Once() | |||
| 568 | + | factory.EXPECT().NewImage().Return(logo).Once() | |||
| 569 | + | logo.EXPECT().SetHalign(mock.Anything).Once() | |||
| 570 | + | logo.EXPECT().SetValign(mock.Anything).Once() | |||
| 571 | + | logo.EXPECT().SetCanFocus(false).Once() | |||
| 572 | + | logo.EXPECT().SetCanTarget(false).Once() | |||
| 573 | + | logo.EXPECT().SetSizeRequest(256, 256).Once() | |||
| 574 | + | logo.EXPECT().SetPixelSize(256).Once() | |||
| 575 | + | logo.EXPECT().AddCssClass("loading-skeleton-logo").Once() | |||
| 576 | + | logo.EXPECT().SetFromPaintable(mock.Anything).Maybe() | |||
| 577 | + | factory.EXPECT().NewSpinner().Return(spinner).Once() | |||
| 578 | + | spinner.EXPECT().SetHalign(mock.Anything).Once() | |||
| 579 | + | spinner.EXPECT().SetValign(mock.Anything).Once() | |||
| 580 | + | spinner.EXPECT().SetCanFocus(false).Once() | |||
| 581 | + | spinner.EXPECT().SetCanTarget(false).Once() | |||
| 582 | + | spinner.EXPECT().SetSizeRequest(32, 32).Once() | |||
| 583 | + | spinner.EXPECT().AddCssClass("loading-skeleton-spinner").Once() | |||
| 584 | + | factory.EXPECT().NewLabel(mock.Anything).Return(version).Once() | |||
| 585 | + | version.EXPECT().SetHalign(mock.Anything).Once() | |||
| 586 | + | version.EXPECT().SetValign(mock.Anything).Once() | |||
| 587 | + | version.EXPECT().SetCanFocus(false).Once() | |||
| 588 | + | version.EXPECT().SetCanTarget(false).Once() | |||
| 589 | + | version.EXPECT().SetMaxWidthChars(mock.Anything).Once() | |||
| 590 | + | version.EXPECT().SetEllipsize(mock.Anything).Once() | |||
| 591 | + | version.EXPECT().AddCssClass("loading-skeleton-version").Once() | |||
| 592 | + | loadingContent.EXPECT().Append(logo).Once() | |||
| 593 | + | loadingContent.EXPECT().Append(spinner).Once() | |||
| 594 | + | loadingContent.EXPECT().Append(version).Once() | |||
| 595 | + | loadingContainer.EXPECT().Append(loadingContent).Once() | |||
| 596 | + | loadingContainer.EXPECT().SetVisible(true).Once() | |||
| 597 | + | spinner.EXPECT().Start().Once() | |||
| 598 | + | paneOverlay.EXPECT().AddOverlay(loadingContainer).Once() | |||
| 599 | + | paneOverlay.EXPECT().SetClipOverlay(loadingContainer, false).Once() | |||
| 600 | + | paneOverlay.EXPECT().SetMeasureOverlay(loadingContainer, false).Once() | |||
| 601 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(border).Once() | |||
| 602 | + | border.EXPECT().SetCanFocus(false).Once() | |||
| 603 | + | border.EXPECT().SetCanTarget(false).Once() | |||
| 604 | + | border.EXPECT().AddCssClass("pane-border").Once() | |||
| 605 | + | border.EXPECT().SetHexpand(true).Once() | |||
| 606 | + | border.EXPECT().SetVexpand(true).Once() | |||
| 607 | + | paneOverlay.EXPECT().AddOverlay(border).Once() | |||
| 608 | + | paneOverlay.EXPECT().SetClipOverlay(border, false).Once() | |||
| 609 | + | paneOverlay.EXPECT().SetMeasureOverlay(border, false).Once() | |||
| 610 | + | ||||
| 611 | + | stackBox := layoutmocks.NewMockBoxWidget(t) | |||
| 612 | + | titleBar := layoutmocks.NewMockBoxWidget(t) | |||
| 613 | + | favicon := layoutmocks.NewMockImageWidget(t) | |||
| 614 | + | title := layoutmocks.NewMockLabelWidget(t) | |||
| 615 | + | closeButton := layoutmocks.NewMockButtonWidget(t) | |||
| 616 | + | factory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(stackBox).Once() | |||
| 617 | + | stackBox.EXPECT().SetHexpand(true).Once() | |||
| 618 | + | stackBox.EXPECT().SetVexpand(true).Once() | |||
| 619 | + | factory.EXPECT().NewBox(layout.OrientationHorizontal, 4).Return(titleBar).Once() | |||
| 620 | + | titleBar.EXPECT().AddCssClass("stacked-pane-titlebar").Once() | |||
| 621 | + | titleBar.EXPECT().AddCssClass("stacked-pane-title-clickable").Once() | |||
| 622 | + | titleBar.EXPECT().SetVexpand(false).Once() | |||
| 623 | + | titleBar.EXPECT().SetHexpand(true).Once() | |||
| 624 | + | factory.EXPECT().NewImage().Return(favicon).Once() | |||
| 625 | + | favicon.EXPECT().SetFromIconName(mock.Anything).Once() | |||
| 626 | + | favicon.EXPECT().SetPixelSize(16).Once() | |||
| 627 | + | titleBar.EXPECT().Append(favicon).Once() | |||
| 628 | + | factory.EXPECT().NewLabel(mock.Anything).Return(title).Once() | |||
| 629 | + | title.EXPECT().SetEllipsize(layout.EllipsizeEnd).Once() | |||
| 630 | + | title.EXPECT().SetMaxWidthChars(30).Once() | |||
| 631 | + | title.EXPECT().SetHexpand(true).Once() | |||
| 632 | + | title.EXPECT().SetXalign(float32(0)).Once() | |||
| 633 | + | titleBar.EXPECT().Append(title).Once() | |||
| 634 | + | factory.EXPECT().NewButton().Return(closeButton).Once() | |||
| 635 | + | closeButton.EXPECT().SetIconName("window-close-symbolic").Once() | |||
| 636 | + | closeButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |||
| 637 | + | closeButton.EXPECT().SetFocusOnClick(false).Once() | |||
| 638 | + | closeButton.EXPECT().SetVexpand(false).Once() | |||
| 639 | + | closeButton.EXPECT().SetHexpand(false).Once() | |||
| 640 | + | titleBar.EXPECT().Append(closeButton).Once() | |||
| 641 | + | titleBar.EXPECT().AddController(mock.Anything).Once() | |||
| 642 | + | closeButton.EXPECT().ConnectClicked(mock.Anything).Return(uint(2)).Once() | |||
| 643 | + | closeButton.EXPECT().GtkWidget().Return(nil).Maybe() | |||
| 644 | + | stackBox.EXPECT().Append(titleBar).Once() | |||
| 645 | + | stackBox.EXPECT().Append(paneOverlay).Once() | |||
| 646 | + | titleBar.EXPECT().SetVisible(false).Once() | |||
| 647 | + | paneOverlay.EXPECT().SetVisible(true).Once() | |||
| 648 | + | titleBar.EXPECT().AddCssClass("active").Once() | |||
| 649 | + | paneOverlay.EXPECT().GtkWidget().Return(nil).Maybe() | |||
| 650 | + | stackBox.EXPECT().SetVisible(true).Once() | |||
| 651 | + | workspaceContainer.EXPECT().Append(stackBox).Once() | |||
| 652 | + | workspaceContainer.EXPECT().AddCssClass("single-pane").Once() | |||
| 653 | + | border.EXPECT().AddCssClass("pane-active").Once() | |||
| 654 | + | ||||
| 655 | + | workspace := &entity.Workspace{ | |||
| 656 | + | ID: "workspace-1", | |||
| 657 | + | Root: &entity.PaneNode{ID: "node-1", Pane: entity.NewPane(paneID)}, | |||
| 658 | + | ActivePaneID: paneID, | |||
| 659 | + | } | |||
| 660 | + | require.NoError(t, wsView.SetWorkspace(ctx, workspace)) | |||
| 661 | + | return wsView, workspace, paneOverlay, loadingContainer, spinner | |||
| 662 | + | } | |||
M internal/ui/coordinator/content/navigation.go +102 -23
| @@ -17,7 +17,7 @@ import ( | |||
| 17 | 17 | // WebKit may reset zoom during document transitions, so we reapply after LoadCommitted. | |
| 18 | 18 | // History is recorded here because the URI is guaranteed to be correct after commit. | |
| 19 | 19 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | |
| 20 | - | func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | |
| 20 | + | func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity) { | |
| 21 | 21 | log := logging.FromContext(ctx) | |
| 22 | 22 | logging.Trace().Mark("load_committed") | |
| 23 | 23 | ||
| @@ -69,7 +69,7 @@ func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, | |||
| 69 | 69 | // Avoid updating UI/domain state to about:blank when we know the pane is | |
| 70 | 70 | // navigating to a different URL. This prevents the omnibox/window title from | |
| 71 | 71 | // briefly showing about:blank on cold start. | |
| 72 | - | c.clearPendingReveal(paneID) | |
| 72 | + | c.clearPendingReveal(paneID, identity) | |
| 73 | 73 | return | |
| 74 | 74 | } | |
| 75 | 75 | ||
| @@ -77,8 +77,8 @@ func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, | |||
| 77 | 77 | c.applyCurrentTheme(ctx, paneID, wv) | |
| 78 | 78 | } | |
| 79 | 79 | ||
| 80 | - | c.markPendingReveal(paneID) | |
| 81 | - | c.revealIfPending(ctx, paneID, uri, "progress-after-commit") | |
| 80 | + | c.markPendingReveal(paneID, identity) | |
| 81 | + | c.revealIfPending(ctx, paneID, wv, identity, uri, "progress-after-commit") | |
| 82 | 82 | ||
| 83 | 83 | // Update domain model with current URI for session snapshots | |
| 84 | 84 | c.updatePaneURI(paneID, uri) | |
| @@ -202,7 +202,7 @@ func (c *Coordinator) onLoadStarted(paneID entity.PaneID) { | |||
| 202 | 202 | } | |
| 203 | 203 | ||
| 204 | 204 | // onLoadFinished hides the progress bar when page loading completes. | |
| 205 | - | func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | |
| 205 | + | func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity) { | |
| 206 | 206 | _, wsView := c.getActiveWS() | |
| 207 | 207 | var paneView *component.PaneView | |
| 208 | 208 | if wsView != nil { | |
| @@ -216,7 +216,7 @@ func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, | |||
| 216 | 216 | paneView.SetLoading(false) | |
| 217 | 217 | } | |
| 218 | 218 | ||
| 219 | - | c.revealIfPending(ctx, paneID, "", "load-finished") | |
| 219 | + | c.revealIfPending(ctx, paneID, wv, identity, "", "load-finished") | |
| 220 | 220 | if c.shouldSkipAboutBlankAppearance(paneID, wv) { | |
| 221 | 221 | return | |
| 222 | 222 | } | |
| @@ -225,9 +225,9 @@ func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, | |||
| 225 | 225 | } | |
| 226 | 226 | ||
| 227 | 227 | // onProgressChanged updates the progress bar with current load progress. | |
| 228 | - | func (c *Coordinator) onProgressChanged(paneID entity.PaneID, progress float64) { | |
| 228 | + | func (c *Coordinator) onProgressChanged(paneID entity.PaneID, wv port.WebView, identity webViewIdentity, progress float64) { | |
| 229 | 229 | if progress > 0 { | |
| 230 | - | c.revealIfPending(context.Background(), paneID, "", "progress") | |
| 230 | + | c.revealIfPending(context.Background(), paneID, wv, identity, "", "progress") | |
| 231 | 231 | } | |
| 232 | 232 | ||
| 233 | 233 | _, wsView := c.getActiveWS() | |
| @@ -244,37 +244,112 @@ func (c *Coordinator) onProgressChanged(paneID entity.PaneID, progress float64) | |||
| 244 | 244 | } | |
| 245 | 245 | } | |
| 246 | 246 | ||
| 247 | - | func (c *Coordinator) markPendingReveal(paneID entity.PaneID) { | |
| 247 | + | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, identity webViewIdentity) { | |
| 248 | + | c.webViewsMu.RLock() | |
| 248 | 249 | c.revealMu.Lock() | |
| 250 | + | defer c.revealMu.Unlock() | |
| 251 | + | defer c.webViewsMu.RUnlock() | |
| 252 | + | currentWebView := c.webViews[paneID] | |
| 253 | + | if currentWebView == nil { | |
| 254 | + | return | |
| 255 | + | } | |
| 256 | + | currentIdentity, ok := identityForWebView(currentWebView) | |
| 257 | + | if !ok || currentIdentity != identity { | |
| 258 | + | return | |
| 259 | + | } | |
| 260 | + | c.ensureRevealMapsLocked() | |
| 249 | 261 | c.pendingReveal[paneID] = true | |
| 250 | - | c.revealMu.Unlock() | |
| 262 | + | c.pendingRevealIdentity[paneID] = identity | |
| 251 | 263 | } | |
| 252 | 264 | ||
| 253 | - | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID) { | |
| 265 | + | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewIdentity) { | |
| 266 | + | c.webViewsMu.RLock() | |
| 254 | 267 | c.revealMu.Lock() | |
| 268 | + | defer c.revealMu.Unlock() | |
| 269 | + | defer c.webViewsMu.RUnlock() | |
| 270 | + | if c.pendingRevealIdentity[paneID] != identity { | |
| 271 | + | return | |
| 272 | + | } | |
| 255 | 273 | delete(c.pendingReveal, paneID) | |
| 256 | - | c.revealMu.Unlock() | |
| 274 | + | delete(c.pendingRevealIdentity, paneID) | |
| 275 | + | } | |
| 276 | + | ||
| 277 | + | // WebViewRevealed reports whether this pane's current WebView has completed a | |
| 278 | + | // reveal. State is identity-bound so a replacement cannot inherit it. | |
| 279 | + | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | |
| 280 | + | return c.webViewRevealed(paneID, nil) | |
| 257 | 281 | } | |
| 258 | 282 | ||
| 259 | - | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, uri, reason string) { | |
| 283 | + | // webViewRevealed verifies the identity being attached when one is supplied. | |
| 284 | + | // This prevents a rebuild from applying a replacement's reveal state to a | |
| 285 | + | // previously read pooled WebView. | |
| 286 | + | func (c *Coordinator) webViewRevealed(paneID entity.PaneID, expected port.WebView) bool { | |
| 287 | + | c.webViewsMu.RLock() | |
| 260 | 288 | c.revealMu.Lock() | |
| 261 | - | pending := c.pendingReveal[paneID] | |
| 262 | - | if pending { | |
| 263 | - | delete(c.pendingReveal, paneID) | |
| 289 | + | defer c.revealMu.Unlock() | |
| 290 | + | defer c.webViewsMu.RUnlock() | |
| 291 | + | current := c.webViews[paneID] | |
| 292 | + | identity, ok := identityForWebView(current) | |
| 293 | + | if !ok || expected != nil { | |
| 294 | + | expectedIdentity, expectedOK := identityForWebView(expected) | |
| 295 | + | if !expectedOK || expectedIdentity != identity { | |
| 296 | + | return false | |
| 297 | + | } | |
| 264 | 298 | } | |
| 265 | - | c.revealMu.Unlock() | |
| 299 | + | return c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity | |
| 300 | + | } | |
| 266 | 301 | ||
| 267 | - | if !pending { | |
| 302 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webViewIdentity) bool { | |
| 303 | + | c.webViewsMu.RLock() | |
| 304 | + | c.revealMu.Lock() | |
| 305 | + | defer c.revealMu.Unlock() | |
| 306 | + | defer c.webViewsMu.RUnlock() | |
| 307 | + | currentIdentity, current := identityForWebView(c.webViews[paneID]) | |
| 308 | + | if !current || currentIdentity != identity { | |
| 309 | + | return false | |
| 310 | + | } | |
| 311 | + | c.ensureRevealMapsLocked() | |
| 312 | + | c.revealedWebViews[paneID] = true | |
| 313 | + | c.revealedWebViewIdentity[paneID] = identity | |
| 314 | + | return true | |
| 315 | + | } | |
| 316 | + | ||
| 317 | + | func (c *Coordinator) revealIfPending( | |
| 318 | + | ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri, reason string, | |
| 319 | + | ) { | |
| 320 | + | // Mapping replacement and release acquire this gate too. No ordinary map | |
| 321 | + | // lock is held across the native call, but the gate keeps the validated | |
| 322 | + | // identity current until its widget visibility has been changed. | |
| 323 | + | c.revealMutationMu.Lock() | |
| 324 | + | ||
| 325 | + | c.webViewsMu.RLock() | |
| 326 | + | c.revealMu.Lock() | |
| 327 | + | if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { | |
| 328 | + | c.revealMu.Unlock() | |
| 329 | + | c.webViewsMu.RUnlock() | |
| 330 | + | c.revealMutationMu.Unlock() | |
| 331 | + | return | |
| 332 | + | } | |
| 333 | + | currentIdentity, current := identityForWebView(c.webViews[paneID]) | |
| 334 | + | if !current || currentIdentity != identity { | |
| 335 | + | c.revealMu.Unlock() | |
| 336 | + | c.webViewsMu.RUnlock() | |
| 337 | + | c.revealMutationMu.Unlock() | |
| 268 | 338 | return | |
| 269 | 339 | } | |
| 340 | + | delete(c.pendingReveal, paneID) | |
| 341 | + | delete(c.pendingRevealIdentity, paneID) | |
| 342 | + | c.revealMu.Unlock() | |
| 343 | + | c.webViewsMu.RUnlock() | |
| 270 | 344 | ||
| 271 | - | wv := c.getWebViewLocked(paneID) | |
| 272 | - | if wv == nil || wv.IsDestroyed() { | |
| 345 | + | if wv.IsDestroyed() { | |
| 346 | + | c.revealMutationMu.Unlock() | |
| 273 | 347 | return | |
| 274 | 348 | } | |
| 275 | 349 | ||
| 276 | - | // Use NativeWidgetProvider to make the widget visible (engine-agnostic) | |
| 277 | - | if nwp, ok := wv.(port.NativeWidgetProvider); ok { | |
| 350 | + | if c.setWebViewVisible != nil { | |
| 351 | + | c.setWebViewVisible(wv) | |
| 352 | + | } else if nwp, ok := wv.(port.NativeWidgetProvider); ok { | |
| 278 | 353 | if ptr := nwp.NativeWidget(); ptr != 0 { | |
| 279 | 354 | gtkWidget := >k.Widget{} | |
| 280 | 355 | gtkWidget.Ptr = ptr | |
| @@ -288,7 +363,11 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||
| 288 | 363 | } | |
| 289 | 364 | } | |
| 290 | 365 | ||
| 291 | - | if c.onWebViewShown != nil { | |
| 366 | + | // Keep the presentation record in the same serialized operation as the | |
| 367 | + | // native mutation. User callbacks run only after the gate is released. | |
| 368 | + | revealed := c.markWebViewRevealed(paneID, identity) | |
| 369 | + | c.revealMutationMu.Unlock() | |
| 370 | + | if revealed && c.onWebViewShown != nil { | |
| 292 | 371 | c.onWebViewShown(paneID) | |
| 293 | 372 | } | |
| 294 | 373 | } | |
| @@ -17,7 +17,7 @@ import ( | |||||
| 17 | // WebKit may reset zoom during document transitions, so we reapply after LoadCommitted. | 17 | // WebKit may reset zoom during document transitions, so we reapply after LoadCommitted. | ||
| 18 | // History is recorded here because the URI is guaranteed to be correct after commit. | 18 | // History is recorded here because the URI is guaranteed to be correct after commit. | ||
| 19 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | 19 | // Also shows the WebView widget (it's hidden during creation to avoid white flash). | ||
| 20 | - | func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | 20 | + | func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity) { |
| 21 | log := logging.FromContext(ctx) | 21 | log := logging.FromContext(ctx) | ||
| 22 | logging.Trace().Mark("load_committed") | 22 | logging.Trace().Mark("load_committed") | ||
| 23 | 23 | ||||
| @@ -69,7 +69,7 @@ func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, | |||||
| 69 | // Avoid updating UI/domain state to about:blank when we know the pane is | 69 | // Avoid updating UI/domain state to about:blank when we know the pane is | ||
| 70 | // navigating to a different URL. This prevents the omnibox/window title from | 70 | // navigating to a different URL. This prevents the omnibox/window title from | ||
| 71 | // briefly showing about:blank on cold start. | 71 | // briefly showing about:blank on cold start. | ||
| 72 | - | c.clearPendingReveal(paneID) | 72 | + | c.clearPendingReveal(paneID, identity) |
| 73 | return | 73 | return | ||
| 74 | } | 74 | } | ||
| 75 | 75 | ||||
| @@ -77,8 +77,8 @@ func (c *Coordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, | |||||
| 77 | c.applyCurrentTheme(ctx, paneID, wv) | 77 | c.applyCurrentTheme(ctx, paneID, wv) | ||
| 78 | } | 78 | } | ||
| 79 | 79 | ||||
| 80 | - | c.markPendingReveal(paneID) | 80 | + | c.markPendingReveal(paneID, identity) |
| 81 | - | c.revealIfPending(ctx, paneID, uri, "progress-after-commit") | 81 | + | c.revealIfPending(ctx, paneID, wv, identity, uri, "progress-after-commit") |
| 82 | 82 | ||||
| 83 | // Update domain model with current URI for session snapshots | 83 | // Update domain model with current URI for session snapshots | ||
| 84 | c.updatePaneURI(paneID, uri) | 84 | c.updatePaneURI(paneID, uri) | ||
| @@ -202,7 +202,7 @@ func (c *Coordinator) onLoadStarted(paneID entity.PaneID) { | |||||
| 202 | } | 202 | } | ||
| 203 | 203 | ||||
| 204 | // onLoadFinished hides the progress bar when page loading completes. | 204 | // onLoadFinished hides the progress bar when page loading completes. | ||
| 205 | - | func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, wv port.WebView) { | 205 | + | func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity) { |
| 206 | _, wsView := c.getActiveWS() | 206 | _, wsView := c.getActiveWS() | ||
| 207 | var paneView *component.PaneView | 207 | var paneView *component.PaneView | ||
| 208 | if wsView != nil { | 208 | if wsView != nil { | ||
| @@ -216,7 +216,7 @@ func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, | |||||
| 216 | paneView.SetLoading(false) | 216 | paneView.SetLoading(false) | ||
| 217 | } | 217 | } | ||
| 218 | 218 | ||||
| 219 | - | c.revealIfPending(ctx, paneID, "", "load-finished") | 219 | + | c.revealIfPending(ctx, paneID, wv, identity, "", "load-finished") |
| 220 | if c.shouldSkipAboutBlankAppearance(paneID, wv) { | 220 | if c.shouldSkipAboutBlankAppearance(paneID, wv) { | ||
| 221 | return | 221 | return | ||
| 222 | } | 222 | } | ||
| @@ -225,9 +225,9 @@ func (c *Coordinator) onLoadFinished(ctx context.Context, paneID entity.PaneID, | |||||
| 225 | } | 225 | } | ||
| 226 | 226 | ||||
| 227 | // onProgressChanged updates the progress bar with current load progress. | 227 | // onProgressChanged updates the progress bar with current load progress. | ||
| 228 | - | func (c *Coordinator) onProgressChanged(paneID entity.PaneID, progress float64) { | 228 | + | func (c *Coordinator) onProgressChanged(paneID entity.PaneID, wv port.WebView, identity webViewIdentity, progress float64) { |
| 229 | if progress > 0 { | 229 | if progress > 0 { | ||
| 230 | - | c.revealIfPending(context.Background(), paneID, "", "progress") | 230 | + | c.revealIfPending(context.Background(), paneID, wv, identity, "", "progress") |
| 231 | } | 231 | } | ||
| 232 | 232 | ||||
| 233 | _, wsView := c.getActiveWS() | 233 | _, wsView := c.getActiveWS() | ||
| @@ -244,37 +244,112 @@ func (c *Coordinator) onProgressChanged(paneID entity.PaneID, progress float64) | |||||
| 244 | } | 244 | } | ||
| 245 | } | 245 | } | ||
| 246 | 246 | ||||
| 247 | - | func (c *Coordinator) markPendingReveal(paneID entity.PaneID) { | 247 | + | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, identity webViewIdentity) { |
| 248 | + | c.webViewsMu.RLock() | |||
| 248 | c.revealMu.Lock() | 249 | c.revealMu.Lock() | ||
| 250 | + | defer c.revealMu.Unlock() | |||
| 251 | + | defer c.webViewsMu.RUnlock() | |||
| 252 | + | currentWebView := c.webViews[paneID] | |||
| 253 | + | if currentWebView == nil { | |||
| 254 | + | return | |||
| 255 | + | } | |||
| 256 | + | currentIdentity, ok := identityForWebView(currentWebView) | |||
| 257 | + | if !ok || currentIdentity != identity { | |||
| 258 | + | return | |||
| 259 | + | } | |||
| 260 | + | c.ensureRevealMapsLocked() | |||
| 249 | c.pendingReveal[paneID] = true | 261 | c.pendingReveal[paneID] = true | ||
| 250 | - | c.revealMu.Unlock() | 262 | + | c.pendingRevealIdentity[paneID] = identity |
| 251 | } | 263 | } | ||
| 252 | 264 | ||||
| 253 | - | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID) { | 265 | + | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewIdentity) { |
| 266 | + | c.webViewsMu.RLock() | |||
| 254 | c.revealMu.Lock() | 267 | c.revealMu.Lock() | ||
| 268 | + | defer c.revealMu.Unlock() | |||
| 269 | + | defer c.webViewsMu.RUnlock() | |||
| 270 | + | if c.pendingRevealIdentity[paneID] != identity { | |||
| 271 | + | return | |||
| 272 | + | } | |||
| 255 | delete(c.pendingReveal, paneID) | 273 | delete(c.pendingReveal, paneID) | ||
| 256 | - | c.revealMu.Unlock() | 274 | + | delete(c.pendingRevealIdentity, paneID) |
| 275 | + | } | |||
| 276 | + | ||||
| 277 | + | // WebViewRevealed reports whether this pane's current WebView has completed a | |||
| 278 | + | // reveal. State is identity-bound so a replacement cannot inherit it. | |||
| 279 | + | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | |||
| 280 | + | return c.webViewRevealed(paneID, nil) | |||
| 257 | } | 281 | } | ||
| 258 | 282 | ||||
| 259 | - | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, uri, reason string) { | 283 | + | // webViewRevealed verifies the identity being attached when one is supplied. |
| 284 | + | // This prevents a rebuild from applying a replacement's reveal state to a | |||
| 285 | + | // previously read pooled WebView. | |||
| 286 | + | func (c *Coordinator) webViewRevealed(paneID entity.PaneID, expected port.WebView) bool { | |||
| 287 | + | c.webViewsMu.RLock() | |||
| 260 | c.revealMu.Lock() | 288 | c.revealMu.Lock() | ||
| 261 | - | pending := c.pendingReveal[paneID] | 289 | + | defer c.revealMu.Unlock() |
| 262 | - | if pending { | 290 | + | defer c.webViewsMu.RUnlock() |
| 263 | - | delete(c.pendingReveal, paneID) | 291 | + | current := c.webViews[paneID] |
| 292 | + | identity, ok := identityForWebView(current) | |||
| 293 | + | if !ok || expected != nil { | |||
| 294 | + | expectedIdentity, expectedOK := identityForWebView(expected) | |||
| 295 | + | if !expectedOK || expectedIdentity != identity { | |||
| 296 | + | return false | |||
| 297 | + | } | |||
| 264 | } | 298 | } | ||
| 265 | - | c.revealMu.Unlock() | 299 | + | return c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity |
| 300 | + | } | |||
| 266 | 301 | ||||
| 267 | - | if !pending { | 302 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webViewIdentity) bool { |
| 303 | + | c.webViewsMu.RLock() | |||
| 304 | + | c.revealMu.Lock() | |||
| 305 | + | defer c.revealMu.Unlock() | |||
| 306 | + | defer c.webViewsMu.RUnlock() | |||
| 307 | + | currentIdentity, current := identityForWebView(c.webViews[paneID]) | |||
| 308 | + | if !current || currentIdentity != identity { | |||
| 309 | + | return false | |||
| 310 | + | } | |||
| 311 | + | c.ensureRevealMapsLocked() | |||
| 312 | + | c.revealedWebViews[paneID] = true | |||
| 313 | + | c.revealedWebViewIdentity[paneID] = identity | |||
| 314 | + | return true | |||
| 315 | + | } | |||
| 316 | + | ||||
| 317 | + | func (c *Coordinator) revealIfPending( | |||
| 318 | + | ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri, reason string, | |||
| 319 | + | ) { | |||
| 320 | + | // Mapping replacement and release acquire this gate too. No ordinary map | |||
| 321 | + | // lock is held across the native call, but the gate keeps the validated | |||
| 322 | + | // identity current until its widget visibility has been changed. | |||
| 323 | + | c.revealMutationMu.Lock() | |||
| 324 | + | ||||
| 325 | + | c.webViewsMu.RLock() | |||
| 326 | + | c.revealMu.Lock() | |||
| 327 | + | if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { | |||
| 328 | + | c.revealMu.Unlock() | |||
| 329 | + | c.webViewsMu.RUnlock() | |||
| 330 | + | c.revealMutationMu.Unlock() | |||
| 331 | + | return | |||
| 332 | + | } | |||
| 333 | + | currentIdentity, current := identityForWebView(c.webViews[paneID]) | |||
| 334 | + | if !current || currentIdentity != identity { | |||
| 335 | + | c.revealMu.Unlock() | |||
| 336 | + | c.webViewsMu.RUnlock() | |||
| 337 | + | c.revealMutationMu.Unlock() | |||
| 268 | return | 338 | return | ||
| 269 | } | 339 | } | ||
| 340 | + | delete(c.pendingReveal, paneID) | |||
| 341 | + | delete(c.pendingRevealIdentity, paneID) | |||
| 342 | + | c.revealMu.Unlock() | |||
| 343 | + | c.webViewsMu.RUnlock() | |||
| 270 | 344 | ||||
| 271 | - | wv := c.getWebViewLocked(paneID) | 345 | + | if wv.IsDestroyed() { |
| 272 | - | if wv == nil || wv.IsDestroyed() { | 346 | + | c.revealMutationMu.Unlock() |
| 273 | return | 347 | return | ||
| 274 | } | 348 | } | ||
| 275 | 349 | ||||
| 276 | - | // Use NativeWidgetProvider to make the widget visible (engine-agnostic) | 350 | + | if c.setWebViewVisible != nil { |
| 277 | - | if nwp, ok := wv.(port.NativeWidgetProvider); ok { | 351 | + | c.setWebViewVisible(wv) |
| 352 | + | } else if nwp, ok := wv.(port.NativeWidgetProvider); ok { | |||
| 278 | if ptr := nwp.NativeWidget(); ptr != 0 { | 353 | if ptr := nwp.NativeWidget(); ptr != 0 { | ||
| 279 | gtkWidget := >k.Widget{} | 354 | gtkWidget := >k.Widget{} | ||
| 280 | gtkWidget.Ptr = ptr | 355 | gtkWidget.Ptr = ptr | ||
| @@ -288,7 +363,11 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||||
| 288 | } | 363 | } | ||
| 289 | } | 364 | } | ||
| 290 | 365 | ||||
| 291 | - | if c.onWebViewShown != nil { | 366 | + | // Keep the presentation record in the same serialized operation as the |
| 367 | + | // native mutation. User callbacks run only after the gate is released. | |||
| 368 | + | revealed := c.markWebViewRevealed(paneID, identity) | |||
| 369 | + | c.revealMutationMu.Unlock() | |||
| 370 | + | if revealed && c.onWebViewShown != nil { | |||
| 292 | c.onWebViewShown(paneID) | 371 | c.onWebViewShown(paneID) | ||
| 293 | } | 372 | } | ||
| 294 | } | 373 | } | ||
M internal/ui/coordinator/content/popup_test.go +7 -7
| @@ -137,7 +137,7 @@ func newPopupCreateCoordinatorForTest(t *testing.T, popupID port.WebViewID) (con | |||
| 137 | 137 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 138 | 138 | ||
| 139 | 139 | popupWV := mocks.NewMockWebView(t) | |
| 140 | - | popupWV.EXPECT().ID().Return(popupID).Once() | |
| 140 | + | popupWV.EXPECT().ID().Return(popupID).Maybe() | |
| 141 | 141 | ||
| 142 | 142 | factory := mocks.NewMockWebViewFactory(t) | |
| 143 | 143 | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | |
| @@ -158,8 +158,8 @@ func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) { | |||
| 158 | 158 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 159 | 159 | ||
| 160 | 160 | popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)} | |
| 161 | - | popupWV.EXPECT().ID().Return(port.WebViewID(149)).Once() | |
| 162 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 161 | + | popupWV.EXPECT().ID().Return(port.WebViewID(149)).Maybe() | |
| 162 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 163 | 163 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 164 | 164 | ||
| 165 | 165 | factory := mocks.NewMockWebViewFactory(t) | |
| @@ -187,7 +187,7 @@ func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) { | |||
| 187 | 187 | ||
| 188 | 188 | func TestHandlePopupCreate_RegistersPopupWebViewBeforeWorkspaceInsertion(t *testing.T) { | |
| 189 | 189 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(150)) | |
| 190 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 190 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 191 | 191 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 192 | 192 | popupWV.EXPECT().IsLoading().Return(false).Once() | |
| 193 | 193 | popupWV.EXPECT().URI().Return("").Once() | |
| @@ -211,7 +211,7 @@ func TestHandlePopupCreate_RegistersPopupWebViewBeforeWorkspaceInsertion(t *test | |||
| 211 | 211 | ||
| 212 | 212 | func TestHandlePopupCreate_CleansUpPreRegisteredPopupWebViewWhenInsertionFails(t *testing.T) { | |
| 213 | 213 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(151)) | |
| 214 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 214 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 215 | 215 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 216 | 216 | popupWV.EXPECT().Destroy().Once() | |
| 217 | 217 | ||
| @@ -238,8 +238,8 @@ func TestHandlePopupCreate_UsesRelatedWebViewWhenPopupDisablesJavaScriptAccess(t | |||
| 238 | 238 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | |
| 239 | 239 | ||
| 240 | 240 | popupWV := mocks.NewMockWebView(t) | |
| 241 | - | popupWV.EXPECT().ID().Return(port.WebViewID(150)).Once() | |
| 242 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | |
| 241 | + | popupWV.EXPECT().ID().Return(port.WebViewID(150)).Maybe() | |
| 242 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() | |
| 243 | 243 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | |
| 244 | 244 | popupWV.EXPECT().IsLoading().Return(false).Once() | |
| 245 | 245 | popupWV.EXPECT().URI().Return("").Once() | |
| @@ -137,7 +137,7 @@ func newPopupCreateCoordinatorForTest(t *testing.T, popupID port.WebViewID) (con | |||||
| 137 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | 137 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | ||
| 138 | 138 | ||||
| 139 | popupWV := mocks.NewMockWebView(t) | 139 | popupWV := mocks.NewMockWebView(t) | ||
| 140 | - | popupWV.EXPECT().ID().Return(popupID).Once() | 140 | + | popupWV.EXPECT().ID().Return(popupID).Maybe() |
| 141 | 141 | ||||
| 142 | factory := mocks.NewMockWebViewFactory(t) | 142 | factory := mocks.NewMockWebViewFactory(t) | ||
| 143 | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | 143 | factory.EXPECT().CreateRelated(mock.Anything, port.WebViewID(101)).Return(popupWV, nil).Once() | ||
| @@ -158,8 +158,8 @@ func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) { | |||||
| 158 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | 158 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | ||
| 159 | 159 | ||||
| 160 | popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)} | 160 | popupWV := &popupNavigationWebViewStub{MockWebView: mocks.NewMockWebView(t)} | ||
| 161 | - | popupWV.EXPECT().ID().Return(port.WebViewID(149)).Once() | 161 | + | popupWV.EXPECT().ID().Return(port.WebViewID(149)).Maybe() |
| 162 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | 162 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() |
| 163 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | 163 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | ||
| 164 | 164 | ||||
| 165 | factory := mocks.NewMockWebViewFactory(t) | 165 | factory := mocks.NewMockWebViewFactory(t) | ||
| @@ -187,7 +187,7 @@ func TestHandlePopupCreate_PrimesPopupNavigationCapability(t *testing.T) { | |||||
| 187 | 187 | ||||
| 188 | func TestHandlePopupCreate_RegistersPopupWebViewBeforeWorkspaceInsertion(t *testing.T) { | 188 | func TestHandlePopupCreate_RegistersPopupWebViewBeforeWorkspaceInsertion(t *testing.T) { | ||
| 189 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(150)) | 189 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(150)) | ||
| 190 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | 190 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() |
| 191 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | 191 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | ||
| 192 | popupWV.EXPECT().IsLoading().Return(false).Once() | 192 | popupWV.EXPECT().IsLoading().Return(false).Once() | ||
| 193 | popupWV.EXPECT().URI().Return("").Once() | 193 | popupWV.EXPECT().URI().Return("").Once() | ||
| @@ -211,7 +211,7 @@ func TestHandlePopupCreate_RegistersPopupWebViewBeforeWorkspaceInsertion(t *test | |||||
| 211 | 211 | ||||
| 212 | func TestHandlePopupCreate_CleansUpPreRegisteredPopupWebViewWhenInsertionFails(t *testing.T) { | 212 | func TestHandlePopupCreate_CleansUpPreRegisteredPopupWebViewWhenInsertionFails(t *testing.T) { | ||
| 213 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(151)) | 213 | ctx, parentPaneID, parentWV, popupWV, c := newPopupCreateCoordinatorForTest(t, port.WebViewID(151)) | ||
| 214 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | 214 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() |
| 215 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | 215 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | ||
| 216 | popupWV.EXPECT().Destroy().Once() | 216 | popupWV.EXPECT().Destroy().Once() | ||
| 217 | 217 | ||||
| @@ -238,8 +238,8 @@ func TestHandlePopupCreate_UsesRelatedWebViewWhenPopupDisablesJavaScriptAccess(t | |||||
| 238 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | 238 | parentWV.EXPECT().ID().Return(port.WebViewID(101)).Once() | ||
| 239 | 239 | ||||
| 240 | popupWV := mocks.NewMockWebView(t) | 240 | popupWV := mocks.NewMockWebView(t) | ||
| 241 | - | popupWV.EXPECT().ID().Return(port.WebViewID(150)).Once() | 241 | + | popupWV.EXPECT().ID().Return(port.WebViewID(150)).Maybe() |
| 242 | - | popupWV.EXPECT().Generation().Return(uint64(1)).Once() | 242 | + | popupWV.EXPECT().Generation().Return(uint64(1)).Maybe() |
| 243 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | 243 | popupWV.EXPECT().SetCallbacks(mock.Anything).Once() | ||
| 244 | popupWV.EXPECT().IsLoading().Return(false).Once() | 244 | popupWV.EXPECT().IsLoading().Return(false).Once() | ||
| 245 | popupWV.EXPECT().URI().Return("").Once() | 245 | popupWV.EXPECT().URI().Return("").Once() | ||