Skip to content

Commit 59fc766

Browse files
authored
[web] Fix potential race condition in ClickDebouncer (flutter#173294)
Based on Gemini's comment: flutter#173072 (comment)
1 parent efa5893 commit 59fc766

File tree

2 files changed

+44
-0
lines changed

2 files changed

+44
-0
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,12 @@ class ClickDebouncer {
350350
///
351351
/// This method is called asynchronously from [_maybeStartDebouncing].
352352
void _doStartDebouncing(DomEvent event, List<ui.PointerData> data) {
353+
// It's possible that debouncing was canceled between the pointerdown event and the execution
354+
// of this method.
355+
if (!isDebouncing) {
356+
return;
357+
}
358+
353359
_state = (
354360
target: event.target!,
355361
// The 200ms duration was chosen empirically by testing tapping, mouse

engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,6 +2651,44 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
26512651
expect(semanticsActions, isEmpty);
26522652
});
26532653

2654+
testWithSemantics('Does not start debouncing if reset before scheduled execution', () async {
2655+
expect(EnginePlatformDispatcher.instance.semanticsEnabled, isTrue);
2656+
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
2657+
expect(PointerBinding.clickDebouncer.debugState, isNull);
2658+
2659+
final DomElement testElement = createDomElement('flt-semantics');
2660+
testElement.setAttribute('flt-tappable', '');
2661+
view.dom.semanticsHost.appendChild(testElement);
2662+
2663+
// 1. Trigger _maybeStartDebouncing, which sets _isDebouncing = true and schedules _doStartDebouncing.
2664+
testElement.dispatchEvent(context.primaryDown());
2665+
2666+
// At this point, _isDebouncing is true, but _doStartDebouncing (which sets _state and creates the Timer)
2667+
// has not yet executed because it was scheduled with Timer.run().
2668+
expect(PointerBinding.clickDebouncer.isDebouncing, isTrue);
2669+
expect(PointerBinding.clickDebouncer.debugState, isNull); // _state is still null
2670+
2671+
// 2. Simulate a scenario where reset() is called before _doStartDebouncing gets a chance to run.
2672+
// This could happen due to a hot restart or other lifecycle events.
2673+
PointerBinding.clickDebouncer.reset();
2674+
2675+
// After reset(), _isDebouncing should be false and _state should still be null.
2676+
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
2677+
expect(PointerBinding.clickDebouncer.debugState, isNull);
2678+
2679+
// 3. Allow the scheduled _doStartDebouncing to run. With the fix, it should now check
2680+
// `!isDebouncing` and return early.
2681+
await nextEventLoop();
2682+
2683+
// Verify that _doStartDebouncing did not proceed to set _state or create a Timer.
2684+
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
2685+
expect(PointerBinding.clickDebouncer.debugState, isNull);
2686+
2687+
// Ensure no events were sent to the framework as debouncing was effectively cancelled.
2688+
expect(pointerPackets, isEmpty);
2689+
expect(semanticsActions, isEmpty);
2690+
});
2691+
26542692
testWithSemantics('Starts debouncing after event loop', () async {
26552693
expect(EnginePlatformDispatcher.instance.semanticsEnabled, isTrue);
26562694
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);

0 commit comments

Comments
 (0)