fix(ui): serialize webview reveal mutations
- commit
- 9baacd8a6899d7d6080974b3d7017064649ef3ed
- author
- brice · · 2026-07-15 04:35 UTC
- parent
- fdb7c64
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +183 -57
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, wv, 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, wv, 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 +106 -7
| @@ -5,10 +5,12 @@ import ( | |||
| 5 | 5 | "testing" | |
| 6 | 6 | ||
| 7 | 7 | "github.com/stretchr/testify/assert" | |
| 8 | + | "github.com/stretchr/testify/mock" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| 10 | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | 12 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | + | "github.com/bnema/dumber/internal/ui/component" | |
| 12 | 14 | ) | |
| 13 | 15 | ||
| 14 | 16 | func newRevealTestCoordinator() *Coordinator { | |
| @@ -38,14 +40,16 @@ func TestRevealState_IsBoundToCurrentWebViewAcrossReplacementPaths(t *testing.T) | |||
| 38 | 40 | c := newRevealTestCoordinator() | |
| 39 | 41 | ||
| 40 | 42 | c.setWebViewLocked(paneID, first) | |
| 41 | - | c.markWebViewRevealed(paneID, first) | |
| 43 | + | firstIdentity, _ := identityForWebView(first) | |
| 44 | + | c.markWebViewRevealed(paneID, firstIdentity) | |
| 42 | 45 | assert.True(t, c.WebViewRevealed(paneID)) | |
| 43 | 46 | ||
| 44 | 47 | // A direct replacement for the same pane must show the fresh loading state. | |
| 45 | 48 | c.setWebViewLocked(paneID, directReplacement) | |
| 46 | 49 | assert.False(t, c.WebViewRevealed(paneID)) | |
| 47 | 50 | ||
| 48 | - | c.markWebViewRevealed(paneID, directReplacement) | |
| 51 | + | directReplacementIdentity, _ := identityForWebView(directReplacement) | |
| 52 | + | c.markWebViewRevealed(paneID, directReplacementIdentity) | |
| 49 | 53 | c.RegisterPopupWebView(paneID, popupReplacement) | |
| 50 | 54 | assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") | |
| 51 | 55 | } | |
| @@ -55,7 +59,8 @@ func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { | |||
| 55 | 59 | wv := revealTestWebView(t, 101, 1) | |
| 56 | 60 | c := newRevealTestCoordinator() | |
| 57 | 61 | c.setWebViewLocked(paneID, wv) | |
| 58 | - | c.markWebViewRevealed(paneID, wv) | |
| 62 | + | identity, _ := identityForWebView(wv) | |
| 63 | + | c.markWebViewRevealed(paneID, identity) | |
| 59 | 64 | ||
| 60 | 65 | // Model an earlier partial cleanup that lost the WebView mapping. Release | |
| 61 | 66 | // remains responsible for clearing presentation state on its early return. | |
| @@ -70,6 +75,23 @@ func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { | |||
| 70 | 75 | assert.False(t, retained) | |
| 71 | 76 | } | |
| 72 | 77 | ||
| 78 | + | func TestRevealState_ReleaseMappedWebViewClearsRevealStateAndReturnsToPool(t *testing.T) { | |
| 79 | + | paneID := entity.PaneID("pane-1") | |
| 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)) | |
| 93 | + | } | |
| 94 | + | ||
| 73 | 95 | func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | |
| 74 | 96 | paneID := entity.PaneID("pane-1") | |
| 75 | 97 | oldWV := revealTestWebView(t, 101, 1) | |
| @@ -77,10 +99,11 @@ func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | |||
| 77 | 99 | oldWV.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 78 | 100 | c := newRevealTestCoordinator() | |
| 79 | 101 | c.setWebViewLocked(paneID, oldWV) | |
| 80 | - | c.markPendingReveal(paneID, oldWV) | |
| 102 | + | oldIdentity, _ := identityForWebView(oldWV) | |
| 103 | + | c.markPendingReveal(paneID, oldIdentity) | |
| 81 | 104 | c.setWebViewLocked(paneID, newWV) | |
| 82 | 105 | ||
| 83 | - | c.revealIfPending(context.Background(), paneID, oldWV, "", "stale") | |
| 106 | + | c.revealIfPending(context.Background(), paneID, oldWV, oldIdentity, "", "stale") | |
| 84 | 107 | assert.False(t, c.WebViewRevealed(paneID)) | |
| 85 | 108 | } | |
| 86 | 109 | ||
| @@ -89,9 +112,85 @@ func TestRevealState_RevealedCurrentWebViewPersistsForWorkspaceRebuild(t *testin | |||
| 89 | 112 | wv := revealTestWebView(t, 101, 1) | |
| 90 | 113 | c := newRevealTestCoordinator() | |
| 91 | 114 | c.setWebViewLocked(paneID, wv) | |
| 92 | - | c.markWebViewRevealed(paneID, wv) | |
| 115 | + | identity, _ := identityForWebView(wv) | |
| 116 | + | c.markWebViewRevealed(paneID, identity) | |
| 93 | 117 | ||
| 94 | 118 | // AttachToWorkspace asks this identity-aware query when rebuilding a pane, | |
| 95 | 119 | // so the replacement PaneView hides its skeleton for this same WebView. | |
| 96 | - | assert.True(t, c.WebViewRevealed(paneID)) | |
| 120 | + | assert.True(t, c.webViewRevealed(paneID, wv)) | |
| 121 | + | ||
| 122 | + | replacement := revealTestWebView(t, 102, 1) | |
| 123 | + | assert.False(t, c.webViewRevealed(paneID, replacement)) | |
| 124 | + | } | |
| 125 | + | ||
| 126 | + | func TestRevealCallback_CapturesGenerationBeforeSameObjectPoolReuse(t *testing.T) { | |
| 127 | + | paneID := entity.PaneID("pane-1") | |
| 128 | + | wv := mocks.NewMockWebView(t) | |
| 129 | + | generation := uint64(1) | |
| 130 | + | wv.EXPECT().ID().Return(port.WebViewID(101)).Maybe() | |
| 131 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() | |
| 132 | + | wv.EXPECT().IsDestroyed().Return(false).Maybe() | |
| 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)) | |
| 154 | + | } | |
| 155 | + | ||
| 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 | |
| 172 | + | } | |
| 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: | |
| 189 | + | } | |
| 190 | + | ||
| 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") | |
| 97 | 196 | } | |
| @@ -5,10 +5,12 @@ import ( | |||||
| 5 | "testing" | 5 | "testing" | ||
| 6 | 6 | ||||
| 7 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| 8 | + | "github.com/stretchr/testify/mock" | |||
| 8 | 9 | ||||
| 9 | "github.com/bnema/dumber/internal/application/port" | 10 | "github.com/bnema/dumber/internal/application/port" | ||
| 10 | "github.com/bnema/dumber/internal/application/port/mocks" | 11 | "github.com/bnema/dumber/internal/application/port/mocks" | ||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 12 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 13 | + | "github.com/bnema/dumber/internal/ui/component" | |||
| 12 | ) | 14 | ) | ||
| 13 | 15 | ||||
| 14 | func newRevealTestCoordinator() *Coordinator { | 16 | func newRevealTestCoordinator() *Coordinator { | ||
| @@ -38,14 +40,16 @@ func TestRevealState_IsBoundToCurrentWebViewAcrossReplacementPaths(t *testing.T) | |||||
| 38 | c := newRevealTestCoordinator() | 40 | c := newRevealTestCoordinator() | ||
| 39 | 41 | ||||
| 40 | c.setWebViewLocked(paneID, first) | 42 | c.setWebViewLocked(paneID, first) | ||
| 41 | - | c.markWebViewRevealed(paneID, first) | 43 | + | firstIdentity, _ := identityForWebView(first) |
| 44 | + | c.markWebViewRevealed(paneID, firstIdentity) | |||
| 42 | assert.True(t, c.WebViewRevealed(paneID)) | 45 | assert.True(t, c.WebViewRevealed(paneID)) | ||
| 43 | 46 | ||||
| 44 | // A direct replacement for the same pane must show the fresh loading state. | 47 | // A direct replacement for the same pane must show the fresh loading state. | ||
| 45 | c.setWebViewLocked(paneID, directReplacement) | 48 | c.setWebViewLocked(paneID, directReplacement) | ||
| 46 | assert.False(t, c.WebViewRevealed(paneID)) | 49 | assert.False(t, c.WebViewRevealed(paneID)) | ||
| 47 | 50 | ||||
| 48 | - | c.markWebViewRevealed(paneID, directReplacement) | 51 | + | directReplacementIdentity, _ := identityForWebView(directReplacement) |
| 52 | + | c.markWebViewRevealed(paneID, directReplacementIdentity) | |||
| 49 | c.RegisterPopupWebView(paneID, popupReplacement) | 53 | c.RegisterPopupWebView(paneID, popupReplacement) | ||
| 50 | assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") | 54 | assert.False(t, c.WebViewRevealed(paneID), "popup registration must not inherit a prior WebView reveal") | ||
| 51 | } | 55 | } | ||
| @@ -55,7 +59,8 @@ func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { | |||||
| 55 | wv := revealTestWebView(t, 101, 1) | 59 | wv := revealTestWebView(t, 101, 1) | ||
| 56 | c := newRevealTestCoordinator() | 60 | c := newRevealTestCoordinator() | ||
| 57 | c.setWebViewLocked(paneID, wv) | 61 | c.setWebViewLocked(paneID, wv) | ||
| 58 | - | c.markWebViewRevealed(paneID, wv) | 62 | + | identity, _ := identityForWebView(wv) |
| 63 | + | c.markWebViewRevealed(paneID, identity) | |||
| 59 | 64 | ||||
| 60 | // Model an earlier partial cleanup that lost the WebView mapping. Release | 65 | // Model an earlier partial cleanup that lost the WebView mapping. Release | ||
| 61 | // remains responsible for clearing presentation state on its early return. | 66 | // remains responsible for clearing presentation state on its early return. | ||
| @@ -70,6 +75,23 @@ func TestRevealState_ReleaseClearsStateWithoutWebViewMapping(t *testing.T) { | |||||
| 70 | assert.False(t, retained) | 75 | assert.False(t, retained) | ||
| 71 | } | 76 | } | ||
| 72 | 77 | ||||
| 78 | + | func TestRevealState_ReleaseMappedWebViewClearsRevealStateAndReturnsToPool(t *testing.T) { | |||
| 79 | + | paneID := entity.PaneID("pane-1") | |||
| 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)) | |||
| 93 | + | } | |||
| 94 | + | ||||
| 73 | func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | 95 | func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | ||
| 74 | paneID := entity.PaneID("pane-1") | 96 | paneID := entity.PaneID("pane-1") | ||
| 75 | oldWV := revealTestWebView(t, 101, 1) | 97 | oldWV := revealTestWebView(t, 101, 1) | ||
| @@ -77,10 +99,11 @@ func TestRevealState_StaleCallbackCannotRevealReplacement(t *testing.T) { | |||||
| 77 | oldWV.EXPECT().IsDestroyed().Return(false).Maybe() | 99 | oldWV.EXPECT().IsDestroyed().Return(false).Maybe() | ||
| 78 | c := newRevealTestCoordinator() | 100 | c := newRevealTestCoordinator() | ||
| 79 | c.setWebViewLocked(paneID, oldWV) | 101 | c.setWebViewLocked(paneID, oldWV) | ||
| 80 | - | c.markPendingReveal(paneID, oldWV) | 102 | + | oldIdentity, _ := identityForWebView(oldWV) |
| 103 | + | c.markPendingReveal(paneID, oldIdentity) | |||
| 81 | c.setWebViewLocked(paneID, newWV) | 104 | c.setWebViewLocked(paneID, newWV) | ||
| 82 | 105 | ||||
| 83 | - | c.revealIfPending(context.Background(), paneID, oldWV, "", "stale") | 106 | + | c.revealIfPending(context.Background(), paneID, oldWV, oldIdentity, "", "stale") |
| 84 | assert.False(t, c.WebViewRevealed(paneID)) | 107 | assert.False(t, c.WebViewRevealed(paneID)) | ||
| 85 | } | 108 | } | ||
| 86 | 109 | ||||
| @@ -89,9 +112,85 @@ func TestRevealState_RevealedCurrentWebViewPersistsForWorkspaceRebuild(t *testin | |||||
| 89 | wv := revealTestWebView(t, 101, 1) | 112 | wv := revealTestWebView(t, 101, 1) | ||
| 90 | c := newRevealTestCoordinator() | 113 | c := newRevealTestCoordinator() | ||
| 91 | c.setWebViewLocked(paneID, wv) | 114 | c.setWebViewLocked(paneID, wv) | ||
| 92 | - | c.markWebViewRevealed(paneID, wv) | 115 | + | identity, _ := identityForWebView(wv) |
| 116 | + | c.markWebViewRevealed(paneID, identity) | |||
| 93 | 117 | ||||
| 94 | // AttachToWorkspace asks this identity-aware query when rebuilding a pane, | 118 | // AttachToWorkspace asks this identity-aware query when rebuilding a pane, | ||
| 95 | // so the replacement PaneView hides its skeleton for this same WebView. | 119 | // so the replacement PaneView hides its skeleton for this same WebView. | ||
| 96 | - | assert.True(t, c.WebViewRevealed(paneID)) | 120 | + | assert.True(t, c.webViewRevealed(paneID, wv)) |
| 121 | + | ||||
| 122 | + | replacement := revealTestWebView(t, 102, 1) | |||
| 123 | + | assert.False(t, c.webViewRevealed(paneID, replacement)) | |||
| 124 | + | } | |||
| 125 | + | ||||
| 126 | + | func TestRevealCallback_CapturesGenerationBeforeSameObjectPoolReuse(t *testing.T) { | |||
| 127 | + | paneID := entity.PaneID("pane-1") | |||
| 128 | + | wv := mocks.NewMockWebView(t) | |||
| 129 | + | generation := uint64(1) | |||
| 130 | + | wv.EXPECT().ID().Return(port.WebViewID(101)).Maybe() | |||
| 131 | + | wv.EXPECT().Generation().RunAndReturn(func() uint64 { return generation }).Maybe() | |||
| 132 | + | wv.EXPECT().IsDestroyed().Return(false).Maybe() | |||
| 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)) | |||
| 154 | + | } | |||
| 155 | + | ||||
| 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 | |||
| 172 | + | } | |||
| 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: | |||
| 189 | + | } | |||
| 190 | + | ||||
| 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") | |||
| 97 | } | 196 | } | ||
M internal/ui/coordinator/content/coordinator.go +12 -0
| @@ -70,6 +70,14 @@ type Coordinator struct { | |||
| 70 | 70 | // Callback when the WebView becomes visible (first real commit) | |
| 71 | 71 | onWebViewShown func(paneID entity.PaneID) | |
| 72 | 72 | ||
| 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 | + | ||
| 73 | 81 | // revealMu is always acquired after webViewsMu. Reveal state is keyed by the | |
| 74 | 82 | // WebView identity (ID plus pool-reuse generation), never by pane alone. | |
| 75 | 83 | revealMu sync.Mutex | |
| @@ -348,6 +356,8 @@ func (c *Coordinator) ensureRevealMapsLocked() { | |||
| 348 | 356 | // setWebViewLocked replaces the pane mapping and resets all reveal state in | |
| 349 | 357 | // one critical section. It is used by normal acquisition and popup paths. | |
| 350 | 358 | func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |
| 359 | + | c.revealMutationMu.Lock() | |
| 360 | + | defer c.revealMutationMu.Unlock() | |
| 351 | 361 | c.webViewsMu.Lock() | |
| 352 | 362 | c.revealMu.Lock() | |
| 353 | 363 | defer c.revealMu.Unlock() | |
| @@ -372,6 +382,8 @@ func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |||
| 372 | 382 | // deleteWebViewLocked removes both the mapping and any presentation state, | |
| 373 | 383 | // including callers which release after another path already removed the map. | |
| 374 | 384 | func (c *Coordinator) deleteWebViewLocked(paneID entity.PaneID) port.WebView { | |
| 385 | + | c.revealMutationMu.Lock() | |
| 386 | + | defer c.revealMutationMu.Unlock() | |
| 375 | 387 | c.webViewsMu.Lock() | |
| 376 | 388 | c.revealMu.Lock() | |
| 377 | 389 | defer c.revealMu.Unlock() | |
| @@ -70,6 +70,14 @@ 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 | + | // 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 | + | ||||
| 73 | // revealMu is always acquired after webViewsMu. Reveal state is keyed by the | 81 | // revealMu is always acquired after webViewsMu. Reveal state is keyed by the | ||
| 74 | // WebView identity (ID plus pool-reuse generation), never by pane alone. | 82 | // WebView identity (ID plus pool-reuse generation), never by pane alone. | ||
| 75 | revealMu sync.Mutex | 83 | revealMu sync.Mutex | ||
| @@ -348,6 +356,8 @@ func (c *Coordinator) ensureRevealMapsLocked() { | |||||
| 348 | // setWebViewLocked replaces the pane mapping and resets all reveal state in | 356 | // setWebViewLocked replaces the pane mapping and resets all reveal state in | ||
| 349 | // one critical section. It is used by normal acquisition and popup paths. | 357 | // one critical section. It is used by normal acquisition and popup paths. | ||
| 350 | 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() | |||
| 351 | c.webViewsMu.Lock() | 361 | c.webViewsMu.Lock() | ||
| 352 | c.revealMu.Lock() | 362 | c.revealMu.Lock() | ||
| 353 | defer c.revealMu.Unlock() | 363 | defer c.revealMu.Unlock() | ||
| @@ -372,6 +382,8 @@ func (c *Coordinator) setWebViewLocked(paneID entity.PaneID, wv port.WebView) { | |||||
| 372 | // deleteWebViewLocked removes both the mapping and any presentation state, | 382 | // deleteWebViewLocked removes both the mapping and any presentation state, | ||
| 373 | // including callers which release after another path already removed the map. | 383 | // including callers which release after another path already removed the map. | ||
| 374 | 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() | |||
| 375 | c.webViewsMu.Lock() | 387 | c.webViewsMu.Lock() | ||
| 376 | c.revealMu.Lock() | 388 | c.revealMu.Lock() | ||
| 377 | defer c.revealMu.Unlock() | 389 | defer c.revealMu.Unlock() | ||
M internal/ui/coordinator/content/lifecycle.go +1 -1
| @@ -140,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||
| 140 | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | |
| 141 | 141 | Msg("wrapped webview widget prepared for pane") | |
| 142 | 142 | ||
| 143 | - | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.WebViewRevealed(pane.ID)); err != nil { | |
| 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.webViewRevealed(pane.ID, wv)); err != nil { | |
| 144 | 144 | log.Warn().Err(err).Str("pane_id", string(pane.ID)).Msg("failed to attach webview widget") | |
| 145 | 145 | continue | |
| 146 | 146 | } | |
| @@ -140,7 +140,7 @@ func (c *Coordinator) AttachToWorkspace(ctx context.Context, ws *entity.Workspac | |||||
| 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | 140 | Int("wrapped_widget_alloc_height", widget.GetAllocatedHeight()). | ||
| 141 | Msg("wrapped webview widget prepared for pane") | 141 | Msg("wrapped webview widget prepared for pane") | ||
| 142 | 142 | ||||
| 143 | - | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.WebViewRevealed(pane.ID)); err != nil { | 143 | + | if err := wsView.AttachWebViewWidget(pane.ID, widget, c.webViewRevealed(pane.ID, wv)); err != nil { |
| 144 | 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") | ||
| 145 | continue | 145 | continue | ||
| 146 | } | 146 | } | ||
M internal/ui/coordinator/content/lifecycle_test.go +2 -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) | |
| @@ -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) | ||
M internal/ui/coordinator/content/navigation.go +46 -36
| @@ -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, wv) | |
| 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, wv) | |
| 81 | - | c.revealIfPending(ctx, paneID, wv, 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, wv, "", "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, wv port.WebView, 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, wv, "", "progress") | |
| 230 | + | c.revealIfPending(context.Background(), paneID, wv, identity, "", "progress") | |
| 231 | 231 | } | |
| 232 | 232 | ||
| 233 | 233 | _, wsView := c.getActiveWS() | |
| @@ -244,11 +244,7 @@ func (c *Coordinator) onProgressChanged(paneID entity.PaneID, wv port.WebView, p | |||
| 244 | 244 | } | |
| 245 | 245 | } | |
| 246 | 246 | ||
| 247 | - | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, wv port.WebView) { | |
| 248 | - | identity, ok := identityForWebView(wv) | |
| 249 | - | if !ok { | |
| 250 | - | return | |
| 251 | - | } | |
| 247 | + | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, identity webViewIdentity) { | |
| 252 | 248 | c.webViewsMu.RLock() | |
| 253 | 249 | c.revealMu.Lock() | |
| 254 | 250 | defer c.revealMu.Unlock() | |
| @@ -266,11 +262,7 @@ func (c *Coordinator) markPendingReveal(paneID entity.PaneID, wv port.WebView) { | |||
| 266 | 262 | c.pendingRevealIdentity[paneID] = identity | |
| 267 | 263 | } | |
| 268 | 264 | ||
| 269 | - | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, wv port.WebView) { | |
| 270 | - | identity, ok := identityForWebView(wv) | |
| 271 | - | if !ok { | |
| 272 | - | return | |
| 273 | - | } | |
| 265 | + | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewIdentity) { | |
| 274 | 266 | c.webViewsMu.RLock() | |
| 275 | 267 | c.revealMu.Lock() | |
| 276 | 268 | defer c.revealMu.Unlock() | |
| @@ -285,19 +277,29 @@ func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, wv port.WebView) | |||
| 285 | 277 | // WebViewRevealed reports whether this pane's current WebView has completed a | |
| 286 | 278 | // reveal. State is identity-bound so a replacement cannot inherit it. | |
| 287 | 279 | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | |
| 280 | + | return c.webViewRevealed(paneID, nil) | |
| 281 | + | } | |
| 282 | + | ||
| 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 { | |
| 288 | 287 | c.webViewsMu.RLock() | |
| 289 | 288 | c.revealMu.Lock() | |
| 290 | 289 | defer c.revealMu.Unlock() | |
| 291 | 290 | defer c.webViewsMu.RUnlock() | |
| 292 | - | identity, ok := identityForWebView(c.webViews[paneID]) | |
| 293 | - | return ok && c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity | |
| 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 | + | } | |
| 298 | + | } | |
| 299 | + | return c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity | |
| 294 | 300 | } | |
| 295 | 301 | ||
| 296 | - | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, wv port.WebView) bool { | |
| 297 | - | identity, ok := identityForWebView(wv) | |
| 298 | - | if !ok { | |
| 299 | - | return false | |
| 300 | - | } | |
| 302 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webViewIdentity) bool { | |
| 301 | 303 | c.webViewsMu.RLock() | |
| 302 | 304 | c.revealMu.Lock() | |
| 303 | 305 | defer c.revealMu.Unlock() | |
| @@ -312,22 +314,27 @@ func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, wv port.WebView) | |||
| 312 | 314 | return true | |
| 313 | 315 | } | |
| 314 | 316 | ||
| 315 | - | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri, reason string) { | |
| 316 | - | identity, ok := identityForWebView(wv) | |
| 317 | - | if !ok { | |
| 318 | - | return | |
| 319 | - | } | |
| 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 | + | ||
| 320 | 325 | c.webViewsMu.RLock() | |
| 321 | 326 | c.revealMu.Lock() | |
| 322 | 327 | if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { | |
| 323 | 328 | c.revealMu.Unlock() | |
| 324 | 329 | c.webViewsMu.RUnlock() | |
| 330 | + | c.revealMutationMu.Unlock() | |
| 325 | 331 | return | |
| 326 | 332 | } | |
| 327 | 333 | currentIdentity, current := identityForWebView(c.webViews[paneID]) | |
| 328 | 334 | if !current || currentIdentity != identity { | |
| 329 | 335 | c.revealMu.Unlock() | |
| 330 | 336 | c.webViewsMu.RUnlock() | |
| 337 | + | c.revealMutationMu.Unlock() | |
| 331 | 338 | return | |
| 332 | 339 | } | |
| 333 | 340 | delete(c.pendingReveal, paneID) | |
| @@ -336,11 +343,13 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||
| 336 | 343 | c.webViewsMu.RUnlock() | |
| 337 | 344 | ||
| 338 | 345 | if wv.IsDestroyed() { | |
| 346 | + | c.revealMutationMu.Unlock() | |
| 339 | 347 | return | |
| 340 | 348 | } | |
| 341 | 349 | ||
| 342 | - | // Use NativeWidgetProvider to make the widget visible (engine-agnostic) | |
| 343 | - | 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 { | |
| 344 | 353 | if ptr := nwp.NativeWidget(); ptr != 0 { | |
| 345 | 354 | gtkWidget := >k.Widget{} | |
| 346 | 355 | gtkWidget.Ptr = ptr | |
| @@ -354,10 +363,11 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||
| 354 | 363 | } | |
| 355 | 364 | } | |
| 356 | 365 | ||
| 357 | - | // Record presentation state before notifying the UI. The callback may run | |
| 358 | - | // while a WorkspaceView is being rebuilt, and the next PaneView must inherit | |
| 359 | - | // this state even though the native widget was unparented during cleanup. | |
| 360 | - | if c.markWebViewRevealed(paneID, wv) && 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 { | |
| 361 | 371 | c.onWebViewShown(paneID) | |
| 362 | 372 | } | |
| 363 | 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, wv) | 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, wv) | 80 | + | c.markPendingReveal(paneID, identity) |
| 81 | - | c.revealIfPending(ctx, paneID, wv, 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, wv, "", "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, wv port.WebView, 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, wv, "", "progress") | 230 | + | c.revealIfPending(context.Background(), paneID, wv, identity, "", "progress") |
| 231 | } | 231 | } | ||
| 232 | 232 | ||||
| 233 | _, wsView := c.getActiveWS() | 233 | _, wsView := c.getActiveWS() | ||
| @@ -244,11 +244,7 @@ func (c *Coordinator) onProgressChanged(paneID entity.PaneID, wv port.WebView, p | |||||
| 244 | } | 244 | } | ||
| 245 | } | 245 | } | ||
| 246 | 246 | ||||
| 247 | - | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, wv port.WebView) { | 247 | + | func (c *Coordinator) markPendingReveal(paneID entity.PaneID, identity webViewIdentity) { |
| 248 | - | identity, ok := identityForWebView(wv) | |||
| 249 | - | if !ok { | |||
| 250 | - | return | |||
| 251 | - | } | |||
| 252 | c.webViewsMu.RLock() | 248 | c.webViewsMu.RLock() | ||
| 253 | c.revealMu.Lock() | 249 | c.revealMu.Lock() | ||
| 254 | defer c.revealMu.Unlock() | 250 | defer c.revealMu.Unlock() | ||
| @@ -266,11 +262,7 @@ func (c *Coordinator) markPendingReveal(paneID entity.PaneID, wv port.WebView) { | |||||
| 266 | c.pendingRevealIdentity[paneID] = identity | 262 | c.pendingRevealIdentity[paneID] = identity | ||
| 267 | } | 263 | } | ||
| 268 | 264 | ||||
| 269 | - | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, wv port.WebView) { | 265 | + | func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, identity webViewIdentity) { |
| 270 | - | identity, ok := identityForWebView(wv) | |||
| 271 | - | if !ok { | |||
| 272 | - | return | |||
| 273 | - | } | |||
| 274 | c.webViewsMu.RLock() | 266 | c.webViewsMu.RLock() | ||
| 275 | c.revealMu.Lock() | 267 | c.revealMu.Lock() | ||
| 276 | defer c.revealMu.Unlock() | 268 | defer c.revealMu.Unlock() | ||
| @@ -285,19 +277,29 @@ func (c *Coordinator) clearPendingReveal(paneID entity.PaneID, wv port.WebView) | |||||
| 285 | // WebViewRevealed reports whether this pane's current WebView has completed a | 277 | // WebViewRevealed reports whether this pane's current WebView has completed a | ||
| 286 | // reveal. State is identity-bound so a replacement cannot inherit it. | 278 | // reveal. State is identity-bound so a replacement cannot inherit it. | ||
| 287 | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | 279 | func (c *Coordinator) WebViewRevealed(paneID entity.PaneID) bool { | ||
| 280 | + | return c.webViewRevealed(paneID, nil) | |||
| 281 | + | } | |||
| 282 | + | ||||
| 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 { | |||
| 288 | c.webViewsMu.RLock() | 287 | c.webViewsMu.RLock() | ||
| 289 | c.revealMu.Lock() | 288 | c.revealMu.Lock() | ||
| 290 | defer c.revealMu.Unlock() | 289 | defer c.revealMu.Unlock() | ||
| 291 | defer c.webViewsMu.RUnlock() | 290 | defer c.webViewsMu.RUnlock() | ||
| 292 | - | identity, ok := identityForWebView(c.webViews[paneID]) | 291 | + | current := c.webViews[paneID] |
| 293 | - | return ok && c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity | 292 | + | identity, ok := identityForWebView(current) |
| 293 | + | if !ok || expected != nil { | |||
| 294 | + | expectedIdentity, expectedOK := identityForWebView(expected) | |||
| 295 | + | if !expectedOK || expectedIdentity != identity { | |||
| 296 | + | return false | |||
| 297 | + | } | |||
| 298 | + | } | |||
| 299 | + | return c.revealedWebViews[paneID] && c.revealedWebViewIdentity[paneID] == identity | |||
| 294 | } | 300 | } | ||
| 295 | 301 | ||||
| 296 | - | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, wv port.WebView) bool { | 302 | + | func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, identity webViewIdentity) bool { |
| 297 | - | identity, ok := identityForWebView(wv) | |||
| 298 | - | if !ok { | |||
| 299 | - | return false | |||
| 300 | - | } | |||
| 301 | c.webViewsMu.RLock() | 303 | c.webViewsMu.RLock() | ||
| 302 | c.revealMu.Lock() | 304 | c.revealMu.Lock() | ||
| 303 | defer c.revealMu.Unlock() | 305 | defer c.revealMu.Unlock() | ||
| @@ -312,22 +314,27 @@ func (c *Coordinator) markWebViewRevealed(paneID entity.PaneID, wv port.WebView) | |||||
| 312 | return true | 314 | return true | ||
| 313 | } | 315 | } | ||
| 314 | 316 | ||||
| 315 | - | func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, wv port.WebView, uri, reason string) { | 317 | + | func (c *Coordinator) revealIfPending( |
| 316 | - | identity, ok := identityForWebView(wv) | 318 | + | ctx context.Context, paneID entity.PaneID, wv port.WebView, identity webViewIdentity, uri, reason string, |
| 317 | - | if !ok { | 319 | + | ) { |
| 318 | - | return | 320 | + | // Mapping replacement and release acquire this gate too. No ordinary map |
| 319 | - | } | 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 | + | ||||
| 320 | c.webViewsMu.RLock() | 325 | c.webViewsMu.RLock() | ||
| 321 | c.revealMu.Lock() | 326 | c.revealMu.Lock() | ||
| 322 | if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { | 327 | if c.pendingRevealIdentity[paneID] != identity || !c.pendingReveal[paneID] { | ||
| 323 | c.revealMu.Unlock() | 328 | c.revealMu.Unlock() | ||
| 324 | c.webViewsMu.RUnlock() | 329 | c.webViewsMu.RUnlock() | ||
| 330 | + | c.revealMutationMu.Unlock() | |||
| 325 | return | 331 | return | ||
| 326 | } | 332 | } | ||
| 327 | currentIdentity, current := identityForWebView(c.webViews[paneID]) | 333 | currentIdentity, current := identityForWebView(c.webViews[paneID]) | ||
| 328 | if !current || currentIdentity != identity { | 334 | if !current || currentIdentity != identity { | ||
| 329 | c.revealMu.Unlock() | 335 | c.revealMu.Unlock() | ||
| 330 | c.webViewsMu.RUnlock() | 336 | c.webViewsMu.RUnlock() | ||
| 337 | + | c.revealMutationMu.Unlock() | |||
| 331 | return | 338 | return | ||
| 332 | } | 339 | } | ||
| 333 | delete(c.pendingReveal, paneID) | 340 | delete(c.pendingReveal, paneID) | ||
| @@ -336,11 +343,13 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||||
| 336 | c.webViewsMu.RUnlock() | 343 | c.webViewsMu.RUnlock() | ||
| 337 | 344 | ||||
| 338 | if wv.IsDestroyed() { | 345 | if wv.IsDestroyed() { | ||
| 346 | + | c.revealMutationMu.Unlock() | |||
| 339 | return | 347 | return | ||
| 340 | } | 348 | } | ||
| 341 | 349 | ||||
| 342 | - | // Use NativeWidgetProvider to make the widget visible (engine-agnostic) | 350 | + | if c.setWebViewVisible != nil { |
| 343 | - | if nwp, ok := wv.(port.NativeWidgetProvider); ok { | 351 | + | c.setWebViewVisible(wv) |
| 352 | + | } else if nwp, ok := wv.(port.NativeWidgetProvider); ok { | |||
| 344 | if ptr := nwp.NativeWidget(); ptr != 0 { | 353 | if ptr := nwp.NativeWidget(); ptr != 0 { | ||
| 345 | gtkWidget := >k.Widget{} | 354 | gtkWidget := >k.Widget{} | ||
| 346 | gtkWidget.Ptr = ptr | 355 | gtkWidget.Ptr = ptr | ||
| @@ -354,10 +363,11 @@ func (c *Coordinator) revealIfPending(ctx context.Context, paneID entity.PaneID, | |||||
| 354 | } | 363 | } | ||
| 355 | } | 364 | } | ||
| 356 | 365 | ||||
| 357 | - | // Record presentation state before notifying the UI. The callback may run | 366 | + | // Keep the presentation record in the same serialized operation as the |
| 358 | - | // while a WorkspaceView is being rebuilt, and the next PaneView must inherit | 367 | + | // native mutation. User callbacks run only after the gate is released. |
| 359 | - | // this state even though the native widget was unparented during cleanup. | 368 | + | revealed := c.markWebViewRevealed(paneID, identity) |
| 360 | - | if c.markWebViewRevealed(paneID, wv) && c.onWebViewShown != nil { | 369 | + | c.revealMutationMu.Unlock() |
| 370 | + | if revealed && c.onWebViewShown != nil { | |||
| 361 | c.onWebViewShown(paneID) | 371 | c.onWebViewShown(paneID) | ||
| 362 | } | 372 | } | ||
| 363 | } | 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() | ||