[web] Fix potential race condition in ClickDebouncer (#173294)

Based on Gemini's comment:
https://github.com/flutter/flutter/pull/173072#discussion_r2246216031
This commit is contained in:
Mouad Debbar 2025-08-05 17:31:15 -04:00 committed by GitHub
parent efa5893347
commit 59fc766c6f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 44 additions and 0 deletions

View File

@ -350,6 +350,12 @@ class ClickDebouncer {
///
/// This method is called asynchronously from [_maybeStartDebouncing].
void _doStartDebouncing(DomEvent event, List<ui.PointerData> data) {
// It's possible that debouncing was canceled between the pointerdown event and the execution
// of this method.
if (!isDebouncing) {
return;
}
_state = (
target: event.target!,
// The 200ms duration was chosen empirically by testing tapping, mouse

View File

@ -2651,6 +2651,44 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) {
expect(semanticsActions, isEmpty);
});
testWithSemantics('Does not start debouncing if reset before scheduled execution', () async {
expect(EnginePlatformDispatcher.instance.semanticsEnabled, isTrue);
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
expect(PointerBinding.clickDebouncer.debugState, isNull);
final DomElement testElement = createDomElement('flt-semantics');
testElement.setAttribute('flt-tappable', '');
view.dom.semanticsHost.appendChild(testElement);
// 1. Trigger _maybeStartDebouncing, which sets _isDebouncing = true and schedules _doStartDebouncing.
testElement.dispatchEvent(context.primaryDown());
// At this point, _isDebouncing is true, but _doStartDebouncing (which sets _state and creates the Timer)
// has not yet executed because it was scheduled with Timer.run().
expect(PointerBinding.clickDebouncer.isDebouncing, isTrue);
expect(PointerBinding.clickDebouncer.debugState, isNull); // _state is still null
// 2. Simulate a scenario where reset() is called before _doStartDebouncing gets a chance to run.
// This could happen due to a hot restart or other lifecycle events.
PointerBinding.clickDebouncer.reset();
// After reset(), _isDebouncing should be false and _state should still be null.
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
expect(PointerBinding.clickDebouncer.debugState, isNull);
// 3. Allow the scheduled _doStartDebouncing to run. With the fix, it should now check
// `!isDebouncing` and return early.
await nextEventLoop();
// Verify that _doStartDebouncing did not proceed to set _state or create a Timer.
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);
expect(PointerBinding.clickDebouncer.debugState, isNull);
// Ensure no events were sent to the framework as debouncing was effectively cancelled.
expect(pointerPackets, isEmpty);
expect(semanticsActions, isEmpty);
});
testWithSemantics('Starts debouncing after event loop', () async {
expect(EnginePlatformDispatcher.instance.semanticsEnabled, isTrue);
expect(PointerBinding.clickDebouncer.isDebouncing, isFalse);