Gesture recognizer cleanup (#81884)

* Gesture recognizer cleanup

1) Make OneSequenceGestureRecognizer.addAllowedPointer()
   call startTrackingPointer(), and change subclasses to
   call super.addAllowedPointer() in place of manually
   calling startTrackingPointer().
2) Fix addAllowedPointer overrides to take PointerDownEvent
   where some were taking PointerEvent.
3) Add API documentation to OneSequenceGestureRecognizer
4) Make the following fields in OneSequenceGestureRecognizer
   private with public getters instead of publicly writable:
   `state`, `primaryPointer`, and `initialPosition`.
5) Clean up gesture recognizer state in
   OneSequenceGestureRecognizer.didStopTrackingLastPointer.

Fixes #81883

packages/flutter/lib/src/gestures/eager.dart

@@ -27,8 +27,7 @@ class EagerGestureRecognizer extends OneSequenceGestureRecognizer {
 
   @override
   void addAllowedPointer(PointerDownEvent event) {
-    // We call startTrackingPointer as this is where OneSequenceGestureRecognizer joins the arena.
-    startTrackingPointer(event.pointer, event.transform);
+    super.addAllowedPointer(event);
     resolve(GestureDisposition.accepted);
     stopTrackingPointer(event.pointer);
   }

packages/flutter/lib/src/gestures/force_press.dart

@@ -216,14 +216,14 @@ class ForcePressGestureRecognizer extends OneSequenceGestureRecognizer {
   _ForceState _state = _ForceState.ready;
 
   @override
-  void addAllowedPointer(PointerEvent event) {
+  void addAllowedPointer(PointerDownEvent event) {
     // If the device has a maximum pressure of less than or equal to 1, it
     // doesn't have touch pressure sensing capabilities. Do not participate
     // in the gesture arena.
-    if (event is! PointerUpEvent && event.pressureMax <= 1.0) {
+    if (event.pressureMax <= 1.0) {
       resolve(GestureDisposition.rejected);
     } else {
-      startTrackingPointer(event.pointer, event.transform);
+      super.addAllowedPointer(event);
       if (_state == _ForceState.ready) {
         _state = _ForceState.possible;
         _lastPosition = OffsetPair.fromEventPosition(event);

packages/flutter/lib/src/gestures/monodrag.dart

@@ -262,8 +262,8 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer {
   }
 
   @override
-  void addAllowedPointer(PointerEvent event) {
-    startTrackingPointer(event.pointer, event.transform);
+  void addAllowedPointer(PointerDownEvent event) {
+    super.addAllowedPointer(event);
     _velocityTrackers[event.pointer] = velocityTrackerBuilder(event);
     if (_state == _DragState.ready) {
       _state = _DragState.possible;

packages/flutter/lib/src/gestures/recognizer.dart

@@ -248,11 +248,32 @@ abstract class OneSequenceGestureRecognizer extends GestureRecognizer {
   final Set<int> _trackedPointers = HashSet<int>();
 
   @override
+  @protected
+  void addAllowedPointer(PointerDownEvent event) {
+    startTrackingPointer(event.pointer, event.transform);
+  }
+
+  @override
+  @protected
   void handleNonAllowedPointer(PointerDownEvent event) {
     resolve(GestureDisposition.rejected);
   }
 
   /// Called when a pointer event is routed to this recognizer.
+  ///
+  /// This will be called for every pointer event while the pointer is being
+  /// tracked. Typically, this recognizer will start tracking the pointer in
+  /// [addAllowedPointer], which means that [handleEvent] will be called
+  /// starting with the [PointerDownEvent] that was passed to [addAllowedPointer].
+  ///
+  /// See also:
+  ///
+  ///  * [startTrackingPointer], which causes pointer events to be routed to
+  ///    this recognizer.
+  ///  * [stopTrackingPointer], which stops events from being routed to this
+  ///    recognizer.
+  ///  * [stopTrackingIfPointerNoLongerDown], which conditionally stops events
+  ///    from being routed to this recognizer.
   @protected
   void handleEvent(PointerEvent event);
 
@@ -338,6 +359,11 @@ abstract class OneSequenceGestureRecognizer extends GestureRecognizer {
   /// null if no transformation is necessary.
   ///
   /// Use [stopTrackingPointer] to remove the route added by this function.
+  ///
+  /// This method also adds this recognizer (or its [team] if it's non-null) to
+  /// the gesture arena for the specified pointer.
+  ///
+  /// This is called by [OneSequenceGestureRecognizer.addAllowedPointer].
   @protected
   void startTrackingPointer(int pointer, [Matrix4? transform]) {
     GestureBinding.instance!.pointerRouter.addRoute(pointer, handleEvent, transform);
@@ -466,13 +492,27 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
   /// The current state of the recognizer.
   ///
   /// See [GestureRecognizerState] for a description of the states.
-  GestureRecognizerState state = GestureRecognizerState.ready;
+  GestureRecognizerState get state => _state;
+  GestureRecognizerState _state = GestureRecognizerState.ready;
 
   /// The ID of the primary pointer this recognizer is tracking.
-  int? primaryPointer;
+  ///
+  /// If this recognizer is no longer tracking any pointers, this field holds
+  /// the ID of the primary pointer this recognizer was most recently tracking.
+  /// This enables the recognizer to know which pointer it was most recently
+  /// tracking when [acceptGesture] or [rejectGesture] is called (which may be
+  /// called after the recognizer is no longer tracking a pointer if, e.g.
+  /// [GestureArenaManager.hold] has been called, or if there are other
+  /// recognizers keeping the arena open).
+  int? get primaryPointer => _primaryPointer;
+  int? _primaryPointer;
 
   /// The location at which the primary pointer contacted the screen.
-  OffsetPair? initialPosition;
+  ///
+  /// This will only be non-null while this recognizer is tracking at least
+  /// one pointer.
+  OffsetPair? get initialPosition => _initialPosition;
+  OffsetPair? _initialPosition;
 
   // Whether this pointer is accepted by winning the arena or as defined by
   // a subclass calling acceptGesture.
@@ -481,11 +521,11 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
 
   @override
   void addAllowedPointer(PointerDownEvent event) {
-    startTrackingPointer(event.pointer, event.transform);
+    super.addAllowedPointer(event);
     if (state == GestureRecognizerState.ready) {
-      state = GestureRecognizerState.possible;
-      primaryPointer = event.pointer;
-      initialPosition = OffsetPair(local: event.localPosition, global: event.position);
+      _state = GestureRecognizerState.possible;
+      _primaryPointer = event.pointer;
+      _initialPosition = OffsetPair(local: event.localPosition, global: event.position);
       if (deadline != null)
         _timer = Timer(deadline!, () => didExceedDeadlineWithEvent(event));
     }
@@ -528,7 +568,8 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
   /// Override to be notified when [deadline] is exceeded.
   ///
   /// You must override this method or [didExceedDeadlineWithEvent] if you
-  /// supply a [deadline].
+  /// supply a [deadline]. Subclasses that override this method must _not_
+  /// call `super.didExceedDeadline()`.
   @protected
   void didExceedDeadline() {
     assert(deadline == null);
@@ -538,7 +579,8 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
   /// gesture.
   ///
   /// You must override this method or [didExceedDeadline] if you supply a
-  /// [deadline].
+  /// [deadline]. Subclasses that override this method must _not_ call
+  /// `super.didExceedDeadlineWithEvent(event)`.
   @protected
   void didExceedDeadlineWithEvent(PointerDownEvent event) {
     didExceedDeadline();
@@ -556,7 +598,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
   void rejectGesture(int pointer) {
     if (pointer == primaryPointer && state == GestureRecognizerState.possible) {
       _stopTimer();
-      state = GestureRecognizerState.defunct;
+      _state = GestureRecognizerState.defunct;
     }
   }
 
@@ -564,7 +606,9 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
   void didStopTrackingLastPointer(int pointer) {
     assert(state != GestureRecognizerState.ready);
     _stopTimer();
-    state = GestureRecognizerState.ready;
+    _state = GestureRecognizerState.ready;
+    _initialPosition = null;
+    _gestureAccepted = false;
   }
 
   @override
@@ -597,6 +641,7 @@ abstract class PrimaryPointerGestureRecognizer extends OneSequenceGestureRecogni
 /// Usually, the [global] [Offset] is in the coordinate space of the screen
 /// after conversion to logical pixels and the [local] offset is the same
 /// [Offset], but transformed to a local coordinate space.
+@immutable
 class OffsetPair {
   /// Creates a [OffsetPair] combining a [local] and [global] [Offset].
   const OffsetPair({

packages/flutter/lib/src/gestures/scale.dart

@@ -350,8 +350,8 @@ class ScaleGestureRecognizer extends OneSequenceGestureRecognizer {
   }
 
   @override
-  void addAllowedPointer(PointerEvent event) {
-    startTrackingPointer(event.pointer, event.transform);
+  void addAllowedPointer(PointerDownEvent event) {
+    super.addAllowedPointer(event);
     _velocityTrackers[event.pointer] = VelocityTracker.withKind(event.kind);
     if (_state == _ScaleState.ready) {
       _state = _ScaleState.possible;

packages/flutter/lib/src/rendering/platform_view.dart

@@ -456,7 +456,7 @@ class _UiKitViewGestureRecognizer extends OneSequenceGestureRecognizer {
 
   @override
   void addAllowedPointer(PointerDownEvent event) {
-    startTrackingPointer(event.pointer, event.transform);
+    super.addAllowedPointer(event);
     for (final OneSequenceGestureRecognizer recognizer in _gestureRecognizers) {
       recognizer.addPointer(event);
     }
@@ -544,7 +544,7 @@ class _PlatformViewGestureRecognizer extends OneSequenceGestureRecognizer {
 
   @override
   void addAllowedPointer(PointerDownEvent event) {
-    startTrackingPointer(event.pointer, event.transform);
+    super.addAllowedPointer(event);
     for (final OneSequenceGestureRecognizer recognizer in _gestureRecognizers) {
       recognizer.addPointer(event);
     }

packages/flutter/test/gestures/recognizer_test.dart

@@ -2,26 +2,48 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:ui' show VoidCallback;
+
 import 'package:flutter/gestures.dart';
 import 'package:flutter_test/flutter_test.dart';
 
-class TestGestureRecognizer extends GestureRecognizer {
-  TestGestureRecognizer({ Object? debugOwner }) : super(debugOwner: debugOwner);
+import 'gesture_tester.dart';
 
-  @override
-  String get debugDescription => 'debugDescription content';
+// Down/move/up pair 1: normal tap sequence
+const PointerDownEvent down = PointerDownEvent(
+  pointer: 5,
+  position: Offset(10, 10),
+);
 
-  @override
-  void addPointer(PointerDownEvent event) { }
+const PointerMoveEvent move = PointerMoveEvent(
+  pointer: 5,
+  position: Offset(15, 15),
+);
 
-  @override
-  void acceptGesture(int pointer) { }
+const PointerUpEvent up = PointerUpEvent(
+  pointer: 5,
+  position: Offset(15, 15),
+);
 
-  @override
-  void rejectGesture(int pointer) { }
-}
+// Down/move/up pair 2: tap sequence with a large move in the middle
+const PointerDownEvent down2 = PointerDownEvent(
+  pointer: 6,
+  position: Offset(10, 10),
+);
+
+const PointerMoveEvent move2 = PointerMoveEvent(
+  pointer: 6,
+  position: Offset(100, 200),
+);
+
+const PointerUpEvent up2 = PointerUpEvent(
+  pointer: 6,
+  position: Offset(100, 200),
+);
 
 void main() {
+  setUp(ensureGestureBinding);
+
   test('GestureRecognizer smoketest', () {
     final TestGestureRecognizer recognizer = TestGestureRecognizer(debugOwner: 0);
     expect(recognizer, hasAGoodToStringDeep);
@@ -64,4 +86,187 @@ void main() {
       ),
     );
   });
+
+  group('PrimaryPointerGestureRecognizer', () {
+    testGesture('cleans up state after winning arena', (GestureTester tester) {
+      final List<String> resolutions = <String>[];
+      final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer();
+      final TestPrimaryPointerGestureRecognizer<PointerUpEvent> accepting = TestPrimaryPointerGestureRecognizer<PointerUpEvent>(
+        GestureDisposition.accepted,
+        onAcceptGesture: () => resolutions.add('accepted'),
+        onRejectGesture: () => resolutions.add('rejected'),
+      );
+      expect(accepting.state, GestureRecognizerState.ready);
+      expect(accepting.primaryPointer, isNull);
+      expect(accepting.initialPosition, isNull);
+      expect(resolutions, <String>[]);
+
+      indefinite.addPointer(down);
+      accepting.addPointer(down);
+      expect(accepting.state, GestureRecognizerState.possible);
+      expect(accepting.primaryPointer, 5);
+      expect(accepting.initialPosition!.global, down.position);
+      expect(accepting.initialPosition!.local, down.localPosition);
+      expect(resolutions, <String>[]);
+
+      tester.closeArena(5);
+      tester.async.flushMicrotasks();
+      tester.route(down);
+      tester.route(up);
+      expect(accepting.state, GestureRecognizerState.ready);
+      expect(accepting.primaryPointer, 5);
+      expect(accepting.initialPosition, isNull);
+      expect(resolutions, <String>['accepted']);
+    });
+
+    testGesture('cleans up state after losing arena', (GestureTester tester) {
+      final List<String> resolutions = <String>[];
+      final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer();
+      final TestPrimaryPointerGestureRecognizer<PointerMoveEvent> rejecting = TestPrimaryPointerGestureRecognizer<PointerMoveEvent>(
+        GestureDisposition.rejected,
+        onAcceptGesture: () => resolutions.add('accepted'),
+        onRejectGesture: () => resolutions.add('rejected'),
+      );
+      expect(rejecting.state, GestureRecognizerState.ready);
+      expect(rejecting.primaryPointer, isNull);
+      expect(rejecting.initialPosition, isNull);
+      expect(resolutions, <String>[]);
+
+      indefinite.addPointer(down);
+      rejecting.addPointer(down);
+      expect(rejecting.state, GestureRecognizerState.possible);
+      expect(rejecting.primaryPointer, 5);
+      expect(rejecting.initialPosition!.global, down.position);
+      expect(rejecting.initialPosition!.local, down.localPosition);
+      expect(resolutions, <String>[]);
+
+      tester.closeArena(5);
+      tester.async.flushMicrotasks();
+      tester.route(down);
+      tester.route(move);
+      expect(rejecting.state, GestureRecognizerState.defunct);
+      expect(rejecting.primaryPointer, 5);
+      expect(rejecting.initialPosition!.global, down.position);
+      expect(rejecting.initialPosition!.local, down.localPosition);
+      expect(resolutions, <String>['rejected']);
+
+      tester.route(up);
+      expect(rejecting.state, GestureRecognizerState.ready);
+      expect(rejecting.primaryPointer, 5);
+      expect(rejecting.initialPosition, isNull);
+      expect(resolutions, <String>['rejected']);
+    });
+
+    testGesture('works properly when recycled', (GestureTester tester) {
+      final List<String> resolutions = <String>[];
+      final IndefiniteGestureRecognizer indefinite = IndefiniteGestureRecognizer();
+      final TestPrimaryPointerGestureRecognizer<PointerUpEvent> accepting = TestPrimaryPointerGestureRecognizer<PointerUpEvent>(
+        GestureDisposition.accepted,
+        preAcceptSlopTolerance: 15,
+        postAcceptSlopTolerance: 1000,
+        onAcceptGesture: () => resolutions.add('accepted'),
+        onRejectGesture: () => resolutions.add('rejected'),
+      );
+
+      // Send one complete pointer sequence
+      indefinite.addPointer(down);
+      accepting.addPointer(down);
+      tester.closeArena(5);
+      tester.async.flushMicrotasks();
+      tester.route(down);
+      tester.route(up);
+      expect(resolutions, <String>['accepted']);
+      resolutions.clear();
+
+      // Send a follow-on sequence that breaks preAcceptSlopTolerance
+      indefinite.addPointer(down2);
+      accepting.addPointer(down2);
+      tester.closeArena(6);
+      tester.async.flushMicrotasks();
+      tester.route(down2);
+      tester.route(move2);
+      expect(resolutions, <String>['rejected']);
+      tester.route(up2);
+      expect(resolutions, <String>['rejected']);
+    });
+  });
+}
+
+class TestGestureRecognizer extends GestureRecognizer {
+  TestGestureRecognizer({ Object? debugOwner }) : super(debugOwner: debugOwner);
+
+  @override
+  String get debugDescription => 'debugDescription content';
+
+  @override
+  void addPointer(PointerDownEvent event) { }
+
+  @override
+  void acceptGesture(int pointer) { }
+
+  @override
+  void rejectGesture(int pointer) { }
+}
+
+/// Gesture recognizer that adds itself to the gesture arena but never
+/// resolves itself.
+class IndefiniteGestureRecognizer extends GestureRecognizer {
+  @override
+  void addAllowedPointer(PointerDownEvent event) {
+    GestureBinding.instance!.gestureArena.add(event.pointer, this);
+  }
+
+  @override
+  void acceptGesture(int pointer) { }
+
+  @override
+  void rejectGesture(int pointer) { }
+
+  @override
+  String get debugDescription => 'Unresolving';
+}
+
+/// Gesture recognizer that resolves with [resolution] when it handles an event
+/// on the primary pointer of type [T]
+class TestPrimaryPointerGestureRecognizer<T extends PointerEvent> extends PrimaryPointerGestureRecognizer {
+  TestPrimaryPointerGestureRecognizer(
+    this.resolution, {
+    this.onAcceptGesture,
+    this.onRejectGesture,
+    double? preAcceptSlopTolerance,
+    double? postAcceptSlopTolerance,
+  }) : super(
+    preAcceptSlopTolerance: preAcceptSlopTolerance,
+    postAcceptSlopTolerance: postAcceptSlopTolerance,
+  );
+
+  final GestureDisposition resolution;
+  final VoidCallback? onAcceptGesture;
+  final VoidCallback? onRejectGesture;
+
+  @override
+  void acceptGesture(int pointer) {
+    super.acceptGesture(pointer);
+    if (onAcceptGesture != null) {
+      onAcceptGesture!();
+    }
+  }
+
+  @override
+  void rejectGesture(int pointer) {
+    super.rejectGesture(pointer);
+    if (onRejectGesture != null) {
+      onRejectGesture!();
+    }
+  }
+
+  @override
+  void handlePrimaryPointer(PointerEvent event) {
+    if (event is T) {
+      resolve(resolution);
+    }
+  }
+
+  @override
+  String get debugDescription => 'TestPrimaryPointer';
 }