fix(cef): prevent queued taps from stalling
- commit
- 4733629ab8cf5d405066b01e9ff25ead31ec6ed0
- author
- brice · · 2026-08-16 15:43 UTC
- parent
- 55df320
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +44 -8
M internal/infrastructure/cef/webview.go +13 -8
| @@ -2510,12 +2510,18 @@ func (wv *WebView) ensurePageScrollFlush() { | |||
| 2510 | 2510 | } | |
| 2511 | 2511 | ||
| 2512 | 2512 | func (wv *WebView) drainPageScrollSynchronously() { | |
| 2513 | - | for wv.drainPageScrollOnce() { | |
| 2514 | - | } | |
| 2515 | 2513 | q := &wv.pageScrollQueue | |
| 2516 | - | q.mu.Lock() | |
| 2517 | - | q.flushPending = false | |
| 2518 | - | q.mu.Unlock() | |
| 2514 | + | for { | |
| 2515 | + | wv.drainPageScrollOnce() | |
| 2516 | + | q.mu.Lock() | |
| 2517 | + | hasWork := q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 | |
| 2518 | + | if !hasWork { | |
| 2519 | + | q.flushPending = false | |
| 2520 | + | q.mu.Unlock() | |
| 2521 | + | return | |
| 2522 | + | } | |
| 2523 | + | q.mu.Unlock() | |
| 2524 | + | } | |
| 2519 | 2525 | } | |
| 2520 | 2526 | ||
| 2521 | 2527 | func (wv *WebView) flushPageScroll() { | |
| @@ -2531,7 +2537,7 @@ func (wv *WebView) flushPageScroll() { | |||
| 2531 | 2537 | } | |
| 2532 | 2538 | } | |
| 2533 | 2539 | ||
| 2534 | - | func (wv *WebView) drainPageScrollOnce() bool { | |
| 2540 | + | func (wv *WebView) drainPageScrollOnce() { | |
| 2535 | 2541 | q := &wv.pageScrollQueue | |
| 2536 | 2542 | q.mu.Lock() | |
| 2537 | 2543 | tapDX, tapDY := q.tapDX, q.tapDY | |
| @@ -2540,7 +2546,7 @@ func (wv *WebView) drainPageScrollOnce() bool { | |||
| 2540 | 2546 | beforeExecute := q.beforeExecute | |
| 2541 | 2547 | if tapDX == 0 && tapDY == 0 && heldDX == 0 && heldDY == 0 { | |
| 2542 | 2548 | q.mu.Unlock() | |
| 2543 | - | return false | |
| 2549 | + | return | |
| 2544 | 2550 | } | |
| 2545 | 2551 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |
| 2546 | 2552 | q.mu.Unlock() | |
| @@ -2563,7 +2569,6 @@ func (wv *WebView) drainPageScrollOnce() bool { | |||
| 2563 | 2569 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | |
| 2564 | 2570 | } | |
| 2565 | 2571 | q.commitMu.Unlock() | |
| 2566 | - | return true | |
| 2567 | 2572 | } | |
| 2568 | 2573 | ||
| 2569 | 2574 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | |
| @@ -2510,12 +2510,18 @@ func (wv *WebView) ensurePageScrollFlush() { | |||||
| 2510 | } | 2510 | } | ||
| 2511 | 2511 | ||||
| 2512 | func (wv *WebView) drainPageScrollSynchronously() { | 2512 | func (wv *WebView) drainPageScrollSynchronously() { | ||
| 2513 | - | for wv.drainPageScrollOnce() { | |||
| 2514 | - | } | |||
| 2515 | q := &wv.pageScrollQueue | 2513 | q := &wv.pageScrollQueue | ||
| 2516 | - | q.mu.Lock() | 2514 | + | for { |
| 2517 | - | q.flushPending = false | 2515 | + | wv.drainPageScrollOnce() |
| 2518 | - | q.mu.Unlock() | 2516 | + | q.mu.Lock() |
| 2517 | + | hasWork := q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 | |||
| 2518 | + | if !hasWork { | |||
| 2519 | + | q.flushPending = false | |||
| 2520 | + | q.mu.Unlock() | |||
| 2521 | + | return | |||
| 2522 | + | } | |||
| 2523 | + | q.mu.Unlock() | |||
| 2524 | + | } | |||
| 2519 | } | 2525 | } | ||
| 2520 | 2526 | ||||
| 2521 | func (wv *WebView) flushPageScroll() { | 2527 | func (wv *WebView) flushPageScroll() { | ||
| @@ -2531,7 +2537,7 @@ func (wv *WebView) flushPageScroll() { | |||||
| 2531 | } | 2537 | } | ||
| 2532 | } | 2538 | } | ||
| 2533 | 2539 | ||||
| 2534 | - | func (wv *WebView) drainPageScrollOnce() bool { | 2540 | + | func (wv *WebView) drainPageScrollOnce() { |
| 2535 | q := &wv.pageScrollQueue | 2541 | q := &wv.pageScrollQueue | ||
| 2536 | q.mu.Lock() | 2542 | q.mu.Lock() | ||
| 2537 | tapDX, tapDY := q.tapDX, q.tapDY | 2543 | tapDX, tapDY := q.tapDX, q.tapDY | ||
| @@ -2540,7 +2546,7 @@ func (wv *WebView) drainPageScrollOnce() bool { | |||||
| 2540 | beforeExecute := q.beforeExecute | 2546 | beforeExecute := q.beforeExecute | ||
| 2541 | if tapDX == 0 && tapDY == 0 && heldDX == 0 && heldDY == 0 { | 2547 | if tapDX == 0 && tapDY == 0 && heldDX == 0 && heldDY == 0 { | ||
| 2542 | q.mu.Unlock() | 2548 | q.mu.Unlock() | ||
| 2543 | - | return false | 2549 | + | return |
| 2544 | } | 2550 | } | ||
| 2545 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | 2551 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | ||
| 2546 | q.mu.Unlock() | 2552 | q.mu.Unlock() | ||
| @@ -2563,7 +2569,6 @@ func (wv *WebView) drainPageScrollOnce() bool { | |||||
| 2563 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | 2569 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | ||
| 2564 | } | 2570 | } | ||
| 2565 | q.commitMu.Unlock() | 2571 | q.commitMu.Unlock() | ||
| 2566 | - | return true | |||
| 2567 | } | 2572 | } | ||
| 2568 | 2573 | ||||
| 2569 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | 2574 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | ||
M internal/infrastructure/cef/webview_scroll_test.go +31 -0
| @@ -326,6 +326,37 @@ func TestCancelPageScroll_DropsHeldDeltaButPreservesTap(t *testing.T) { | |||
| 326 | 326 | } | |
| 327 | 327 | } | |
| 328 | 328 | ||
| 329 | + | func TestDrainPageScrollSynchronously_DrainsTapEnqueuedDuringExecution(t *testing.T) { | |
| 330 | + | browser := cefmocks.NewMockBrowser(t) | |
| 331 | + | frame := cefmocks.NewMockFrame(t) | |
| 332 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |
| 333 | + | ||
| 334 | + | wv := &WebView{browser: browser} | |
| 335 | + | calls := 0 | |
| 336 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(_, _ string, _ int32) { | |
| 337 | + | calls++ | |
| 338 | + | if calls == 1 { | |
| 339 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |
| 340 | + | t.Errorf("enqueue tap during execution: %v", err) | |
| 341 | + | } | |
| 342 | + | } | |
| 343 | + | }).Twice() | |
| 344 | + | ||
| 345 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |
| 346 | + | t.Fatal(err) | |
| 347 | + | } | |
| 348 | + | if calls != 2 { | |
| 349 | + | t.Fatalf("ExecuteJavaScript calls=%d, want 2", calls) | |
| 350 | + | } | |
| 351 | + | ||
| 352 | + | q := &wv.pageScrollQueue | |
| 353 | + | q.mu.Lock() | |
| 354 | + | defer q.mu.Unlock() | |
| 355 | + | if q.flushPending || q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 { | |
| 356 | + | t.Fatalf("queue not drained: pending=%t tap=(%d,%d) held=(%d,%d)", q.flushPending, q.tapDX, q.tapDY, q.heldDX, q.heldDY) | |
| 357 | + | } | |
| 358 | + | } | |
| 359 | + | ||
| 329 | 360 | func TestCancelPageScroll_AfterDrainDropsOldHeldAndPreservesTapAndNextGesture(t *testing.T) { | |
| 330 | 361 | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 331 | 362 | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| @@ -326,6 +326,37 @@ func TestCancelPageScroll_DropsHeldDeltaButPreservesTap(t *testing.T) { | |||||
| 326 | } | 326 | } | ||
| 327 | } | 327 | } | ||
| 328 | 328 | ||||
| 329 | + | func TestDrainPageScrollSynchronously_DrainsTapEnqueuedDuringExecution(t *testing.T) { | |||
| 330 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 331 | + | frame := cefmocks.NewMockFrame(t) | |||
| 332 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |||
| 333 | + | ||||
| 334 | + | wv := &WebView{browser: browser} | |||
| 335 | + | calls := 0 | |||
| 336 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(_, _ string, _ int32) { | |||
| 337 | + | calls++ | |||
| 338 | + | if calls == 1 { | |||
| 339 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |||
| 340 | + | t.Errorf("enqueue tap during execution: %v", err) | |||
| 341 | + | } | |||
| 342 | + | } | |||
| 343 | + | }).Twice() | |||
| 344 | + | ||||
| 345 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |||
| 346 | + | t.Fatal(err) | |||
| 347 | + | } | |||
| 348 | + | if calls != 2 { | |||
| 349 | + | t.Fatalf("ExecuteJavaScript calls=%d, want 2", calls) | |||
| 350 | + | } | |||
| 351 | + | ||||
| 352 | + | q := &wv.pageScrollQueue | |||
| 353 | + | q.mu.Lock() | |||
| 354 | + | defer q.mu.Unlock() | |||
| 355 | + | if q.flushPending || q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 { | |||
| 356 | + | t.Fatalf("queue not drained: pending=%t tap=(%d,%d) held=(%d,%d)", q.flushPending, q.tapDX, q.tapDY, q.heldDX, q.heldDY) | |||
| 357 | + | } | |||
| 358 | + | } | |||
| 359 | + | ||||
| 329 | func TestCancelPageScroll_AfterDrainDropsOldHeldAndPreservesTapAndNextGesture(t *testing.T) { | 360 | func TestCancelPageScroll_AfterDrainDropsOldHeldAndPreservesTapAndNextGesture(t *testing.T) { | ||
| 330 | oldNewTask, oldPostTask := cefNewTask, cefPostTask | 361 | oldNewTask, oldPostTask := cefNewTask, cefPostTask | ||
| 331 | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | 362 | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | ||