AppBar size can be lost after a Navigator.pop() (#4594)

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

@@ -320,6 +320,8 @@ class Scaffold extends StatefulWidget {
 /// the current [BuildContext] using [Scaffold.of].
 class ScaffoldState extends State<Scaffold> {
 
+  static final Object _kScaffoldStorageIdentifier = new Object();
+
   // APPBAR API
 
   AnimationController _appBarController;
@@ -511,7 +513,11 @@ class ScaffoldState extends State<Scaffold> {
   void initState() {
     super.initState();
     _appBarController = new AnimationController();
-    List<double> scrollValues = PageStorage.of(context)?.readState(context);
+    // Use an explicit identifier to guard against the possibility that the
+    // Scaffold's key is recreated by the Widget that creates the Scaffold.
+    List<double> scrollValues = PageStorage.of(context)?.readState(context,
+      identifier: _kScaffoldStorageIdentifier
+    );
     if (scrollValues != null) {
       assert(scrollValues.length == 2);
       _scrollOffset = scrollValues[0];
@@ -526,7 +532,9 @@ class ScaffoldState extends State<Scaffold> {
     _snackBarController = null;
     _snackBarTimer?.cancel();
     _snackBarTimer = null;
-    PageStorage.of(context)?.writeState(context, <double>[_scrollOffset, _scrollOffsetDelta]);
+    PageStorage.of(context)?.writeState(context, <double>[_scrollOffset, _scrollOffsetDelta],
+      identifier: _kScaffoldStorageIdentifier
+    );
     super.dispose();
   }
 

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

@@ -74,19 +74,28 @@ class PageStorageBucket {
     return result;
   }
 
-  Map<_StorageEntryIdentifier, dynamic> _storage;
+  Map<Object, dynamic> _storage;
 
   /// Write the given data into this page storage bucket using an identifier
-  /// computed from the given context.
-  void writeState(BuildContext context, dynamic data) {
-    _storage ??= <_StorageEntryIdentifier, dynamic>{};
-    _storage[_computeStorageIdentifier(context)] = data;
+  /// 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.
+  ///
+  /// 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.
+  void writeState(BuildContext context, dynamic data, { Object identifier }) {
+    _storage ??= <Object, dynamic>{};
+    _storage[identifier ?? _computeStorageIdentifier(context)] = data;
   }
 
   /// Read given data from into this page storage bucket using an identifier
-  /// computed from the given context.
-  dynamic readState(BuildContext context) {
-    return _storage != null ? _storage[_computeStorageIdentifier(context)] : null;
+  /// computed from the given context. More about [identifier] in [writeState].
+  dynamic readState(BuildContext context, { Object identifier }) {
+    return _storage != null ? _storage[identifier ?? _computeStorageIdentifier(context)] : null;
   }
 }
 

packages/flutter/test/widget/page_storage_test.dart

@@ -0,0 +1,82 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/material.dart';
+
+void main() {
+  testWidgets('PageStorage read and write', (WidgetTester tester) async {
+    final Key builderKey = new Key('builderKey');
+    StateSetter setState;
+    int storedValue = 0;
+
+    await tester.pumpWidget(
+      new MaterialApp(
+        home: new StatefulBuilder(
+          key: builderKey,
+          builder: (BuildContext context, StateSetter setter) {
+            PageStorage.of(context).writeState(context, storedValue);
+            setState = setter;
+            return new Center(
+              child: new Text('storedValue: $storedValue')
+            );
+          }
+        )
+      )
+    );
+
+    Element builderElement = tester.element(find.byKey(builderKey));
+    expect(PageStorage.of(builderElement), isNotNull);
+    expect(PageStorage.of(builderElement).readState(builderElement), equals(storedValue));
+
+    setState(() {
+      storedValue = 1;
+    });
+    await tester.pump();
+    expect(PageStorage.of(builderElement).readState(builderElement), equals(storedValue));
+  });
+
+  testWidgets('PageStorage read and write by identifier', (WidgetTester tester) async {
+    StateSetter setState;
+    int storedValue = 0;
+
+    Widget buildWidthKey(Key key) {
+      return new MaterialApp(
+        home: new StatefulBuilder(
+          key: key,
+          builder: (BuildContext context, StateSetter setter) {
+            PageStorage.of(context).writeState(context, storedValue, identifier: 123);
+            setState = setter;
+            return new Center(
+              child: new Text('storedValue: $storedValue')
+            );
+          }
+        )
+      );
+    }
+
+    Key key = new Key('Key one');
+    await tester.pumpWidget(buildWidthKey(key));
+    Element builderElement = tester.element(find.byKey(key));
+    expect(PageStorage.of(builderElement), isNotNull);
+    expect(PageStorage.of(builderElement).readState(builderElement), isNull);
+    expect(PageStorage.of(builderElement).readState(builderElement, identifier: 123), equals(storedValue));
+
+    // New StatefulBuilder widget - different key - but the same PageStorage identifier.
+
+    key = new Key('Key two');
+    await tester.pumpWidget(buildWidthKey(key));
+    builderElement = tester.element(find.byKey(key));
+    expect(PageStorage.of(builderElement), isNotNull);
+    expect(PageStorage.of(builderElement).readState(builderElement), isNull);
+    expect(PageStorage.of(builderElement).readState(builderElement, identifier: 123), equals(storedValue));
+
+    setState(() {
+      storedValue = 1;
+    });
+    await tester.pump();
+    expect(PageStorage.of(builderElement).readState(builderElement, identifier: 123), equals(storedValue));
+  });
+
+}