Unverified Commit 55fd5f15 authored by Tong Mu's avatar Tong Mu Committed by GitHub

Fix mouse region crash when using closures (#37342)

This PR fixes an issue where MouseRegion crashes when being passed with closures instead of methods.

It changes how a RenderMouseRegion handles its MouseTrackingAnnotation.

Instead of creating a new annotation every time it becomes active and destroys it when deactivated, it now creates an annotation during the constructor and holds onto it until the end of its lifecycle.
Instead of directly passing the argument callbacks to the annotation, it proxies them using methods.
parent e4a909fb
...@@ -2568,15 +2568,13 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2568,15 +2568,13 @@ class RenderMouseRegion extends RenderProxyBox {
}) : _onEnter = onEnter, }) : _onEnter = onEnter,
_onHover = onHover, _onHover = onHover,
_onExit = onExit, _onExit = onExit,
_annotationIsActive = false,
super(child) { super(child) {
if (_onEnter != null || _onHover != null || _onExit != null) { _hoverAnnotation = MouseTrackerAnnotation(
_hoverAnnotation = MouseTrackerAnnotation( onEnter: _handleEnter,
onEnter: _onEnter, onHover: _handleHover,
onHover: _onHover, onExit: _handleExit,
onExit: _onExit, );
);
}
_mouseIsConnected = RendererBinding.instance.mouseTracker.mouseIsConnected;
} }
/// Called when a hovering pointer enters the region for this widget. /// Called when a hovering pointer enters the region for this widget.
...@@ -2591,6 +2589,10 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2591,6 +2589,10 @@ class RenderMouseRegion extends RenderProxyBox {
} }
} }
PointerEnterEventListener _onEnter; PointerEnterEventListener _onEnter;
void _handleEnter(PointerEnterEvent event) {
if (_onEnter != null)
_onEnter(event);
}
/// Called when a pointer that has not triggered an [onPointerDown] changes /// Called when a pointer that has not triggered an [onPointerDown] changes
/// position. /// position.
...@@ -2604,6 +2606,10 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2604,6 +2606,10 @@ class RenderMouseRegion extends RenderProxyBox {
} }
} }
PointerHoverEventListener _onHover; PointerHoverEventListener _onHover;
void _handleHover(PointerHoverEvent event) {
if (_onHover != null)
_onHover(event);
}
/// Called when a hovering pointer leaves the region for this widget. /// Called when a hovering pointer leaves the region for this widget.
/// ///
...@@ -2617,6 +2623,10 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2617,6 +2623,10 @@ class RenderMouseRegion extends RenderProxyBox {
} }
} }
PointerExitEventListener _onExit; PointerExitEventListener _onExit;
void _handleExit(PointerExitEvent event) {
if (_onExit != null)
_onExit(event);
}
// Object used for annotation of the layer used for hover hit detection. // Object used for annotation of the layer used for hover hit detection.
MouseTrackerAnnotation _hoverAnnotation; MouseTrackerAnnotation _hoverAnnotation;
...@@ -2629,45 +2639,22 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2629,45 +2639,22 @@ class RenderMouseRegion extends RenderProxyBox {
MouseTrackerAnnotation get hoverAnnotation => _hoverAnnotation; MouseTrackerAnnotation get hoverAnnotation => _hoverAnnotation;
void _updateAnnotations() { void _updateAnnotations() {
assert(_hoverAnnotation == null || _onEnter != _hoverAnnotation.onEnter || _onHover != _hoverAnnotation.onHover || _onExit != _hoverAnnotation.onExit, final bool annotationWasActive = _annotationIsActive;
"Shouldn't call _updateAnnotations if nothing has changed."); final bool annotationWillBeActive = (
bool changed = false; _onEnter != null ||
final bool hadHoverAnnotation = _hoverAnnotation != null; _onHover != null ||
if (_hoverAnnotation != null && attached) { _onExit != null
RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation); ) &&
changed = true; RendererBinding.instance.mouseTracker.mouseIsConnected;
} if (annotationWasActive != annotationWillBeActive) {
if (_onEnter != null || _onHover != null || _onExit != null) {
_hoverAnnotation = MouseTrackerAnnotation(
onEnter: _onEnter,
onHover: _onHover,
onExit: _onExit,
);
if (attached) {
RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
changed = true;
}
} else {
_hoverAnnotation = null;
}
if (changed) {
markNeedsPaint(); markNeedsPaint();
}
final bool hasHoverAnnotation = _hoverAnnotation != null;
if (hadHoverAnnotation != hasHoverAnnotation) {
markNeedsCompositingBitsUpdate(); markNeedsCompositingBitsUpdate();
} if (annotationWillBeActive) {
} RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
} else {
bool _mouseIsConnected; RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
void _handleMouseTrackerChanged() {
final bool newState = RendererBinding.instance.mouseTracker.mouseIsConnected;
if (newState != _mouseIsConnected) {
_mouseIsConnected = newState;
if (_hoverAnnotation != null) {
markNeedsCompositingBitsUpdate();
markNeedsPaint();
} }
_annotationIsActive = annotationWillBeActive;
} }
} }
...@@ -2675,52 +2662,50 @@ class RenderMouseRegion extends RenderProxyBox { ...@@ -2675,52 +2662,50 @@ class RenderMouseRegion extends RenderProxyBox {
void attach(PipelineOwner owner) { void attach(PipelineOwner owner) {
super.attach(owner); super.attach(owner);
// Add a listener to listen for changes in mouseIsConnected. // Add a listener to listen for changes in mouseIsConnected.
RendererBinding.instance.mouseTracker.addListener(_handleMouseTrackerChanged); RendererBinding.instance.mouseTracker.addListener(_updateAnnotations);
postActivate(); _updateAnnotations();
} }
/// Attaches the annotation for this render object, if any. /// Attaches the annotation for this render object, if any.
/// ///
/// This is called by [attach] to attach any new annotations. /// This is called by the [MouseRegion]'s [Element] to tell this
/// /// [RenderMouseRegion] that it has transitioned from "inactive"
/// This is also called by the [Listener]'s [Element] to tell this /// state to "active". We call it here so that
/// [RenderPointerListener] that it will shortly be attached. That way,
/// [MouseTrackerAnnotation.onEnter] isn't called during the build step for /// [MouseTrackerAnnotation.onEnter] isn't called during the build step for
/// the widget that provided the callback, and [State.setState] can safely be /// the widget that provided the callback, and [State.setState] can safely be
/// called within that callback. /// called within that callback.
void postActivate() { void postActivate() {
if (_hoverAnnotation != null) { if (_annotationIsActive)
RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation); RendererBinding.instance.mouseTracker.attachAnnotation(_hoverAnnotation);
}
} }
/// Detaches the annotation for this render object, if any. /// Detaches the annotation for this render object, if any.
/// ///
/// This is called by the [Listener]'s [Element] to tell this /// This is called by the [MouseRegion]'s [Element] to tell this
/// [RenderPointerListener] that it will shortly be attached. That way, /// [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 /// [MouseTrackerAnnotation.onExit] isn't called during the build step for the
/// widget that provided the callback, and [State.setState] can safely be /// widget that provided the callback, and [State.setState] can safely be
/// called within that callback. /// called within that callback.
void preDeactivate() { void preDeactivate() {
if (_hoverAnnotation != null) { if (_annotationIsActive)
RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation); RendererBinding.instance.mouseTracker.detachAnnotation(_hoverAnnotation);
}
} }
@override @override
void detach() { void detach() {
RendererBinding.instance.mouseTracker.removeListener(_handleMouseTrackerChanged); RendererBinding.instance.mouseTracker.removeListener(_updateAnnotations);
super.detach(); super.detach();
} }
bool get _hasActiveAnnotation => _hoverAnnotation != null && _mouseIsConnected; bool _annotationIsActive;
@override @override
bool get needsCompositing => super.needsCompositing || _hasActiveAnnotation; bool get needsCompositing => super.needsCompositing || _annotationIsActive;
@override @override
void paint(PaintingContext context, Offset offset) { void paint(PaintingContext context, Offset offset) {
if (_hasActiveAnnotation) { if (_annotationIsActive) {
final AnnotatedRegionLayer<MouseTrackerAnnotation> layer = AnnotatedRegionLayer<MouseTrackerAnnotation>( final AnnotatedRegionLayer<MouseTrackerAnnotation> layer = AnnotatedRegionLayer<MouseTrackerAnnotation>(
_hoverAnnotation, _hoverAnnotation,
size: size, size: size,
......
...@@ -297,7 +297,7 @@ void main() { ...@@ -297,7 +297,7 @@ void main() {
), ),
); );
await tester.tap(find.text('PUSH')); await tester.tap(find.text('PUSH'));
expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 3); expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2);
expect(find.text('PUSH'), findsNothing); expect(find.text('PUSH'), findsNothing);
expect(find.text('HELLO'), findsOneWidget); expect(find.text('HELLO'), findsOneWidget);
final Offset helloPosition1 = tester.getCenter(find.text('HELLO')); final Offset helloPosition1 = tester.getCenter(find.text('HELLO'));
...@@ -342,7 +342,7 @@ void main() { ...@@ -342,7 +342,7 @@ void main() {
expect(helloPosition3.dy, helloPosition4.dy); expect(helloPosition3.dy, helloPosition4.dy);
await gesture.moveBy(const Offset(500.0, 0.0)); await gesture.moveBy(const Offset(500.0, 0.0));
await gesture.up(); await gesture.up();
expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 3); expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 2);
expect(find.text('PUSH'), findsOneWidget); expect(find.text('PUSH'), findsOneWidget);
expect(find.text('HELLO'), findsNothing); expect(find.text('HELLO'), findsNothing);
}); });
......
...@@ -276,6 +276,7 @@ void main() { ...@@ -276,6 +276,7 @@ void main() {
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer); addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(400.0, 0.0)); await gesture.moveTo(const Offset(400.0, 0.0));
addTearDown(gesture.removePointer);
await tester.pump(); await tester.pump();
await tester.pumpWidget( await tester.pumpWidget(
Column( Column(
...@@ -424,6 +425,7 @@ void main() { ...@@ -424,6 +425,7 @@ void main() {
expect(bottomLeft.dy - topLeft.dy, scaleFactor * localHeight); expect(bottomLeft.dy - topLeft.dy, scaleFactor * localHeight);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.addPointer(); await gesture.addPointer();
addTearDown(gesture.removePointer); addTearDown(gesture.removePointer);
await gesture.moveTo(topLeft - const Offset(1, 1)); await gesture.moveTo(topLeft - const Offset(1, 1));
...@@ -452,6 +454,7 @@ void main() { ...@@ -452,6 +454,7 @@ void main() {
testWidgets('needsCompositing updates correctly and is respected', (WidgetTester tester) async { testWidgets('needsCompositing updates correctly and is respected', (WidgetTester tester) async {
// Pretend that we have a mouse connected. // Pretend that we have a mouse connected.
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.addPointer(); await gesture.addPointer();
addTearDown(gesture.removePointer); addTearDown(gesture.removePointer);
...@@ -497,6 +500,7 @@ void main() { ...@@ -497,6 +500,7 @@ void main() {
testWidgets("Callbacks aren't called during build", (WidgetTester tester) async { testWidgets("Callbacks aren't called during build", (WidgetTester tester) async {
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.addPointer(); await gesture.addPointer();
addTearDown(gesture.removePointer); addTearDown(gesture.removePointer);
...@@ -537,6 +541,7 @@ void main() { ...@@ -537,6 +541,7 @@ void main() {
testWidgets("MouseRegion activate/deactivate don't duplicate annotations", (WidgetTester tester) async { testWidgets("MouseRegion activate/deactivate don't duplicate annotations", (WidgetTester tester) async {
final GlobalKey feedbackKey = GlobalKey(); final GlobalKey feedbackKey = GlobalKey();
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.addPointer(); await gesture.addPointer();
addTearDown(gesture.removePointer); addTearDown(gesture.removePointer);
...@@ -622,6 +627,24 @@ void main() { ...@@ -622,6 +627,24 @@ void main() {
expect(exit.single.position, const Offset(400.0, 300.0)); expect(exit.single.position, const Offset(400.0, 300.0));
expect(exit.single.delta, const Offset(0.0, 0.0)); expect(exit.single.delta, const Offset(0.0, 0.0));
}); });
testWidgets('detects pointer enter with closure arguments', (WidgetTester tester) async {
await tester.pumpWidget(_HoverClientWithClosures());
expect(find.text('not hovering'), findsOneWidget);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.addPointer();
// Move to a position out of MouseRegion
await gesture.moveTo(tester.getBottomRight(find.byType(MouseRegion)) + const Offset(10, -10));
await tester.pumpAndSettle();
expect(find.text('not hovering'), findsOneWidget);
// Move into MouseRegion
await gesture.moveBy(const Offset(-20, 0));
await tester.pumpAndSettle();
expect(find.text('HOVERING'), findsOneWidget);
});
}); });
group('MouseRegion paints child once and only once', () { group('MouseRegion paints child once and only once', () {
...@@ -712,3 +735,25 @@ class _PaintCallbackObject extends RenderProxyBox { ...@@ -712,3 +735,25 @@ class _PaintCallbackObject extends RenderProxyBox {
super.paint(context, offset); super.paint(context, offset);
} }
} }
class _HoverClientWithClosures extends StatefulWidget {
@override
_HoverClientWithClosuresState createState() => _HoverClientWithClosuresState();
}
class _HoverClientWithClosuresState extends State<_HoverClientWithClosures> {
bool _hovering = false;
@override
Widget build(BuildContext context) {
return Directionality(
textDirection: TextDirection.ltr,
child: MouseRegion(
onEnter: (PointerEnterEvent _) { setState(() { _hovering = true; }); },
onExit: (PointerExitEvent _) { setState(() { _hovering = false; }); },
child: Text(_hovering ? 'HOVERING' : 'not hovering'),
),
);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment