fix(cef): cancel JS dialogs on main-frame load and reject dialogs after destroy
CEF drops its handler after a suppress or rejected beforeunload, so no reset follows a navigation; cancel the pending dialog on main-frame OnLoadStart. Close the dialog state atomically in Destroy so begin cannot race it, and correct the reset-ordering comment.
- commit
- 9c48b2156bca02a2da7a3d5db06d441971f7879c
- author
- brice · · 2026-10-03 07:37 UTC
- committer
- brice · · 2026-10-03 07:50 UTC
- parent
- dc2703a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +110 -24
M internal/infrastructure/cef/jsdialog_handler.go +54 -19
| @@ -21,13 +21,20 @@ import ( | |||
| 21 | 21 | // are suppressed (suppress_message=true) instead of being queued, so a page | |
| 22 | 22 | // cannot stack or spam dialogs. | |
| 23 | 23 | // | |
| 24 | - | // CEF reset semantics (javascript_dialog_manager.cc): OnResetDialogState is | |
| 25 | - | // also delivered (a) right after a suppressed OnJsdialog and (b) at the end of | |
| 26 | - | // every DialogClosed, i.e. inside/after each callback Cont. Those are not real | |
| 27 | - | // resets and must not cancel the dialog the user is looking at, so | |
| 28 | - | // jsDialogState carries a one-shot ignoreReset flag armed before suppressing, | |
| 29 | - | // rejecting or continuing. Only WebView destruction and renderer termination | |
| 30 | - | // cancel unconditionally. | |
| 24 | + | // CEF reset semantics (javascript_dialog_manager.cc): while CEF's handler_ is | |
| 25 | + | // set, OnResetDialogState is also delivered (a) right after a suppressed | |
| 26 | + | // OnJsdialog and (b) at the end of DialogClosed, i.e. inside/after a callback | |
| 27 | + | // Cont. Those are not real resets and must not cancel the dialog the user is | |
| 28 | + | // looking at, so jsDialogState carries a one-shot ignoreReset flag armed before | |
| 29 | + | // suppressing, rejecting or continuing (it may stay unconsumed when CEF's | |
| 30 | + | // handler_ was already cleared; begin resets it). | |
| 31 | + | // | |
| 32 | + | // After a suppress or a rejected beforeunload CEF clears handler_, so a later | |
| 33 | + | // navigation sends no reset for the still-open dialog, and Chromium's | |
| 34 | + | // main-frame commit does not reset state either. Main-frame OnLoadStart | |
| 35 | + | // therefore cancels a pending dialog too. Destroy and renderer termination | |
| 36 | + | // cancel unconditionally; Destroy also closes the state so no dialog can begin | |
| 37 | + | // afterwards. | |
| 31 | 38 | ||
| 32 | 39 | // continueCEFJSDialog resolves a CEF dialog callback. It is a seam for tests. | |
| 33 | 40 | // CEF's callback implementation re-posts to the CEF UI thread by itself. | |
| @@ -52,8 +59,18 @@ type jsDialogState struct { | |||
| 52 | 59 | mu sync.Mutex | |
| 53 | 60 | current *jsDialogCall | |
| 54 | 61 | ignoreReset bool // swallow the next OnResetDialogState (see file comment) | |
| 62 | + | closed bool // WebView destroyed: begin always fails | |
| 55 | 63 | } | |
| 56 | 64 | ||
| 65 | + | // jsDialogBegin is the outcome of jsDialogState.begin. | |
| 66 | + | type jsDialogBegin int | |
| 67 | + | ||
| 68 | + | const ( | |
| 69 | + | jsDialogBeginOK jsDialogBegin = iota | |
| 70 | + | jsDialogBeginBusy // another dialog is open | |
| 71 | + | jsDialogBeginClosed // WebView destroyed | |
| 72 | + | ) | |
| 73 | + | ||
| 57 | 74 | // armIgnoreReset makes the next OnResetDialogState a no-op. | |
| 58 | 75 | func (s *jsDialogState) armIgnoreReset() { | |
| 59 | 76 | s.mu.Lock() | |
| @@ -70,19 +87,22 @@ func (s *jsDialogState) consumeIgnoreReset() bool { | |||
| 70 | 87 | return was | |
| 71 | 88 | } | |
| 72 | 89 | ||
| 73 | - | // begin registers a new pending dialog; false if one is already open. | |
| 74 | - | func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, bool) { | |
| 90 | + | // begin registers a new pending dialog. | |
| 91 | + | func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, jsDialogBegin) { | |
| 75 | 92 | s.mu.Lock() | |
| 76 | 93 | defer s.mu.Unlock() | |
| 94 | + | if s.closed { | |
| 95 | + | return nil, jsDialogBeginClosed | |
| 96 | + | } | |
| 77 | 97 | if s.current != nil { | |
| 78 | - | return nil, false | |
| 98 | + | return nil, jsDialogBeginBusy | |
| 79 | 99 | } | |
| 80 | 100 | call := &jsDialogCall{callback: cb} | |
| 81 | 101 | s.current = call | |
| 82 | 102 | // A new dialog means any reset owed by the previous one has been delivered | |
| 83 | 103 | // (the page is blocked until then); never carry a stale flag across. | |
| 84 | 104 | s.ignoreReset = false | |
| 85 | - | return call, true | |
| 105 | + | return call, jsDialogBeginOK | |
| 86 | 106 | } | |
| 87 | 107 | ||
| 88 | 108 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | |
| @@ -110,9 +130,14 @@ func (s *jsDialogState) resolve(call *jsDialogCall, ok bool, input string) bool | |||
| 110 | 130 | return true | |
| 111 | 131 | } | |
| 112 | 132 | ||
| 113 | - | // cancelPending resolves the pending dialog (if any) with a cancel. | |
| 114 | - | func (s *jsDialogState) cancelPending() bool { | |
| 133 | + | // cancelPending resolves the pending dialog (if any) with a cancel. When | |
| 134 | + | // closing is true it also rejects every later begin (closing is atomic with | |
| 135 | + | // reading the pending call, so a concurrent begin cannot slip in). | |
| 136 | + | func (s *jsDialogState) cancelPending(closing bool) bool { | |
| 115 | 137 | s.mu.Lock() | |
| 138 | + | if closing { | |
| 139 | + | s.closed = true | |
| 140 | + | } | |
| 116 | 141 | call := s.current | |
| 117 | 142 | s.mu.Unlock() | |
| 118 | 143 | if call == nil { | |
| @@ -128,11 +153,15 @@ func (wv *WebView) jsDialogUICallbacks() *port.WebViewCallbacks { | |||
| 128 | 153 | } | |
| 129 | 154 | ||
| 130 | 155 | // cancelJSDialogs resolves any pending dialog and tells the UI to hide it. | |
| 131 | - | func (wv *WebView) cancelJSDialogs() { | |
| 156 | + | func (wv *WebView) cancelJSDialogs() { wv.cancelJSDialogsClosing(false) } | |
| 157 | + | ||
| 158 | + | // cancelJSDialogsClosing is cancelJSDialogs; with closing it also forbids any | |
| 159 | + | // further dialog (WebView destruction). | |
| 160 | + | func (wv *WebView) cancelJSDialogsClosing(closing bool) { | |
| 132 | 161 | if wv == nil { | |
| 133 | 162 | return | |
| 134 | 163 | } | |
| 135 | - | if !wv.jsDialogs.cancelPending() { | |
| 164 | + | if !wv.jsDialogs.cancelPending(closing) { | |
| 136 | 165 | return | |
| 137 | 166 | } | |
| 138 | 167 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | |
| @@ -196,8 +225,11 @@ func (h *handlerSet) OnJsdialog( | |||
| 196 | 225 | if ui == nil || ui.OnJSDialog == nil { | |
| 197 | 226 | return 0 // no UI: let CEF apply its default behavior | |
| 198 | 227 | } | |
| 199 | - | call, ok := h.wv.jsDialogs.begin(callback) | |
| 200 | - | if !ok { | |
| 228 | + | call, begin := h.wv.jsDialogs.begin(callback) | |
| 229 | + | if begin == jsDialogBeginClosed { | |
| 230 | + | return 0 | |
| 231 | + | } | |
| 232 | + | if begin == jsDialogBeginBusy { | |
| 201 | 233 | // A dialog is already open: suppress rather than stack/spam. CEF will | |
| 202 | 234 | // follow up with OnResetDialogState, which must not cancel that dialog. | |
| 203 | 235 | h.wv.jsDialogs.armIgnoreReset() | |
| @@ -235,8 +267,11 @@ func (h *handlerSet) OnBeforeUnloadDialog( | |||
| 235 | 267 | if ui == nil || ui.OnJSDialog == nil { | |
| 236 | 268 | return false | |
| 237 | 269 | } | |
| 238 | - | call, ok := h.wv.jsDialogs.begin(callback) | |
| 239 | - | if !ok { | |
| 270 | + | call, begin := h.wv.jsDialogs.begin(callback) | |
| 271 | + | if begin == jsDialogBeginClosed { | |
| 272 | + | return false | |
| 273 | + | } | |
| 274 | + | if begin == jsDialogBeginBusy { | |
| 240 | 275 | // Another dialog is open: stay on the page rather than stacking. | |
| 241 | 276 | // The Cont below makes CEF send a reset that must not cancel it. | |
| 242 | 277 | h.wv.jsDialogs.armIgnoreReset() | |
| @@ -21,13 +21,20 @@ import ( | |||||
| 21 | // are suppressed (suppress_message=true) instead of being queued, so a page | 21 | // are suppressed (suppress_message=true) instead of being queued, so a page | ||
| 22 | // cannot stack or spam dialogs. | 22 | // cannot stack or spam dialogs. | ||
| 23 | // | 23 | // | ||
| 24 | - | // CEF reset semantics (javascript_dialog_manager.cc): OnResetDialogState is | 24 | + | // CEF reset semantics (javascript_dialog_manager.cc): while CEF's handler_ is |
| 25 | - | // also delivered (a) right after a suppressed OnJsdialog and (b) at the end of | 25 | + | // set, OnResetDialogState is also delivered (a) right after a suppressed |
| 26 | - | // every DialogClosed, i.e. inside/after each callback Cont. Those are not real | 26 | + | // OnJsdialog and (b) at the end of DialogClosed, i.e. inside/after a callback |
| 27 | - | // resets and must not cancel the dialog the user is looking at, so | 27 | + | // Cont. Those are not real resets and must not cancel the dialog the user is |
| 28 | - | // jsDialogState carries a one-shot ignoreReset flag armed before suppressing, | 28 | + | // looking at, so jsDialogState carries a one-shot ignoreReset flag armed before |
| 29 | - | // rejecting or continuing. Only WebView destruction and renderer termination | 29 | + | // suppressing, rejecting or continuing (it may stay unconsumed when CEF's |
| 30 | - | // cancel unconditionally. | 30 | + | // handler_ was already cleared; begin resets it). |
| 31 | + | // | |||
| 32 | + | // After a suppress or a rejected beforeunload CEF clears handler_, so a later | |||
| 33 | + | // navigation sends no reset for the still-open dialog, and Chromium's | |||
| 34 | + | // main-frame commit does not reset state either. Main-frame OnLoadStart | |||
| 35 | + | // therefore cancels a pending dialog too. Destroy and renderer termination | |||
| 36 | + | // cancel unconditionally; Destroy also closes the state so no dialog can begin | |||
| 37 | + | // afterwards. | |||
| 31 | 38 | ||||
| 32 | // continueCEFJSDialog resolves a CEF dialog callback. It is a seam for tests. | 39 | // continueCEFJSDialog resolves a CEF dialog callback. It is a seam for tests. | ||
| 33 | // CEF's callback implementation re-posts to the CEF UI thread by itself. | 40 | // CEF's callback implementation re-posts to the CEF UI thread by itself. | ||
| @@ -52,8 +59,18 @@ type jsDialogState struct { | |||||
| 52 | mu sync.Mutex | 59 | mu sync.Mutex | ||
| 53 | current *jsDialogCall | 60 | current *jsDialogCall | ||
| 54 | ignoreReset bool // swallow the next OnResetDialogState (see file comment) | 61 | ignoreReset bool // swallow the next OnResetDialogState (see file comment) | ||
| 62 | + | closed bool // WebView destroyed: begin always fails | |||
| 55 | } | 63 | } | ||
| 56 | 64 | ||||
| 65 | + | // jsDialogBegin is the outcome of jsDialogState.begin. | |||
| 66 | + | type jsDialogBegin int | |||
| 67 | + | ||||
| 68 | + | const ( | |||
| 69 | + | jsDialogBeginOK jsDialogBegin = iota | |||
| 70 | + | jsDialogBeginBusy // another dialog is open | |||
| 71 | + | jsDialogBeginClosed // WebView destroyed | |||
| 72 | + | ) | |||
| 73 | + | ||||
| 57 | // armIgnoreReset makes the next OnResetDialogState a no-op. | 74 | // armIgnoreReset makes the next OnResetDialogState a no-op. | ||
| 58 | func (s *jsDialogState) armIgnoreReset() { | 75 | func (s *jsDialogState) armIgnoreReset() { | ||
| 59 | s.mu.Lock() | 76 | s.mu.Lock() | ||
| @@ -70,19 +87,22 @@ func (s *jsDialogState) consumeIgnoreReset() bool { | |||||
| 70 | return was | 87 | return was | ||
| 71 | } | 88 | } | ||
| 72 | 89 | ||||
| 73 | - | // begin registers a new pending dialog; false if one is already open. | 90 | + | // begin registers a new pending dialog. |
| 74 | - | func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, bool) { | 91 | + | func (s *jsDialogState) begin(cb purecef.JsdialogCallback) (*jsDialogCall, jsDialogBegin) { |
| 75 | s.mu.Lock() | 92 | s.mu.Lock() | ||
| 76 | defer s.mu.Unlock() | 93 | defer s.mu.Unlock() | ||
| 94 | + | if s.closed { | |||
| 95 | + | return nil, jsDialogBeginClosed | |||
| 96 | + | } | |||
| 77 | if s.current != nil { | 97 | if s.current != nil { | ||
| 78 | - | return nil, false | 98 | + | return nil, jsDialogBeginBusy |
| 79 | } | 99 | } | ||
| 80 | call := &jsDialogCall{callback: cb} | 100 | call := &jsDialogCall{callback: cb} | ||
| 81 | s.current = call | 101 | s.current = call | ||
| 82 | // A new dialog means any reset owed by the previous one has been delivered | 102 | // A new dialog means any reset owed by the previous one has been delivered | ||
| 83 | // (the page is blocked until then); never carry a stale flag across. | 103 | // (the page is blocked until then); never carry a stale flag across. | ||
| 84 | s.ignoreReset = false | 104 | s.ignoreReset = false | ||
| 85 | - | return call, true | 105 | + | return call, jsDialogBeginOK |
| 86 | } | 106 | } | ||
| 87 | 107 | ||||
| 88 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | 108 | func (s *jsDialogState) isDone(call *jsDialogCall) bool { | ||
| @@ -110,9 +130,14 @@ func (s *jsDialogState) resolve(call *jsDialogCall, ok bool, input string) bool | |||||
| 110 | return true | 130 | return true | ||
| 111 | } | 131 | } | ||
| 112 | 132 | ||||
| 113 | - | // cancelPending resolves the pending dialog (if any) with a cancel. | 133 | + | // cancelPending resolves the pending dialog (if any) with a cancel. When |
| 114 | - | func (s *jsDialogState) cancelPending() bool { | 134 | + | // closing is true it also rejects every later begin (closing is atomic with |
| 135 | + | // reading the pending call, so a concurrent begin cannot slip in). | |||
| 136 | + | func (s *jsDialogState) cancelPending(closing bool) bool { | |||
| 115 | s.mu.Lock() | 137 | s.mu.Lock() | ||
| 138 | + | if closing { | |||
| 139 | + | s.closed = true | |||
| 140 | + | } | |||
| 116 | call := s.current | 141 | call := s.current | ||
| 117 | s.mu.Unlock() | 142 | s.mu.Unlock() | ||
| 118 | if call == nil { | 143 | if call == nil { | ||
| @@ -128,11 +153,15 @@ func (wv *WebView) jsDialogUICallbacks() *port.WebViewCallbacks { | |||||
| 128 | } | 153 | } | ||
| 129 | 154 | ||||
| 130 | // cancelJSDialogs resolves any pending dialog and tells the UI to hide it. | 155 | // cancelJSDialogs resolves any pending dialog and tells the UI to hide it. | ||
| 131 | - | func (wv *WebView) cancelJSDialogs() { | 156 | + | func (wv *WebView) cancelJSDialogs() { wv.cancelJSDialogsClosing(false) } |
| 157 | + | ||||
| 158 | + | // cancelJSDialogsClosing is cancelJSDialogs; with closing it also forbids any | |||
| 159 | + | // further dialog (WebView destruction). | |||
| 160 | + | func (wv *WebView) cancelJSDialogsClosing(closing bool) { | |||
| 132 | if wv == nil { | 161 | if wv == nil { | ||
| 133 | return | 162 | return | ||
| 134 | } | 163 | } | ||
| 135 | - | if !wv.jsDialogs.cancelPending() { | 164 | + | if !wv.jsDialogs.cancelPending(closing) { |
| 136 | return | 165 | return | ||
| 137 | } | 166 | } | ||
| 138 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | 167 | if cb := wv.jsDialogUICallbacks(); cb != nil && cb.OnJSDialogReset != nil { | ||
| @@ -196,8 +225,11 @@ func (h *handlerSet) OnJsdialog( | |||||
| 196 | if ui == nil || ui.OnJSDialog == nil { | 225 | if ui == nil || ui.OnJSDialog == nil { | ||
| 197 | return 0 // no UI: let CEF apply its default behavior | 226 | return 0 // no UI: let CEF apply its default behavior | ||
| 198 | } | 227 | } | ||
| 199 | - | call, ok := h.wv.jsDialogs.begin(callback) | 228 | + | call, begin := h.wv.jsDialogs.begin(callback) |
| 200 | - | if !ok { | 229 | + | if begin == jsDialogBeginClosed { |
| 230 | + | return 0 | |||
| 231 | + | } | |||
| 232 | + | if begin == jsDialogBeginBusy { | |||
| 201 | // A dialog is already open: suppress rather than stack/spam. CEF will | 233 | // A dialog is already open: suppress rather than stack/spam. CEF will | ||
| 202 | // follow up with OnResetDialogState, which must not cancel that dialog. | 234 | // follow up with OnResetDialogState, which must not cancel that dialog. | ||
| 203 | h.wv.jsDialogs.armIgnoreReset() | 235 | h.wv.jsDialogs.armIgnoreReset() | ||
| @@ -235,8 +267,11 @@ func (h *handlerSet) OnBeforeUnloadDialog( | |||||
| 235 | if ui == nil || ui.OnJSDialog == nil { | 267 | if ui == nil || ui.OnJSDialog == nil { | ||
| 236 | return false | 268 | return false | ||
| 237 | } | 269 | } | ||
| 238 | - | call, ok := h.wv.jsDialogs.begin(callback) | 270 | + | call, begin := h.wv.jsDialogs.begin(callback) |
| 239 | - | if !ok { | 271 | + | if begin == jsDialogBeginClosed { |
| 272 | + | return false | |||
| 273 | + | } | |||
| 274 | + | if begin == jsDialogBeginBusy { | |||
| 240 | // Another dialog is open: stay on the page rather than stacking. | 275 | // Another dialog is open: stay on the page rather than stacking. | ||
| 241 | // The Cont below makes CEF send a reset that must not cancel it. | 276 | // The Cont below makes CEF send a reset that must not cancel it. | ||
| 242 | h.wv.jsDialogs.armIgnoreReset() | 277 | h.wv.jsDialogs.armIgnoreReset() | ||
M internal/infrastructure/cef/jsdialog_handler_test.go +53 -4
| @@ -246,8 +246,8 @@ func TestJSDialogCancelledBeforeUIRunsIsNotShown(t *testing.T) { | |||
| 246 | 246 | ui := &jsDialogUIRecorder{handled: true} | |
| 247 | 247 | wv := newJSDialogWebView(ui) | |
| 248 | 248 | cb := &stubJSDialogCallback{} | |
| 249 | - | call, ok := wv.jsDialogs.begin(cb) | |
| 250 | - | require.True(t, ok) | |
| 249 | + | call, begin := wv.jsDialogs.begin(cb) | |
| 250 | + | require.Equal(t, jsDialogBeginOK, begin) | |
| 251 | 251 | wv.jsDialogs.resolve(call, false, "") | |
| 252 | 252 | ||
| 253 | 253 | wv.presentJSDialog(call, port.JSDialogRequest{}, wv.callbacks) | |
| @@ -303,13 +303,62 @@ func TestCEFCloseEchoDoesNotCancelNewerDialog(t *testing.T) { | |||
| 303 | 303 | require.Equal(t, []jsDialogAnswer{{ok: true}}, first.calls) | |
| 304 | 304 | require.Zero(t, ui.resets, "echo must not hide the UI") | |
| 305 | 305 | ||
| 306 | - | // A newer dialog is unaffected by a trailing echo that arrives late. | |
| 306 | + | // The echo consumed the flag, so a genuine reset on a newer dialog cancels it. | |
| 307 | 307 | second := &stubJSDialogCallback{} | |
| 308 | 308 | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "2", "", second, new(int32)) | |
| 309 | - | h.OnResetDialogState(nil) // a genuine reset | |
| 309 | + | h.OnResetDialogState(nil) | |
| 310 | 310 | require.Equal(t, []jsDialogAnswer{{ok: false}}, second.calls) | |
| 311 | 311 | } | |
| 312 | 312 | ||
| 313 | + | func TestDeferredResetEchoIsConsumed(t *testing.T) { | |
| 314 | + | useDirectJSDialogContinue(t) | |
| 315 | + | ui := &jsDialogUIRecorder{handled: true} | |
| 316 | + | h := &handlerSet{wv: newJSDialogWebView(ui)} | |
| 317 | + | first := &stubJSDialogCallback{} // no synchronous echo | |
| 318 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "1", "", first, new(int32)) | |
| 319 | + | ui.respond[0](true, "") | |
| 320 | + | ||
| 321 | + | // The echo arrives later, while no dialog is open: consumed, UI untouched. | |
| 322 | + | h.OnDialogClosed(nil) | |
| 323 | + | h.OnResetDialogState(nil) | |
| 324 | + | require.Zero(t, ui.resets) | |
| 325 | + | require.Len(t, first.calls, 1) | |
| 326 | + | require.False(t, h.wv.jsDialogs.consumeIgnoreReset(), "flag must have been consumed") | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | func TestMainFrameLoadStartCancelsPendingDialog(t *testing.T) { | |
| 330 | + | useDirectJSDialogContinue(t) | |
| 331 | + | ui := &jsDialogUIRecorder{handled: true} | |
| 332 | + | wv := newJSDialogWebView(ui) | |
| 333 | + | h := &handlerSet{wv: wv} | |
| 334 | + | first := &stubJSDialogCallback{} | |
| 335 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "1", "", first, new(int32)) | |
| 336 | + | // A suppressed spammer makes CEF drop its handler_, so no reset will follow. | |
| 337 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "2", "", &stubJSDialogCallback{}, new(int32)) | |
| 338 | + | ||
| 339 | + | h.OnLoadStart(nil, stubFrame{main: false, url: "https://sub.example.com"}, 0) | |
| 340 | + | require.Empty(t, first.calls, "subframe loads must not cancel") | |
| 341 | + | ||
| 342 | + | h.OnLoadStart(nil, stubFrame{main: true, url: "https://example.com/next"}, 0) | |
| 343 | + | require.Equal(t, []jsDialogAnswer{{ok: false}}, first.calls) | |
| 344 | + | require.Equal(t, 1, ui.resets) | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | func TestDestroyedStateRejectsNewDialog(t *testing.T) { | |
| 348 | + | useDirectJSDialogContinue(t) | |
| 349 | + | ui := &jsDialogUIRecorder{handled: true} | |
| 350 | + | wv := newJSDialogWebView(ui) | |
| 351 | + | h := &handlerSet{wv: wv} | |
| 352 | + | wv.cancelJSDialogsClosing(true) // Destroy raced ahead of the destroyed check | |
| 353 | + | cb := &stubJSDialogCallback{} | |
| 354 | + | var suppress int32 | |
| 355 | + | ||
| 356 | + | require.Zero(t, h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "m", "", cb, &suppress)) | |
| 357 | + | require.False(t, h.OnBeforeUnloadDialog(nil, "", 0, cb)) | |
| 358 | + | require.Zero(t, suppress) | |
| 359 | + | require.Empty(t, ui.reqs) | |
| 360 | + | } | |
| 361 | + | ||
| 313 | 362 | func TestNoStaleIgnoreAcrossDialogs(t *testing.T) { | |
| 314 | 363 | useDirectJSDialogContinue(t) | |
| 315 | 364 | ui := &jsDialogUIRecorder{handled: true} | |
| @@ -246,8 +246,8 @@ func TestJSDialogCancelledBeforeUIRunsIsNotShown(t *testing.T) { | |||||
| 246 | ui := &jsDialogUIRecorder{handled: true} | 246 | ui := &jsDialogUIRecorder{handled: true} | ||
| 247 | wv := newJSDialogWebView(ui) | 247 | wv := newJSDialogWebView(ui) | ||
| 248 | cb := &stubJSDialogCallback{} | 248 | cb := &stubJSDialogCallback{} | ||
| 249 | - | call, ok := wv.jsDialogs.begin(cb) | 249 | + | call, begin := wv.jsDialogs.begin(cb) |
| 250 | - | require.True(t, ok) | 250 | + | require.Equal(t, jsDialogBeginOK, begin) |
| 251 | wv.jsDialogs.resolve(call, false, "") | 251 | wv.jsDialogs.resolve(call, false, "") | ||
| 252 | 252 | ||||
| 253 | wv.presentJSDialog(call, port.JSDialogRequest{}, wv.callbacks) | 253 | wv.presentJSDialog(call, port.JSDialogRequest{}, wv.callbacks) | ||
| @@ -303,13 +303,62 @@ func TestCEFCloseEchoDoesNotCancelNewerDialog(t *testing.T) { | |||||
| 303 | require.Equal(t, []jsDialogAnswer{{ok: true}}, first.calls) | 303 | require.Equal(t, []jsDialogAnswer{{ok: true}}, first.calls) | ||
| 304 | require.Zero(t, ui.resets, "echo must not hide the UI") | 304 | require.Zero(t, ui.resets, "echo must not hide the UI") | ||
| 305 | 305 | ||||
| 306 | - | // A newer dialog is unaffected by a trailing echo that arrives late. | 306 | + | // The echo consumed the flag, so a genuine reset on a newer dialog cancels it. |
| 307 | second := &stubJSDialogCallback{} | 307 | second := &stubJSDialogCallback{} | ||
| 308 | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "2", "", second, new(int32)) | 308 | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "2", "", second, new(int32)) | ||
| 309 | - | h.OnResetDialogState(nil) // a genuine reset | 309 | + | h.OnResetDialogState(nil) |
| 310 | require.Equal(t, []jsDialogAnswer{{ok: false}}, second.calls) | 310 | require.Equal(t, []jsDialogAnswer{{ok: false}}, second.calls) | ||
| 311 | } | 311 | } | ||
| 312 | 312 | ||||
| 313 | + | func TestDeferredResetEchoIsConsumed(t *testing.T) { | |||
| 314 | + | useDirectJSDialogContinue(t) | |||
| 315 | + | ui := &jsDialogUIRecorder{handled: true} | |||
| 316 | + | h := &handlerSet{wv: newJSDialogWebView(ui)} | |||
| 317 | + | first := &stubJSDialogCallback{} // no synchronous echo | |||
| 318 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "1", "", first, new(int32)) | |||
| 319 | + | ui.respond[0](true, "") | |||
| 320 | + | ||||
| 321 | + | // The echo arrives later, while no dialog is open: consumed, UI untouched. | |||
| 322 | + | h.OnDialogClosed(nil) | |||
| 323 | + | h.OnResetDialogState(nil) | |||
| 324 | + | require.Zero(t, ui.resets) | |||
| 325 | + | require.Len(t, first.calls, 1) | |||
| 326 | + | require.False(t, h.wv.jsDialogs.consumeIgnoreReset(), "flag must have been consumed") | |||
| 327 | + | } | |||
| 328 | + | ||||
| 329 | + | func TestMainFrameLoadStartCancelsPendingDialog(t *testing.T) { | |||
| 330 | + | useDirectJSDialogContinue(t) | |||
| 331 | + | ui := &jsDialogUIRecorder{handled: true} | |||
| 332 | + | wv := newJSDialogWebView(ui) | |||
| 333 | + | h := &handlerSet{wv: wv} | |||
| 334 | + | first := &stubJSDialogCallback{} | |||
| 335 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "1", "", first, new(int32)) | |||
| 336 | + | // A suppressed spammer makes CEF drop its handler_, so no reset will follow. | |||
| 337 | + | h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "2", "", &stubJSDialogCallback{}, new(int32)) | |||
| 338 | + | ||||
| 339 | + | h.OnLoadStart(nil, stubFrame{main: false, url: "https://sub.example.com"}, 0) | |||
| 340 | + | require.Empty(t, first.calls, "subframe loads must not cancel") | |||
| 341 | + | ||||
| 342 | + | h.OnLoadStart(nil, stubFrame{main: true, url: "https://example.com/next"}, 0) | |||
| 343 | + | require.Equal(t, []jsDialogAnswer{{ok: false}}, first.calls) | |||
| 344 | + | require.Equal(t, 1, ui.resets) | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | func TestDestroyedStateRejectsNewDialog(t *testing.T) { | |||
| 348 | + | useDirectJSDialogContinue(t) | |||
| 349 | + | ui := &jsDialogUIRecorder{handled: true} | |||
| 350 | + | wv := newJSDialogWebView(ui) | |||
| 351 | + | h := &handlerSet{wv: wv} | |||
| 352 | + | wv.cancelJSDialogsClosing(true) // Destroy raced ahead of the destroyed check | |||
| 353 | + | cb := &stubJSDialogCallback{} | |||
| 354 | + | var suppress int32 | |||
| 355 | + | ||||
| 356 | + | require.Zero(t, h.OnJsdialog(nil, "", purecef.JsdialogTypeJsdialogtypeAlert, "m", "", cb, &suppress)) | |||
| 357 | + | require.False(t, h.OnBeforeUnloadDialog(nil, "", 0, cb)) | |||
| 358 | + | require.Zero(t, suppress) | |||
| 359 | + | require.Empty(t, ui.reqs) | |||
| 360 | + | } | |||
| 361 | + | ||||
| 313 | func TestNoStaleIgnoreAcrossDialogs(t *testing.T) { | 362 | func TestNoStaleIgnoreAcrossDialogs(t *testing.T) { | ||
| 314 | useDirectJSDialogContinue(t) | 363 | useDirectJSDialogContinue(t) | ||
| 315 | ui := &jsDialogUIRecorder{handled: true} | 364 | ui := &jsDialogUIRecorder{handled: true} | ||
M internal/infrastructure/cef/webview.go +1 -1
| @@ -1333,7 +1333,7 @@ func (wv *WebView) Destroy() { | |||
| 1333 | 1333 | // calling thread; GTK-only cleanup follows through the owning | |
| 1334 | 1334 | // dispatcher without waiting for the deferred native browser close. | |
| 1335 | 1335 | wv.invalidateScrollMotion() | |
| 1336 | - | wv.cancelJSDialogs() | |
| 1336 | + | wv.cancelJSDialogsClosing(true) | |
| 1337 | 1337 | wv.shutdownAccessibilityCapture() | |
| 1338 | 1338 | wv.resetPageScrollQueue() | |
| 1339 | 1339 | wv.syntheticPopupMu.Lock() | |
| @@ -1333,7 +1333,7 @@ func (wv *WebView) Destroy() { | |||||
| 1333 | // calling thread; GTK-only cleanup follows through the owning | 1333 | // calling thread; GTK-only cleanup follows through the owning | ||
| 1334 | // dispatcher without waiting for the deferred native browser close. | 1334 | // dispatcher without waiting for the deferred native browser close. | ||
| 1335 | wv.invalidateScrollMotion() | 1335 | wv.invalidateScrollMotion() | ||
| 1336 | - | wv.cancelJSDialogs() | 1336 | + | wv.cancelJSDialogsClosing(true) |
| 1337 | wv.shutdownAccessibilityCapture() | 1337 | wv.shutdownAccessibilityCapture() | ||
| 1338 | wv.resetPageScrollQueue() | 1338 | wv.resetPageScrollQueue() | ||
| 1339 | wv.syntheticPopupMu.Lock() | 1339 | wv.syntheticPopupMu.Lock() | ||
M internal/infrastructure/cef/webview_handlers.go +2 -0
| @@ -531,6 +531,8 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||
| 531 | 531 | // Post-commit fallback for navigation paths that bypass OnBeforeBrowse | |
| 532 | 532 | // (same-page, error, and helper-driven commits). Main frame only. | |
| 533 | 533 | if h.wv != nil { | |
| 534 | + | // A new document replaces the page that asked for any open JS dialog. | |
| 535 | + | h.wv.cancelJSDialogs() | |
| 534 | 536 | h.wv.invalidateScrollMotion() | |
| 535 | 537 | h.wv.mu.Lock() | |
| 536 | 538 | h.wv.documentSeq++ | |
| @@ -531,6 +531,8 @@ func (h *handlerSet) OnLoadStart(_ purecef.Browser, frame purecef.Frame, _ purec | |||||
| 531 | // Post-commit fallback for navigation paths that bypass OnBeforeBrowse | 531 | // Post-commit fallback for navigation paths that bypass OnBeforeBrowse | ||
| 532 | // (same-page, error, and helper-driven commits). Main frame only. | 532 | // (same-page, error, and helper-driven commits). Main frame only. | ||
| 533 | if h.wv != nil { | 533 | if h.wv != nil { | ||
| 534 | + | // A new document replaces the page that asked for any open JS dialog. | |||
| 535 | + | h.wv.cancelJSDialogs() | |||
| 534 | h.wv.invalidateScrollMotion() | 536 | h.wv.invalidateScrollMotion() | ||
| 535 | h.wv.mu.Lock() | 537 | h.wv.mu.Lock() | ||
| 536 | h.wv.documentSeq++ | 538 | h.wv.documentSeq++ | ||