fix(cef): skip stale JS dialog reset when a newer dialog is pending
The UI reset posted by cancelJSDialogsClosing carries no dialog identity; if a new dialog began before the GTK thread ran it, it would hide that dialog. Re-check pending state under the dialog mutex on the GTK thread and skip the reset when a dialog is pending.
- commit
- f72eaf648f3ce06bfd01419eeff8e03142200cb0
- author
- brice · · 2026-10-04 09:49 UTC
- parent
- b8b745a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- feat/cef-js-dialogs
2 files · +46 -1
M internal/infrastructure/cef/jsdialog_handler.go +20 -1
| @@ -105,6 +105,13 @@ func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, jsDia | |||
| 105 | 105 | return call, jsDialogBeginOK | |
| 106 | 106 | } | |
| 107 | 107 | ||
| 108 | + | // hasPending reports whether an unresolved dialog is registered. | |
| 109 | + | func (s *jsDialogState) hasPending() bool { | |
| 110 | + | s.mu.Lock() | |
| 111 | + | defer s.mu.Unlock() | |
| 112 | + | return s.current != nil | |
| 113 | + | } | |
| 114 | + | ||
| 108 | 115 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | |
| 109 | 116 | s.mu.Lock() | |
| 110 | 117 | defer s.mu.Unlock() | |
| @@ -165,8 +172,20 @@ func (wv *WebView) cancelJSDialogsClosing(closing bool) { | |||
| 165 | 172 | return | |
| 166 | 173 | } | |
| 167 | 174 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | |
| 168 | - | wv.runOnGTK(cb.OnJSDialogReset) | |
| 175 | + | wv.runOnGTK(func() { wv.resetJSDialogUI(cb.OnJSDialogReset) }) | |
| 176 | + | } | |
| 177 | + | } | |
| 178 | + | ||
| 179 | + | // resetJSDialogUI runs on the GTK thread, after a cancel posted it. The reset | |
| 180 | + | // carries no dialog identity, so if a newer dialog began in the meantime it is | |
| 181 | + | // stale and must be skipped, or it would hide that dialog. A dialog that | |
| 182 | + | // begins after this check has its presentation queued behind us on the GTK | |
| 183 | + | // thread, so it is never hidden either. | |
| 184 | + | func (wv *WebView) resetJSDialogUI(reset func()) { | |
| 185 | + | if wv.jsDialogs.hasPending() { | |
| 186 | + | return | |
| 169 | 187 | } | |
| 188 | + | reset() | |
| 170 | 189 | } | |
| 171 | 190 | ||
| 172 | 191 | func jsDialogTypeFromCEF(t purecef.JsdialogType) (port.JSDialogType, bool) { | |
| @@ -105,6 +105,13 @@ func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, jsDia | |||||
| 105 | return call, jsDialogBeginOK | 105 | return call, jsDialogBeginOK | ||
| 106 | } | 106 | } | ||
| 107 | 107 | ||||
| 108 | + | // hasPending reports whether an unresolved dialog is registered. | |||
| 109 | + | func (s *jsDialogState) hasPending() bool { | |||
| 110 | + | s.mu.Lock() | |||
| 111 | + | defer s.mu.Unlock() | |||
| 112 | + | return s.current != nil | |||
| 113 | + | } | |||
| 114 | + | ||||
| 108 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | 115 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | ||
| 109 | s.mu.Lock() | 116 | s.mu.Lock() | ||
| 110 | defer s.mu.Unlock() | 117 | defer s.mu.Unlock() | ||
| @@ -165,8 +172,20 @@ func (wv *WebView) cancelJSDialogsClosing(closing bool) { | |||||
| 165 | return | 172 | return | ||
| 166 | } | 173 | } | ||
| 167 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | 174 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | ||
| 168 | - | wv.runOnGTK(cb.OnJSDialogReset) | 175 | + | wv.runOnGTK(func() { wv.resetJSDialogUI(cb.OnJSDialogReset) }) |
| 176 | + | } | |||
| 177 | + | } | |||
| 178 | + | ||||
| 179 | + | // resetJSDialogUI runs on the GTK thread, after a cancel posted it. The reset | |||
| 180 | + | // carries no dialog identity, so if a newer dialog began in the meantime it is | |||
| 181 | + | // stale and must be skipped, or it would hide that dialog. A dialog that | |||
| 182 | + | // begins after this check has its presentation queued behind us on the GTK | |||
| 183 | + | // thread, so it is never hidden either. | |||
| 184 | + | func (wv *WebView) resetJSDialogUI(reset func()) { | |||
| 185 | + | if wv.jsDialogs.hasPending() { | |||
| 186 | + | return | |||
| 169 | } | 187 | } | ||
| 188 | + | reset() | |||
| 170 | } | 189 | } | ||
| 171 | 190 | ||||
| 172 | func jsDialogTypeFromCEF(t purecef.JsdialogType) (port.JSDialogType, bool) { | 191 | func jsDialogTypeFromCEF(t purecef.JsdialogType) (port.JSDialogType, bool) { | ||
M internal/infrastructure/cef/jsdialog_handler_test.go +26 -0
| @@ -256,6 +256,32 @@ func TestJSDialogCancelledBeforeUIRunsIsNotShown(t *testing.T) { | |||
| 256 | 256 | require.Len(t, cb.calls, 1) | |
| 257 | 257 | } | |
| 258 | 258 | ||
| 259 | + | func TestStaleUIResetSkippedWhenNewerDialogPending(t *testing.T) { | |
| 260 | + | useDirectJSDialogContinue(t) | |
| 261 | + | ui := &jsDialogUIRecorder{handled: true} | |
| 262 | + | wv := newJSDialogWebView(ui) | |
| 263 | + | h := &handlerSet{wv: wv} | |
| 264 | + | old := &stubJSDialogCallback{} | |
| 265 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "old", "", old, new(int32)) | |
| 266 | + | ||
| 267 | + | // Cancel resolves the old dialog; its UI reset is still queued for the | |
| 268 | + | // GTK thread (modeled by calling resetJSDialogUI later). | |
| 269 | + | require.True(t, wv.jsDialogs.cancelPending(false)) | |
| 270 | + | ||
| 271 | + | // A newer dialog arrives before the queued reset runs. | |
| 272 | + | newer := &stubJSDialogCallback{} | |
| 273 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "new", "", newer, new(int32)) | |
| 274 | + | require.Len(t, ui.reqs, 2) | |
| 275 | + | ||
| 276 | + | wv.resetJSDialogUI(wv.callbacks.OnJSDialogReset) | |
| 277 | + | require.Zero(t, ui.resets, "stale reset must not hide the newer dialog") | |
| 278 | + | ||
| 279 | + | // Once nothing is pending the reset goes through. | |
| 280 | + | ui.respond[1](true, "") | |
| 281 | + | wv.resetJSDialogUI(wv.callbacks.OnJSDialogReset) | |
| 282 | + | require.Equal(t, 1, ui.resets) | |
| 283 | + | } | |
| 284 | + | ||
| 259 | 285 | func TestResetEchoAfterSuppressDoesNotCancelOpenDialog(t *testing.T) { | |
| 260 | 286 | useDirectJSDialogContinue(t) | |
| 261 | 287 | ui := &jsDialogUIRecorder{handled: true} | |
| @@ -256,6 +256,32 @@ func TestJSDialogCancelledBeforeUIRunsIsNotShown(t *testing.T) { | |||||
| 256 | require.Len(t, cb.calls, 1) | 256 | require.Len(t, cb.calls, 1) | ||
| 257 | } | 257 | } | ||
| 258 | 258 | ||||
| 259 | + | func TestStaleUIResetSkippedWhenNewerDialogPending(t *testing.T) { | |||
| 260 | + | useDirectJSDialogContinue(t) | |||
| 261 | + | ui := &jsDialogUIRecorder{handled: true} | |||
| 262 | + | wv := newJSDialogWebView(ui) | |||
| 263 | + | h := &handlerSet{wv: wv} | |||
| 264 | + | old := &stubJSDialogCallback{} | |||
| 265 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "old", "", old, new(int32)) | |||
| 266 | + | ||||
| 267 | + | // Cancel resolves the old dialog; its UI reset is still queued for the | |||
| 268 | + | // GTK thread (modeled by calling resetJSDialogUI later). | |||
| 269 | + | require.True(t, wv.jsDialogs.cancelPending(false)) | |||
| 270 | + | ||||
| 271 | + | // A newer dialog arrives before the queued reset runs. | |||
| 272 | + | newer := &stubJSDialogCallback{} | |||
| 273 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "new", "", newer, new(int32)) | |||
| 274 | + | require.Len(t, ui.reqs, 2) | |||
| 275 | + | ||||
| 276 | + | wv.resetJSDialogUI(wv.callbacks.OnJSDialogReset) | |||
| 277 | + | require.Zero(t, ui.resets, "stale reset must not hide the newer dialog") | |||
| 278 | + | ||||
| 279 | + | // Once nothing is pending the reset goes through. | |||
| 280 | + | ui.respond[1](true, "") | |||
| 281 | + | wv.resetJSDialogUI(wv.callbacks.OnJSDialogReset) | |||
| 282 | + | require.Equal(t, 1, ui.resets) | |||
| 283 | + | } | |||
| 284 | + | ||||
| 259 | func TestResetEchoAfterSuppressDoesNotCancelOpenDialog(t *testing.T) { | 285 | func TestResetEchoAfterSuppressDoesNotCancelOpenDialog(t *testing.T) { | ||
| 260 | useDirectJSDialogContinue(t) | 286 | useDirectJSDialogContinue(t) | ||
| 261 | ui := &jsDialogUIRecorder{handled: true} | 287 | ui := &jsDialogUIRecorder{handled: true} | ||