MouseTracker no longer requires annotations attached (#48453)

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

@@ -10,7 +10,6 @@ import 'package:flutter/gestures.dart';
 import 'package:flutter/semantics.dart';
 import 'package:flutter/services.dart';
 
-import 'binding.dart';
 import 'box.dart';
 import 'layer.dart';
 import 'object.dart';
@@ -838,13 +837,11 @@ mixin _PlatformViewGestureMixin on RenderBox {
       if (_handlePointerEvent != null)
         _handlePointerEvent(event);
     });
-    RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
   }
 
   @override
   void detach() {
     _gestureRecognizer.reset();
-    RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
     _hoverAnnotation = null;
     super.detach();
   }

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

@@ -2659,6 +2659,9 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
 ///    [RenderMouseRegion].
 class RenderMouseRegion extends RenderProxyBox {
   /// Creates a render object that forwards pointer events to callbacks.
+  ///
+  /// All parameters are optional. By default this method creates an opaque
+  /// mouse region with no callbacks.
   RenderMouseRegion({
     PointerEnterEventListener onEnter,
     PointerHoverEventListener onHover,
@@ -2698,17 +2701,23 @@ class RenderMouseRegion extends RenderProxyBox {
   set opaque(bool value) {
     if (_opaque != value) {
       _opaque = value;
-      _updateAnnotations();
+      _markPropertyUpdated(mustRepaint: true);
     }
   }
 
-  /// Called when a mouse pointer enters the region (with or without buttons
-  /// pressed).
+  /// Called when a mouse pointer starts being contained by the region (with or
+  /// without buttons pressed) for any reason.
+  ///
+  /// This callback is always matched by a later [onExit].
+  ///
+  /// See also:
+  ///
+  ///  * [MouseRegion.onEnter], which uses this callback.
   PointerEnterEventListener get onEnter => _onEnter;
   set onEnter(PointerEnterEventListener value) {
     if (_onEnter != value) {
       _onEnter = value;
-      _updateAnnotations();
+      _markPropertyUpdated(mustRepaint: false);
     }
   }
   PointerEnterEventListener _onEnter;
@@ -2723,7 +2732,7 @@ class RenderMouseRegion extends RenderProxyBox {
   set onHover(PointerHoverEventListener value) {
     if (_onHover != value) {
       _onHover = value;
-      _updateAnnotations();
+      _markPropertyUpdated(mustRepaint: false);
     }
   }
   PointerHoverEventListener _onHover;
@@ -2732,13 +2741,20 @@ class RenderMouseRegion extends RenderProxyBox {
       _onHover(event);
   }
 
-  /// Called when a pointer leaves the region (with or without buttons pressed)
-  /// and the annotation is still attached.
+  /// Called when a pointer is no longer contained by the region (with or
+  /// without buttons pressed) for any reason.
+  ///
+  /// This callback is always matched by an earlier [onEnter].
+  ///
+  /// See also:
+  ///
+  ///  * [MouseRegion.onExit], which uses this callback, but is not triggered in
+  ///    certain cases and does not always match its earier [MouseRegion.onEnter].
   PointerExitEventListener get onExit => _onExit;
   set onExit(PointerExitEventListener value) {
     if (_onExit != value) {
       _onExit = value;
-      _updateAnnotations();
+      _markPropertyUpdated(mustRepaint: false);
     }
   }
   PointerExitEventListener _onExit;
@@ -2757,64 +2773,52 @@ class RenderMouseRegion extends RenderProxyBox {
   @visibleForTesting
   MouseTrackerAnnotation get hoverAnnotation => _hoverAnnotation;
 
-  void _updateAnnotations() {
-    final bool annotationWasActive = _annotationIsActive;
-    final bool annotationWillBeActive = (
+  // Call this method when a property has changed and might affect the
+  // `_annotationIsActive` bit.
+  //
+  // If `mustRepaint` is false, this method does NOT call `markNeedsPaint`
+  // unless the `_annotationIsActive` bit is changed. If there is a property
+  // that needs updating while `_annotationIsActive` stays true, make
+  // `mustRepaint` true.
+  //
+  // This method must not be called during `paint`.
+  void _markPropertyUpdated({@required bool mustRepaint}) {
+    assert(owner == null || !owner.debugDoingPaint);
+    final bool newAnnotationIsActive = (
         _onEnter != null ||
         _onHover != null ||
         _onExit != null ||
         opaque
-      ) &&
-      RendererBinding.instance.mouseTracker.mouseIsConnected;
-    if (annotationWasActive != annotationWillBeActive) {
+      ) && RendererBinding.instance.mouseTracker.mouseIsConnected;
+    _setAnnotationIsActive(newAnnotationIsActive);
+    if (mustRepaint)
+      markNeedsPaint();
+  }
+
+  void _setAnnotationIsActive(bool value) {
+    final bool annotationWasActive = _annotationIsActive;
+    _annotationIsActive = value;
+    if (annotationWasActive != value) {
       markNeedsPaint();
       markNeedsCompositingBitsUpdate();
-      if (annotationWillBeActive) {
-        RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
-      } else {
-        RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
-      }
-      _annotationIsActive = annotationWillBeActive;
     }
   }
 
+  void _handleUpdatedMouseIsConnected() {
+    _markPropertyUpdated(mustRepaint: false);
+  }
+
   @override
   void attach(PipelineOwner owner) {
     super.attach(owner);
     // Add a listener to listen for changes in mouseIsConnected.
-    RendererBinding.instance.mouseTracker.addListener(_updateAnnotations);
-    _updateAnnotations();
-  }
-
-  /// Attaches the annotation for this render object, if any.
-  ///
-  /// This is called by the [MouseRegion]'s [Element] to tell this
-  /// [RenderMouseRegion] that it has transitioned from "inactive"
-  /// state to "active". We call it here so that
-  /// [MouseTrackerAnnotation.onEnter] isn't called during the build step for
-  /// the widget that provided the callback, and [State.setState] can safely be
-  /// called within that callback.
-  void postActivate() {
-    if (_annotationIsActive)
-      RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
-  }
-
-  /// Detaches the annotation for this render object, if any.
-  ///
-  /// This is called by the [MouseRegion]'s [Element] to tell this
-  /// [RenderMouseRegion] that it will shortly be transitioned from "active"
-  /// state to "inactive". We call it here so that
-  /// [MouseTrackerAnnotation.onExit] isn't called during the build step for the
-  /// widget that provided the callback, and [State.setState] can safely be
-  /// called within that callback.
-  void preDeactivate() {
-    if (_annotationIsActive)
-      RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
+    RendererBinding.instance.mouseTracker.addListener(_handleUpdatedMouseIsConnected);
+    _markPropertyUpdated(mustRepaint: false);
   }
 
   @override
   void detach() {
-    RendererBinding.instance.mouseTracker.removeListener(_updateAnnotations);
+    RendererBinding.instance.mouseTracker.removeListener(_handleUpdatedMouseIsConnected);
     super.detach();
   }
 

packages/flutter/test/rendering/proxy_box_test.dart

@@ -467,6 +467,20 @@ void main() {
     // transform -> clip
     _testFittedBoxWithClipRectLayer();
   });
+
+  test('RenderMouseRegion can change properties when detached', () {
+    renderer.initMouseTracker(MouseTracker(
+      renderer.pointerRouter,
+      (_) => <MouseTrackerAnnotation>[],
+    ));
+    final RenderMouseRegion object = RenderMouseRegion();
+    object
+      ..opaque = false
+      ..onEnter = (_) {}
+      ..onExit = (_) {}
+      ..onHover = (_) {};
+    // Passes if no error is thrown
+  });
 }
 
 class _TestRectClipper extends CustomClipper<Rect> {

packages/flutter/test/widgets/listener_deprecated_test.dart

@@ -162,7 +162,6 @@ void main() {
           ),
         ),
       );
-      final RenderMouseRegion renderListener = tester.renderObject(find.byType(MouseRegion));
       final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
       await gesture.addPointer(location: const Offset(400.0, 300.0));
       addTearDown(gesture.removePointer);
@@ -178,7 +177,6 @@ void main() {
         ),
       ));
       expect(exit, isNull);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener.hoverAnnotation), isFalse);
     });
     testWidgets('Hover works with nested listeners', (WidgetTester tester) async {
       final UniqueKey key1 = UniqueKey();
@@ -229,9 +227,6 @@ void main() {
           ],
         ),
       );
