Revert "Fix RangeMaintainingScrollPhysics" (#63611)

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

@@ -412,45 +412,6 @@ class ScrollPhysics {
 /// These physics should be combined with other scroll physics, e.g.
 /// [BouncingScrollPhysics] or [ClampingScrollPhysics], to obtain a complete
 /// description of typical scroll physics. See [applyTo].
-///
-/// ## Implementation details
-///
-/// Specifically, these physics perform two adjustments.
-///
-/// The first is to maintain overscroll when the position is out of range.
-///
-/// The second is to enforce the boundary when the position is in range.
-///
-/// If the current velocity is zero, neither adjustment is made. The
-/// assumption is that there is an ongoing animation and therefore
-/// further changing the scroll position would disrupt the experience.
-///
-/// If the extents haven't changed, then the overscroll adjustment is
-/// not made. The assumption is that if the position is overscrolled,
-/// it is intentional, otherwise the position could not have reached
-/// that position. (Consider [ClampingScrollPhysics] vs
-/// [BouncingScrollPhysics] for example.)
-///
-/// If the position itself changed since the last animation frame,
-/// then the overscroll is not maintained. The assumption is similar
-/// to the previous case: the position would not have been placed out
-/// of range unless it was intentional.
-///
-/// In addition, if the position changed and the boundaries were and
-/// still are finite, then the boundary isn't enforced either, for
-/// the same reason. However, if any of the boundaries were or are
-/// now infinite, the boundary _is_ enforced, on the assumption that
-/// infinite boundaries indicate a lazy-loading scroll view, which
-/// cannot enforce boundaries while the full list has not loaded.
-///
-/// If the range was out of range, then the boundary is not enforced
-/// even if the range is not maintained. If the range is maintained,
-/// then the distance between the old position and the old boundary is
-/// applied to the new boundary to obtain the new position.
-///
-/// If the range was in range, and the boundary is to be enforced,
-/// then the new position is obtained by deferring to the other physics,
-/// if any, and then clamped to the new range.
 class RangeMaintainingScrollPhysics extends ScrollPhysics {
   /// Creates scroll physics that maintain the scroll position in range.
   const RangeMaintainingScrollPhysics({ ScrollPhysics parent }) : super(parent: parent);
@@ -467,60 +428,18 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics {
     @required bool isScrolling,
     @required double velocity,
   }) {
-    bool maintainOverscroll = true;
+    if (velocity != 0.0 || ((oldPosition.minScrollExtent == newPosition.minScrollExtent) && (oldPosition.maxScrollExtent == newPosition.maxScrollExtent))) {
-    bool enforceBoundary = true;
+      return super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity);
-    if (velocity != 0.0) {
-      // Don't try to adjust an animating position, the jumping around
-      // would be distracting.
-      maintainOverscroll = false;
-      enforceBoundary = false;
     }
-    if ((oldPosition.minScrollExtent == newPosition.minScrollExtent) &&
+    if (oldPosition.pixels < oldPosition.minScrollExtent) {
-        (oldPosition.maxScrollExtent == newPosition.maxScrollExtent)) {
+      final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels;
-      // If the extents haven't changed then ignore overscroll.
+      return newPosition.minScrollExtent - oldDelta;
-      maintainOverscroll = false;
     }
-    if (oldPosition.pixels != newPosition.pixels) {
+    if (oldPosition.pixels > oldPosition.maxScrollExtent) {
-      // If the position has been changed already, then it might have
+      final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent;
-      // been adjusted to expect new overscroll, so don't try to
+      return newPosition.maxScrollExtent + oldDelta;
-      // maintain the relative overscroll.
-      maintainOverscroll = false;
-      if (oldPosition.minScrollExtent.isFinite && oldPosition.maxScrollExtent.isFinite &&
-          newPosition.minScrollExtent.isFinite && newPosition.maxScrollExtent.isFinite) {
-        // In addition, if the position changed then we only enforce
-        // the new boundary if the previous boundary was not entirely
-        // finite. A common case where the position changes while one
-        // of the extents is infinite is a lazily-loaded list. (If the
-        // boundaries were finite, and the position changed, then we
-        // assume it was intentional.)
-        enforceBoundary = false;
-      }
     }
-    if ((oldPosition.pixels < oldPosition.minScrollExtent) &&
+    return newPosition.pixels.clamp(newPosition.minScrollExtent, newPosition.maxScrollExtent) as double;
-        (oldPosition.pixels > oldPosition.maxScrollExtent)) {
-      // If the old position was out of range, then we should
-      // not try to keep the new position in range.
-      enforceBoundary = false;
-    }
-    if (maintainOverscroll) {
-      // Force the new position to be no more out of range
-      // than it was before, if it was overscrolled.
-      if (oldPosition.pixels < oldPosition.minScrollExtent) {
-        final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels;
-        return newPosition.minScrollExtent - oldDelta;
-      }
-      if (oldPosition.pixels > oldPosition.maxScrollExtent) {
-        final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent;
-        return newPosition.maxScrollExtent + oldDelta;
-      }
-    }
-    // If we're not forcing the overscroll, defer to other physics.
-    double result = super.adjustPositionForNewDimensions(oldPosition: oldPosition, newPosition: newPosition, isScrolling: isScrolling, velocity: velocity);
-    if (enforceBoundary) {
-      // ...but if they put us out of range then reinforce the boundary.
-      result = result.clamp(newPosition.minScrollExtent, newPosition.maxScrollExtent) as double;
-    }
-    return result;
   }
 }
 

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

@@ -226,7 +226,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// If there is any overscroll, it is reported using [didOverscrollBy].
   double setPixels(double newPixels) {
     assert(_pixels != null);
-    assert(SchedulerBinding.instance.schedulerPhase != SchedulerPhase.persistentCallbacks, 'A scrollable\'s position should not change during the build, layout, and paint phases, otherwise the rendering will be confused.');
+    assert(SchedulerBinding.instance.schedulerPhase.index <= SchedulerPhase.transientCallbacks.index);
     if (newPixels != pixels) {
       final double overscroll = applyBoundaryConditions(newPixels);
       assert(() {
@@ -471,37 +471,27 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
     return true;
   }
 
-  bool _pendingDimensions = false;
-  ScrollMetrics _lastMetrics;
-
   @override
   bool applyContentDimensions(double minScrollExtent, double maxScrollExtent) {
     assert(minScrollExtent != null);
     assert(maxScrollExtent != null);
-    assert(haveDimensions == (_lastMetrics != null));
     if (!nearEqual(_minScrollExtent, minScrollExtent, Tolerance.defaultTolerance.distance) ||
         !nearEqual(_maxScrollExtent, maxScrollExtent, Tolerance.defaultTolerance.distance) ||
         _didChangeViewportDimensionOrReceiveCorrection) {
       assert(minScrollExtent != null);
       assert(maxScrollExtent != null);
       assert(minScrollExtent <= maxScrollExtent);
+      final ScrollMetrics oldPosition = haveDimensions ? copyWith() : null;
       _minScrollExtent = minScrollExtent;
       _maxScrollExtent = maxScrollExtent;
-      final ScrollMetrics currentMetrics = haveDimensions ? copyWith() : null;
+      final ScrollMetrics newPosition = haveDimensions ? copyWith() : null;
       _didChangeViewportDimensionOrReceiveCorrection = false;
-      _pendingDimensions = true;
+      if (haveDimensions && !correctForNewDimensions(oldPosition, newPosition))
-      if (haveDimensions && !correctForNewDimensions(_lastMetrics, currentMetrics)) {
         return false;
-      }
       _haveDimensions = true;
-    }
-    assert(haveDimensions);
-    if (_pendingDimensions) {
       applyNewDimensions();
-      _pendingDimensions = false;
     }
     assert(!_didChangeViewportDimensionOrReceiveCorrection, 'Use correctForNewDimensions() (and return true) to change the scroll offset during applyContentDimensions().');
-    _lastMetrics = copyWith();
     return true;
   }
 
@@ -556,7 +546,6 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   @mustCallSuper
   void applyNewDimensions() {
     assert(pixels != null);
-    assert(_pendingDimensions);
     activity.applyNewDimensions();
     _updateSemanticActions(); // will potentially request a semantics update.
   }

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

@@ -694,7 +694,7 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
         key: _scrollSemanticsKey,
         child: result,
         position: position,
-        allowImplicitScrolling: _physics.allowImplicitScrolling,
+        allowImplicitScrolling: widget?.physics?.allowImplicitScrolling ?? _physics.allowImplicitScrolling,
         semanticChildCount: widget.semanticChildCount,
       );
     }
@@ -706,7 +706,6 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
   void debugFillProperties(DiagnosticPropertiesBuilder properties) {
     super.debugFillProperties(properties);
     properties.add(DiagnosticsProperty<ScrollPosition>('position', position));
-    properties.add(DiagnosticsProperty<ScrollPhysics>('effective physics', _physics));
   }
 
   @override
@@ -974,7 +973,7 @@ class ScrollAction extends Action<ScrollIntent> {
     assert(state.position.viewportDimension != null);
     assert(state.position.maxScrollExtent != null);
     assert(state.position.minScrollExtent != null);
-    assert(state._physics == null || state._physics.shouldAcceptUserOffset(state.position));
+    assert(state.widget.physics == null || state.widget.physics.shouldAcceptUserOffset(state.position));
     if (state.widget.incrementCalculator != null) {
       return state.widget.incrementCalculator(
         ScrollIncrementDetails(
@@ -1063,7 +1062,7 @@ class ScrollAction extends Action<ScrollIntent> {
     assert(state.position.minScrollExtent != null);
 
     // Don't do anything if the user isn't allowed to scroll.
-    if (state._physics != null && !state._physics.shouldAcceptUserOffset(state.position)) {
+    if (state.widget.physics != null && !state.widget.physics.shouldAcceptUserOffset(state.position)) {
       return;
     }
     final double increment = _getIncrement(state, intent);

packages/flutter/test/widgets/range_maintaining_scroll_physics_test.dart

@@ -219,45 +219,4 @@ void main() {
     expect(position.maxScrollExtent, 100.0);
     expect(position.pixels, 0.0);
   });
-
-  testWidgets('expanding page views', (WidgetTester tester) async {
-    await tester.pumpWidget(Padding(padding: const EdgeInsets.only(right: 200.0), child: TabBarDemo()));
-    await tester.tap(find.text('bike'));
-    await tester.pump();
-    await tester.pump(const Duration(seconds: 1));
-    final Rect bike1 = tester.getRect(find.byIcon(Icons.directions_bike));
-    await tester.pumpWidget(Padding(padding: EdgeInsets.zero, child: TabBarDemo()));
-    final Rect bike2 = tester.getRect(find.byIcon(Icons.directions_bike));
-    expect(bike2.center, bike1.shift(const Offset(100.0, 0.0)).center);
-  });
 }
-
-class TabBarDemo extends StatelessWidget {
-  @override
-  Widget build(BuildContext context) {
-    return MaterialApp(
-      home: DefaultTabController(
-        length: 3,
-        child: Scaffold(
-          appBar: AppBar(
-            bottom: const TabBar(
-              tabs: <Widget>[
-                Tab(text: 'car'),
-                Tab(text: 'transit'),
-                Tab(text: 'bike'),
-              ],
-            ),
-            title: const Text('Tabs Demo'),
-          ),
-          body: const TabBarView(
-            children: <Widget>[
-              Icon(Icons.directions_car),
-              Icon(Icons.directions_transit),
-              Icon(Icons.directions_bike),
-            ],
-          ),
-        ),
-      ),
-    );
-  }
-}

packages/flutter/test/widgets/scroll_activity_test.dart

@@ -1,264 +0,0 @@
-// Copyright 2014 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// @dart = 2.8
-
-import 'package:flutter/material.dart';
-import 'package:flutter/scheduler.dart';
-import 'package:flutter_test/flutter_test.dart';
-
-List<Widget> children(int n) {
-  return List<Widget>.generate(n, (int i) {
-    return Container(height: 100.0, child: Text('$i'));
-  });
-}
-
-void main() {
-  testWidgets('Scrolling with list view changes', (WidgetTester tester) async {
-    final ScrollController controller = ScrollController();
-    await tester.pumpWidget(MaterialApp(home: ListView(children: children(30), controller: controller)));
-    final double thirty = controller.position.maxScrollExtent;
-    controller.jumpTo(thirty);
-    await tester.pump();
-    controller.jumpTo(thirty + 100.0); // past the end
-    await tester.pump();
-    await tester.pumpWidget(MaterialApp(home: ListView(children: children(31), controller: controller)));
-    expect(controller.position.pixels, thirty + 200.0); // same distance past the end
-    expect(await tester.pumpAndSettle(), 7); // now it goes ballistic...
-    expect(controller.position.pixels, thirty + 100.0); // and ends up at the end
-  });
-
-  testWidgets('Ability to keep a PageView at the end manually (issue 62209)', (WidgetTester tester) async {
-    await tester.pumpWidget(const MaterialApp(home: PageView62209()));
-    expect(find.text('Page 1'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 2'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 3'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 4'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 5'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 5'), findsNothing);
-    expect(find.text('Page 100'), findsOneWidget);
-    await tester.tap(find.byType(FlatButton)); // 6
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 6'), findsNothing);
-    expect(find.text('Page 5'), findsNothing);
-    expect(find.text('Page 100'), findsOneWidget);
-    await tester.tap(find.byType(FlatButton)); // 7
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 6'), findsNothing);
-    expect(find.text('Page 7'), findsNothing);
-    expect(find.text('Page 5'), findsNothing);
-    expect(find.text('Page 100'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 5'), findsOneWidget);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 4'), findsOneWidget);
-    expect(find.text('Page 5'), findsNothing);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.tap(find.byType(FlatButton)); // 8
-    await tester.pump();
-    expect(find.text('Page 1'), findsNothing);
-    expect(find.text('Page 8'), findsNothing);
-    expect(find.text('Page 4'), findsOneWidget);
-    expect(find.text('Page 5'), findsNothing);
-    expect(find.text('Page 100'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 3'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 2'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 6'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 7'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 8'), findsOneWidget);
-    await tester.drag(find.byType(PageView62209), const Offset(800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 1'), findsOneWidget);
-    await tester.tap(find.byType(FlatButton)); // 9
-    await tester.pump();
-    expect(find.text('Page 1'), findsOneWidget);
-    expect(find.text('Page 9'), findsNothing);
-    await tester.drag(find.byType(PageView62209), const Offset(-800.0, 0.0));
-    await tester.pump();
-    expect(find.text('Page 9'), findsOneWidget);
-  });
-}
-
-class PageView62209 extends StatefulWidget {
-  const PageView62209();
-
-  @override
-  _PageView62209State createState() => _PageView62209State();
-}
-
-class _PageView62209State extends State<PageView62209> {
-  int _nextPageNum = 1;
-  final List<Carousel62209Page> _pages = <Carousel62209Page>[];
-
-  @override
-  void initState() {
-    super.initState();
-    for (int i = 0; i < 5; i++) {
-      _pages.add(Carousel62209Page(
-        key: Key('$_nextPageNum'),
-        number: _nextPageNum++,
-      ));
-    }
-    _pages.add(const Carousel62209Page(number: 100));
-  }
-
-  @override
-  Widget build(BuildContext context) {
-    return Scaffold(
-      body: Column(
-        children: <Widget>[
-          Expanded(child: Carousel62209(pages: _pages)),
-          FlatButton(
-            child: const Text('ADD PAGE'),
-            onPressed: () {
-              setState(() {
-                _pages.insert(
-                  1,
-                  Carousel62209Page(
-                    key: Key('$_nextPageNum'),
-                    number: _nextPageNum++,
-                  ),
-                );
-              });
-            },
-          )
-        ],
-      ),
-    );
-  }
-}
-
-class Carousel62209Page extends StatelessWidget {
-  const Carousel62209Page({this.number, Key key}) : super(key: key);
-
-  final int number;
-
-  @override
-  Widget build(BuildContext context) {
-    return Center(child: Text('Page $number'));
-  }
-}
-
-class Carousel62209 extends StatefulWidget {
-  const Carousel62209({Key key, this.pages}) : super(key: key);
-
-  final List<Carousel62209Page> pages;
-
-  @override
-  _Carousel62209State createState() => _Carousel62209State();
-}
-
-class _Carousel62209State extends State<Carousel62209> {
-  // page variables
-  PageController _pageController;
-  int _currentPage = 0;
-
-  // controls updates outside of user interaction
-  List<Carousel62209Page> _pages;
-  bool _jumpingToPage = false;
-
-  @override
-  void initState() {
-    super.initState();
-    _pages = widget.pages.toList();
-    _pageController = PageController(initialPage: 0, keepPage: false);
-  }
-
-  @override
-  void didUpdateWidget(Carousel62209 oldWidget) {
-    super.didUpdateWidget(oldWidget);
-    if (!_jumpingToPage) {
-      int newPage = -1;
-      for (int i = 0; i < widget.pages.length; i++) {
-        if (widget.pages[i].number == _pages[_currentPage].number) {
-          newPage = i;
-        }
-      }
-      if (newPage == _currentPage) {
-        _pages = widget.pages.toList();
-      } else {
-        _jumpingToPage = true;
-        SchedulerBinding.instance.addPostFrameCallback((_) {
-          if (mounted) {
-            setState(() {
-              _pages = widget.pages.toList();
-              _currentPage = newPage;
-              _pageController.jumpToPage(_currentPage);
-              SchedulerBinding.instance.addPostFrameCallback((_) {
-                _jumpingToPage = false;
-              });
-            });
-          }
-        });
-      }
-    }
-  }
-
-  @override
-  void dispose() {
-    _pageController.dispose();
-    super.dispose();
-  }
-
-  bool _handleScrollNotification(ScrollNotification notification) {
-    if (notification is ScrollUpdateNotification) {
-      final int page = _pageController.page.round();
-      if (!_jumpingToPage && _currentPage != page) {
-        _currentPage = page;
-      }
-    }
-    return true;
-  }
-
-  @override
-  Widget build(BuildContext context) {
-    return NotificationListener<ScrollNotification>(
-      onNotification: _handleScrollNotification,
-      child: PageView.builder(
-        controller: _pageController,
-        itemCount: _pages.length,
-        itemBuilder: (BuildContext context, int index) {
-          return _pages[index];
-        },
-      ),
-    );
-  }
-}