Fix ScrollbarPainter thumbExtent calculation and add padding (#31763)

- Fixed extentInside calculation in ScrollMetrics
- Added asserts to extentInside getter, as well as ScrollPosition.applyContentDimensions to enforce minScrollExtent <= maxScrollExtent
- Added padding to ScrollbarPainter, updated implementation. Took care of some edge cases.
- Changed some scroll bar constants on Cupertino side.

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

@@ -8,15 +8,22 @@ import 'package:flutter/widgets.dart';
 
 // All values eyeballed.
 const Color _kScrollbarColor = Color(0x99777777);
-const double _kScrollbarThickness = 2.5;
-const double _kScrollbarMainAxisMargin = 4.0;
-const double _kScrollbarCrossAxisMargin = 2.5;
 const double _kScrollbarMinLength = 36.0;
 const double _kScrollbarMinOverscrollLength = 8.0;
 const Radius _kScrollbarRadius = Radius.circular(1.25);
 const Duration _kScrollbarTimeToFade = Duration(milliseconds: 50);
 const Duration _kScrollbarFadeDuration = Duration(milliseconds: 250);
 
+// These values are measured using screenshots from an iPhone XR 12.1 simulator.
+const double _kScrollbarThickness = 2.5;
+// This is the amount of space from the top of a vertical scrollbar to the
+// top edge of the scrollable, measured when the vertical scrollbar overscrolls
+// to the top.
+// TODO(LongCatIsLooong): fix https://github.com/flutter/flutter/issues/32175
+const double _kScrollbarMainAxisMargin = 3.0;
+const double _kScrollbarCrossAxisMargin = 3.0;
+
+
 /// An iOS style scrollbar.
 ///
 /// A scrollbar indicates which portion of a [Scrollable] widget is actually
@@ -89,6 +96,7 @@ class _CupertinoScrollbarState extends State<CupertinoScrollbar> with TickerProv
       mainAxisMargin: _kScrollbarMainAxisMargin,
       crossAxisMargin: _kScrollbarCrossAxisMargin,
       radius: _kScrollbarRadius,
+      padding: MediaQuery.of(context).padding,
       minLength: _kScrollbarMinLength,
       minOverscrollLength: _kScrollbarMinOverscrollLength,
     );

packages/flutter/lib/src/material/scrollbar.dart

@@ -104,6 +104,7 @@ class _ScrollbarState extends State<Scrollbar> with TickerProviderStateMixin {
         textDirection: _textDirection,
         thickness: _kScrollbarThickness,
         fadeoutOpacityAnimation: _fadeoutOpacityAnimation,
+        padding: MediaQuery.of(context).padding,
       );
   }
 

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

