Fix 25807: implement move for sliver multibox widget (#29188)

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

@@ -1116,6 +1116,7 @@ class _TabBarViewState extends State<TabBarView> {
   TabController _controller;
   PageController _pageController;
   List<Widget> _children;
+  List<Widget> _childrenWithKey;
   int _currentIndex;
   int _warpUnderwayCount = 0;
 
@@ -1157,7 +1158,7 @@ class _TabBarViewState extends State<TabBarView> {
   @override
   void initState() {
     super.initState();
-    _children = widget.children;
+    _updateChildren();
   }
 
   @override
@@ -1174,7 +1175,7 @@ class _TabBarViewState extends State<TabBarView> {
     if (widget.controller != oldWidget.controller)
       _updateTabController();
     if (widget.children != oldWidget.children && _warpUnderwayCount == 0)
-      _children = widget.children;
+      _updateChildren();
   }
 
   @override
@@ -1185,6 +1186,11 @@ class _TabBarViewState extends State<TabBarView> {
     super.dispose();
   }
 
+  void _updateChildren() {
+    _children = widget.children;
+    _childrenWithKey = KeyedSubtree.ensureUniqueKeysForList(widget.children);
+  }
+
   void _handleTabControllerAnimationTick() {
     if (_warpUnderwayCount > 0 || !_controller.indexIsChanging)
       return; // This widget is driving the controller's animation.
@@ -1207,28 +1213,30 @@ class _TabBarViewState extends State<TabBarView> {
       return _pageController.animateToPage(_currentIndex, duration: kTabScrollDuration, curve: Curves.ease);
 
     assert((_currentIndex - previousIndex).abs() > 1);
-    int initialPage;
+    final int initialPage = _currentIndex > previousIndex
+        ? _currentIndex - 1
+        : _currentIndex + 1;
+    final List<Widget> originalChildren = _childrenWithKey;
     setState(() {
       _warpUnderwayCount += 1;
-      _children = List<Widget>.from(widget.children, growable: false);
-      if (_currentIndex > previousIndex) {
-        _children[_currentIndex - 1] = _children[previousIndex];
-        initialPage = _currentIndex - 1;
-      } else {
-        _children[_currentIndex + 1] = _children[previousIndex];
-        initialPage = _currentIndex + 1;
-      }
-    });
 
+      _childrenWithKey = List<Widget>.from(_childrenWithKey, growable: false);
+      final Widget temp = _childrenWithKey[initialPage];
+      _childrenWithKey[initialPage] = _childrenWithKey[previousIndex];
+      _childrenWithKey[previousIndex] = temp;
+    });
     _pageController.jumpToPage(initialPage);
 
     await _pageController.animateToPage(_currentIndex, duration: kTabScrollDuration, curve: Curves.ease);
     if (!mounted)
       return Future<void>.value();
-
     setState(() {
       _warpUnderwayCount -= 1;
-      _children = widget.children;
+      if (widget.children != _children) {
+        _updateChildren();
+      } else {
+        _childrenWithKey = originalChildren;
+      }
     });
   }
 
@@ -1264,7 +1272,7 @@ class _TabBarViewState extends State<TabBarView> {
         dragStartBehavior: widget.dragStartBehavior,
         controller: _pageController,
         physics: widget.physics == null ? _kTabBarViewPhysics : _kTabBarViewPhysics.applyTo(widget.physics),
-        children: _children,
+        children: _childrenWithKey,
       ),
     );
   }

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

