Do not read or write state if PageStorageKeys cannot be found (#10612)

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

@@ -39,21 +39,19 @@ class PageStorageKey<T> extends ValueKey<T> {
 }
 
 class _StorageEntryIdentifier {
-  _StorageEntryIdentifier(this.clientType, this.keys) {
-    assert(clientType != null);
+  _StorageEntryIdentifier(this.keys) {
     assert(keys != null);
   }
 
-  final Type clientType;
   final List<PageStorageKey<dynamic>> keys;
 
+  bool get isNotEmpty => keys.isNotEmpty;
+
   @override
   bool operator ==(dynamic other) {
     if (other.runtimeType != runtimeType)
       return false;
     final _StorageEntryIdentifier typedOther = other;
-    if (clientType != typedOther.clientType || keys.length != typedOther.keys.length)
-      return false;
     for (int index = 0; index < keys.length; index += 1) {
       if (keys[index] != typedOther.keys[index])
         return false;
@@ -62,11 +60,11 @@ class _StorageEntryIdentifier {
   }
 
   @override
-  int get hashCode => hashValues(clientType, hashList(keys));
+  int get hashCode => hashList(keys);
 
   @override
   String toString() {
-    return 'StorageEntryIdentifier($clientType, ${keys?.join(":")})';
+    return 'StorageEntryIdentifier(${keys?.join(":")})';
   }
 }
 
@@ -94,31 +92,45 @@ class PageStorageBucket {
   }
 
   _StorageEntryIdentifier _computeIdentifier(BuildContext context) {
-    return new _StorageEntryIdentifier(context.widget.runtimeType, _allKeys(context));
+    return new _StorageEntryIdentifier(_allKeys(context));
   }
 
   Map<Object, dynamic> _storage;
 
-  /// Write the given data into this page storage bucket using an identifier
-  /// computed from the given context. The identifier is based on the keys
-  /// found in the path from context to the root of the widget tree for this
-  /// page. Keys are collected until the widget tree's root is reached or
-  /// a GlobalKey is found.
+  /// Write the given data into this page storage bucket using the
+  /// specified identifier or an identifier computed from the given context.
+  /// The computed identifier is based on the [PageStorageKey]s
+  /// found in the path from context to the [PageStorage] widget that
+  /// owns this page storage bucket.
   ///
-  /// An explicit identifier can be used in cases where the list of keys
-  /// is not stable. For example if the path concludes with a GlobalKey
-  /// that's created by a stateful widget, if the stateful widget is
-  /// recreated when it's exposed by [Navigator.pop], then its storage
-  /// identifier will change.
+  /// If an explicit identifier is not provided and no [PageStorageKey]s
+  /// are found, then the `data` is not saved.
   void writeState(BuildContext context, dynamic data, { Object identifier }) {
     _storage ??= <Object, dynamic>{};
-    _storage[identifier ?? _computeIdentifier(context)] = data;
+    if (identifier != null) {
+      _storage[identifier] = data;
+    } else {
+      final _StorageEntryIdentifier contextIdentifier = _computeIdentifier(context);
+      if (contextIdentifier.isNotEmpty)
+        _storage[contextIdentifier] = data;
+    }
   }
 
-  /// Read given data from into this page storage bucket using an identifier
-  /// computed from the given context. More about [identifier] in [writeState].
+  /// Read given data from into this page storage bucket using the specified
+  /// identifier or an identifier computed from the given context.
+  /// The computed identifier is based on the [PageStorageKey]s
+  /// found in the path from context to the [PageStorage] widget that
+  /// owns this page storage bucket.
+  ///
+  /// If an explicit identifier is not provided and no [PageStorageKey]s
+  /// are found, then null is returned.
   dynamic readState(BuildContext context, { Object identifier }) {
-    return _storage != null ? _storage[identifier ?? _computeIdentifier(context)] : null;
+    if (_storage == null)
+      return null;
+    if (identifier != null)
+      return _storage[identifier];
+    final _StorageEntryIdentifier contextIdentifier = _computeIdentifier(context);
+    return contextIdentifier.isNotEmpty ? _storage[contextIdentifier] : null;
   }
 }
 

packages/flutter/test/widgets/page_storage_test.dart

@@ -7,7 +7,7 @@ import 'package:flutter/material.dart';
 
 void main() {
   testWidgets('PageStorage read and write', (WidgetTester tester) async {
-    final Key builderKey = const Key('builderKey');
+    final Key builderKey = const PageStorageKey<String>('builderKey');
     StateSetter setState;
     int storedValue = 0;
 

packages/flutter/test/widgets/page_view_test.dart

@@ -407,6 +407,7 @@ void main() {
       new PageStorage(
         bucket: bucket,
         child: new PageView(
+          key: const PageStorageKey<String>('PageView'),
           controller: controller,
           children: <Widget>[
             const Placeholder(),
@@ -431,6 +432,7 @@ void main() {
       new PageStorage(
         bucket: bucket,
         child: new PageView(
+          key: const PageStorageKey<String>('PageView'),
           controller: controller,
           children: <Widget>[
             const Placeholder(),
@@ -447,7 +449,7 @@ void main() {
       new PageStorage(
         bucket: bucket,
         child: new PageView(
-          key: const Key('Check it again against your list and see consistency!'),
+          key: const PageStorageKey<String>('Check it again against your list and see consistency!'),
           controller: controller2,
           children: <Widget>[
             const Placeholder(),

packages/flutter/test/widgets/remember_scroll_position_test.dart

@@ -18,6 +18,7 @@ class ThePositiveNumbers extends StatelessWidget {
   @override
   Widget build(BuildContext context) {
     return new ListView.builder(
+      key: const PageStorageKey<String>('ThePositiveNumbers'),
       itemExtent: 100.0,
       controller: _controller,
       itemBuilder: (BuildContext context, int index) {

packages/flutter/test/widgets/scroll_controller_test.dart

@@ -266,12 +266,15 @@ void main() {
     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(),
+        child: new KeyedSubtree(
+          key: const PageStorageKey<String>('ListView'),
+          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(),
+          ),
         ),
       );
     }