@@ -123,8 +123,9 @@ abstract class ViewportOffset extends ChangeNotifier {
   /// Called when the viewport's content extents are established.
   ///
   /// The arguments are the minimum and maximum scroll extents respectively. The
-  /// minimum will be equal to or less than zero, the maximum will be equal to
-  /// or greater than zero.
+  /// minimum will be equal to or less than the maximum. In the case of slivers,
+  /// the minimum will be equal to or less than zero, the maximum will be equal
+  /// to or greater than zero.
   ///
   /// The maximum scroll extent has the viewport dimension subtracted from it.
   /// For instance, if there is 100.0 pixels of scrollable content, and the

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

@@ -60,14 +60,16 @@ abstract class ScrollMetrics {
   ///
   /// The actual [pixels] value might be [outOfRange].
   ///
-  /// This value can be negative infinity, if the scroll is unbounded.
+  /// This value should typically be non-null and less than or equal to
+  /// [maxScrollExtent]. It can be negative infinity, if the scroll is unbounded.
   double get minScrollExtent;
 
   /// The maximum in-range value for [pixels].
   ///
   /// The actual [pixels] value might be [outOfRange].
   ///
-  /// This value can be infinity, if the scroll is unbounded.
+  /// This value should typically be non-null and greater than or equal to
+  /// [minScrollExtent]. It can be infinity, if the scroll is unbounded.
   double get maxScrollExtent;
 
   /// The current scroll position, in logical pixels along the [axisDirection].
@@ -90,25 +92,27 @@ abstract class ScrollMetrics {
   /// [maxScrollExtent].
   bool get atEdge => pixels == minScrollExtent || pixels == maxScrollExtent;
 
-  /// The quantity of content conceptually "above" the currently visible content
-  /// of the viewport in the scrollable. This is the content above the content
-  /// described by [extentInside].
+  /// The quantity of content conceptually "above" the viewport in the scrollable.
+  /// This is the content above the content described by [extentInside].
   double get extentBefore => math.max(pixels - minScrollExtent, 0.0);
 
-  /// The quantity of visible content.
+  /// The quantity of content conceptually "inside" the viewport in the scrollable.
   ///
-  /// If [extentBefore] and [extentAfter] are non-zero, then this is typically
-  /// the height of the viewport. It could be less if there is less content
-  /// visible than the size of the viewport.
+  /// The value is typically the height of the viewport when [outOfRange] is false.
+  /// It could be less if there is less content visible than the size of the
+  /// viewport, such as when overscrolling.
+  ///
+  /// The value is always non-negative, and less than or equal to [viewportDimension].
   double get extentInside {
-    return math.min(pixels, maxScrollExtent) -
-           math.max(pixels, minScrollExtent) +
-           math.min(viewportDimension, maxScrollExtent - minScrollExtent);
+    return viewportDimension
+      // "above" overscroll value
+      - (minScrollExtent - pixels).clamp(0, viewportDimension)
+      // "below" overscroll value
+      - (pixels - maxScrollExtent).clamp(0, viewportDimension);
   }
 
-  /// The quantity of content conceptually "below" the currently visible content
-  /// of the viewport in the scrollable. This is the content below the content
-  /// described by [extentInside].
+  /// The quantity of content conceptually "below" the viewport in the scrollable.
+  /// This is the content below the content described by [extentInside].
   double get extentAfter => math.max(maxScrollExtent - pixels, 0.0);
 }
 

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

@@ -450,6 +450,9 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
     if (!nearEqual(_minScrollExtent, minScrollExtent, Tolerance.defaultTolerance.distance) ||
         !nearEqual(_maxScrollExtent, maxScrollExtent, Tolerance.defaultTolerance.distance) ||
         _didChangeViewportDimensionOrReceiveCorrection) {
+      assert(minScrollExtent != null);
+      assert(maxScrollExtent != null);
+      assert(minScrollExtent <= maxScrollExtent);
       _minScrollExtent = minScrollExtent;
       _maxScrollExtent = maxScrollExtent;
       _haveDimensions = true;

packages/flutter/test/cupertino/scrollbar_paint_test.dart

@@ -7,32 +7,90 @@ import 'package:flutter_test/flutter_test.dart';
 
 import '../rendering/mock_canvas.dart';
 
+const Color _kScrollbarColor = Color(0x99777777);
+
+// The `y` offset has to be larger than `ScrollDragController._bigThresholdBreakDistance`
+// to prevent [motionStartDistanceThreshold] from affecting the actual drag distance.
+const Offset _kGestureOffset = Offset(0, -25);
+
 void main() {
   testWidgets('Paints iOS spec', (WidgetTester tester) async {
-    await tester.pumpWidget(const Directionality(
-      textDirection: TextDirection.ltr,
-      child: CupertinoScrollbar(
-        child: SingleChildScrollView(
-          child: SizedBox(width: 4000.0, height: 4000.0),
+    await tester.pumpWidget(
+      const Directionality(
+        textDirection: TextDirection.ltr,
+        child: MediaQuery(
+          data: MediaQueryData(),
+          child: CupertinoScrollbar(
+            child: SingleChildScrollView(
+              child: SizedBox(width: 4000.0, height: 4000.0),
+            ),
+          ),
         ),
       ),
-    ));
+    );
+
     expect(find.byType(CupertinoScrollbar), isNot(paints..rrect()));
+
     final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(SingleChildScrollView)));
-    await gesture.moveBy(const Offset(0.0, -10.0));
+    await gesture.moveBy(_kGestureOffset);
     // Move back to original position.
-    await gesture.moveBy(const Offset(0.0, 10.0));
+    await gesture.moveBy(Offset.zero.translate(-_kGestureOffset.dx, -_kGestureOffset.dy));
     await tester.pump();
     await tester.pump(const Duration(milliseconds: 500));
     expect(find.byType(CupertinoScrollbar), paints..rrect(
-      color: const Color(0x99777777),
+      color: _kScrollbarColor,
       rrect: RRect.fromRectAndRadius(
         const Rect.fromLTWH(
-          800.0 - 2.5 - 2.5, // Screen width - margin - thickness.
-          4.0, // Initial position is the top margin.
+          800.0 - 3 - 2.5, // Screen width - margin - thickness.
+          3.0, // Initial position is the top margin.
           2.5, // Thickness.
           // Fraction in viewport * scrollbar height - top, bottom margin.
-          600.0 / 4000.0 * 600.0 - 4.0 - 4.0,
+          600.0 / 4000.0 * (600.0 - 2 * 3),
+        ),
+        const Radius.circular(1.25),
+      ),
+    ));
+  });
+
+  testWidgets('Paints iOS spec with nav bar', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      CupertinoApp(
+        home: MediaQuery(
+          data: const MediaQueryData(
+            padding: EdgeInsets.fromLTRB(0, 20, 0, 34),
+          ),
+          child: CupertinoPageScaffold(
+            navigationBar: const CupertinoNavigationBar(
+              middle: Text('Title'),
+              backgroundColor: Color(0x11111111),
+            ),
+            child: CupertinoScrollbar(
+              child: ListView(
+                children: const <Widget> [SizedBox(width: 4000, height: 4000)]
+              ),
+            ),
+          ),
+        ),
+      ),
+    );
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(ListView)));
+    await gesture.moveBy(_kGestureOffset);
+    // Move back to original position.
+    await gesture.moveBy(Offset.zero.translate(-_kGestureOffset.dx, -_kGestureOffset.dy));
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds: 500));
+
+    expect(find.byType(CupertinoScrollbar), paints..rrect(
+      color: _kScrollbarColor,
+      rrect: RRect.fromRectAndRadius(
+        const Rect.fromLTWH(
+          800.0 - 3 - 2.5, // Screen width - margin - thickness.
+          44 + 20 + 3.0, // nav bar height + top margin
+          2.5, // Thickness.
+          // Fraction visible * (viewport size - padding - margin)
+          // where Fraction visible = (viewport size - padding) / content size
+          (600.0 - 34 - 44 - 20) / 4000.0 * (600.0 - 2 * 3 - 34 - 44 - 20),
         ),
         const Radius.circular(1.25),
       ),

packages/flutter/test/cupertino/scrollbar_test.dart

@@ -9,14 +9,17 @@ import '../rendering/mock_canvas.dart';
 
 void main() {
   testWidgets('Scrollbar never goes away until finger lift', (WidgetTester tester) async {
-    await tester.pumpWidget(const Directionality(
-      textDirection: TextDirection.ltr,
-      child: CupertinoScrollbar(
-        child: SingleChildScrollView(
-          child: SizedBox(width: 4000.0, height: 4000.0),
+    await tester.pumpWidget(
+      const Directionality(
+        textDirection: TextDirection.ltr,
+        child: MediaQuery(
+          data: MediaQueryData(),
+          child: CupertinoScrollbar(
+            child: SingleChildScrollView(child: SizedBox(width: 4000.0, height: 4000.0)),
+          ),
         ),
       ),
-    ));
+    );
     final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(SingleChildScrollView)));
     await gesture.moveBy(const Offset(0.0, -10.0));
     await tester.pump();
@@ -42,25 +45,4 @@ void main() {
       color: const Color(0x15777777),
     ));
   });
-
-  testWidgets('Scrollbar is not smaller than minLength with large scroll views', (WidgetTester tester) async {
-    await tester.pumpWidget(const Directionality(
-      textDirection: TextDirection.ltr,
-      child: CupertinoScrollbar(
-        child: SingleChildScrollView(
-          child: SizedBox(width: 800.0, height: 20000.0),
-        ),
-      ),
-    ));
-    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(SingleChildScrollView)));
-    await gesture.moveBy(const Offset(0.0, -10.0));
-    await tester.pump();
-    await tester.pump(const Duration(milliseconds: 200));
-
-    // Height is 36.0.
-    const Rect scrollbarRect = Rect.fromLTWH(795.0, 4.28659793814433, 2.5, 36.0);
-    expect(find.byType(CupertinoScrollbar), paints..rrect(
-      rrect: RRect.fromRectAndRadius(scrollbarRect, const Radius.circular(1.25)),
-    ));
-  });
 }

