Improve MouseTracker lifecycle: Move checks to post-frame (#44631)

This PR rewrites MouseTracker's lifecycle, so that mouse callbacks are all triggered in post frame, instead of the current one where some are triggered during the build phase. This PR also changes the onExit callback to MouseRegion, RenderMouseRegion, and MouseTrackerAnnotation, so that it is no longer triggered on dispose.

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

@@ -281,6 +281,7 @@ mixin RendererBinding on BindingBase, ServicesBinding, SchedulerBinding, Gesture
 
   void _handlePersistentFrameCallback(Duration timeStamp) {
     drawFrame();
+    _mouseTracker.schedulePostFrameCheck();
   }
 
   /// Pump the rendering pipeline to generate a frame.

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

@@ -2685,7 +2685,8 @@ class RenderMouseRegion extends RenderProxyBox {
       _onHover(event);
   }
 
-  /// Called when a pointer leaves the region (with or without buttons pressed).
+  /// Called when a pointer leaves the region (with or without buttons pressed)
+  /// and the annotation is still attached.
   PointerExitEventListener get onExit => _onExit;
   set onExit(PointerExitEventListener value) {
     if (_onExit != value) {

packages/flutter/lib/src/widgets/actions.dart

@@ -2,9 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
-
 import 'package:flutter/foundation.dart';
+import 'package:flutter/scheduler.dart';
 import 'package:flutter/gestures.dart';
 
 import 'basic.dart';
@@ -550,10 +549,15 @@ class FocusableActionDetector extends StatefulWidget {
   /// {@macro flutter.widgets.shortcuts.shortcuts}
   final Map<LogicalKeySet, Intent> shortcuts;
 
-  /// A function that will be called when the focus highlight should be shown or hidden.
+  /// A function that will be called when the focus highlight should be shown or
+  /// hidden.
+  ///
+  /// This method is not triggered at the unmount of the widget.
   final ValueChanged<bool> onShowFocusHighlight;
 
   /// A function that will be called when the hover highlight should be shown or hidden.
+  ///
+  /// This method is not triggered at the unmount of the widget.
   final ValueChanged<bool> onShowHoverHighlight;
 
   /// A function that will be called when the focus changes.
@@ -574,7 +578,9 @@ class _FocusableActionDetectorState extends State<FocusableActionDetector> {
   @override
   void initState() {
     super.initState();
-    _updateHighlightMode(FocusManager.instance.highlightMode);
+    SchedulerBinding.instance.addPostFrameCallback((Duration duration) {
+      _updateHighlightMode(FocusManager.instance.highlightMode);
+    });
     FocusManager.instance.addHighlightModeListener(_handleFocusHighlightModeChange);
   }
 
@@ -586,23 +592,22 @@ class _FocusableActionDetectorState extends State<FocusableActionDetector> {
 
   bool _canShowHighlight = false;
   void _updateHighlightMode(FocusHighlightMode mode) {
-    final bool couldShowHighlight = _canShowHighlight;
-    switch (FocusManager.instance.highlightMode) {
-      case FocusHighlightMode.touch:
-        _canShowHighlight = false;
-        break;
-      case FocusHighlightMode.traditional:
-        _canShowHighlight = true;
-        break;
-    }
-    if  (couldShowHighlight != _canShowHighlight) {
-      _handleShowFocusHighlight();
-      _handleShowHoverHighlight();
-    }
+    _mayTriggerCallback(task: () {
+      switch (FocusManager.instance.highlightMode) {
+        case FocusHighlightMode.touch:
+          _canShowHighlight = false;
+          break;
+        case FocusHighlightMode.traditional:
+          _canShowHighlight = true;
+          break;
+      }
+    });
   }
 
-  /// Have to have this separate from the _updateHighlightMode because it gets
-  /// called in initState, where things aren't mounted yet.
+  // Have to have this separate from the _updateHighlightMode because it gets
+  // called in initState, where things aren't mounted yet.
+  // Since this method is a highlight mode listener, it is only called
+  // immediately following pointer events.
   void _handleFocusHighlightModeChange(FocusHighlightMode mode) {
     if (!mounted) {
       return;
@@ -614,36 +619,67 @@ class _FocusableActionDetectorState extends State<FocusableActionDetector> {
   void _handleMouseEnter(PointerEnterEvent event) {
     assert(widget.onShowHoverHighlight != null);
     if (!_hovering) {
-      // TODO(gspencergoog): remove scheduleMicrotask once MouseRegion event timing has changed.
-      scheduleMicrotask(() { setState(() { _hovering = true; _handleShowHoverHighlight(); }); });
+      _mayTriggerCallback(task: () {
+        _hovering = true;
+      });
     }
   }
 
   void _handleMouseExit(PointerExitEvent event) {
     assert(widget.onShowHoverHighlight != null);
     if (_hovering) {
-      // TODO(gspencergoog): remove scheduleMicrotask once MouseRegion event timing has changed.
-      scheduleMicrotask(() { setState(() { _hovering = false; _handleShowHoverHighlight(); }); });
+      _mayTriggerCallback(task: () {
+        _hovering = false;
+      });
     }
   }
 
   bool _focused = false;
   void _handleFocusChange(bool focused) {
     if (_focused != focused) {
-      setState(() {
+      _mayTriggerCallback(task: () {
         _focused = focused;
-        _handleShowFocusHighlight();
-        widget.onFocusChange?.call(_focused);
       });
+      widget.onFocusChange?.call(_focused);
     }
   }
 
-  void _handleShowHoverHighlight() {
-    widget.onShowHoverHighlight?.call(_hovering && widget.enabled && _canShowHighlight);
+  // Record old states, do `task` if not null, then compare old states with the
+  // new states, and trigger callbacks if necessary.
+  //
+  // The old states are collected from `oldWidget` if it is provided, or the
+  // current widget (before doing `task`) otherwise. The new states are always
+  // collected from the current widget.
+  void _mayTriggerCallback({VoidCallback task, FocusableActionDetector oldWidget}) {
+    bool shouldShowHoverHighlight(FocusableActionDetector target) {
+      return _hovering && target.enabled && _canShowHighlight;
+    }
+    bool shouldShowFocusHighlight(FocusableActionDetector target) {
+      return _focused && target.enabled && _canShowHighlight;
+    }
+
+    assert(SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks);
+    final FocusableActionDetector oldTarget = oldWidget ?? widget;
+    final bool didShowHoverHighlight = shouldShowHoverHighlight(oldTarget);
+    final bool didShowFocusHighlight = shouldShowFocusHighlight(oldTarget);
+    if (task != null)
+      task();
+    final bool doShowHoverHighlight = shouldShowHoverHighlight(widget);
+    final bool doShowFocusHighlight = shouldShowFocusHighlight(widget);
+    if (didShowFocusHighlight != doShowFocusHighlight)
+      widget.onShowFocusHighlight?.call(doShowFocusHighlight);
+    if (didShowHoverHighlight != doShowHoverHighlight)
+      widget.onShowHoverHighlight?.call(doShowHoverHighlight);
   }
 
-  void _handleShowFocusHighlight() {
-    widget.onShowFocusHighlight?.call(_focused && widget.enabled && _canShowHighlight);
+  @override
+  void didUpdateWidget(FocusableActionDetector oldWidget) {
+    super.didUpdateWidget(oldWidget);
+    if (widget.enabled != oldWidget.enabled) {
+      SchedulerBinding.instance.addPostFrameCallback((Duration duration) {
+        _mayTriggerCallback(oldWidget: oldWidget);
+      });
+    }
   }
 
   @override

packages/flutter/lib/src/widgets/basic.dart

@@ -5842,18 +5842,71 @@ class MouseRegion extends SingleChildRenderObjectWidget {
   }) : assert(opaque != null),
        super(key: key, child: child);
 
-  /// Called when a mouse pointer (with or without buttons pressed) enters the
-  /// region defined by this widget, or when the widget appears under the
-  /// pointer.
+  /// Called when a mouse pointer, with or without buttons pressed, has
+  /// entered this widget.
+  ///
+  /// This callback is triggered when the pointer has started to be contained
+  /// by the region of this widget. More specifically, the callback is triggered
+  /// by the following cases:
+  ///
+  ///  * This widget has appeared under a pointer.
+  ///  * This widget has moved to under a pointer.
+  ///  * A new pointer has been added to somewhere within this widget.
+  ///  * An existing pointer has moved into this widget.
+  ///
+  /// This callback is not always matched by an [onExit]. If the [MouseRegion]
+  /// is unmounted while being hovered by a pointer, the [onExit] of the widget
+  /// callback will never called, despite the earlier call of [onEnter]. For
+  /// more details, see [onExit].
+  ///
+  /// See also:
+  ///
+  ///  * [onExit], which is triggered when a mouse pointer exits the region.
+  ///  * [MouseTrackerAnnotation.onEnter], which is how this callback is
+  ///    internally implemented.
   final PointerEnterEventListener onEnter;
 
-  /// Called when a mouse pointer (with or without buttons pressed) changes
-  /// position, and the new position is within the region defined by this widget.
+  /// Called when a mouse pointer changes position without buttons pressed, and
+  /// the new position is within the region defined by this widget.
+  ///
+  /// This callback is triggered when:
+  ///
+  ///  * An annotation that did not contain the pointer has moved to under a
+  ///    pointer that has no buttons pressed.
+  ///  * A pointer has moved onto, or moved within an annotation without buttons
+  ///    pressed.
+  ///
+  /// This callback is not triggered when
+  ///
+  ///  * An annotation that is containing the pointer has moved, and still
+  ///    contains the pointer.
   final PointerHoverEventListener onHover;
 
-  /// Called when a mouse pointer (with or without buttons pressed) leaves the
-  /// region defined by this widget, or when the widget disappears from under
-  /// the pointer.
+  /// Called when a mouse pointer, with or without buttons pressed, has exited
+  /// this widget when the widget is still mounted.
+  ///
+  /// This callback is triggered when the pointer has stopped to be contained
+  /// by the region of this widget, except when it's caused by the removal of
+  /// this widget. More specifically, the callback is triggered by
+  /// the following cases:
+  ///
+  ///  * This widget, which used to contain a pointer, has moved away.
+  ///  * A pointer that used to be within this widget has been removed.
+  ///  * A pointer that used to be within this widget has moved away.
+  ///
+  /// And is __not__ triggered by the following case,
+  ///
+  ///  * This widget, which used to contain a pointer, has disappeared.
+  ///
+  /// The last case is the only case when [onExit] does not match an earlier
+  /// [onEnter].
+  /// {@macro flutter.mouseTracker.onExit}
+  ///
+  /// See also:
+  ///
+  ///  * [onEnter], which is triggered when a mouse pointer enters the region.
+  ///  * [MouseTrackerAnnotation.onExit], which is how this callback is
+  ///    internally implemented.
   final PointerExitEventListener onExit;
 
   /// Whether this widget should prevent other [MouseRegion]s visually behind it

packages/flutter/test/cupertino/text_field_test.dart

@@ -2596,10 +2596,11 @@ void main() {
 
       // Double tap at the end of text.
       final Offset textEndPos = textOffsetToPosition(tester, 11); // Position at the end of text.
-      TestGesture gesture = await tester.startGesture(
+      final TestGesture gesture = await tester.startGesture(
         textEndPos,
         kind: PointerDeviceKind.mouse,
       );
+      addTearDown(gesture.removePointer);
       await tester.pump(const Duration(milliseconds: 50));
       await gesture.up();
       await tester.pump();
@@ -2614,11 +2615,7 @@ void main() {
       final Offset hPos = textOffsetToPosition(tester, 9); // Position of 'h'.
 
       // Double tap on 'h' to select 'ghi'.
-      gesture = await tester.startGesture(
-        hPos,
-        kind: PointerDeviceKind.mouse,
-      );
-      addTearDown(gesture.removePointer);
+      await gesture.down(hPos);
       await tester.pump(const Duration(milliseconds: 50));
       await gesture.up();
       await tester.pump();

packages/flutter/test/material/ink_well_test.dart

@@ -385,6 +385,7 @@ void main() {
     await tester.pumpAndSettle();
     expect(focusNode.hasPrimaryFocus, isTrue);
     final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+    await gesture.addPointer();
     addTearDown(gesture.removePointer);
     await gesture.moveTo(tester.getCenter(find.byKey(childKey)));
     await tester.pumpAndSettle();

packages/flutter/test/material/radio_test.dart

@@ -522,4 +522,3 @@ void main() {
   });
 
 }
-

packages/flutter/test/material/switch_test.dart

@@ -744,6 +744,7 @@ void main() {
 
     // Start hovering
     final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+    await gesture.addPointer();
     addTearDown(gesture.removePointer);
     await gesture.moveTo(tester.getCenter(find.byType(Switch)));
 

packages/flutter/test/widgets/drawer_test.dart

@@ -82,10 +82,9 @@ void main() {
     final GlobalKey<ScaffoldState> scaffoldKey = GlobalKey<ScaffoldState>();
     final List<String> logs = <String>[];
     final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
-    addTearDown(gesture.removePointer);
-
     // Start out of hoverTarget
-    await gesture.moveTo(const Offset(100, 100));
+    await gesture.addPointer(location: const Offset(100, 100));
+    addTearDown(gesture.removePointer);
 
     await tester.pumpWidget(
       MaterialApp(

packages/flutter/test/widgets/listener_deprecated_test.dart

@@ -145,7 +145,7 @@ void main() {
       expect(exit, isNotNull);
       expect(exit.position, equals(const Offset(1.0, 1.0)));
     });
-    testWidgets('detects pointer exit when widget disappears', (WidgetTester tester) async {
+    testWidgets('does not detect pointer exit when widget disappears', (WidgetTester tester) async {
       PointerEnterEvent enter;
       PointerHoverEvent move;
       PointerExitEvent exit;
@@ -177,8 +177,7 @@ void main() {
           height: 100.0,
         ),
       ));
-      expect(exit, isNotNull);
-      expect(exit.position, equals(const Offset(400.0, 300.0)));
+      expect(exit, isNull);
       expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener.hoverAnnotation), isFalse);
     });
     testWidgets('Hover works with nested listeners', (WidgetTester tester) async {
@@ -529,14 +528,15 @@ void main() {
       );
       await tester.pump();
       expect(HoverClientState.numEntries, equals(1));
-      expect(HoverClientState.numExits, equals(1));
+      // Unmounting a MouseRegion doesn't trigger onExit
+      expect(HoverClientState.numExits, equals(0));
 
       await tester.pumpWidget(
         const Center(child: HoverFeedback()),
       );
       await tester.pump();
       expect(HoverClientState.numEntries, equals(2));
-      expect(HoverClientState.numExits, equals(1));
+      expect(HoverClientState.numExits, equals(0));
     });
 
     testWidgets("Listener activate/deactivate don't duplicate annotations", (WidgetTester tester) async {
@@ -559,14 +559,15 @@ void main() {
         Center(child: Container(child: HoverFeedback(key: feedbackKey))),
       );
       await tester.pump();
-      expect(HoverClientState.numEntries, equals(2));
-      expect(HoverClientState.numExits, equals(1));
+      expect(HoverClientState.numEntries, equals(1));
+      expect(HoverClientState.numExits, equals(0));
       await tester.pumpWidget(
         Container(),
       );
       await tester.pump();
-      expect(HoverClientState.numEntries, equals(2));
-      expect(HoverClientState.numExits, equals(2));
+      expect(HoverClientState.numEntries, equals(1));
+      // Unmounting a MouseRegion doesn't trigger onExit
+      expect(HoverClientState.numExits, equals(0));
     });
 
     testWidgets('Exit event when unplugging mouse should have a position', (WidgetTester tester) async {

packages/flutter_test/lib/src/test_pointer.dart

@@ -337,12 +337,16 @@ class TestGesture {
        _dispatcher = dispatcher,
        _hitTester = hitTester,
        _pointer = TestPointer(pointer, kind, device, buttons),
+       _added = false,
        _result = null;
 
   /// Dispatch a pointer down event at the given `downLocation`, caching the
   /// hit test result.
-  Future<void> down(Offset downLocation) async {
+  ///
+  /// If the pointer has not been added, an added event will be dispatched first.
+  Future<void> down(Offset downLocation) {
     return TestAsyncUtils.guard<void>(() async {
+      await _ensureAdded(location: downLocation);
       _result = _hitTester(downLocation);
       return _dispatcher(_pointer.down(downLocation), _result);
     });
@@ -350,9 +354,12 @@ class TestGesture {
 
   /// Dispatch a pointer down event at the given `downLocation`, caching the
   /// hit test result with a custom down event.
-  Future<void> downWithCustomEvent(Offset downLocation, PointerDownEvent event) async {
-    _pointer.setDownInfo(event, downLocation);
+  ///
+  /// If the pointer has not been added, an added event will be dispatched first.
+  Future<void> downWithCustomEvent(Offset downLocation, PointerDownEvent event) {
     return TestAsyncUtils.guard<void>(() async {
+      await _ensureAdded(location: downLocation);
+      _pointer.setDownInfo(event, downLocation);
       _result = _hitTester(downLocation);
       return _dispatcher(event, _result);
     });
@@ -362,10 +369,22 @@ class TestGesture {
   final HitTester _hitTester;
   final TestPointer _pointer;
   HitTestResult _result;
+  bool _added;
+
+  Future<void> _ensureAdded({ Offset location }) async {
+    if (!_added) {
+      await addPointer(location: location ?? _pointer.location);
+    }
+  }
 
   /// In a test, send a move event that moves the pointer by the given offset.
+  ///
+  /// If the pointer has not been added, and the subject event is not an added
+  /// event, an added event will be dispatched first.
   @visibleForTesting
-  Future<void> updateWithCustomEvent(PointerEvent event, { Duration timeStamp = Duration.zero }) {
+  Future<void> updateWithCustomEvent(PointerEvent event, { Duration timeStamp = Duration.zero }) async {
+    if (event is! PointerAddedEvent)
+      await _ensureAdded(location: event.position);
     _pointer.setDownInfo(event, event.position);
     return TestAsyncUtils.guard<void>(() {
       return _dispatcher(event, _result);
@@ -373,21 +392,34 @@ class TestGesture {
   }
 
   /// In a test, send a pointer add event for this pointer.
+  ///
+  /// If a pointer has been added, the pointer will be removed first.
   Future<void> addPointer({ Duration timeStamp = Duration.zero, Offset location }) {
-    return TestAsyncUtils.guard<void>(() {
+    return TestAsyncUtils.guard<void>(() async {
+      if (_added) {
+        await removePointer(timeStamp: timeStamp);
+      }
+      _added = true;
       return _dispatcher(_pointer.addPointer(timeStamp: timeStamp, location: location ?? _pointer.location), null);
     });
   }
 
   /// In a test, send a pointer remove event for this pointer.
+  ///
+  /// If no pointer has been added, the call will be a no-op.
   Future<void> removePointer({ Duration timeStamp = Duration.zero, Offset location }) {
-    return TestAsyncUtils.guard<void>(() {
-      return _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: location ?? _pointer.location), null);
+    return TestAsyncUtils.guard<void>(() async {
+      if (!_added)
+        return;
+      _added = false;
+      await _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: location ?? _pointer.location), null);
     });
   }
 
   /// Send a move event moving the pointer by the given offset.
   ///
+  /// If the pointer has not been added, an added event will be dispatched first.
+  ///
   /// If the pointer is down, then a move event is dispatched. If the pointer is
   /// up, then a hover event is dispatched. Touch devices are not able to send
   /// hover events.
@@ -397,11 +429,14 @@ class TestGesture {
 
   /// Send a move event moving the pointer to the given location.
   ///
+  /// If the pointer has not been added, an added event will be dispatched first.
+  ///
   /// If the pointer is down, then a move event is dispatched. If the pointer is
   /// up, then a hover event is dispatched. Touch devices are not able to send
   /// hover events.
   Future<void> moveTo(Offset location, { Duration timeStamp = Duration.zero }) {
-    return TestAsyncUtils.guard<void>(() {
+    return TestAsyncUtils.guard<void>(() async {
+      await _ensureAdded(location: location);
       if (_pointer._isDown) {
         assert(_result != null,
             'Move events with the pointer down must be preceded by a down '
@@ -416,8 +451,11 @@ class TestGesture {
   }
 
   /// End the gesture by releasing the pointer.
+  ///
+  /// If the pointer has not been added, an added event will be dispatched first.
   Future<void> up() {
     return TestAsyncUtils.guard<void>(() async {
+      await _ensureAdded();
       assert(_pointer._isDown);
       await _dispatcher(_pointer.up(), _result);
       assert(!_pointer._isDown);
@@ -428,8 +466,11 @@ class TestGesture {
   /// End the gesture by canceling the pointer (as would happen if the
   /// system showed a modal dialog on top of the Flutter application,
   /// for instance).
+  ///
+  /// If the pointer has not been added, an added event will be dispatched first.
   Future<void> cancel() {
     return TestAsyncUtils.guard<void>(() async {
+      await _ensureAdded();
       assert(_pointer._isDown);
       await _dispatcher(_pointer.cancel(), _result);
       assert(!_pointer._isDown);