TabView should keep state

Previously, we lost sync with the tab view contents when switching tabs. Now we
key the subtrees to make sure they keep their state across tab animations.

Fixes #3147

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

@@ -959,9 +959,12 @@ class _TabBarViewState<T> extends PageableListState<TabBarView<T>> implements Ta
 
   void _updateItemsFromChildren(int first, int second, [int third]) {
     List<Widget> widgets = config.children;
-    _items = <Widget>[widgets[first], widgets[second]];
+    _items = <Widget>[
+      new KeyedSubtree.wrap(widgets[first], first),
+      new KeyedSubtree.wrap(widgets[second], second),
+    ];
     if (third != null)
-      _items.add(widgets[third]);
+      _items.add(new KeyedSubtree.wrap(widgets[third], third));
   }
 
   void _updateItemsForSelectedIndex(int selectedIndex) {

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

@@ -2616,13 +2616,23 @@ class MetaData extends SingleChildRenderObjectWidget {
   }
 }
 
+/// Always builds the given child.
+///
+/// Useful for attaching a key to an existing widget.
 class KeyedSubtree extends StatelessWidget {
+  /// Creates a widget that always builds the given child.
   KeyedSubtree({ Key key, this.child })
     : super(key: key);
 
   /// The widget below this widget in the tree.
   final Widget child;
 
+  /// Creates a KeyedSubtree for child with a key that's based on the child's existing key or childIndex.
+  factory KeyedSubtree.wrap(Widget child, int childIndex) {
+    Key key = child.key != null ? new ValueKey<Key>(child.key) : new ValueKey<int>(childIndex);
+    return new KeyedSubtree(key: key, child: child);
+  }
+
   /// Wrap each item in a KeyedSubtree whose key is based on the item's existing key or
   /// its list index + baseIndex.
   static List<Widget> ensureUniqueKeysForList(Iterable<Widget> items, { int baseIndex: 0 }) {
@@ -2631,11 +2641,8 @@ class KeyedSubtree extends StatelessWidget {
 
     List<Widget> itemsWithUniqueKeys = <Widget>[];
     int itemIndex = baseIndex;
-    for(Widget item in items) {
-      itemsWithUniqueKeys.add(new KeyedSubtree(
-        key: item.key != null ? new ValueKey<Key>(item.key) : new ValueKey<int>(itemIndex),
-        child: item
-      ));
+    for (Widget item in items) {
+      itemsWithUniqueKeys.add(new KeyedSubtree.wrap(item, itemIndex));
       itemIndex += 1;
     }
 
@@ -2643,7 +2650,6 @@ class KeyedSubtree extends StatelessWidget {
     return itemsWithUniqueKeys;
   }
 
-
   @override
   Widget build(BuildContext context) => child;
 }

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

@@ -1158,6 +1158,9 @@ abstract class Element implements BuildContext {
     return renderObjectAncestor?.renderObject;
   }
 
+  /// Calls visitor for each ancestor element.
+  ///
+  /// Continues until visitor reaches the root or until visitor returns false.
   @override
   void visitAncestorElements(bool visitor(Element element)) {
     Element ancestor = _parent;

packages/flutter/test/widget/tabs_test.dart → packages/flutter/test/material/tabs_test.dart

@@ -7,6 +7,26 @@ import 'package:flutter/material.dart';
 import 'package:flutter/widgets.dart';
 import 'package:test/test.dart';
 
+class StateMarker extends StatefulWidget {
+  StateMarker({ Key key, this.child }) : super(key: key);
+
+  final Widget child;
+
+  @override
+  StateMarkerState createState() => new StateMarkerState();
+}
+
+class StateMarkerState extends State<StateMarker> {
+  String marker;
+
+  @override
+  Widget build(BuildContext context) {
+    if (config.child != null)
+      return config.child;
+    return new Container();
+  }
+}
+
 Widget buildFrame({ List<String> tabs, String value, bool isScrollable: false, Key tabBarKey }) {
   return new Material(
     child: new TabBarSelection<String>(
@@ -143,4 +163,90 @@ void main() {
       expect(selection.value, equals('AAAAAA'));
     });
   });
+
+  test('TabView maintains state', () {
+    testWidgets((WidgetTester tester) {
+      List<String> tabs = <String>['AAAAAA', 'BBBBBB', 'CCCCCC', 'DDDDDD', 'EEEEEE'];
+      String value = tabs[0];
+
+      void onTabSelectionChanged(String newValue) {
+        value = newValue;
+      }
+
+      Widget builder() {
+        return new Material(
+          child: new TabBarSelection<String>(
+            value: value,
+            values: tabs,
+            onChanged: onTabSelectionChanged,
+            child: new TabBarView<String>(
+              children: tabs.map((String name) {
+                return new StateMarker(
+                  child: new Text(name)
+                );
+              }).toList()
+            )
+          )
+        );
+      }
+
+      StateMarkerState findStateMarkerState(String name) {
+        Element secondLabel = tester.findText(name);
+        expect(secondLabel, isNotNull);
+        StatefulElement secondMarker;
+        secondLabel.visitAncestorElements((Element element) {
+          if (element.widget is StateMarker) {
+            secondMarker = element;
+            return false;
+          }
+          return true;
+        });
+        expect(secondMarker, isNotNull);
+        return secondMarker.state;
+      }
+
+      tester.pumpWidget(builder());
+      TestGesture gesture = tester.startGesture(tester.getCenter(tester.findText(tabs[0])));
+      gesture.moveBy(new Offset(-600.0, 0.0));
+      tester.pump();
+      expect(value, equals(tabs[0]));
+      findStateMarkerState(tabs[1]).marker = 'marked';
+      gesture.up();
+      tester.pump();
+      tester.pump(const Duration(seconds: 1));
+      expect(value, equals(tabs[1]));
+      tester.pumpWidget(builder());
+      expect(findStateMarkerState(tabs[1]).marker, equals('marked'));
+
+      // Move to the third tab.
+
+      gesture = tester.startGesture(tester.getCenter(tester.findText(tabs[1])));
+      gesture.moveBy(new Offset(-600.0, 0.0));
+      gesture.up();
+      tester.pump();
+      expect(findStateMarkerState(tabs[1]).marker, equals('marked'));
+      tester.pump(const Duration(seconds: 1));
+      expect(value, equals(tabs[2]));
+      tester.pumpWidget(builder());
+
+      // The state is now gone.
+
+      expect(tester.findText(tabs[1]), isNull);
+
+      // Move back to the second tab.
+
+      gesture = tester.startGesture(tester.getCenter(tester.findText(tabs[2])));
+      gesture.moveBy(new Offset(600.0, 0.0));
+      tester.pump();
+      StateMarkerState markerState = findStateMarkerState(tabs[1]);
+      expect(markerState.marker, isNull);
+      markerState.marker = 'marked';
+      gesture.up();
+      tester.pump();
+      tester.pump(const Duration(seconds: 1));
+      expect(value, equals(tabs[1]));
+      tester.pumpWidget(builder());
+      expect(findStateMarkerState(tabs[1]).marker, equals('marked'));
+    });
+  });
 }