From a53b78ddfbdf008a14e7bf37f3376836945daca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Gawron?= Date: Fri, 24 May 2024 18:26:07 +0200 Subject: [PATCH] Fix TwoDimensionalViewport's keep alive child not always removed (when no longer should be kept alive) (#148298) - Fixes a child not removed from `_keepAliveBucket` when widget is no longer kept alive offscreen. Bug was triggering assert in performLayout. - Adds test to cover the case from bug report Fixes #138977 --- .../src/widgets/two_dimensional_viewport.dart | 4 ++ .../test/widgets/two_dimensional_utils.dart | 36 +++++++++++ .../two_dimensional_viewport_test.dart | 60 +++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart b/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart index 356a8dbab2a..403a1be71ea 100644 --- a/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart +++ b/packages/flutter/lib/src/widgets/two_dimensional_viewport.dart @@ -1649,6 +1649,10 @@ abstract class RenderTwoDimensionalViewport extends RenderBox implements RenderA _children.remove(slot); } assert(_debugTrackOrphans(noLongerOrphan: child)); + if (_keepAliveBucket[childParentData.vicinity] == child) { + _keepAliveBucket.remove(childParentData.vicinity); + } + assert(_keepAliveBucket[childParentData.vicinity] != child); dropChild(child); return; } diff --git a/packages/flutter/test/widgets/two_dimensional_utils.dart b/packages/flutter/test/widgets/two_dimensional_utils.dart index a56f4f8994d..b6beae9b139 100644 --- a/packages/flutter/test/widgets/two_dimensional_utils.dart +++ b/packages/flutter/test/widgets/two_dimensional_utils.dart @@ -511,3 +511,39 @@ class TestParentDataWidget extends ParentDataWidget { @override Type get debugTypicalAncestorWidgetClass => SimpleBuilderTableViewport; } + +class KeepAliveOnlyWhenHovered extends StatefulWidget { + const KeepAliveOnlyWhenHovered({ required this.child, super.key }); + + final Widget child; + + @override + KeepAliveOnlyWhenHoveredState createState() => KeepAliveOnlyWhenHoveredState(); +} + +class KeepAliveOnlyWhenHoveredState extends State with AutomaticKeepAliveClientMixin { + bool _hovered = false; + + @override + bool get wantKeepAlive => _hovered; + + @override + Widget build(BuildContext context) { + super.build(context); + return MouseRegion( + onEnter: (_) { + setState(() { + _hovered = true; + updateKeepAlive(); + }); + }, + onExit: (_) { + setState(() { + _hovered = false; + updateKeepAlive(); + }); + }, + child: widget.child, + ); + } +} diff --git a/packages/flutter/test/widgets/two_dimensional_viewport_test.dart b/packages/flutter/test/widgets/two_dimensional_viewport_test.dart index 70c4a6f9b7b..4c02adb5daf 100644 --- a/packages/flutter/test/widgets/two_dimensional_viewport_test.dart +++ b/packages/flutter/test/widgets/two_dimensional_viewport_test.dart @@ -731,6 +731,66 @@ void main() { ); }); + testWidgets('Ensure KeepAlive widget is not held onto when it no longer should be kept alive offscreen', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/138977 + final UniqueKey checkBoxKey = UniqueKey(); + final Widget originCell = KeepAliveOnlyWhenHovered( + key: checkBoxKey, + child: const SizedBox.square(dimension: 200), + ); + const Widget otherCell = SizedBox.square(dimension: 200, child: Placeholder()); + final ScrollController verticalController = ScrollController(); + addTearDown(verticalController.dispose); + final TwoDimensionalChildListDelegate listDelegate = TwoDimensionalChildListDelegate( + children: >[ + [originCell, otherCell, otherCell, otherCell, otherCell], + [otherCell, otherCell, otherCell, otherCell, otherCell], + [otherCell, otherCell, otherCell, otherCell, otherCell], + [otherCell, otherCell, otherCell, otherCell, otherCell], + [otherCell, otherCell, otherCell, otherCell, otherCell], + ], + ); + addTearDown(listDelegate.dispose); + + await tester.pumpWidget(simpleListTest( + delegate: listDelegate, + verticalDetails: ScrollableDetails.vertical(controller: verticalController), + )); + await tester.pumpAndSettle(); + expect(find.byKey(checkBoxKey), findsOneWidget); + + // Scroll away, should not be kept alive (disposed). + verticalController.jumpTo(verticalController.position.maxScrollExtent); + await tester.pump(); + expect(find.byKey(checkBoxKey), findsNothing); + + // Bring back into view + verticalController.jumpTo(0.0); + await tester.pump(); + expect(find.byKey(checkBoxKey), findsOneWidget); + + // Hover over widget to make it keep alive. + final TestGesture gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(location: Offset.zero); + addTearDown(gesture.removePointer); + await tester.pump(); + await gesture.moveTo(tester.getCenter(find.byKey(checkBoxKey))); + await tester.pump(); + + // Scroll away, should be kept alive still. + verticalController.jumpTo(verticalController.position.maxScrollExtent); + await tester.pump(); + expect(find.byKey(checkBoxKey), findsOneWidget); + + // Move the pointer outside the widget bounds to trigger exit event + // and remove it from keep alive bucket. + await gesture.moveTo(const Offset(300, 300)); + await tester.pump(); + expect(find.byKey(checkBoxKey), findsNothing); + }); + testWidgets('list delegate will not add automatic keep alives', (WidgetTester tester) async { final UniqueKey checkBoxKey = UniqueKey(); final Widget originCell = SizedBox.square(