Let scrollOffsetCorrection on small lists not leave the list stuck in overscroll (#17599)

packages/flutter/lib/src/cupertino/refresh.dart

@@ -282,7 +282,8 @@ class CupertinoRefreshControl extends StatefulWidget {
 
   /// The amount of overscroll the scrollable must be dragged to trigger a reload.
   ///
-  /// Must not be null, must be larger than 0.0 and larger than [refreshIndicatorExtent].
+  /// Must not be null, must be larger than 0.0 and larger than
+  /// [refreshIndicatorExtent]. Defaults to 100px when not specified.
   ///
   /// When overscrolled past this distance, [onRefresh] will be called if not
   /// null and the [builder] will build in the [RefreshIndicatorMode.armed] state.
@@ -293,6 +294,7 @@ class CupertinoRefreshControl extends StatefulWidget {
   ///
   /// Must not be null and must be positive, but can be 0.0, in which case the
   /// sliver will start retracting back to 0.0 as soon as the refresh is started.
+  /// Defaults to 60px when not specified.
   ///
   /// Must be smaller than [refreshTriggerPullDistance], since the sliver
   /// shouldn't grow further after triggering the refresh.

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

@@ -150,10 +150,20 @@ abstract class ViewportOffset extends ChangeNotifier {
   /// [RenderViewport], before [applyContentDimensions]. After this method is
   /// called, the layout will be recomputed and that may result in this method
   /// being called again, though this should be very rare.
+  ///
+  /// See also:
+  ///
+  ///  * [jumpTo], for also changing the scroll position when not in layout.
+  ///    [jumpTo] applies the change immediately and notifies its listeners.
   void correctBy(double correction);
 
   /// Jumps the scroll position from its current value to the given value,
   /// without animation, and without checking if the new value is in range.
+  ///
+  /// See also:
+  ///
+  ///  * [correctBy], for changing the current offset in the middle of layout
+  ///    and that defers the notification of its listeners until after layout.
   void jumpTo(double pixels);
 
   /// The direction in which the user is trying to change [pixels], relative to

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

@@ -250,13 +250,43 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// To force the [pixels] to a particular value without honoring the normal
   /// conventions for changing the scroll offset, consider [forcePixels]. (But
   /// see the discussion there for why that might still be a bad idea.)
+  ///
+  /// See also:
+  ///
+  ///  * The method [correctBy], which is a method of [ViewportOffset] used
+  ///    by viewport render objects to correct the offset during layout
+  ///    without notifying its listeners.
+  ///  * The method [jumpTo], for making changes to position while not in the
+  ///    middle of layout and applying the new position immediately.
+  ///  * The method [animateTo], which is like [jumpTo] but animating to the
+  ///    distination offset.
   void correctPixels(double value) {
     _pixels = value;
   }
 
+  /// Apply a layout-time correction to the scroll offset.
+  ///
+  /// This method should change the [pixels] value by `correction`, but without
+  /// calling [notifyListeners]. It is called during layout by the
+  /// [RenderViewport], before [applyContentDimensions]. After this method is
+  /// called, the layout will be recomputed and that may result in this method
+  /// being called again, though this should be very rare.
+  ///
+  /// See also:
+  ///
+  ///  * [jumpTo], for also changing the scroll position when not in layout.
+  ///    [jumpTo] applies the change immediately and notifies its listeners.
+  ///  * [correctPixels], which is used by the [ScrollPosition] itself to
+  ///    set the offset initially during construction or after
+  ///    [applyViewportDimension] or [applyContentDimensions] is called.
   @override
   void correctBy(double correction) {
+    assert(
+      _pixels != null,
+      'An initial pixels value must exist by caling correctPixels on the ScrollPosition',
+    );
     _pixels += correction;
+    _didChangeViewportDimensionOrReceiveCorrection = true;
   }
 
   /// Change the value of [pixels] to the new value, and notify any customers,
@@ -360,13 +390,13 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
     return result;
   }
 
-  bool _didChangeViewportDimension = true;
+  bool _didChangeViewportDimensionOrReceiveCorrection = true;
 
   @override
   bool applyViewportDimension(double viewportDimension) {
     if (_viewportDimension != viewportDimension) {
       _viewportDimension = viewportDimension;
-      _didChangeViewportDimension = true;
+      _didChangeViewportDimensionOrReceiveCorrection = true;
       // If this is called, you can rely on applyContentDimensions being called
       // soon afterwards in the same layout phase. So we put all the logic that
       // relies on both values being computed into applyContentDimensions.
@@ -419,12 +449,12 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) {
     if (!nearEqual(_minScrollExtent, minScrollExtent, Tolerance.defaultTolerance.distance) ||
         !nearEqual(_maxScrollExtent, maxScrollExtent, Tolerance.defaultTolerance.distance) ||
-        _didChangeViewportDimension) {
+        _didChangeViewportDimensionOrReceiveCorrection) {
       _minScrollExtent = minScrollExtent;
       _maxScrollExtent = maxScrollExtent;
       _haveDimensions = true;
       applyNewDimensions();
-      _didChangeViewportDimension = false;
+      _didChangeViewportDimensionOrReceiveCorrection = false;
     }
     return true;
   }

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

@@ -84,11 +84,6 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc
     return super.setPixels(newPixels);
   }
 
-  @override
-  void correctBy(double correction) {
-    correctPixels(pixels + correction);
-  }
-
   @override
   void absorb(ScrollPosition other) {
     super.absorb(other);

packages/flutter/test/cupertino/refresh_test.dart

@@ -62,6 +62,7 @@ void main() {
     when(mockHelper.refreshTask()).thenAnswer((_) => refreshCompleter.future);
   });
 
+  int testListLength = 10;
   SliverList buildAListOfStuff() {
     return new SliverList(
       delegate: new SliverChildBuilderDelegate(
@@ -71,12 +72,12 @@ void main() {
             child: new Center(child: new Text(index.toString())),
           );
         },
-        childCount: 10,
+        childCount: testListLength,
       ),
     );
   }
 
-  group('UI tests', () {
+  final Function uiTestGroup = () {
     testWidgets("doesn't invoke anything without user interaction", (WidgetTester tester) async {
       debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
 
@@ -682,6 +683,11 @@ void main() {
     testWidgets(
       'sliver scrolled away when task completes properly removes itself',
       (WidgetTester tester) async {
+        if (testListLength < 4) {
+          // This test only makes sense when the list is long enough that
+          // the indicator can be scrolled away while refreshing.
+          return;
+        }
         debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
 
         refreshIndicator = const Center(child: const Text('-1'));
@@ -849,11 +855,9 @@ void main() {
         debugDefaultTargetPlatformOverride = null;
       }
     );
-  });
+  };
 
-  // Test the internal state machine directly to make sure the UI aren't just
+  final Function stateMachineTestGroup = () {
-  // correct by coincidence.
-  group('state machine test', () {
     testWidgets('starts in inactive state', (WidgetTester tester) async {
       debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
 
@@ -1228,7 +1232,22 @@ void main() {
         debugDefaultTargetPlatformOverride = null;
       }
     );
-  });
+  };
+
+  group('UI tests long list', uiTestGroup);
+
+  // Test the internal state machine directly to make sure the UI aren't just
+  // correct by coincidence.
+  group('state machine test long list', stateMachineTestGroup);
+
+  // Retest everything and make sure that it still works when the whole list
+  // is smaller than the viewport size.
+  testListLength = 2;
+  group('UI tests short list', uiTestGroup);
+
+  // Test the internal state machine directly to make sure the UI aren't just
+  // correct by coincidence.
+  group('state machine test short list', stateMachineTestGroup);
 }
 
 class MockHelper extends Mock {