packages/flutter/test/material/scrollbar_paint_test.dart

@@ -7,15 +7,26 @@ import 'package:flutter/material.dart';
 
 import '../rendering/mock_canvas.dart';
 
-void main() {
-  testWidgets('Viewport basic test (LTR)', (WidgetTester tester) async {
-    await tester.pumpWidget(const Directionality(
-      textDirection: TextDirection.ltr,
+Widget _buildSingleChildScrollViewWithScrollbar({
+  TextDirection textDirection = TextDirection.ltr,
+  EdgeInsets padding = EdgeInsets.zero,
+  Widget child}
+) {
+  return Directionality(
+    textDirection: textDirection,
+    child: MediaQuery(
+      data: MediaQueryData(padding: padding),
       child: Scrollbar(
-        child: SingleChildScrollView(
-          child: SizedBox(width: 4000.0, height: 4000.0),
-        ),
+        child: SingleChildScrollView(child: child),
       ),
+    ),
+  );
+}
+
+void main() {
+  testWidgets('Viewport basic test (LTR)', (WidgetTester tester) async {
+    await tester.pumpWidget(_buildSingleChildScrollViewWithScrollbar(
+      child: const SizedBox(width: 4000.0, height: 4000.0),
     ));
     expect(find.byType(Scrollbar), isNot(paints..rect()));
     await tester.fling(find.byType(SingleChildScrollView), const Offset(0.0, -10.0), 10.0);
@@ -23,16 +34,47 @@ void main() {
   });
 
   testWidgets('Viewport basic test (RTL)', (WidgetTester tester) async {
-    await tester.pumpWidget(const Directionality(
+    await tester.pumpWidget(_buildSingleChildScrollViewWithScrollbar(
       textDirection: TextDirection.rtl,
-      child: Scrollbar(
-        child: SingleChildScrollView(
-          child: SizedBox(width: 4000.0, height: 4000.0),
-        ),
-      ),
+      child: const SizedBox(width: 4000.0, height: 4000.0),
     ));
     expect(find.byType(Scrollbar), isNot(paints..rect()));
     await tester.fling(find.byType(SingleChildScrollView), const Offset(0.0, -10.0), 10.0);
     expect(find.byType(Scrollbar), paints..rect(rect: const Rect.fromLTRB(0.0, 1.5, 6.0, 91.5)));
   });
+
+  testWidgets('workds with MaterialApp and Scaffold', (WidgetTester tester) async {
+    await tester.pumpWidget(MaterialApp(
+      home: MediaQuery(
+        data: const MediaQueryData(
+          padding: EdgeInsets.fromLTRB(0, 20, 0, 34)
+        ),
+        child: Scaffold(
+          appBar: AppBar(title: const Text('Title')),
+          body: Scrollbar(
+            child: ListView(
+              children: const <Widget>[SizedBox(width: 4000, height: 4000)]
+            ),
+          ),
+        ),
+      ),
+    ));
+
+    final TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(ListView)));
+    // On Android it should not overscroll.
+    await gesture.moveBy(const Offset(0, 100));
+    // Trigger fade in animation.
+    await tester.pump();
+    await tester.pump(const Duration(milliseconds: 500));
+
+    expect(find.byType(Scrollbar), paints..rect(
+      rect: const Rect.fromLTWH(
+        800.0 - 6, // screen width - thickness
+        0,         // the paint area starts from the bottom of the app bar
+        6,         // thickness
+        // 56 being the height of the app bar
+        (600.0 - 56 - 34 - 20) / 4000 * (600 - 56 - 34 - 20),
+      ),
+    ));
+  });
 }

packages/flutter/test/material/scrollbar_test.dart

@@ -19,34 +19,49 @@ class TestCanvas implements Canvas {
   }
 }
 
+Widget _buildBoilerplate({
+    TextDirection textDirection = TextDirection.ltr,
+    EdgeInsets padding = EdgeInsets.zero,
+    Widget child
+}) {
+  return Directionality(
+    textDirection: textDirection,
+    child: MediaQuery(
+      data: MediaQueryData(padding: padding),
+      child: child,
+    ),
+  );
+}
+
 void main() {
   testWidgets('Scrollbar doesn\'t show when tapping list', (WidgetTester tester) async {
-    await tester.pumpWidget(Directionality(
-      textDirection: TextDirection.ltr,
-      child: Center(
-        child: Container(
-          decoration: BoxDecoration(
-            border: Border.all(color: const Color(0xFFFFFF00))
-          ),
-          height: 200.0,
-          width: 300.0,
-          child: Scrollbar(
-            child: ListView(
-              children: <Widget>[
-                Container(height: 40.0, child: const Text('0')),
-                Container(height: 40.0, child: const Text('1')),
-                Container(height: 40.0, child: const Text('2')),
-                Container(height: 40.0, child: const Text('3')),
-                Container(height: 40.0, child: const Text('4')),
-                Container(height: 40.0, child: const Text('5')),
-                Container(height: 40.0, child: const Text('6')),
-                Container(height: 40.0, child: const Text('7')),
-              ],
+    await tester.pumpWidget(
+      _buildBoilerplate(
+        child: Center(
+          child: Container(
+            decoration: BoxDecoration(
+              border: Border.all(color: const Color(0xFFFFFF00))
+            ),
+            height: 200.0,
+            width: 300.0,
+            child: Scrollbar(
+              child: ListView(
+                children: <Widget>[
+                  Container(height: 40.0, child: const Text('0')),
+                  Container(height: 40.0, child: const Text('1')),
+                  Container(height: 40.0, child: const Text('2')),
+                  Container(height: 40.0, child: const Text('3')),
+                  Container(height: 40.0, child: const Text('4')),
+                  Container(height: 40.0, child: const Text('5')),
+                  Container(height: 40.0, child: const Text('6')),
+                  Container(height: 40.0, child: const Text('7')),
+                ],
+              ),
             ),
           ),
-        ),
-      ),
-    ));
+        )
+      )
+    );
 
     SchedulerBinding.instance.debugAssertNoTransientCallbacks('Building a list with a scrollbar triggered an animation.');
     await tester.tap(find.byType(ListView));
@@ -64,9 +79,8 @@ void main() {
   });
 
   testWidgets('ScrollbarPainter does not divide by zero', (WidgetTester tester) async {
-    await tester.pumpWidget(Directionality(
-      textDirection: TextDirection.ltr,
-      child: Container(
+    await tester.pumpWidget(
+      _buildBoilerplate(child: Container(
         height: 200.0,
         width: 300.0,
         child: Scrollbar(
@@ -76,8 +90,8 @@ void main() {
             ],
           ),
         ),
-      ),
-    ));
+      ))
+    );
 
     final CustomPaint custom = tester.widget(find.descendant(
       of: find.byType(Scrollbar),
@@ -107,8 +121,7 @@ void main() {
 
   testWidgets('Adaptive scrollbar', (WidgetTester tester) async {
     Widget viewWithScroll(TargetPlatform platform) {
-      return Directionality(
-        textDirection: TextDirection.ltr,
+      return _buildBoilerplate(
         child: Theme(
           data: ThemeData(
             platform: platform