-      final List<RenderObject> listeners = tester.renderObjectList(find.byType(MouseRegion)).toList();
-      final RenderMouseRegion renderListener1 = listeners[0] as RenderMouseRegion;
-      final RenderMouseRegion renderListener2 = listeners[1] as RenderMouseRegion;
       Offset center = tester.getCenter(find.byKey(key2));
       await gesture.moveTo(center);
       await tester.pump();
@@ -243,8 +238,6 @@ void main() {
       expect(enter1, isNotEmpty);
       expect(enter1.last.position, equals(center));
       expect(exit1, isEmpty);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
       clearLists();
 
       // Now make sure that exiting the child only triggers the child exit, not
@@ -259,8 +252,6 @@ void main() {
       expect(move1.last.position, equals(center));
       expect(enter1, isEmpty);
       expect(exit1, isEmpty);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
       clearLists();
     });
     testWidgets('Hover transfers between two listeners', (WidgetTester tester) async {
@@ -314,9 +305,6 @@ void main() {
           ],
         ),
       );
-      final List<RenderObject> listeners = tester.renderObjectList(find.byType(MouseRegion)).toList();
-      final RenderMouseRegion renderListener1 = listeners[0] as RenderMouseRegion;
-      final RenderMouseRegion renderListener2 = listeners[1] as RenderMouseRegion;
       final Offset center1 = tester.getCenter(find.byKey(key1));
       final Offset center2 = tester.getCenter(find.byKey(key2));
       await gesture.moveTo(center1);
@@ -329,8 +317,6 @@ void main() {
       expect(move2, isEmpty);
       expect(enter2, isEmpty);
       expect(exit2, isEmpty);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
       clearLists();
       await gesture.moveTo(center2);
       await tester.pump();
@@ -343,8 +329,6 @@ void main() {
       expect(enter2, isNotEmpty);
       expect(enter2.last.position, equals(center2));
       expect(exit2, isEmpty);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
       clearLists();
       await gesture.moveTo(const Offset(400.0, 450.0));
       await tester.pump();
@@ -355,8 +339,6 @@ void main() {
       expect(enter2, isEmpty);
       expect(exit2, isNotEmpty);
       expect(exit2.last.position, equals(const Offset(400.0, 450.0)));
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
       clearLists();
       await tester.pumpWidget(Container());
       expect(move1, isEmpty);
@@ -365,8 +347,6 @@ void main() {
       expect(move2, isEmpty);
       expect(enter2, isEmpty);
       expect(exit2, isEmpty);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isFalse);
-      expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isFalse);
     });
 
     testWidgets('needsCompositing set when parent class needsCompositing is set', (WidgetTester tester) async {