@@ -85,7 +85,8 @@ abstract class RenderSliverBoxChildManager {
   /// list).
   int get childCount;
 
-  /// Called during [RenderSliverMultiBoxAdaptor.adoptChild].
+  /// Called during [RenderSliverMultiBoxAdaptor.adoptChild] or
+  /// [RenderSliverMultiBoxAdaptor.move].
   ///
   /// Subclasses must ensure that the [SliverMultiBoxAdaptorParentData.index]
   /// field of the child's [RenderObject.parentData] accurately reflects the
@@ -193,7 +194,12 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
   RenderSliverMultiBoxAdaptor({
     @required RenderSliverBoxChildManager childManager,
   }) : assert(childManager != null),
-       _childManager = childManager;
+       _childManager = childManager {
+    assert(() {
+      _debugDanglingKeepAlives = <RenderBox>[];
+      return true;
+    }());
+  }
 
   @override
   void setupParentData(RenderObject child) {
@@ -214,6 +220,27 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
   /// The nodes being kept alive despite not being visible.
   final Map<int, RenderBox> _keepAliveBucket = <int, RenderBox>{};
 
+  List<RenderBox> _debugDanglingKeepAlives;
+
+  /// Indicates whether integrity check is enabled.
+  ///
+  /// Setting this property to true will immediately perform an integrity check.
+  ///
+  /// The integrity check consists of:
+  ///
+  /// 1. Verify that the children index in childList is in ascending order.
+  /// 2. Verify that there is no dangling keepalive child as the result of [move].
+  bool get debugChildIntegrityEnabled => _debugChildIntegrityEnabled;
+  bool _debugChildIntegrityEnabled = true;
+  set debugChildIntegrityEnabled(bool enabled) {
+    assert(enabled != null);
+    assert(() {
+      _debugChildIntegrityEnabled = enabled;
+      return _debugVerifyChildOrder() &&
+        (!_debugChildIntegrityEnabled || _debugDanglingKeepAlives.isEmpty);
+    }());
+  }
+
   @override
   void adoptChild(RenderObject child) {
     super.adoptChild(child);
@@ -224,21 +251,70 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
 
   bool _debugAssertChildListLocked() => childManager.debugAssertChildListLocked();
 
+  /// Verify that the child list index is in strictly increasing order.
+  ///
+  /// This has no effect in release builds.
+  bool _debugVerifyChildOrder(){
+    if (_debugChildIntegrityEnabled) {
+      RenderBox child = firstChild;
+      int index;
+      while (child != null) {
+        index = indexOf(child);
+        child = childAfter(child);
+        assert(child == null || indexOf(child) > index);
+      }
+    }
+    return true;
+  }
+
   @override
   void insert(RenderBox child, { RenderBox after }) {
     assert(!_keepAliveBucket.containsValue(child));
     super.insert(child, after: after);
     assert(firstChild != null);
-    assert(() {
-      int index = indexOf(firstChild);
-      RenderBox child = childAfter(firstChild);
-      while (child != null) {
-        assert(indexOf(child) > index);
-        index = indexOf(child);
-        child = childAfter(child);
+    assert(_debugVerifyChildOrder());
+  }
+
+  @override
+  void move(RenderBox child, { RenderBox after }) {
+    // There are two scenarios:
+    //
+    // 1. The child is not keptAlive.
+    // The child is in the childList maintained by ContainerRenderObjectMixin.
+    // We can call super.move and update parentData with the new slot.
+    //
+    // 2. The child is keptAlive.
+    // In this case, the child is no longer in the childList but might be stored in
+    // [_keepAliveBucket]. We need to update the location of the child in the bucket.
+    final SliverMultiBoxAdaptorParentData childParentData = child.parentData;
+    if (!childParentData.keptAlive) {
+      super.move(child, after: after);
+      childManager.didAdoptChild(child); // updates the slot in the parentData
+      // Its slot may change even if super.move does not change the position.
+      // In this case, we still want to mark as needs layout.
+      markNeedsLayout();
+    } else {
+      // If the child in the bucket is not current child, that means someone has
+      // already moved and replaced current child, and we cannot remove this child.
+      if (_keepAliveBucket[childParentData.index] == child) {
+        _keepAliveBucket.remove(childParentData.index);
       }
-      return true;
-    }());
+      assert(() {
+        _debugDanglingKeepAlives.remove(child);
+        return true;
+      }());
+      // Update the slot and reinsert back to _keepAliveBucket in the new slot.
+      childManager.didAdoptChild(child);
+      // If there is an existing child in the new slot, that mean that child will
+      // be moved to other index. In other cases, the existing child should have been
+      // removed by updateChild. Thus, it is ok to overwrite it.
+      assert(() {
+        if (_keepAliveBucket.containsKey(childParentData.index))
+          _debugDanglingKeepAlives.add(_keepAliveBucket[childParentData.index]);
+        return true;
+      }());
+      _keepAliveBucket[childParentData.index] = child;
+    }
   }
 
   @override
@@ -249,6 +325,10 @@ abstract class RenderSliverMultiBoxAdaptor extends RenderSliver
       return;
     }
     assert(_keepAliveBucket[childParentData.index] == child);
+    assert(() {
+      _debugDanglingKeepAlives.remove(child);
+      return true;
+    }());
     _keepAliveBucket.remove(childParentData.index);
     dropChild(child);
   }

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

@@ -149,6 +149,13 @@ abstract class GlobalKey<T extends State<StatefulWidget>> extends Key {
     assert(() {
       assert(parent != null);
       if (_debugReservations.containsKey(this) && _debugReservations[this] != parent) {
+        // Reserving a new parent while the old parent is not attached is ok.
+        // This can happen when a renderObject detaches and re-attaches to rendering
+        // tree multiple times.
+        if (_debugReservations[this].renderObject?.attached == false) {
+          _debugReservations[this] = parent;
+          return true;
+        }
         // It's possible for an element to get built multiple times in one
         // frame, in which case it'll reserve the same child's key multiple
         // times. We catch multiple children of one widget having the same key

packages/flutter/test/material/tabs_test.dart

@@ -49,6 +49,7 @@ class StateMarkerState extends State<StateMarker> {
 }
 
 class AlwaysKeepAliveWidget extends StatefulWidget {
+  const AlwaysKeepAliveWidget({ Key key}) : super(key: key);
   static String text = 'AlwaysKeepAlive';
   @override
   AlwaysKeepAliveState createState() => AlwaysKeepAliveState();
@@ -1987,6 +1988,56 @@ void main() {
 
   });
 
+  testWidgets('Skipping tabs with global key does not crash', (WidgetTester tester) async {
+    // Regression test for https://github.com/flutter/flutter/issues/24660
+    final List<String> tabs = <String>[
+      'Tab1',
+      'Tab2',
+      'Tab3',
+      'Tab4',
+    ];
+    final TabController controller = TabController(
+      vsync: const TestVSync(),
+      length: tabs.length,
+    );
+    await tester.pumpWidget(
+      MaterialApp(
+        home: Align(
+          alignment: Alignment.topLeft,
+          child: SizedBox(
+            width: 300.0,
+            height: 200.0,
+            child: Scaffold(
+              appBar: AppBar(
+                title: const Text('tabs'),
+                bottom: TabBar(
+                  controller: controller,
+                  tabs: tabs.map<Widget>((String tab) => Tab(text: tab)).toList(),
+                ),
+              ),
+              body: TabBarView(
+                controller: controller,
+                children: <Widget>[
+                  Text('1', key: GlobalKey()),
+                  Text('2', key: GlobalKey()),
+                  Text('3', key: GlobalKey()),
+                  Text('4', key: GlobalKey()),
+                ],
+              ),
+            ),
+          ),
+        ),
+      ),
+    );
+    expect(find.text('1'), findsOneWidget);
+    expect(find.text('4'), findsNothing);
+    await tester.tap(find.text('Tab4'));
+    await tester.pumpAndSettle();
+    expect(controller.index, 3);
+    expect(find.text('4'), findsOneWidget);
+    expect(find.text('1'), findsNothing);
+  });
+
   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>[
@@ -2018,7 +2069,7 @@ void main() {
               body: TabBarView(
                 controller: controller,
                 children: <Widget>[
-                  AlwaysKeepAliveWidget(),
+                  AlwaysKeepAliveWidget(key: UniqueKey()),
                   const Text('2'),
                   const Text('3'),
                   const Text('4'),

packages/flutter/test/widgets/box_sliver_mismatch_test.dart

@@ -9,10 +9,10 @@ import 'package:flutter/widgets.dart';
 void main() {
   testWidgets('Sliver in a box', (WidgetTester tester) async {
     await tester.pumpWidget(
-      const DecoratedBox(
-        decoration: BoxDecoration(),
+      DecoratedBox(
+        decoration: const BoxDecoration(),
         child: SliverList(
-          delegate: SliverChildListDelegate(<Widget>[]),
+          delegate: SliverChildListDelegate(const <Widget>[]),
         ),
       ),
     );
@@ -21,9 +21,9 @@ void main() {
 
     await tester.pumpWidget(
       Row(
-        children: const <Widget>[
+        children: <Widget>[
           SliverList(
-            delegate: SliverChildListDelegate(<Widget>[]),
+            delegate: SliverChildListDelegate(const <Widget>[]),
           ),
         ],
       ),

packages/flutter/test/widgets/framework_test.dart

@@ -451,6 +451,41 @@ void main() {
     expect(count, 2);
   });
 
+  testWidgets('GlobalKey - dettach and re-attach child to different parents', (WidgetTester tester) async {
+    await tester.pumpWidget(Directionality(
+      textDirection: TextDirection.ltr,
+      child: Center(
+        child: Container(
+          height: 100,
+          child: CustomScrollView(
+            controller: ScrollController(),
+            slivers: <Widget>[
+              SliverList(
+                delegate: SliverChildListDelegate(<Widget>[
+                  Text('child', key: GlobalKey()),
+                ]),
+              )
+            ],
+          ),
+        ),
+      ),
+    ));
+    final SliverMultiBoxAdaptorElement element = tester.element(find.byType(SliverList));
+    Element childElement;
+    // Removing and recreating child with same Global Key should not trigger
+    // duplicate key error.
+    element.visitChildren((Element e) {
+      childElement = e;
+    });
+    element.removeChild(childElement.renderObject);
+    element.createChild(0, after: null);
+    element.visitChildren((Element e) {
+      childElement = e;
+    });
+    element.removeChild(childElement.renderObject);
+    element.createChild(0, after: null);
+  });
+
   testWidgets('Defunct setState throws exception', (WidgetTester tester) async {
     StateSetter setState;
 

packages/flutter/test/widgets/slivers_appbar_floating_pinned_test.dart

@@ -22,12 +22,12 @@ void main() {
             data: const MediaQueryData(),
             child: CustomScrollView(
               controller: controller,
-              slivers: const <Widget>[
-                SliverAppBar(floating: true, pinned: true, expandedHeight: 200.0, title: Text('A')),
-                SliverAppBar(primary: false, pinned: true, title: Text('B')),
+              slivers: <Widget>[
+                const SliverAppBar(floating: true, pinned: true, expandedHeight: 200.0, title: Text('A')),
+                const SliverAppBar(primary: false, pinned: true, title: Text('B')),
                 SliverList(
                   delegate: SliverChildListDelegate(
-                    <Widget>[
+                    const <Widget>[
                       Text('C'),
                       Text('D'),
                       SizedBox(height: 500.0),

packages/flutter/test/widgets/slivers_appbar_floating_test.dart

@@ -203,9 +203,9 @@ void main() {
           physics: const BouncingScrollPhysics(),
           slivers: <Widget>[
             SliverPersistentHeader(delegate: TestDelegate(), floating: true),
-            const SliverList(
+            SliverList(
               delegate: SliverChildListDelegate(<Widget>[
-                SizedBox(
+                const SizedBox(
                   height: 300.0,
                   child: Text('X'),
                 ),

packages/flutter/test/widgets/slivers_appbar_pinned_test.dart

@@ -258,9 +258,9 @@ void main() {
           physics: const BouncingScrollPhysics(),
           slivers: <Widget>[
             SliverPersistentHeader(delegate: TestDelegate(), pinned: true),
-            const SliverList(
+            SliverList(
               delegate: SliverChildListDelegate(<Widget>[
-                SizedBox(
+                const SizedBox(
                   height: 300.0,
                   child: Text('X'),
                 ),

packages/flutter/test/widgets/slivers_appbar_scrolling_test.dart

@@ -82,8 +82,40 @@ void main() {
           physics: const BouncingScrollPhysics(),
           slivers: <Widget>[
             SliverPersistentHeader(delegate: TestDelegate()),
-            const SliverList(
+            SliverList(
               delegate: SliverChildListDelegate(<Widget>[
+                const SizedBox(
+                  height: 300.0,
+                  child: Text('X'),
+                ),
+              ]),
+            ),
+          ],
+        ),
+      ),
+    );
+
+    expect(tester.getTopLeft(find.byType(Container)), Offset.zero);
+    expect(tester.getTopLeft(find.text('X')), const Offset(0.0, 200.0));
+
+    final ScrollPosition position = tester.state<ScrollableState>(find.byType(Scrollable)).position;
+    position.jumpTo(-50.0);
+    await tester.pump();
+
+    expect(tester.getTopLeft(find.byType(Container)), Offset.zero);
+    expect(tester.getTopLeft(find.text('X')), const Offset(0.0, 250.0));
+  });
+
+  testWidgets('Sliver appbars const child delegate - scrolling - overscroll gap is below header', (WidgetTester tester) async {
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: CustomScrollView(
+          physics: const BouncingScrollPhysics(),
+          slivers: <Widget>[
+            SliverPersistentHeader(delegate: TestDelegate()),
+            const SliverList(
+              delegate: SliverChildListDelegate.fixed(<Widget>[
                 SizedBox(
                   height: 300.0,
                   child: Text('X'),

packages/flutter/test/widgets/slivers_block_test.dart

@@ -9,6 +9,28 @@ import 'package:flutter/widgets.dart';
 import '../rendering/mock_canvas.dart';
 
 Future<void> test(WidgetTester tester, double offset) {
+  return tester.pumpWidget(
+    Directionality(
+      textDirection: TextDirection.ltr,
+      child: Viewport(
+        offset: ViewportOffset.fixed(offset),
+        slivers: <Widget>[
+          SliverList(
+            delegate: SliverChildListDelegate(const <Widget>[
+              SizedBox(height: 400.0, child: Text('a')),
+              SizedBox(height: 400.0, child: Text('b')),
+              SizedBox(height: 400.0, child: Text('c')),
+              SizedBox(height: 400.0, child: Text('d')),
+              SizedBox(height: 400.0, child: Text('e')),
+            ]),
+          ),
+        ],
+      ),
+    ),
+  );
+}
+
+Future<void> testWithConstChildDelegate(WidgetTester tester, double offset) {
   return tester.pumpWidget(
     Directionality(
       textDirection: TextDirection.ltr,
@@ -16,7 +38,7 @@ Future<void> test(WidgetTester tester, double offset) {
         offset: ViewportOffset.fixed(offset),
         slivers: const <Widget>[
           SliverList(
-            delegate: SliverChildListDelegate(<Widget>[
+            delegate: SliverChildListDelegate.fixed(<Widget>[
               SizedBox(height: 400.0, child: Text('a')),
               SizedBox(height: 400.0, child: Text('b')),
               SizedBox(height: 400.0, child: Text('c')),
@@ -76,6 +98,39 @@ void main() {
     ], 'ab');
   });
 
+  testWidgets('Viewport+SliverBlock basic test with constant SliverChildListDelegate', (WidgetTester tester) async {
+    await testWithConstChildDelegate(tester, 0.0);
+    expect(tester.renderObject<RenderBox>(find.byType(Viewport)).size, equals(const Size(800.0, 600.0)));
+    verify(tester, <Offset>[
+      const Offset(0.0, 0.0),
+      const Offset(0.0, 400.0),
+    ], 'ab');
+
+    await testWithConstChildDelegate(tester, 200.0);
+    verify(tester, <Offset>[
+      const Offset(0.0, -200.0),
+      const Offset(0.0, 200.0),
+    ], 'ab');
+
+    await testWithConstChildDelegate(tester, 600.0);
+    verify(tester, <Offset>[
+      const Offset(0.0, -200.0),
+      const Offset(0.0, 200.0),
+    ], 'bc');
+
+    await testWithConstChildDelegate(tester, 900.0);
+    verify(tester, <Offset>[
+      const Offset(0.0, -100.0),
+      const Offset(0.0, 300.0),
+    ], 'cd');
+
+    await testWithConstChildDelegate(tester, 200.0);
+    verify(tester, <Offset>[
+      const Offset(0.0, -200.0),
+      const Offset(0.0, 200.0),
+    ], 'ab');
+  });
+
   testWidgets('Viewport with GlobalKey reparenting', (WidgetTester tester) async {
     final Key key1 = GlobalKey();
     final ViewportOffset offset = ViewportOffset.zero();
@@ -150,9 +205,9 @@ void main() {
         textDirection: TextDirection.ltr,
         child: Viewport(
           offset: offset,
-          slivers: const <Widget>[
+          slivers: <Widget>[
             SliverList(
-              delegate: SliverChildListDelegate(<Widget>[
+              delegate: SliverChildListDelegate(const <Widget>[
                 SizedBox(height: 251.0, child: Text('a')),
                 SizedBox(height: 252.0, child: Text('b')),
               ]),
@@ -261,9 +316,9 @@ void main() {
         textDirection: TextDirection.ltr,
         child: Viewport(
           offset: ViewportOffset.zero(),
-          slivers: const <Widget>[
+          slivers: <Widget>[
             SliverList(
-              delegate: SliverChildListDelegate(<Widget>[
+              delegate: SliverChildListDelegate(const <Widget>[
                 SizedBox(height: 400.0, child: Text('a')),
               ]),
             ),
@@ -279,9 +334,9 @@ void main() {
         textDirection: TextDirection.ltr,
         child: Viewport(
           offset: ViewportOffset.fixed(100.0),
-          slivers: const <Widget>[
+          slivers: <Widget>[
             SliverList(
-              delegate: SliverChildListDelegate(<Widget>[
+              delegate: SliverChildListDelegate(const <Widget>[
                 SizedBox(height: 400.0, child: Text('a')),
               ]),
             ),
@@ -297,9 +352,9 @@ void main() {
         textDirection: TextDirection.ltr,
         child: Viewport(
           offset: ViewportOffset.fixed(100.0),
-          slivers: const <Widget>[
+          slivers: <Widget>[
             SliverList(
-              delegate: SliverChildListDelegate(<Widget>[
+              delegate: SliverChildListDelegate(const <Widget>[
                 SizedBox(height: 4000.0, child: Text('a')),
               ]),
             ),
@@ -315,9 +370,9 @@ void main() {
         textDirection: TextDirection.ltr,
         child: Viewport(
           offset: ViewportOffset.zero(),
-          slivers: const <Widget>[
+          slivers: <Widget>[
             SliverList(
-              delegate: SliverChildListDelegate(<Widget>[
+              delegate: SliverChildListDelegate(const <Widget>[
                 SizedBox(height: 4000.0, child: Text('a')),
               ]),
             ),

packages/flutter/test/widgets/slivers_test.dart

@@ -206,7 +206,8 @@ void main() {
       addRepaintBoundaries: false,
       addSemanticIndexes: false,
     );
-    expect(builderThrowsDelegate.build(null, 0), errorText);
+    final KeyedSubtree wrapped = builderThrowsDelegate.build(null, 0);
+    expect(wrapped.child, errorText);
     expect(tester.takeException(), 'builder');
     ErrorWidget.builder = oldBuilder;
   });