Better defaulting for scroll view primary-ness (#8506)

* Improved defaults for scroll view primary-ness

* Vertical scroll views default to primary:true.
* Horizontal scroll views default to primary:false.
* If a scroll view is primary and it got a non-null inherited primary
  scroll controller, it introduces a primary scroll controller inherited
  with a value of null for its descendants.

ScrollController now multiplexes writes to all registered positions;
reads of position continue to assert that only one position is
registered. Reads still require a single position.

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

@@ -16,6 +16,12 @@ class PrimaryScrollController extends InheritedWidget {
     assert(controller != null);
   }
 
+  const PrimaryScrollController.none({
+    Key key,
+    @required Widget child
+  }) : controller = null,
+       super(key: key, child: child);
+
   final ScrollController controller;
 
   static ScrollController of(BuildContext context) {
@@ -29,6 +35,6 @@ class PrimaryScrollController extends InheritedWidget {
   @override
   void debugFillDescription(List<String> description) {
     super.debugFillDescription(description);
-    description.add('$controller');
+    description.add('${controller ?? 'no controller'}');
   }
 }

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

@@ -68,7 +68,13 @@ class ScrollController {
   Future<Null> animateTo(double offset, {
     @required Duration duration,
     @required Curve curve,
-  }) => position.animateTo(offset, duration: duration, curve: curve);
+  }) {
+    assert(_positions.isNotEmpty, 'ScrollController not attached to any scroll views.');
+    List<Future<Null>> animations = new List<Future<Null>>(_positions.length);
+    for (int i = 0; i < _positions.length; i++)
+      animations[i] = _positions[i].animateTo(offset, duration: duration, curve: curve);
+    return Future.wait<Null>(animations).then((List<Null> _) => null);
+  }
 
   /// Jumps the scroll position from its current value to the given value,
   /// without animation, and without checking if the new value is in range.
@@ -82,7 +88,11 @@ class ScrollController {
   ///
   /// Immediately after the jump, a ballistic activity is started, in case the
   /// value was out of range.
-  void jumpTo(double value) => position.jumpTo(value);
+  void jumpTo(double value) {
+    assert(_positions.isNotEmpty, 'ScrollController not attached to any scroll views.');
+    for (ScrollPosition p in _positions)
+      p.jumpTo(value);
+  }
 
   /// Register the given position with this controller.
   ///

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

@@ -21,14 +21,15 @@ abstract class ScrollView extends StatelessWidget {
     this.scrollDirection: Axis.vertical,
     this.reverse: false,
     this.controller,
-    this.primary: false,
+    bool primary,
     this.physics,
     this.shrinkWrap: false,
-  }) : super(key: key) {
+  }) : primary = primary ?? controller == null && scrollDirection == Axis.vertical,
-    assert(reverse != null);
+       super(key: key) {
-    assert(shrinkWrap != null);
+    assert(this.reverse != null);
-    assert(primary != null);
+    assert(this.shrinkWrap != null);
-    assert(controller == null || !primary,
+    assert(this.primary != null);
+    assert(this.controller == null || !this.primary,
            'Primary ScrollViews obtain their ScrollController via inheritance from a PrimaryScrollController widget. '
            'You cannot both set primary to true and pass an explicit controller.'
     );
@@ -40,6 +41,14 @@ abstract class ScrollView extends StatelessWidget {
 
   final ScrollController controller;
 
+  /// Whether this is the primary scroll view associated with the parent
+  /// [PrimaryScrollController].
+  ///
+  /// On iOS, this identifies the scroll view that will scroll to top in
+  /// response to a tap in the status bar.
+  ///
+  /// Defaults to true when `scrollDirection` is vertical and `controller` is
+  /// not specified.
   final bool primary;
 
   final ScrollPhysics physics;
@@ -65,9 +74,13 @@ abstract class ScrollView extends StatelessWidget {
   Widget build(BuildContext context) {
     List<Widget> slivers = buildSlivers(context);
     AxisDirection axisDirection = getDirection(context);
-    return new Scrollable(
+
+    ScrollController scrollController = primary
+        ? PrimaryScrollController.of(context)
+        : controller;
+    Scrollable scrollable = new Scrollable(
       axisDirection: axisDirection,
-      controller: controller ?? (primary ? PrimaryScrollController.of(context) : null),
+      controller: scrollController,
       physics: physics,
       viewportBuilder: (BuildContext context, ViewportOffset offset) {
         if (shrinkWrap) {
@@ -85,6 +98,9 @@ abstract class ScrollView extends StatelessWidget {
         }
       }
     );
+    return primary && scrollController != null
+      ? new PrimaryScrollController.none(child: scrollable)
+      : scrollable;
   }
 
   @override
@@ -110,7 +126,7 @@ class CustomScrollView extends ScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     this.slivers: const <Widget>[],
@@ -136,7 +152,7 @@ abstract class BoxScrollView extends ScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     this.padding,
@@ -185,7 +201,7 @@ class ListView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -207,7 +223,7 @@ class ListView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -230,7 +246,7 @@ class ListView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -286,7 +302,7 @@ class GridView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -310,7 +326,7 @@ class GridView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -335,7 +351,7 @@ class GridView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,
@@ -366,7 +382,7 @@ class GridView extends BoxScrollView {
     Axis scrollDirection: Axis.vertical,
     bool reverse: false,
     ScrollController controller,
-    bool primary: false,
+    bool primary,
     ScrollPhysics physics,
     bool shrinkWrap: false,
     EdgeInsets padding,

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

@@ -43,14 +43,15 @@ class SingleChildScrollView extends StatelessWidget {
     this.scrollDirection: Axis.vertical,
     this.reverse: false,
     this.padding,
-    this.primary: false,
+    bool primary,
     this.physics,
     this.controller,
     this.child,
-  }) : super(key: key) {
+  }) : primary = primary ?? controller == null && scrollDirection == Axis.vertical,
-    assert(scrollDirection != null);
+       super(key: key) {
-    assert(primary != null);
+    assert(this.scrollDirection != null);
-    assert(controller == null || !primary,
+    assert(this.primary != null);
+    assert(this.controller == null || !this.primary,
        'Primary ScrollViews obtain their ScrollController via inheritance from a PrimaryScrollController widget. '
        'You cannot both set primary to true and pass an explicit controller.'
     );
@@ -64,6 +65,14 @@ class SingleChildScrollView extends StatelessWidget {
 
   final ScrollController controller;
 
+  /// Whether this is the primary scroll view associated with the parent
+  /// [PrimaryScrollController].
+  ///
+  /// On iOS, this identifies the scroll view that will scroll to top in
+  /// response to a tap in the status bar.
+  ///
+  /// Defaults to true when `scrollDirection` is vertical and `controller` is
+  /// not specified.
   final bool primary;
 
   final ScrollPhysics physics;
@@ -87,9 +96,12 @@ class SingleChildScrollView extends StatelessWidget {
     Widget contents = child;
     if (padding != null)
       contents = new Padding(padding: padding, child: contents);
-    return new Scrollable(
+    ScrollController scrollController = primary
+        ? PrimaryScrollController.of(context)
+        : controller;
+    Scrollable scrollable = new Scrollable(
       axisDirection: axisDirection,
-      controller: controller ?? (primary ? PrimaryScrollController.of(context) : null),
+      controller: scrollController,
       physics: physics,
       viewportBuilder: (BuildContext context, ViewportOffset offset) {
         return new _SingleChildViewport(
@@ -99,6 +111,9 @@ class SingleChildScrollView extends StatelessWidget {
         );
       },
     );
+    return primary && scrollController != null
+      ? new PrimaryScrollController.none(child: scrollable)
+      : scrollable;
   }
 }
 

packages/flutter/test/widgets/scroll_controller_test.dart

@@ -201,7 +201,7 @@ void main() {
     expect(() => controller.jumpTo(1.0), throwsAssertionError);
   });
 
-  testWidgets('Write operations on ScrollControllers with more than one position fail', (WidgetTester tester) async {
+  testWidgets('Write operations on ScrollControllers with more than one position do not throw', (WidgetTester tester) async {
     ScrollController controller = new ScrollController();
     await tester.pumpWidget(new ListView(
       children: <Widget>[
@@ -226,7 +226,9 @@ void main() {
       ],
     ));
 
-    expect(() => controller.jumpTo(1.0), throwsAssertionError);
+    controller.jumpTo(1.0);
-    expect(() => controller.animateTo(1.0, duration: const Duration(seconds: 1), curve: Curves.linear), throwsAssertionError);
+    controller.animateTo(1.0, duration: const Duration(seconds: 1), curve: Curves.linear);
+    await tester.pump();
+    await tester.pumpUntilNoTransientCallbacks();
   });
 }

packages/flutter/test/widgets/scroll_view_test.dart

@@ -169,6 +169,67 @@ void main() {
     expect(log, isEmpty);
   });
 
+  testWidgets('Vertical CustomScrollViews are primary by default', (WidgetTester tester) async {
+    CustomScrollView view = new CustomScrollView(scrollDirection: Axis.vertical);
+    expect(view.primary, isTrue);
+  });
+
+  testWidgets('Vertical ListViews are primary by default', (WidgetTester tester) async {
+    ListView view = new ListView(scrollDirection: Axis.vertical);
+    expect(view.primary, isTrue);
+  });
+
+  testWidgets('Vertical GridViews are primary by default', (WidgetTester tester) async {
+    GridView view = new GridView.count(
+      scrollDirection: Axis.vertical,
+      crossAxisCount: 1,
+    );
+    expect(view.primary, isTrue);
+  });
+
+  testWidgets('Horizontal CustomScrollViews are non-primary by default', (WidgetTester tester) async {
+    CustomScrollView view = new CustomScrollView(scrollDirection: Axis.horizontal);
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('Horizontal ListViews are non-primary by default', (WidgetTester tester) async {
+    ListView view = new ListView(scrollDirection: Axis.horizontal);
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('Horizontal GridViews are non-primary by default', (WidgetTester tester) async {
+    GridView view = new GridView.count(
+      scrollDirection: Axis.horizontal,
+      crossAxisCount: 1,
+    );
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('CustomScrollViews with controllers are non-primary by default', (WidgetTester tester) async {
+    CustomScrollView view = new CustomScrollView(
+      controller: new ScrollController(),
+      scrollDirection: Axis.vertical,
+    );
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('ListViews with controllers are non-primary by default', (WidgetTester tester) async {
+    ListView view = new ListView(
+      controller: new ScrollController(),
+      scrollDirection: Axis.vertical,
+    );
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('GridViews with controllers are non-primary by default', (WidgetTester tester) async {
+    GridView view = new GridView.count(
+      controller: new ScrollController(),
+      scrollDirection: Axis.vertical,
+      crossAxisCount: 1,
+    );
+    expect(view.primary, isFalse);
+  });
+
   testWidgets('CustomScrollView sets PrimaryScrollController when primary', (WidgetTester tester) async {
     ScrollController primaryScrollController = new ScrollController();
     await tester.pumpWidget(new PrimaryScrollController(
@@ -198,4 +259,29 @@ void main() {
     Scrollable scrollable = tester.widget(find.byType(Scrollable));
     expect(scrollable.controller, primaryScrollController);
   });
+
+  testWidgets('Nested scrollables have a null PrimaryScrollController', (WidgetTester tester) async {
+    const Key innerKey = const Key('inner');
+    ScrollController primaryScrollController = new ScrollController();
+    await tester.pumpWidget(new PrimaryScrollController(
+      controller: primaryScrollController,
+      child: new ListView(
+        primary: true,
+        children: <Widget>[
+          new Container(
+            constraints: new BoxConstraints(maxHeight: 200.0),
+            child: new ListView(key: innerKey, primary: true),
+          ),
+        ],
+      ),
+    ));
+
+    Scrollable innerScrollable = tester.widget(
+      find.descendant(
+        of: find.byKey(innerKey),
+        matching: find.byType(Scrollable),
+      ),
+    );
+    expect(innerScrollable.controller, isNull);
+  });
 }

packages/flutter/test/widgets/single_child_scroll_view_test.dart

@@ -136,4 +136,45 @@ void main() {
       ),
     ));
   });
+
+  testWidgets('Vertical SingleChildScrollViews are primary by default', (WidgetTester tester) async {
+    SingleChildScrollView view = new SingleChildScrollView(scrollDirection: Axis.vertical);
+    expect(view.primary, isTrue);
+  });
+
+  testWidgets('Horizontal SingleChildScrollViews are non-primary by default', (WidgetTester tester) async {
+    SingleChildScrollView view = new SingleChildScrollView(scrollDirection: Axis.horizontal);
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('SingleChildScrollViews with controllers are non-primary by default', (WidgetTester tester) async {
+    SingleChildScrollView view = new SingleChildScrollView(
+      controller: new ScrollController(),
+      scrollDirection: Axis.vertical,
+    );
+    expect(view.primary, isFalse);
+  });
+
+  testWidgets('Nested scrollables have a null PrimaryScrollController', (WidgetTester tester) async {
+    const Key innerKey = const Key('inner');
+    ScrollController primaryScrollController = new ScrollController();
+    await tester.pumpWidget(new PrimaryScrollController(
+      controller: primaryScrollController,
+      child: new SingleChildScrollView(
+        primary: true,
+        child: new Container(
+          constraints: new BoxConstraints(maxHeight: 200.0),
+          child: new ListView(key: innerKey, primary: true),
+        ),
+      ),
+    ));
+
+    Scrollable innerScrollable = tester.widget(
+      find.descendant(
+        of: find.byKey(innerKey),
+        matching: find.byType(Scrollable),
+      ),
+    );
+    expect(innerScrollable.controller, isNull);
+  });
 }