Fixes RenderSliverFixedExtentBoxAdaptor correctly calculates leadingGarbage and trailingGarbage. (#36302)
diff --git a/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart b/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart
index 221897f..131cf38 100644
--- a/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart
+++ b/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart
@@ -142,6 +142,26 @@
return childManager.childCount * itemExtent;
}
+ int _calculateLeadingGarbage(int firstIndex) {
+ RenderBox walker = firstChild;
+ int leadingGarbage = 0;
+ while(walker != null && indexOf(walker) < firstIndex){
+ leadingGarbage += 1;
+ walker = childAfter(walker);
+ }
+ return leadingGarbage;
+ }
+
+ int _calculateTrailingGarbage(int targetLastIndex) {
+ RenderBox walker = lastChild;
+ int trailingGarbage = 0;
+ while(walker != null && indexOf(walker) > targetLastIndex){
+ trailingGarbage += 1;
+ walker = childBefore(walker);
+ }
+ return trailingGarbage;
+ }
+
@override
void performLayout() {
childManager.didStartLayout();
@@ -165,10 +185,8 @@
getMaxChildIndexForScrollOffset(targetEndScrollOffset, itemExtent) : null;
if (firstChild != null) {
- final int oldFirstIndex = indexOf(firstChild);
- final int oldLastIndex = indexOf(lastChild);
- final int leadingGarbage = (firstIndex - oldFirstIndex).clamp(0, childCount);
- final int trailingGarbage = targetLastIndex == null ? 0 : (oldLastIndex - targetLastIndex).clamp(0, childCount);
+ final int leadingGarbage = _calculateLeadingGarbage(firstIndex);
+ final int trailingGarbage = _calculateTrailingGarbage(targetLastIndex);
collectGarbage(leadingGarbage, trailingGarbage);
} else {
collectGarbage(0, 0);
diff --git a/packages/flutter/test/widgets/slivers_test.dart b/packages/flutter/test/widgets/slivers_test.dart
index e2c4c93..44a942b 100644
--- a/packages/flutter/test/widgets/slivers_test.dart
+++ b/packages/flutter/test/widgets/slivers_test.dart
@@ -25,6 +25,37 @@
);
}
+Future<void> testSliverFixedExtentList(WidgetTester tester, List<String> items) {
+ return tester.pumpWidget(
+ Directionality(
+ textDirection: TextDirection.ltr,
+ child: CustomScrollView(
+ slivers: <Widget>[
+ SliverFixedExtentList(
+ itemExtent: 900,
+ delegate: SliverChildBuilderDelegate(
+ (BuildContext context, int index) {
+ return Center(
+ key: ValueKey<String>(items[index]),
+ child: KeepAlive(
+ items[index],
+ )
+ );
+ },
+ childCount : items.length,
+ findChildIndexCallback: (Key key) {
+ final ValueKey<String> valueKey = key;
+ final String data = valueKey.value;
+ return items.indexOf(data);
+ }
+ ),
+ ),
+ ],
+ ),
+ ),
+ );
+}
+
void verify(WidgetTester tester, List<Offset> idealPositions, List<bool> idealVisibles) {
final List<Offset> actualPositions = tester.renderObjectList<RenderBox>(find.byType(SizedBox, skipOffstage: false)).map<Offset>(
(RenderBox target) => target.localToGlobal(const Offset(0.0, 0.0))
@@ -196,6 +227,47 @@
expect(find.text('BOTTOM'), findsOneWidget);
});
+ testWidgets('SliverFixedExtentList correctly clears garbage', (WidgetTester tester) async {
+ final List<String> items = <String>['1', '2', '3', '4', '5', '6'];
+ await testSliverFixedExtentList(tester, items);
+ // Keep alive widgets require 1 frame to notify their parents. Pumps in between
+ // drags to ensure widgets are kept alive.
+ await tester.drag(find.byType(CustomScrollView),const Offset(0.0, -1200.0));
+ await tester.pump();
+ await tester.drag(find.byType(CustomScrollView),const Offset(0.0, -1200.0));
+ await tester.pump();
+ await tester.drag(find.byType(CustomScrollView),const Offset(0.0, -800.0));
+ await tester.pump();
+ expect(find.text('1'), findsNothing);
+ expect(find.text('2'), findsNothing);
+ expect(find.text('3'), findsNothing);
+ expect(find.text('4'), findsOneWidget);
+ expect(find.text('5'), findsOneWidget);
+ // Indexes [0, 1, 2] are kept alive and [3, 4] are in viewport, thus the sliver
+ // will need to keep updating the elements at these indexes whenever a rebuild is
+ // triggered. The current child list in RenderSliverFixedExtentList is
+ // '4' -> '5' -> null.
+ //
+ // With the insertion below, all items will get shifted back 1 position. The sliver
+ // will have to update indexes [0, 1, 2, 3, 4, 5]. Since this is the first time
+ // item '0' gets initialized, mounting the element will cause it to attach to
+ // child list in RenderSliverFixedExtentList. This will create a gap.
+ // '0' -> '4' -> '5' -> null.
+ items.insert(0, '0');
+ await testSliverFixedExtentList(tester, items);
+ // Sliver should collect leading and trailing garbage correctly.
+ //
+ // The child list update should occur in following order.
+ // '0' -> '4' -> '5' -> null Started with Original list.
+ // '4' -> null Removed 1 leading garbage and 1 trailing garbage.
+ // '3' -> '4' -> null Prepended '3' because viewport is still at [3, 4].
+ expect(find.text('0'), findsNothing);
+ expect(find.text('1'), findsNothing);
+ expect(find.text('2'), findsNothing);
+ expect(find.text('3'), findsOneWidget);
+ expect(find.text('4'), findsOneWidget);
+ });
+
testWidgets('SliverGrid Correctly layout children after rearranging', (WidgetTester tester) async {
await tester.pumpWidget(const TestSliverGrid(
<Widget>[
@@ -332,3 +404,23 @@
);
}
}
+
+class KeepAlive extends StatefulWidget {
+ const KeepAlive(this.data);
+
+ final String data;
+
+ @override
+ KeepAliveState createState() => KeepAliveState();
+}
+
+class KeepAliveState extends State<KeepAlive> with AutomaticKeepAliveClientMixin {
+ @override
+ bool get wantKeepAlive => true;
+
+ @override
+ Widget build(BuildContext context) {
+ super.build(context);
+ return Text(widget.data);
+ }
+}
