Fix stale _entries and pointer tracking in OneSequenceGestureRecognizer.rejectGesture#187515
Fix stale _entries and pointer tracking in OneSequenceGestureRecognizer.rejectGesture#187515ishaq2321 wants to merge 1 commit into
Conversation
…er.rejectGesture OneSequenceGestureRecognizer.rejectGesture was a no-op, leaving stale _entries and tracked pointers when a recognizer lost the gesture arena. This caused issues particularly in GestureArenaTeam scenarios where losing team members retained per-pointer state indefinitely. Fix the base class cleanup and propagate it through all subclass overrides: - OneSequenceGestureRecognizer.rejectGesture: call stopTrackingPointer and remove the stale _entries entry. Marked @mustCallSuper. - PrimaryPointerGestureRecognizer.rejectGesture: add super.rejectGesture call and @mustCallSuper. - DragGestureRecognizer.rejectGesture: add super.rejectGesture call (before _giveUpPointer to avoid redundant work). - ForcePressGestureRecognizer.rejectGesture: add super.rejectGesture call and @mustCallSuper. - ScaleGestureRecognizer.rejectGesture: add super.rejectGesture call and @mustCallSuper. - PassiveGestureRecognizer (test helper): remove unnecessary override since the base class now handles cleanup. Also fix PrimaryPointerGestureRecognizer state transition: after rejection, didStopTrackingLastPointer now properly transitions to ready state and clears initialPosition, since the base class cleanup calls through the stopTrackingPointer -> didStopTrackingLastPointer chain. Co-authored-by: Muhammad Ishaq Khan <muhammadishaqkhan.2321@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request refactors OneSequenceGestureRecognizer and its subclasses to mark rejectGesture with @mustCallSuper and ensure it performs pointer cleanup. Feedback points out that BaseTapAndDragGestureRecognizer.rejectGesture fails to call super.rejectGesture, which violates the @mustCallSuper contract. Additionally, several subclasses contain redundant pointer cleanup calls that can be removed because they are now handled by the superclass implementation.
| @override | ||
| @mustCallSuper | ||
| void rejectGesture(int pointer) { | ||
| if (pointer != _primaryPointer) { | ||
| return; |
There was a problem hiding this comment.
The BaseTapAndDragGestureRecognizer.rejectGesture method is marked @mustCallSuper but does not actually call super.rejectGesture(pointer). This will trigger a Dart analyzer warning/error and prevent the necessary cleanup of tracked pointers and _entries for this recognizer.
To fix this, ensure super.rejectGesture(pointer) is called for all execution paths:
- Call
super.rejectGesture(pointer)before the early return whenpointer != _primaryPointer. - Call
super.rejectGesture(pointer)at the end of the method (or before any other return points) for the primary pointer case.
@override
@mustCallSuper
void rejectGesture(int pointer) {
if (pointer != _primaryPointer) {
super.rejectGesture(pointer);
return;
}| @override | ||
| @mustCallSuper | ||
| void rejectGesture(int pointer) { | ||
| _pointerPanZooms.remove(pointer); | ||
| _pointerLocations.remove(pointer); | ||
| _pointerQueue.remove(pointer); | ||
| stopTrackingPointer(pointer); | ||
| super.rejectGesture(pointer); | ||
| } |
There was a problem hiding this comment.
The call to stopTrackingPointer(pointer) is redundant here because super.rejectGesture(pointer) already calls stopTrackingPointer(pointer) internally. We can simplify this method by removing the redundant call.
@override
@mustCallSuper
void rejectGesture(int pointer) {
_pointerPanZooms.remove(pointer);
_pointerLocations.remove(pointer);
_pointerQueue.remove(pointer);
super.rejectGesture(pointer);
}| @override | ||
| @mustCallSuper | ||
| void rejectGesture(int pointer) { | ||
| stopTrackingPointer(pointer); | ||
| didStopTrackingLastPointer(pointer); | ||
| super.rejectGesture(pointer); | ||
| } |
There was a problem hiding this comment.
The manual calls to stopTrackingPointer(pointer) and didStopTrackingLastPointer(pointer) are redundant because super.rejectGesture(pointer) already calls stopTrackingPointer(pointer) (which internally triggers didStopTrackingLastPointer when the last pointer is stopped).
You can simplify this to just call super.rejectGesture(pointer). Even better, since this makes the entire override redundant, you can completely remove the rejectGesture method from ForcePressGestureRecognizer.
@override
@mustCallSuper
void rejectGesture(int pointer) {
super.rejectGesture(pointer);
}
Summary
OneSequenceGestureRecognizer.rejectGesturewas a no-op ({}), leaving stale_entriesand tracked pointers when a recognizer lost the gesture arena. This caused pointer route leaks and particularly affectedGestureArenaTeamscenarios where losing team members retained per-pointer state indefinitely.Fixes #187475
Related #117356
Changes
OneSequenceGestureRecognizer.rejectGesture: now callsstopTrackingPointer+ removes_entries, marked@mustCallSuperPrimaryPointerGestureRecognizer.rejectGesture: addedsuper.rejectGesture(pointer)+@mustCallSuperDragGestureRecognizer.rejectGesture: addedsuper.rejectGesture(pointer)+@mustCallSuperForcePressGestureRecognizer.rejectGesture: addedsuper.rejectGesture(pointer)+@mustCallSuperScaleGestureRecognizer.rejectGesture: addedsuper.rejectGesture(pointer)+@mustCallSuper_TapStatusTrackerMixin.rejectGesture: addedsuper.rejectGesture(pointer)to ensure chain reaches baseBaseTapAndDragGestureRecognizer.rejectGesture: added@mustCallSuperPassiveGestureRecognizer(test helper): removed unnecessary overridePrimaryPointerGestureRecognizer: state now transitions toreadyviadidStopTrackingLastPointerafter rejectionlosing team member cleans up per-pointer stateinteam_test.dartPrimaryPointerGestureRecognizerstate expectations inrecognizer_test.dartTests
All 344 gesture tests pass.