Improved support for saving and restoring the scroll offset, etc (#10517)

examples/catalog/lib/expansion_tile_sample.dart

@@ -76,7 +76,7 @@ class EntryItem extends StatelessWidget {
     if (root.children.isEmpty)
       return new ListTile(title: new Text(root.title));
     return new ExpansionTile(
-      key: new ValueKey<Entry>(root),
+      key: new PageStorageKey<Entry>(root),
       title: new Text(root.title),
       children: root.children.map(_buildTiles).toList(),
     );

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

@@ -322,10 +322,10 @@ class LabeledGlobalKey<T extends State<StatefulWidget>> extends GlobalKey<T> {
 
   @override
   String toString() {
-    final String tag = _debugLabel != null ? ' $_debugLabel' : '#$hashCode';
+    final String label = _debugLabel != null ? ' $_debugLabel' : '';
     if (runtimeType == LabeledGlobalKey)
-      return '[GlobalKey$tag]';
-    return '[$runtimeType$tag]';
+      return '[GlobalKey#$hashCode$label]';
+    return '[$runtimeType#$hashCode$label]';
   }
 }
 

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

@@ -6,43 +6,67 @@ import 'package:flutter/foundation.dart';
 
 import 'framework.dart';
 
+/// A [ValueKey] that defines where [PageStorage] values will be saved.
+///
+/// [Scrollable]s ([ScrollPosition]s really) use [PageStorage] to save their
+/// scroll offset. Each time a scroll completes, the scrollable's page
+/// storage is updated.
+///
+/// [PageStorage] is used to save and restore values that can outlive the widget.
+/// The values are stored in a per-route [Map] whose keys are defined by the
+/// [PageStorageKey]s for the widget and its ancestors. To make it possible
+/// for a saved value to be found when a widget is recreated, the key's values
+/// must not be objects whose identity will change each time the widget is created.
+///
+/// For example, to ensure that the scroll offsets for the scrollable within
+/// each `MyScrollableTabView` below are restored when the [TabBarView]
+/// is recreated, we've specified [PageStorageKey]s whose values are the the
+/// tabs' string labels.
+///
+/// ```dart
+/// new TabBarView(
+///   children: myTabs.map((Tab tab) {
+///     new MyScrollableTabView(
+///       key: new PageStorageKey<String>(tab.text), // like 'Tab 1'
+///       tab: tab,
+///    ),
+///  }),
+///)
+/// ```
+class PageStorageKey<T> extends ValueKey<T> {
+  /// Creates a [ValueKey] that defines where [PageStorage] values will be saved.
+  const PageStorageKey(T value) : super(value);
+}
+
 class _StorageEntryIdentifier {
-  Type clientType;
-  List<Key> keys;
-
-  void addKey(Key key) {
-    assert(key != null);
-    assert(key is! GlobalKey);
-    keys ??= <Key>[];
-    keys.add(key);
+  _StorageEntryIdentifier(this.clientType, this.keys) {
+    assert(clientType != null);
+    assert(keys != null);
   }
 
-  GlobalKey scopeKey;
+  final Type clientType;
+  final List<PageStorageKey<dynamic>> keys;
 
   @override
   bool operator ==(dynamic other) {
-    if (other is! _StorageEntryIdentifier)
+    if (other.runtimeType != runtimeType)
       return false;
     final _StorageEntryIdentifier typedOther = other;
-    if (clientType != typedOther.clientType ||
-        scopeKey != typedOther.scopeKey ||
-        keys?.length != typedOther.keys?.length)
+    if (clientType != typedOther.clientType || keys.length != typedOther.keys.length)
       return false;
-    if (keys != null) {
-      for (int index = 0; index < keys.length; index += 1) {
-        if (keys[index] != typedOther.keys[index])
-          return false;
-      }
+    for (int index = 0; index < keys.length; index += 1) {
+      if (keys[index] != typedOther.keys[index])
+        return false;
     }
     return true;
   }
 
   @override
-  int get hashCode => hashValues(clientType, scopeKey, hashList(keys));
+  int get hashCode => hashValues(clientType, hashList(keys));
 
   @override
   String toString() {
-    return 'StorageEntryIdentifier($clientType, $scopeKey, ${keys?.join(":")})';
+    return 'StorageEntryIdentifier($clientType, ${keys?.join(":")})';
   }
 }
 
@@ -51,27 +75,26 @@ class _StorageEntryIdentifier {
 /// Useful for storing per-page state that persists across navigations from one
 /// page to another.
 class PageStorageBucket {
-  _StorageEntryIdentifier _computeStorageIdentifier(BuildContext context) {
-    final _StorageEntryIdentifier result = new _StorageEntryIdentifier();
-    result.clientType = context.widget.runtimeType;
-    Key lastKey = context.widget.key;
-    if (lastKey is! GlobalKey) {
-      if (lastKey != null)
-        result.addKey(lastKey);
+  bool _maybeAddKey(BuildContext context, List<PageStorageKey<dynamic>> keys) {
+    final Widget widget = context.widget;
+    final Key key = widget.key;
+    if (key is PageStorageKey)
+      keys.add(key);
+    return widget is! PageStorage;
+  }
+
+  List<PageStorageKey<dynamic>> _allKeys(BuildContext context) {
+    final List<PageStorageKey<dynamic>> keys = <PageStorageKey<dynamic>>[];
+    if (_maybeAddKey(context, keys)) {
       context.visitAncestorElements((Element element) {
-        if (element.widget.key is GlobalKey) {
-          lastKey = element.widget.key;
-          return false;
-        } else if (element.widget.key != null) {
-          result.addKey(element.widget.key);
-        }
-        return true;
+        return _maybeAddKey(element, keys);
       });
-      return result;
     }
-    assert(lastKey is GlobalKey);
-    result.scopeKey = lastKey;
-    return result;
+    return keys;
+  }
+
+  _StorageEntryIdentifier _computeIdentifier(BuildContext context) {
+    return new _StorageEntryIdentifier(context.widget.runtimeType, _allKeys(context));
   }
 
   Map<Object, dynamic> _storage;
@@ -89,13 +112,13 @@ class PageStorageBucket {
   /// identifier will change.
   void writeState(BuildContext context, dynamic data, { Object identifier }) {
     _storage ??= <Object, dynamic>{};
-    _storage[identifier ?? _computeStorageIdentifier(context)] = data;
+    _storage[identifier ?? _computeIdentifier(context)] = data;
   }
 
   /// Read given data from into this page storage bucket using an identifier
   /// computed from the given context. More about [identifier] in [writeState].
   dynamic readState(BuildContext context, { Object identifier }) {
-    return _storage != null ? _storage[identifier ?? _computeStorageIdentifier(context)] : null;
+    return _storage != null ? _storage[identifier ?? _computeIdentifier(context)] : null;
   }
 }
 

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

@@ -38,17 +38,36 @@ import 'viewport.dart';
 class PageController extends ScrollController {
   /// Creates a page controller.
   ///
-  /// The [initialPage] and [viewportFraction] arguments must not be null.
+  /// The [initialPage], [keepPage], and [viewportFraction] arguments must not be null.
   PageController({
     this.initialPage: 0,
+    this.keepPage: true,
     this.viewportFraction: 1.0,
   }) : assert(initialPage != null),
+       assert(keepPage != null),
        assert(viewportFraction != null),
        assert(viewportFraction > 0.0);
 
   /// The page to show when first creating the [PageView].
   final int initialPage;
 
+  /// Save the current [page] with [PageStorage] and restore it if
+  /// this controller's scrollable is recreated.
+  ///
+  /// If this property is set to false, the current [page] is never saved
+  /// and [initialPage] is always used to initialize the scroll offset.
+  /// If true (the default), the initial page is used the first time the
+  /// controller's scrollable is created, since there's isn't a page to
+  /// restore yet. Subsequently the saved page is restored and
+  /// [initialPage] is ignored.
+  ///
+  /// See also:
+  ///
+  ///  * [PageStorageKey], which should be used when more than one
+  ////   scrollable appears in the same route, to distinguish the [PageStorage]
+  ///    locations used to save scroll offsets.
+  final bool keepPage;
+
   /// The fraction of the viewport that each page should occupy.
   ///
   /// Defaults to 1.0, which means each page fills the viewport in the scrolling
@@ -116,6 +135,7 @@ class PageController extends ScrollController {
       physics: physics,
       context: context,
       initialPage: initialPage,
+      keepPage: keepPage,
       viewportFraction: viewportFraction,
       oldPosition: oldPosition,
     );
@@ -150,9 +170,11 @@ class _PagePosition extends ScrollPositionWithSingleContext {
     ScrollPhysics physics,
     ScrollContext context,
     this.initialPage: 0,
+    bool keepPage: true,
     double viewportFraction: 1.0,
     ScrollPosition oldPosition,
   }) : assert(initialPage != null),
+       assert(keepPage != null),
        assert(viewportFraction != null),
        assert(viewportFraction > 0.0),
        _viewportFraction = viewportFraction,
@@ -161,6 +183,7 @@ class _PagePosition extends ScrollPositionWithSingleContext {
          physics: physics,
          context: context,
          initialPixels: null,
+         keepScrollOffset: keepPage,
          oldPosition: oldPosition,
        );
 

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

@@ -39,20 +39,40 @@ import 'scroll_position_with_single_context.dart';
 class ScrollController extends ChangeNotifier {
   /// Creates a controller for a scrollable widget.
   ///
-  /// The [initialScrollOffset] must not be null.
+  /// The values of `initialScrollOffset` and `keepScrollOffset` must not be null.
   ScrollController({
     this.initialScrollOffset: 0.0,
+    this.keepScrollOffset: true,
     this.debugLabel,
-  }) : assert(initialScrollOffset != null);
+  }) : assert(initialScrollOffset != null),
+       assert(keepScrollOffset != null);
 
   /// The initial value to use for [offset].
   ///
   /// New [ScrollPosition] objects that are created and attached to this
-  /// controller will have their offset initialized to this value.
+  /// controller will have their offset initialized to this value
+  /// if [keepScrollOffset] is false or a scroll offset hasn't been saved yet.
   ///
   /// Defaults to 0.0.
   final double initialScrollOffset;
 
+  /// Each time a scroll completes, save the current scroll [offset] with
+  /// [PageStorage] and restore it if this controller's scrollable is recreated.
+  ///
+  /// If this property is set to false, the scroll offset is never saved
+  /// and [initialScrollOffset] is always used to initialize the scroll
+  /// offset. If true (the default), the initial scroll offset is used the
+  /// first time the controller's scrollable is created, since there's no
+  /// scroll offset to restore yet. Subsequently the saved offset is
+  /// restored and [initialScrollOffset] is ignored.
+  ///
+  /// See also:
+  ///
+  ///  * [PageStorageKey], which should be used when more than one
+  ////   scrollable appears in the same route, to distinguish the [PageStorage]
+  ///    locations used to save scroll offsets.
+  final bool keepScrollOffset;
+
   /// A label that is used in the [toString] output. Intended to aid with
   /// identifying scroll controller instances in debug output.
   final String debugLabel;
@@ -204,6 +224,7 @@ class ScrollController extends ChangeNotifier {
       physics: physics,
       context: context,
       initialPixels: initialScrollOffset,
+      keepScrollOffset: keepScrollOffset,
       oldPosition: oldPosition,
       debugLabel: debugLabel,
     );

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

@@ -61,17 +61,22 @@ export 'scroll_activity.dart' show ScrollHoldController;
 abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// Creates an object that determines which portion of the content is visible
   /// in a scroll view.
+  ///
+  /// The [physics], [context], and [keepScrollOffset] parameters must not be null.
   ScrollPosition({
     @required this.physics,
     @required this.context,
+    this.keepScrollOffset: true,
     ScrollPosition oldPosition,
     this.debugLabel,
   }) : assert(physics != null),
        assert(context != null),
-       assert(context.vsync != null) {
+       assert(context.vsync != null),
+       assert(keepScrollOffset != null) {
     if (oldPosition != null)
       absorb(oldPosition);
-    restoreScrollOffset();
+    if (keepScrollOffset)
+      restoreScrollOffset();
   }
 
   /// How the scroll position should respond to user input.
@@ -85,6 +90,15 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// Typically implemented by [ScrollableState].
   final ScrollContext context;
 
+  /// Save the current scroll [offset] with [PageStorage] and restore it if
+  /// this scroll position's scrollable is recreated.
+  ///
+  /// See also:
+  ///
+  ///  * [ScrollController.keepScrollOffset] and [PageController.keepPage], which
+  ///    create scroll positions and initialize this property.
+  final bool keepScrollOffset;
+
   /// A label that is used in the [toString] output. Intended to aid with
   /// identifying animation controller instances in debug output.
   final String debugLabel;
@@ -539,7 +553,8 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
   /// This also saves the scroll offset using [saveScrollOffset].
   void didEndScroll() {
     activity.dispatchScrollEndNotification(cloneMetrics(), context.notificationContext);
-    saveScrollOffset();
+    if (keepScrollOffset)
+      saveScrollOffset();
   }
 
   /// Called by [setPixels] to report overscroll when an attempt is made to

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

@@ -46,13 +46,24 @@ class ScrollPositionWithSingleContext extends ScrollPosition implements ScrollAc
   /// imperative that the value be set, using [correctPixels], as soon as
   /// [applyNewDimensions] is invoked, before calling the inherited
   /// implementation of that method.
+  ///
+  /// If [keepScrollOffset] is true (the default), the current scroll offset is
+  /// saved with [PageStorage] and restored it if this scroll position's scrollable
+  /// is recreated.
   ScrollPositionWithSingleContext({
     @required ScrollPhysics physics,
     @required ScrollContext context,
     double initialPixels: 0.0,
+    bool keepScrollOffset: true,
     ScrollPosition oldPosition,
     String debugLabel,
-  }) : super(physics: physics, context: context, oldPosition: oldPosition, debugLabel: debugLabel) {
+  }) : super(
+         physics: physics,
+         context: context,
+         keepScrollOffset: keepScrollOffset,
+         oldPosition: oldPosition,
+         debugLabel: debugLabel,
+       ) {
     // If oldPosition is not null, the superclass will first call absorb(),
     // which may set _pixels and _activity.
     if (pixels == null && initialPixels != null)

packages/flutter/test/widgets/page_view_test.dart

@@ -441,12 +441,14 @@ void main() {
       ),
     );
     expect(controller.page, 2);
+
+    final PageController controller2 = new PageController(keepPage: false);
     await tester.pumpWidget(
       new PageStorage(
         bucket: bucket,
         child: new PageView(
           key: const Key('Check it again against your list and see consistency!'),
-          controller: controller,
+          controller: controller2,
           children: <Widget>[
             const Placeholder(),
             const Placeholder(),
@@ -455,6 +457,6 @@ void main() {
         ),
       ),
     );
-    expect(controller.page, 0);
+    expect(controller2.page, 0);
   });
 }

packages/flutter/test/widgets/scroll_controller_test.dart

@@ -259,4 +259,51 @@ void main() {
     await tester.drag(find.byType(ListView), const Offset(0.0, -130.0));
     expect(log, isEmpty);
   });
+
+  testWidgets('keepScrollOffset', (WidgetTester tester) async {
+    final PageStorageBucket bucket = new PageStorageBucket();
+
+    Widget buildFrame(ScrollController controller) {
+      return new PageStorage(
+        bucket: bucket,
+        child: new ListView(
+          key: new UniqueKey(), // it's a different ListView every time
+          controller: controller,
+          children: new List<Widget>.generate(50, (int index) {
+            return new Container(height: 100.0, child: new Text('Item $index'));
+          }).toList(),
+        ),
+      );
+    }
+
+    // keepScrollOffset: true (the default). The scroll offset is restored
+    // when the ListView is recreated with a new ScrollController.
+
+    // The initialScrollOffset is used in this case, because there's no saved
+    // scroll offset.
+    ScrollController controller = new ScrollController(initialScrollOffset: 200.0);
+    await tester.pumpWidget(buildFrame(controller));
+    expect(tester.getTopLeft(find.widgetWithText(Container, 'Item 2')), Offset.zero);
+
+    controller.jumpTo(2000.0);
+    await tester.pump();
+    expect(tester.getTopLeft(find.widgetWithText(Container, 'Item 20')), Offset.zero);
+
+    // The initialScrollOffset isn't used in this case, because the scrolloffset
+    // can be restored.
+    controller = new ScrollController(initialScrollOffset: 25.0);
+    await tester.pumpWidget(buildFrame(controller));
+    expect(controller.offset, 2000.0);
+    expect(tester.getTopLeft(find.widgetWithText(Container, 'Item 20')), Offset.zero);
+
+    // keepScrollOffset: false. The scroll offset is -not- restored
+    // when the ListView is recreated with a new ScrollController and
+    // the initialScrollOffset is used.
+
+    controller = new ScrollController(keepScrollOffset: false, initialScrollOffset: 100.0);
+    await tester.pumpWidget(buildFrame(controller));
+    expect(controller.offset, 100.0);
+    expect(tester.getTopLeft(find.widgetWithText(Container, 'Item 1')), Offset.zero);
+
+  });
 }