Fix onExit calling when the mouse is removed. (#33477)

This PR solves two problems: currently, the onExit is called for a mouse pointer the moment the removal message is received, except that by the time it actually calls it, there is no _lastEvent for it in the mouse tracker (it's already been removed), resulting in an event being passed to the onExit that contains nulls for the position. Also, removePointer events don't actually get created with a position, although they easily could be, so that even the the _lastEvent in the mouse tracker were still populated, it would still give a null position and delta.

This PR adds support for the position and delta in a PointerRemovedEvent, and populates them. In addition, when a remove event is received, it doesn't actually remove the pointer until the mouse position check that gets scheduled actually happens.

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

@@ -386,8 +386,8 @@ class PointerEventConverter {
                 kind: kind,
                 device: datum.device,
                 position: position,
-                buttons: datum.buttons,
                 delta: state.deltaTo(position),
+                buttons: datum.buttons,
                 obscured: datum.obscured,
                 pressureMin: datum.pressureMin,
                 pressureMax: datum.pressureMax,
@@ -408,6 +408,7 @@ class PointerEventConverter {
               timeStamp: timeStamp,
               kind: kind,
               device: datum.device,
+              position: position,
               obscured: datum.obscured,
               pressureMin: datum.pressureMin,
               pressureMax: datum.pressureMax,

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

@@ -240,7 +240,7 @@ abstract class PointerEvent extends Diagnosticable {
   final Offset position;
 
   /// Distance in logical pixels that the pointer moved since the last
-  /// [PointerMoveEvent].
+  /// [PointerMoveEvent] or [PointerHoverEvent].
   ///
   /// This value is always 0.0 for down, up, and cancel events.
   final Offset delta;
@@ -477,6 +477,7 @@ class PointerRemovedEvent extends PointerEvent {
     Duration timeStamp = Duration.zero,
     PointerDeviceKind kind = PointerDeviceKind.touch,
     int device = 0,
+    Offset position = Offset.zero,
     bool obscured = false,
     double pressureMin = 1.0,
     double pressureMax = 1.0,
@@ -487,7 +488,7 @@ class PointerRemovedEvent extends PointerEvent {
          timeStamp: timeStamp,
          kind: kind,
          device: device,
-         position: null,
+         position: position,
          obscured: obscured,
          pressure: 0.0,
          pressureMin: pressureMin,

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

@@ -125,7 +125,9 @@ class MouseTracker extends ChangeNotifier {
     final _TrackedAnnotation trackedAnnotation = _findAnnotation(annotation);
     for (int deviceId in trackedAnnotation.activeDevices) {
       if (annotation.onExit != null) {
-        annotation.onExit(PointerExitEvent.fromMouseEvent(_lastMouseEvent[deviceId]));
+        final PointerEvent event = _lastMouseEvent[deviceId] ?? _pendingRemovals[deviceId];
+        assert(event != null);
+        annotation.onExit(PointerExitEvent.fromMouseEvent(event));
       }
     }
     _trackedAnnotations.remove(annotation);
@@ -153,11 +155,13 @@ class MouseTracker extends ChangeNotifier {
     }
     final int deviceId = event.device;
     if (event is PointerAddedEvent) {
+      // If we are adding the device again, then we're not removing it anymore.
+      _pendingRemovals.remove(deviceId);
       _addMouseEvent(deviceId, event);
       return;
     }
     if (event is PointerRemovedEvent) {
-      _removeMouseEvent(deviceId);
+      _removeMouseEvent(deviceId, event);
       // If the mouse was removed, then we need to schedule one more check to
       // exit any annotations that were active.
       _scheduleMousePositionCheck();
@@ -205,7 +209,9 @@ class MouseTracker extends ChangeNotifier {
   void collectMousePositions() {
     void exitAnnotation(_TrackedAnnotation trackedAnnotation, int deviceId) {
       if (trackedAnnotation.annotation?.onExit != null && trackedAnnotation.activeDevices.contains(deviceId)) {
-        trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(_lastMouseEvent[deviceId]));
+        final PointerEvent event = _lastMouseEvent[deviceId] ?? _pendingRemovals[deviceId];
+        assert(event != null);
+        trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(event));
         trackedAnnotation.activeDevices.remove(deviceId);
       }
     }
@@ -219,75 +225,89 @@ class MouseTracker extends ChangeNotifier {
       }
     }
 
-    // This indicates that all mouse pointers were removed, or none have been
-    // connected yet. If no mouse is connected, then we want to make sure that
-    // all active annotations are exited.
-    if (!mouseIsConnected) {
-      _trackedAnnotations.values.forEach(exitAllDevices);
-      return;
-    }
-
-    for (int deviceId in _lastMouseEvent.keys) {
-      final PointerEvent lastEvent = _lastMouseEvent[deviceId];
-      final Iterable<MouseTrackerAnnotation> hits = annotationFinder(lastEvent.position);
-
-      // No annotations were found at this position for this deviceId, so send an
-      // exit to all active tracked annotations, since none of them were hit.
-      if (hits.isEmpty) {
-        // Send an exit to all tracked animations tracking this deviceId.
-        for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
-          exitAnnotation(trackedAnnotation, deviceId);
-        }
-        continue;
+    try {
+      // This indicates that all mouse pointers were removed, or none have been
+      // connected yet. If no mouse is connected, then we want to make sure that
+      // all active annotations are exited.
+      if (!mouseIsConnected) {
+        _trackedAnnotations.values.forEach(exitAllDevices);
+        return;
       }
 
-      final Set<_TrackedAnnotation> hitAnnotations = hits.map<_TrackedAnnotation>((MouseTrackerAnnotation hit) => _findAnnotation(hit)).toSet();
-      for (_TrackedAnnotation hitAnnotation in hitAnnotations) {
-        if (!hitAnnotation.activeDevices.contains(deviceId)) {
-          // A tracked annotation that just became active and needs to have an enter
-          // event sent to it.
-          hitAnnotation.activeDevices.add(deviceId);
-          if (hitAnnotation.annotation?.onEnter != null) {
-            hitAnnotation.annotation.onEnter(PointerEnterEvent.fromMouseEvent(lastEvent));
+      for (int deviceId in _lastMouseEvent.keys) {
+        final PointerEvent lastEvent = _lastMouseEvent[deviceId];
+        final Iterable<MouseTrackerAnnotation> hits = annotationFinder(lastEvent.position);
+
+        // No annotations were found at this position for this deviceId, so send an
+        // exit to all active tracked annotations, since none of them were hit.
+        if (hits.isEmpty) {
+          // Send an exit to all tracked animations tracking this deviceId.
+          for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
+            exitAnnotation(trackedAnnotation, deviceId);
           }
-        }
-        if (hitAnnotation.annotation?.onHover != null && lastEvent is PointerHoverEvent) {
-          hitAnnotation.annotation.onHover(lastEvent);
+          continue;
         }
 
-        // Tell any tracked annotations that weren't hit that they are no longer
-        // active.
-        for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
-          if (hitAnnotations.contains(trackedAnnotation)) {
-            continue;
+        final Set<_TrackedAnnotation> hitAnnotations = hits.map<_TrackedAnnotation>((MouseTrackerAnnotation hit) => _findAnnotation(hit)).toSet();
+        for (_TrackedAnnotation hitAnnotation in hitAnnotations) {
+          if (!hitAnnotation.activeDevices.contains(deviceId)) {
+            // A tracked annotation that just became active and needs to have an enter
+            // event sent to it.
+            hitAnnotation.activeDevices.add(deviceId);
+            if (hitAnnotation.annotation?.onEnter != null) {
+              hitAnnotation.annotation.onEnter(PointerEnterEvent.fromMouseEvent(lastEvent));
+            }
+          }
+          if (hitAnnotation.annotation?.onHover != null && lastEvent is PointerHoverEvent) {
+            hitAnnotation.annotation.onHover(lastEvent);
           }
-          if (trackedAnnotation.activeDevices.contains(deviceId)) {
-            if (trackedAnnotation.annotation?.onExit != null) {
-              trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(lastEvent));
+
+          // Tell any tracked annotations that weren't hit that they are no longer
+          // active.
+          for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
+            if (hitAnnotations.contains(trackedAnnotation)) {
+              continue;
+            }
+            if (trackedAnnotation.activeDevices.contains(deviceId)) {
+              if (trackedAnnotation.annotation?.onExit != null) {
+                trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(lastEvent));
+              }
+              trackedAnnotation.activeDevices.remove(deviceId);
             }
-            trackedAnnotation.activeDevices.remove(deviceId);
           }
         }
       }
+    } finally {
+      _pendingRemovals.clear();
     }
   }
 
   void _addMouseEvent(int deviceId, PointerEvent event) {
     final bool wasConnected = mouseIsConnected;
+    if (event is PointerAddedEvent) {
+      // If we are adding the device again, then we're not removing it anymore.
+      _pendingRemovals.remove(deviceId);
+    }
     _lastMouseEvent[deviceId] = event;
     if (mouseIsConnected != wasConnected) {
       notifyListeners();
     }
   }
 
-  void _removeMouseEvent(int deviceId) {
+  void _removeMouseEvent(int deviceId, PointerEvent event) {
     final bool wasConnected = mouseIsConnected;
+    assert(event is PointerRemovedEvent);
+    _pendingRemovals[deviceId] = event;
     _lastMouseEvent.remove(deviceId);
     if (mouseIsConnected != wasConnected) {
       notifyListeners();
     }
   }
 
+  // A list of device IDs that should be removed and notified when scheduling a
+  // mouse position check.
+  final Map<int, PointerRemovedEvent> _pendingRemovals = <int, PointerRemovedEvent>{};
+
   /// The most recent mouse event observed for each mouse device ID observed.
   ///
   /// May be null if no mouse is connected, or hasn't produced an event yet.

packages/flutter/test/gestures/mouse_tracking_test.dart

@@ -91,9 +91,11 @@ void main() {
           kind: PointerDeviceKind.mouse,
         ),
       ]);
-      const ui.PointerDataPacket packet3 = ui.PointerDataPacket(data: <ui.PointerData>[
+      final ui.PointerDataPacket packet3 = ui.PointerDataPacket(data: <ui.PointerData>[
         ui.PointerData(
           change: ui.PointerChange.remove,
+          physicalX: 1.0 * ui.window.devicePixelRatio,
+          physicalY: 201.0 * ui.window.devicePixelRatio,
           kind: PointerDeviceKind.mouse,
         ),
       ]);
@@ -101,7 +103,7 @@ void main() {
         ui.PointerData(
           change: ui.PointerChange.hover,
           physicalX: 1.0 * ui.window.devicePixelRatio,
-          physicalY: 201.0 * ui.window.devicePixelRatio,
+          physicalY: 301.0 * ui.window.devicePixelRatio,
           kind: PointerDeviceKind.mouse,
         ),
       ]);
@@ -109,7 +111,7 @@ void main() {
         ui.PointerData(
           change: ui.PointerChange.hover,
           physicalX: 1.0 * ui.window.devicePixelRatio,
-          physicalY: 301.0 * ui.window.devicePixelRatio,
+          physicalY: 401.0 * ui.window.devicePixelRatio,
           kind: PointerDeviceKind.mouse,
           device: 1,
         ),
@@ -144,20 +146,20 @@ void main() {
       expect(enter.length, equals(0), reason: 'enter contains $enter');
       expect(move.length, equals(0), reason: 'move contains $move');
       expect(exit.length, equals(1), reason: 'exit contains $exit');
-      expect(exit.first.position, isNull);
-      expect(exit.first.device, isNull);
+      expect(exit.first.position, equals(const Offset(1.0, 201.0)));
+      expect(exit.first.device, equals(0));
       expect(exit.first.runtimeType, equals(PointerExitEvent));
 
       clear();
       ui.window.onPointerDataPacket(packet4);
       tracker.collectMousePositions();
       expect(enter.length, equals(1), reason: 'enter contains $enter');
-      expect(enter.first.position, equals(const Offset(1.0, 201.0)));
+      expect(enter.first.position, equals(const Offset(1.0, 301.0)));
       expect(enter.first.device, equals(0));
       expect(enter.first.runtimeType, equals(PointerEnterEvent));
       expect(exit.length, equals(0), reason: 'exit contains $exit');
       expect(move.length, equals(1), reason: 'move contains $move');
-      expect(move.first.position, equals(const Offset(1.0, 201.0)));
+      expect(move.first.position, equals(const Offset(1.0, 301.0)));
       expect(move.first.device, equals(0));
       expect(move.first.runtimeType, equals(PointerHoverEvent));
 
@@ -166,15 +168,15 @@ void main() {
       ui.window.onPointerDataPacket(packet5);
       tracker.collectMousePositions();
       expect(enter.length, equals(1), reason: 'enter contains $enter');
-      expect(enter.first.position, equals(const Offset(1.0, 301.0)));
+      expect(enter.first.position, equals(const Offset(1.0, 401.0)));
       expect(enter.first.device, equals(1));
       expect(enter.first.runtimeType, equals(PointerEnterEvent));
       expect(exit.length, equals(0), reason: 'exit contains $exit');
       expect(move.length, equals(2), reason: 'move contains $move');
-      expect(move.first.position, equals(const Offset(1.0, 201.0)));
+      expect(move.first.position, equals(const Offset(1.0, 301.0)));
       expect(move.first.device, equals(0));
       expect(move.first.runtimeType, equals(PointerHoverEvent));
-      expect(move.last.position, equals(const Offset(1.0, 301.0)));
+      expect(move.last.position, equals(const Offset(1.0, 401.0)));
       expect(move.last.device, equals(1));
       expect(move.last.runtimeType, equals(PointerHoverEvent));
     });
@@ -228,7 +230,7 @@ void main() {
       expect(exit.first.device, equals(0));
       expect(exit.first.runtimeType, equals(PointerExitEvent));
 
-      // Actually detatch annotation. Shouldn't receive hit.
+      // Actually detach annotation. Shouldn't receive hit.
       tracker.detachAnnotation(annotation);
       clear();
       isInHitRegionOne = false;
@@ -274,9 +276,11 @@ void main() {
           kind: PointerDeviceKind.mouse,
         ),
       ]);
-      const ui.PointerDataPacket packet2 = ui.PointerDataPacket(data: <ui.PointerData>[
+      final ui.PointerDataPacket packet2 = ui.PointerDataPacket(data: <ui.PointerData>[
         ui.PointerData(
           change: ui.PointerChange.remove,
+          physicalX: 1.0 * ui.window.devicePixelRatio,
+          physicalY: 201.0 * ui.window.devicePixelRatio,
           kind: PointerDeviceKind.mouse,
         ),
       ]);
@@ -288,15 +292,18 @@ void main() {
       tracker.collectMousePositions();
       expect(enter.length, equals(1), reason: 'enter contains $enter');
       expect(enter.first.position, equals(const Offset(1.0, 101.0)));
+      expect(enter.first.delta, equals(const Offset(1.0, 101.0)));
       expect(enter.first.device, equals(0));
       expect(enter.first.runtimeType, equals(PointerEnterEvent));
       expect(move.length, equals(1), reason: 'move contains $move');
       expect(move.first.position, equals(const Offset(1.0, 101.0)));
+      expect(move.first.delta, equals(const Offset(1.0, 101.0)));
       expect(move.first.device, equals(0));
       expect(move.first.runtimeType, equals(PointerHoverEvent));
       expect(exit.length, equals(1), reason: 'exit contains $exit');
-      expect(exit.first.position, isNull);
-      expect(exit.first.device, isNull);
+      expect(exit.first.position, equals(const Offset(1.0, 201.0)));
+      expect(exit.first.delta, equals(const Offset(0.0, 0.0)));
+      expect(exit.first.device, equals(0));
       expect(exit.first.runtimeType, equals(PointerExitEvent));
     });
     test('handles mouse down and move', () {

packages/flutter/test/widgets/listener_test.dart

@@ -594,5 +594,50 @@ void main() {
 
       await gesture.removePointer();
     });
+    testWidgets('Exit event when unplugging mouse should have a position', (WidgetTester tester) async {
+      final List<PointerEnterEvent> enter = <PointerEnterEvent>[];
+      final List<PointerHoverEvent> hover = <PointerHoverEvent>[];
+      final List<PointerExitEvent> exit = <PointerExitEvent>[];
+
+      await tester.pumpWidget(
+        Center(
+          child: Listener(
+            onPointerEnter: (PointerEnterEvent e) => enter.add(e),
+            onPointerHover: (PointerHoverEvent e) => hover.add(e),
+            onPointerExit: (PointerExitEvent e) => exit.add(e),
+            child: Container(
+              height: 100.0,
+              width: 100.0,
+            ),
+          ),
+        ),
+      );
+
+      // Plug-in a mouse and move it to the center of the container.
+      final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+      await gesture.addPointer();
+      await gesture.moveTo(tester.getCenter(find.byType(Container)));
+      await tester.pumpAndSettle();
+
+      expect(enter.length, 1);
+      expect(enter.single.position, const Offset(400.0, 300.0));
+      expect(hover.length, 1);
+      expect(hover.single.position, const Offset(400.0, 300.0));
+      expect(exit.length, 0);
+
+      enter.clear();
+      hover.clear();
+      exit.clear();
+
+      // Unplug the mouse.
+      await gesture.removePointer();
+      await tester.pumpAndSettle();
+
+      expect(enter.length, 0);
+      expect(hover.length, 0);
+      expect(exit.length, 1);
+      expect(exit.single.position, const Offset(400.0, 300.0));
+      expect(exit.single.delta, const Offset(0.0, 0.0));
+    });
   });
 }

packages/flutter_test/lib/src/test_pointer.dart

@@ -209,35 +209,36 @@ class TestPointer {
   ///
   /// By default, the time stamp on the event is [Duration.zero]. You can give a
   /// specific time stamp by passing the `timeStamp` argument.
-  ///
-  /// [isDown] must be false, since hover events can't be sent when the pointer
-  /// is up.
   PointerAddedEvent addPointer({
     Duration timeStamp = Duration.zero,
+    Offset location,
   }) {
     assert(timeStamp != null);
+    _location = location ?? _location;
     return PointerAddedEvent(
       timeStamp: timeStamp,
       kind: kind,
       device: _device,
+      position: _location ?? Offset.zero,
     );
   }
 
-  /// Create a [PointerRemovedEvent] with the kind the pointer was created with.
+  /// Create a [PointerRemovedEvent] with the [PointerDeviceKind] the pointer
+  /// was created with.
   ///
   /// By default, the time stamp on the event is [Duration.zero]. You can give a
   /// specific time stamp by passing the `timeStamp` argument.
-  ///
-  /// [isDown] must be false, since hover events can't be sent when the pointer
-  /// is up.
   PointerRemovedEvent removePointer({
     Duration timeStamp = Duration.zero,
+    Offset location,
   }) {
     assert(timeStamp != null);
+    _location = location ?? _location;
     return PointerRemovedEvent(
       timeStamp: timeStamp,
       kind: kind,
       device: _device,
+      position: _location ?? Offset.zero,
     );
   }
 
@@ -374,14 +375,14 @@ class TestGesture {
   /// In a test, send a pointer add event for this pointer.
   Future<void> addPointer({ Duration timeStamp = Duration.zero }) {
     return TestAsyncUtils.guard<void>(() {
-      return _dispatcher(_pointer.addPointer(timeStamp: timeStamp), null);
+      return _dispatcher(_pointer.addPointer(timeStamp: timeStamp, location: _pointer.location), null);
     });
   }
 
   /// In a test, send a pointer remove event for this pointer.
-  Future<void> removePointer({ Duration timeStamp = Duration.zero }) {
+  Future<void> removePointer({ Duration timeStamp = Duration.zero}) {
     return TestAsyncUtils.guard<void>(() {
-      return _dispatcher(_pointer.removePointer(timeStamp: timeStamp), null);
+      return _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: _pointer.location), null);
     });
   }