Fix keepalive for large jumps on tabs and lists. (#21350)

* Ensure that the _childElements map is properly traversed as a sparse list and not inflated with garbage collected children.

* Add tests to ensure Lists/Tabs with KeepAlive children can make large jumps, don't lose children (including after rebuild).

packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart

@@ -130,8 +130,9 @@ class SliverMultiBoxAdaptorParentData extends SliverLogicalParentData with Conta
   /// Whether to keep the child alive even when it is no longer visible.
   bool keepAlive = false;
 
-  /// Whether the widget is currently in the
-  /// [RenderSliverMultiBoxAdaptor._keepAliveBucket].
+  /// Whether the widget is currently being kept alive, i.e. has [keepAlive] set
+  /// to true and is offscreen.
+  bool get keptAlive => _keptAlive;
   bool _keptAlive = false;
 
   @override
@@ -206,6 +207,7 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
 
   @override
   void insert(RenderBox child, { RenderBox after }) {
+    assert(!_keepAliveBucket.containsValue(child));
     super.insert(child, after: after);
     assert(firstChild != null);
     assert(() {

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

@@ -89,6 +89,9 @@ class _AutomaticKeepAliveState extends State<AutomaticKeepAlive> {
         // build of this subtree. Wait until the end of the frame to update
         // the child when the child is guaranteed to be present.
         SchedulerBinding.instance.addPostFrameCallback((Duration timeStamp) {
+          if (!mounted) {
+            return;
+          }
           final ParentDataElement<SliverMultiBoxAdaptorWidget> childElement = _getChildElement();
           assert(childElement != null);
           _updateParentDataOfChild(childElement);
@@ -103,6 +106,7 @@ class _AutomaticKeepAliveState extends State<AutomaticKeepAlive> {
   /// While this widget is guaranteed to have a child, this may return null if
   /// the first build of that child has not completed yet.
   ParentDataElement<SliverMultiBoxAdaptorWidget> _getChildElement() {
+    assert(mounted);
     final Element element = context;
     Element childElement;
     // We use Element.visitChildren rather than context.visitChildElements

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

@@ -756,24 +756,24 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render
     _currentBeforeChild = null;
     assert(_currentlyUpdatingChildIndex == null);
     try {
-      int firstIndex = _childElements.firstKey();
-      int lastIndex = _childElements.lastKey();
-      if (_childElements.isEmpty) {
-        firstIndex = 0;
-        lastIndex = 0;
-      } else if (_didUnderflow) {
-        lastIndex += 1;
-      }
-      for (int index = firstIndex; index <= lastIndex; ++index) {
+      void processElement(int index) {
         _currentlyUpdatingChildIndex = index;
         final Element newChild = updateChild(_childElements[index], _build(index), index);
         if (newChild != null) {
           _childElements[index] = newChild;
-          _currentBeforeChild = newChild.renderObject;
+          final SliverMultiBoxAdaptorParentData parentData = newChild.renderObject.parentData;
+          if (!parentData.keptAlive)
+            _currentBeforeChild = newChild.renderObject;
         } else {
           _childElements.remove(index);
         }
       }
+      // processElement may modify the Map - need to do a .toList() here.
+      _childElements.keys.toList().forEach(processElement);
+      if (_didUnderflow) {
+        final int lastKey = _childElements.lastKey() ?? -1;
+        processElement(lastKey + 1);
+      }
     } finally {
       _currentlyUpdatingChildIndex = null;
     }

packages/flutter/test/material/tabs_test.dart

@@ -48,6 +48,23 @@ class StateMarkerState extends State<StateMarker> {
   }
 }
 
+class AlwaysKeepAliveWidget extends StatefulWidget {
+  static String text = 'AlwaysKeepAlive';
+  @override
+  AlwaysKeepAliveState createState() => new AlwaysKeepAliveState();
+}
+
+class AlwaysKeepAliveState extends State<AlwaysKeepAliveWidget>
+    with AutomaticKeepAliveClientMixin {
+  @override
+  bool get wantKeepAlive => true;
+
+  @override
+  Widget build(BuildContext context) {
+    return new Text(AlwaysKeepAliveWidget.text);
+  }
+}
+
 Widget buildFrame({
     Key tabBarKey,
     List<String> tabs,
@@ -1754,4 +1771,56 @@ void main() {
 
   });
 
+  testWidgets('Skipping tabs with a KeepAlive child works', (WidgetTester tester) async {
+    // Regression test for https://github.com/flutter/flutter/issues/11895
+    final List<String> tabs = <String>[
+      'Tab1',
+      'Tab2',
+      'Tab3',
+      'Tab4',
+      'Tab5',
+    ];
+    final TabController controller = new TabController(
+      vsync: const TestVSync(),
+      length: tabs.length,
+    );
+    await tester.pumpWidget(
+      new MaterialApp(
+        home: new Align(
+          alignment: Alignment.topLeft,
+          child: new SizedBox(
+            width: 300.0,
+            height: 200.0,
+            child: new Scaffold(
+              appBar: new AppBar(
+                title: const Text('tabs'),
+                bottom: new TabBar(
+                  controller: controller,
+                  tabs: tabs.map((String tab) => new Tab(text: tab)).toList(),
+                ),
+              ),
+              body: new TabBarView(
+                controller: controller,
+                children: <Widget>[
+                  new AlwaysKeepAliveWidget(),
+                  const Text('2'),
+                  const Text('3'),
+                  const Text('4'),
+                  const Text('5'),
+                ],
+              ),
+            ),
+          ),
+        ),
+      ),
+    );
+    expect(find.text(AlwaysKeepAliveWidget.text), findsOneWidget);
+    expect(find.text('4'), findsNothing);
+    await tester.tap(find.text('Tab4'));
+    await tester.pumpAndSettle();
+    await tester.pump();
+    expect(controller.index, 3);
+    expect(find.text(AlwaysKeepAliveWidget.text, skipOffstage: false), findsOneWidget);
+    expect(find.text('4'), findsOneWidget);
+  });
 }

packages/flutter/test/widgets/automatic_keep_alive_test.dart

@@ -496,6 +496,44 @@ void main() {
     expect(find.text('FooBar 1'), findsNothing);
     expect(find.text('FooBar 2'), findsNothing);
   });
+
+  testWidgets('AutomaticKeepAlive with keepAlive set to true before initState and widget goes out of scope', (WidgetTester tester) async {
+    await tester.pumpWidget(new Directionality(
+      textDirection: TextDirection.ltr,
+      child: new ListView.builder(
+        itemCount: 250,
+        itemBuilder: (BuildContext context, int index){
+          if (index % 2 == 0){
+            return new _AlwaysKeepAlive(
+              key: GlobalObjectKey<_AlwaysKeepAliveState>(index),
+            );
+          }
+          return new Container(
+            height: 44.0,
+            child: new Text('FooBar $index'),
+          );
+        },
+      ),
+    ));
+
+    expect(find.text('keep me alive'), findsNWidgets(7));
+    expect(find.text('FooBar 1'), findsOneWidget);
+    expect(find.text('FooBar 3'), findsOneWidget);
+
+    expect(find.byKey(const GlobalObjectKey<_AlwaysKeepAliveState>(0)), findsOneWidget);
+
+    final ScrollableState state = tester.state(find.byType(Scrollable));
+    final ScrollPosition position = state.position;
+    position.jumpTo(3025.0);
+
+    await tester.pump();
+    expect(find.byKey(const GlobalObjectKey<_AlwaysKeepAliveState>(0), skipOffstage: false), findsOneWidget);
+
+    expect(find.text('keep me alive', skipOffstage: false), findsNWidgets(23));
+    expect(find.text('FooBar 1'), findsNothing);
+    expect(find.text('FooBar 3'), findsNothing);
+    expect(find.text('FooBar 73'), findsOneWidget);
+  });
 }
 
 class _AlwaysKeepAlive extends StatefulWidget {

packages/flutter/test/widgets/list_view_test.dart

@@ -18,6 +18,61 @@ class TestSliverChildListDelegate extends SliverChildListDelegate {
   }
 }
 
+class Alive extends StatefulWidget {
+  const Alive(this.alive, this.index);
+  final bool alive;
+  final int index;
+
+  @override
+  AliveState createState() => new AliveState();
+
+  @override
+  String toString({DiagnosticLevel minLevel}) => '$index $alive';
+}
+
+class AliveState extends State<Alive> with AutomaticKeepAliveClientMixin {
+  @override
+  bool get wantKeepAlive => widget.alive;
+
+  @override
+  Widget build(BuildContext context) =>
+     new Text('${widget.index}:$wantKeepAlive');
+}
+
+typedef WhetherToKeepAlive = bool Function(int);
+class _StatefulListView extends StatefulWidget {
+  const _StatefulListView(this.aliveCallback);
+
+  final WhetherToKeepAlive aliveCallback;
+  @override
+  _StatefulListViewState createState() => new _StatefulListViewState();
+}
+
+class _StatefulListViewState extends State<_StatefulListView> {
+  @override
+  Widget build(BuildContext context) {
+    return new GestureDetector(
+      // force a rebuild - the test(s) using this are verifying that the list is
+      // still correct after rebuild
+      onTap: () => setState,
+      child: new Directionality(
+        textDirection: TextDirection.ltr,
+        child: new ListView(
+          children: new List<Widget>.generate(200, (int i) {
+            return new Builder(
+              builder: (BuildContext context) {
+                return new Container(
+                  child: new Alive(widget.aliveCallback(i), i),
+                );
+              },
+            );
+          }),
+        ),
+      ),
+    );
+  }
+}
+
 void main() {
   testWidgets('ListView default control', (WidgetTester tester) async {
     await tester.pumpWidget(
@@ -91,7 +146,7 @@ void main() {
                 return new Container(
                   child: new Text('$i'),
                 );
-              }
+              },
             );
           }),
         ),
@@ -120,6 +175,43 @@ void main() {
     log.clear();
   });
 
+  testWidgets('ListView large scroll jump and keepAlive first child not keepAlive', (WidgetTester tester) async {
+    Future<Null> checkAndScroll([String zero = '0:false']) async {
+      expect(find.text(zero), findsOneWidget);
+      expect(find.text('1:false'), findsOneWidget);
+      expect(find.text('2:false'), findsOneWidget);
+      expect(find.text('3:true'), findsOneWidget);
+      expect(find.text('116:false'), findsNothing);
+      final ScrollableState state = tester.state(find.byType(Scrollable));
+      final ScrollPosition position = state.position;
+      position.jumpTo(1025.0);
+
+      await tester.pump();
+
+      expect(find.text(zero), findsNothing);
+      expect(find.text('1:false'), findsNothing);
+      expect(find.text('2:false'), findsNothing);
+      expect(find.text('3:true', skipOffstage: false), findsOneWidget);
+      expect(find.text('116:false'), findsOneWidget);
+
+      await tester.tapAt(const Offset(100.0, 100.0));
+      position.jumpTo(0.0);
+      await tester.pump();
+      await tester.pump();
+
+      expect(find.text(zero), findsOneWidget);
+      expect(find.text('1:false'), findsOneWidget);
+      expect(find.text('2:false'), findsOneWidget);
+      expect(find.text('3:true'), findsOneWidget);
+    }
+
+    await tester.pumpWidget(new _StatefulListView((int i) => i > 2 && i % 3 == 0));
+    await checkAndScroll();
+
+    await tester.pumpWidget(new _StatefulListView((int i) => i % 3 == 0));
+    await checkAndScroll('0:true');
+  });
+
   testWidgets('ListView can build out of underflow', (WidgetTester tester) async {
     await tester.pumpWidget(
       new Directionality(
@@ -225,11 +317,14 @@ void main() {
 
   testWidgets('didFinishLayout has correct indices', (WidgetTester tester) async {
     final TestSliverChildListDelegate delegate = new TestSliverChildListDelegate(
-      new List<Widget>.generate(20, (int i) {
-        return new Container(
-          child: new Text('$i', textDirection: TextDirection.ltr),
-        );
-      })
+      new List<Widget>.generate(
+        20,
+        (int i) {
+          return new Container(
+            child: new Text('$i', textDirection: TextDirection.ltr),
+          );
+        },
+      )
     );
 
     await tester.pumpWidget(
@@ -345,7 +440,7 @@ void main() {
           ),
         ),
       ),
-  );
+    );
 
     expect(find.byType(Viewport), isNot(paints..clipRect()));
   });