Fix scrolling of multiple SliverLists and SliverGrids (#14449)

* Fix scrolling of multiple SliverLists and SliverGrids

* Update sliver_fixed_extent_list.dart

* Update sliver_grid.dart

examples/flutter_gallery/lib/demo/shrine/shrine_home.dart

@@ -84,9 +84,7 @@ class _ShrineGridLayout extends SliverGridLayout {
   }
 
   @override
-  double estimateMaxScrollOffset(int childCount) {
-    if (childCount == null)
-      return null;
+  double computeMaxScrollOffset(int childCount) {
     if (childCount == 0)
       return 0.0;
     final int rowCount = _rowAtIndex(childCount - 1) + 1;

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

@@ -87,6 +87,11 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
   /// index.
   ///
   /// By default, defers to [RenderSliverBoxChildManager.estimateMaxScrollOffset].
+  ///
+  /// See also:
+  ///
+  ///  * [computeMaxScrollOffset], which is similar but must provide a precise
+  ///    value.
   @protected
   double estimateMaxScrollOffset(SliverConstraints constraints, {
     int firstIndex,
@@ -103,6 +108,31 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
     );
   }
 
+  /// Called to obtain a precise measure of the total scrollable extents of this
+  /// object.
+  ///
+  /// Must return the precise total distance from the start of the child with
+  /// the earliest possible index to the end of the child with the last possible
+  /// index.
+  ///
+  /// This is used when no child is available for the index corresponding to the
+  /// current scroll offset, to determine the precise dimensions of the sliver.
+  /// It must return a precise value. It will not be called if the
+  /// [childManager] returns an infinite number of children for positive
+  /// indices.
+  ///
+  /// By default, multiplies the [itemExtent] by the number of children reported
+  /// by [RenderSliverBoxChildManager.childCount].
+  ///
+  /// See also:
+  ///
+  ///  * [estimateMaxScrollOffset], which is similar but may provide inaccurate
+  ///    values.
+  @protected
+  double computeMaxScrollOffset(SliverConstraints constraints, double itemExtent) {
+    return childManager.childCount * itemExtent;
+  }
+
   @override
   void performLayout() {
     childManager.didStartLayout();
@@ -137,8 +167,12 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda
 
     if (firstChild == null) {
       if (!addInitialChild(index: firstIndex, layoutOffset: indexToLayoutOffset(itemExtent, firstIndex))) {
-        // There are no children.
-        geometry = SliverGeometry.zero;
+        // There are either no children, or we are past the end of all our children.
+        final double max = computeMaxScrollOffset(constraints, itemExtent);
+        geometry = new SliverGeometry(
+          scrollExtent: max,
+          maxPaintExtent: max,
+        );
         childManager.didFinishLayout();
         return;
       }

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

@@ -115,9 +115,11 @@ abstract class SliverGridLayout {
   /// The size and position of the child with the given index.
   SliverGridGeometry getGeometryForChildIndex(int index);
 
-  /// An estimate of the scroll extent needed to fully display all the tiles if
-  /// there are `childCount` children in total.
-  double estimateMaxScrollOffset(int childCount);
+  /// The scroll extent needed to fully display all the tiles if there are
+  /// `childCount` children in total.
+  ///
+  /// The child count will never be null.
+  double computeMaxScrollOffset(int childCount);
 }
 
 /// A [SliverGridLayout] that uses equally sized and spaced tiles.
@@ -221,9 +223,8 @@ class SliverGridRegularTileLayout extends SliverGridLayout {
   }
 
   @override
-  double estimateMaxScrollOffset(int childCount) {
-    if (childCount == null)
-      return null;
+  double computeMaxScrollOffset(int childCount) {
+    assert(childCount != null);
     final int mainAxisCount = ((childCount - 1) ~/ crossAxisCount) + 1;
     final double mainAxisSpacing = mainAxisStride - childMainAxisExtent;
     return mainAxisStride * mainAxisCount - mainAxisSpacing;
@@ -539,10 +540,13 @@ class RenderSliverGrid extends RenderSliverMultiBoxAdaptor {
     double trailingScrollOffset = firstChildGridGeometry.trailingScrollOffset;
 
     if (firstChild == null) {
-      if (!addInitialChild(index: firstIndex,
-          layoutOffset: firstChildGridGeometry.scrollOffset)) {
-        // There are no children.
-        geometry = SliverGeometry.zero;
+      if (!addInitialChild(index: firstIndex, layoutOffset: firstChildGridGeometry.scrollOffset)) {
+        // There are either no children, or we are past the end of all our children.
+        final double max = layout.computeMaxScrollOffset(childManager.childCount);
+        geometry = new SliverGeometry(
+          scrollExtent: max,
+          maxPaintExtent: max,
+        );
         childManager.didFinishLayout();
         return;
       }

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

@@ -43,6 +43,10 @@ abstract class RenderSliverBoxChildManager {
   /// the [RenderSliverMultiBoxAdaptor] object if they were not created during
   /// this frame and have not yet been updated during this frame. It is not
   /// valid to add any other children to this render object.
+  ///
+  /// If this method does not create a child for a given `index` greater than or
+  /// equal to zero, then [computeMaxScrollOffset] must be able to return a
+  /// precise value.
   void createChild(int index, { @required RenderBox after });
 
   /// Remove the given child from the child list.
@@ -68,6 +72,18 @@ abstract class RenderSliverBoxChildManager {
     double trailingScrollOffset,
   });
 
+  /// Called to obtain a precise measure of the total number of children.
+  ///
+  /// Must return the number that is one greater than the greatest `index` for
+  /// which `createChild` will actually create a child.
+  ///
+  /// This is used when [createChild] cannot add a child for a positive `index`,
+  /// to determine the precise dimensions of the sliver. It must return an
+  /// accurate and precise non-null value. It will not be called if
+  /// [createChild] is always able to create a child (e.g. for an infinite
+  /// list).
+  int get childCount;
+
   /// Called during [RenderSliverMultiBoxAdaptor.adoptChild].
   ///
   /// Subclasses must ensure that the [SliverMultiBoxAdaptorParentData.index]

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

@@ -41,7 +41,8 @@ abstract class SliverChildDelegate {
   /// Returns the child with the given index.
   ///
   /// Should return null if asked to build a widget with a greater index than
-  /// exists.
+  /// exists. If this returns null, [estimatedChildCount] must subsequently
+  /// return a precise non-null value.
   ///
   /// Subclasses typically override this function and wrap their children in
   /// [AutomaticKeepAlive] and [RepaintBoundary] widgets.
@@ -54,6 +55,8 @@ abstract class SliverChildDelegate {
   ///
   /// Return null if there are an unbounded number of children or if it would
   /// be too difficult to estimate the number of children.
+  ///
+  /// This must return a precise number once [build] has returned null.
   int get estimatedChildCount => null;
 
   /// Returns an estimate of the max scroll extent for all the children.
@@ -336,8 +339,11 @@ abstract class SliverMultiBoxAdaptorWidget extends RenderObjectWidget {
   /// Subclasses should override this function if they have additional
   /// information about their max scroll extent.
   ///
-  /// The default implementation returns calls
-  /// [SliverChildDelegate.estimateMaxScrollOffset].
+  /// This is used by [SliverMultiBoxAdaptorElement] to implement part of the
+  /// [RenderSliverBoxChildManager] API.
+  ///
+  /// The default implementation defers to [delegate] via its
+  /// [SliverChildDelegate.estimateMaxScrollOffset] method.
   double estimateMaxScrollOffset(
     SliverConstraints constraints,
     int firstIndex,
@@ -592,7 +598,7 @@ class SliverGrid extends SliverMultiBoxAdaptorWidget {
       lastIndex,
       leadingScrollOffset,
       trailingScrollOffset,
-    ) ?? gridDelegate.getLayout(constraints).estimateMaxScrollOffset(delegate.estimatedChildCount);
+    ) ?? gridDelegate.getLayout(constraints).computeMaxScrollOffset(delegate.estimatedChildCount);
   }
 }
 
@@ -765,7 +771,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
     double leadingScrollOffset,
     double trailingScrollOffset,
   ) {
-    final int childCount = widget.delegate.estimatedChildCount;
+    final int childCount = this.childCount;
     if (childCount == null)
       return double.INFINITY;
     if (lastIndex == childCount - 1)
@@ -797,6 +803,9 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
     );
   }
 
+  @override
+  int get childCount => widget.delegate.estimatedChildCount;
+
   @override
   void didStartLayout() {
     assert(debugAssertChildListLocked());

packages/flutter/test/rendering/slivers_block_test.dart

@@ -52,6 +52,9 @@ class TestRenderSliverBoxChildManager extends RenderSliverBoxChildManager {
     return children.length * (trailingScrollOffset - leadingScrollOffset) / (lastIndex - firstIndex + 1);
   }
 
+  @override
+  int get childCount => children.length;
+
   @override
   void didAdoptChild(RenderBox child) {
     assert(_currentlyUpdatingChildIndex != null);

packages/flutter/test/widgets/grid_view_test.dart

@@ -453,7 +453,4 @@ void main() {
     expect(tester.getTopLeft(find.byKey(target)), const Offset(600.0, 0.0));
     expect(tester.getBottomRight(find.byKey(target)), const Offset(800.0, 200.0));
   });
-
-  // TODO(ianh): can you tap a grid cell that is slightly off the bottom of the screen?
-  // (try it with the flutter_gallery Grid demo)
 }

packages/flutter/test/widgets/slivers_test.dart

@@ -114,4 +114,85 @@ void main() {
       const Offset(0.0, 600.0),
     ], <bool>[false, false, true, true, false]);
   });
+
+  testWidgets('Multiple grids and lists', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      new Center(
+        child: new SizedBox(
+          width: 44.4,
+          height: 60.0,
+          child: new Directionality(
+            textDirection: TextDirection.ltr,
+            child: new CustomScrollView(
+              slivers: <Widget>[
+                new SliverList(
+                  delegate: new SliverChildListDelegate(
+                    <Widget>[
+                      new Container(height: 22.2, child: const Text('TOP')),
+                      new Container(height: 22.2),
+                      new Container(height: 22.2),
+                    ],
+                  ),
+                ),
+                new SliverFixedExtentList(
+                  itemExtent: 22.2,
+                  delegate: new SliverChildListDelegate(
+                    <Widget>[
+                      new Container(),
+                      new Container(child: const Text('A')),
+                      new Container(),
+                    ],
+                  ),
+                ),
+                new SliverGrid(
+                  gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
+                    crossAxisCount: 2,
+                  ),
+                  delegate: new SliverChildListDelegate(
+                    <Widget>[
+                      new Container(),
+                      new Container(child: const Text('B')),
+                      new Container(),
+                    ],
+                  ),
+                ),
+                new SliverList(
+                  delegate: new SliverChildListDelegate(
+                    <Widget>[
+                      new Container(height: 22.2),
+                      new Container(height: 22.2),
+                      new Container(height: 22.2, child: const Text('BOTTOM')),
+                    ],
+                  ),
+                ),
+              ],
+            ),
+          ),
+        ),
+      ),
+    );
+    final TestGesture gesture = await tester.startGesture(const Offset(400.0, 300.0));
+    expect(find.text('TOP'), findsOneWidget);
+    expect(find.text('A'), findsNothing);
+    expect(find.text('B'), findsNothing);
+    expect(find.text('BOTTOM'), findsNothing);
+    await gesture.moveBy(const Offset(0.0, -70.0));
+    await tester.pump();
+    expect(find.text('TOP'), findsNothing);
+    expect(find.text('A'), findsOneWidget);
+    expect(find.text('B'), findsNothing);
+    expect(find.text('BOTTOM'), findsNothing);
+    await gesture.moveBy(const Offset(0.0, -70.0));
+    await tester.pump();
+    expect(find.text('TOP'), findsNothing);
+    expect(find.text('A'), findsNothing);
+    expect(find.text('B'), findsOneWidget);
+    expect(find.text('BOTTOM'), findsNothing);
+    await gesture.moveBy(const Offset(0.0, -70.0));
+    await tester.pump();
+    expect(find.text('TOP'), findsNothing);
+    expect(find.text('A'), findsNothing);
+    expect(find.text('B'), findsNothing);
+    expect(find.text('BOTTOM'), findsOneWidget);
+  });
 }