Keep render tree and element tree in sync when re-used elements move in a MultiChildRenderObjectElement's child list (#51674)
diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart
index 5c458aa..7867c11 100644
--- a/packages/flutter/lib/src/widgets/framework.dart
+++ b/packages/flutter/lib/src/widgets/framework.dart
@@ -5433,8 +5433,35 @@
/// acts as if the child was not in `oldChildren`.
///
/// This function is a convenience wrapper around [updateChild], which updates
- /// each individual child. When calling [updateChild], this function uses the
- /// previous element as the `newSlot` argument.
+ /// each individual child. When calling [updateChild], this function uses an
+ /// [IndexedSlot<Element>] as the value for the `newSlot` argument.
+ /// [IndexedSlot.index] is set to the index that the currently processed
+ /// `child` corresponds to in the `newWidgets` list and [IndexedSlot.value] is
+ /// set to the [Element] of the previous widget in that list (or null if it is
+ /// the first child).
+ ///
+ /// When the [slot] value of an [Element] changes, its
+ /// associated [renderObject] needs to move to a new position in the child
+ /// list of its parents. If that [RenderObject] organizes its children in a
+ /// linked list (as is done by the [ContainerRenderObjectMixin]) this can
+ /// be implemented by re-inserting the child [RenderObject] into the
+ /// list after the [RenderObject] associated with the [Element] provided as
+ /// [IndexedSlot.value] in the [slot] object.
+ ///
+ /// Simply using the previous sibling as a [slot] is not enough, though, because
+ /// child [RenderObject]s are only moved around when the [slot] of their
+ /// associated [RenderObjectElement]s is updated. When the order of child
+ /// [Element]s is changed, some elements in the list may move to a new index
+ /// but still have the same previous sibling. For example, when
+ /// `[e1, e2, e3, e4]` is changed to `[e1, e3, e4, e2]` the element e4
+ /// continues to have e3 as a previous sibling even though its index in the list
+ /// has changed and its [RenderObject] needs to move to come before e2's
+ /// [RenderObject]. In order to trigger this move, a new [slot] value needs to
+ /// be assigned to its [Element] whenever its index in its
+ /// parent's child list changes. Using an [IndexedSlot<Element>] achieves
+ /// exactly that and also ensures that the underlying parent [RenderObject]
+ /// knows where a child needs to move to in a linked list by providing its new
+ /// previous sibling.
@protected
List<Element> updateChildren(List<Element> oldChildren, List<Widget> newWidgets, { Set<Element> forgottenChildren }) {
assert(oldChildren != null);
@@ -5492,7 +5519,7 @@
assert(oldChild == null || oldChild._debugLifecycleState == _ElementLifecycle.active);
if (oldChild == null || !Widget.canUpdate(oldChild.widget, newWidget))
break;
- final Element newChild = updateChild(oldChild, newWidget, previousChild);
+ final Element newChild = updateChild(oldChild, newWidget, IndexedSlot<Element>(newChildrenTop, previousChild));
assert(newChild._debugLifecycleState == _ElementLifecycle.active);
newChildren[newChildrenTop] = newChild;
previousChild = newChild;
@@ -5550,7 +5577,7 @@
}
}
assert(oldChild == null || Widget.canUpdate(oldChild.widget, newWidget));
- final Element newChild = updateChild(oldChild, newWidget, previousChild);
+ final Element newChild = updateChild(oldChild, newWidget, IndexedSlot<Element>(newChildrenTop, previousChild));
assert(newChild._debugLifecycleState == _ElementLifecycle.active);
assert(oldChild == newChild || oldChild == null || oldChild._debugLifecycleState != _ElementLifecycle.active);
newChildren[newChildrenTop] = newChild;
@@ -5572,7 +5599,7 @@
assert(oldChild._debugLifecycleState == _ElementLifecycle.active);
final Widget newWidget = newWidgets[newChildrenTop];
assert(Widget.canUpdate(oldChild.widget, newWidget));
- final Element newChild = updateChild(oldChild, newWidget, previousChild);
+ final Element newChild = updateChild(oldChild, newWidget, IndexedSlot<Element>(newChildrenTop, previousChild));
assert(newChild._debugLifecycleState == _ElementLifecycle.active);
assert(oldChild == newChild || oldChild == null || oldChild._debugLifecycleState != _ElementLifecycle.active);
newChildren[newChildrenTop] = newChild;
@@ -5666,10 +5693,12 @@
/// Insert the given child into [renderObject] at the given slot.
///
+ /// {@template flutter.widgets.slots}
/// The semantics of `slot` are determined by this element. For example, if
/// this element has a single child, the slot should always be null. If this
- /// element has a list of children, the previous sibling is a convenient value
- /// for the slot.
+ /// element has a list of children, the previous sibling element wrapped in an
+ /// [IndexedSlot] is a convenient value for the slot.
+ /// {@endtemplate}
@protected
void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot);
@@ -5677,10 +5706,7 @@
///
/// The given child is guaranteed to have [renderObject] as its parent.
///
- /// The semantics of `slot` are determined by this element. For example, if
- /// this element has a single child, the slot should always be null. If this
- /// element has a list of children, the previous sibling is a convenient value
- /// for the slot.
+ /// {@macro flutter.widgets.slots}
///
/// This method is only ever called if [updateChild] can end up being called
/// with an existing [Element] child and a `slot` that differs from the slot
@@ -5840,6 +5866,13 @@
/// RenderObjects use the [ContainerRenderObjectMixin] mixin with a parent data
/// type that implements [ContainerParentDataMixin<RenderObject>]. Such widgets
/// are expected to inherit from [MultiChildRenderObjectWidget].
+///
+/// See also:
+///
+/// * [IndexedSlot], which is used as [Element.slot]s for the children of a
+/// [MultiChildRenderObjectElement].
+/// * [RenderObjectElement.updateChildren], which discusses why [IndexedSlot]
+/// is used for the slots of the children.
class MultiChildRenderObjectElement extends RenderObjectElement {
/// Creates an element that uses the given widget as its configuration.
MultiChildRenderObjectElement(MultiChildRenderObjectWidget widget)
@@ -5863,20 +5896,20 @@
final Set<Element> _forgottenChildren = HashSet<Element>();
@override
- void insertChildRenderObject(RenderObject child, Element slot) {
+ void insertChildRenderObject(RenderObject child, IndexedSlot<Element> slot) {
final ContainerRenderObjectMixin<RenderObject, ContainerParentDataMixin<RenderObject>> renderObject =
this.renderObject as ContainerRenderObjectMixin<RenderObject, ContainerParentDataMixin<RenderObject>>;
assert(renderObject.debugValidateChild(child));
- renderObject.insert(child, after: slot?.renderObject);
+ renderObject.insert(child, after: slot?.value?.renderObject);
assert(renderObject == this.renderObject);
}
@override
- void moveChildRenderObject(RenderObject child, Element slot) {
+ void moveChildRenderObject(RenderObject child, IndexedSlot<Element> slot) {
final ContainerRenderObjectMixin<RenderObject, ContainerParentDataMixin<RenderObject>> renderObject =
this.renderObject as ContainerRenderObjectMixin<RenderObject, ContainerParentDataMixin<RenderObject>>;
assert(child.parent == renderObject);
- renderObject.move(child, after: slot?.renderObject);
+ renderObject.move(child, after: slot?.value?.renderObject);
assert(renderObject == this.renderObject);
}
@@ -5911,7 +5944,7 @@
_children = List<Element>(widget.children.length);
Element previousChild;
for (int i = 0; i < _children.length; i += 1) {
- final Element newChild = inflateWidget(widget.children[i], previousChild);
+ final Element newChild = inflateWidget(widget.children[i], IndexedSlot<Element>(i, previousChild));
_children[i] = newChild;
previousChild = newChild;
}
@@ -5957,3 +5990,41 @@
FlutterError.reportError(details);
return details;
}
+
+/// A value for [Element.slot] used for children of
+/// [MultiChildRenderObjectElement]s.
+///
+/// A slot for a [MultiChildRenderObjectElement] consists of an [index]
+/// identifying where the child occupying this slot is located in the
+/// [MultiChildRenderObjectElement]'s child list and an arbitrary [value] that
+/// can further define where the child occupying this slot fits in its
+/// parent's child list.
+///
+/// See also:
+///
+/// * [RenderObjectElement.updateChildren], which discusses why this class is
+/// used as slot values for the children of a [MultiChildRenderObjectElement].
+@immutable
+class IndexedSlot<T> {
+ /// Creates an [IndexedSlot] with the provided [index] and slot [value].
+ const IndexedSlot(this.index, this.value);
+
+ /// Information to define where the child occupying this slot fits in its
+ /// parent's child list.
+ final T value;
+
+ /// The index of this slot in the parent's child list.
+ final int index;
+
+ @override
+ bool operator ==(Object other) {
+ if (other.runtimeType != runtimeType)
+ return false;
+ return other is IndexedSlot
+ && index == other.index
+ && value == other.value;
+ }
+
+ @override
+ int get hashCode => hashValues(index, value);
+}
diff --git a/packages/flutter/lib/src/widgets/table.dart b/packages/flutter/lib/src/widgets/table.dart
index df42cc8..0d1ea1f 100644
--- a/packages/flutter/lib/src/widgets/table.dart
+++ b/packages/flutter/lib/src/widgets/table.dart
@@ -270,7 +270,7 @@
}
@override
- void insertChildRenderObject(RenderObject child, Element slot) {
+ void insertChildRenderObject(RenderObject child, IndexedSlot<Element> slot) {
renderObject.setupParentData(child);
}
diff --git a/packages/flutter/test/widgets/multichildobject_with_keys_test.dart b/packages/flutter/test/widgets/multichildobject_with_keys_test.dart
new file mode 100644
index 0000000..186025b
--- /dev/null
+++ b/packages/flutter/test/widgets/multichildobject_with_keys_test.dart
@@ -0,0 +1,72 @@
+// Copyright 2014 The Flutter 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/rendering.dart';
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+ testWidgets('Render and element tree stay in sync when keyed children move around', (WidgetTester tester) async {
+ // Regression test for https://github.com/flutter/flutter/issues/48855.
+
+ await tester.pumpWidget(
+ Directionality(
+ textDirection: TextDirection.ltr,
+ child: Column(
+ children: const <Widget>[
+ Text('0', key: ValueKey<int>(0)),
+ Text('1', key: ValueKey<int>(1)),
+ Text('2', key: ValueKey<int>(2)),
+ Text('3', key: ValueKey<int>(3)),
+ Text('4', key: ValueKey<int>(4)),
+ Text('5', key: ValueKey<int>(5)),
+ Text('6', key: ValueKey<int>(6)),
+ Text('7', key: ValueKey<int>(7)),
+ Text('8', key: ValueKey<int>(8)),
+ ],
+ ),
+ ),
+ );
+
+ expect(
+ _getChildOrder(tester.renderObject<RenderFlex>(find.byType(Column))),
+ <String>['0', '1', '2', '3', '4', '5', '6', '7', '8'],
+ );
+
+ await tester.pumpWidget(
+ Directionality(
+ textDirection: TextDirection.ltr,
+ child: Column(
+ children: const <Widget>[
+ Text('0', key: ValueKey<int>(0)),
+ Text('6', key: ValueKey<int>(6)),
+ Text('7', key: ValueKey<int>(7)),
+ Text('8', key: ValueKey<int>(8)),
+ Text('1', key: ValueKey<int>(1)),
+ Text('2', key: ValueKey<int>(2)),
+ Text('3', key: ValueKey<int>(3)),
+ Text('4', key: ValueKey<int>(4)),
+ Text('5', key: ValueKey<int>(5)),
+ ],
+ ),
+ ),
+ );
+
+ expect(
+ _getChildOrder(tester.renderObject<RenderFlex>(find.byType(Column))),
+ <String>['0', '6', '7', '8', '1', '2', '3', '4', '5'],
+ );
+ });
+}
+
+// Do not use tester.renderObjectList(find.byType(RenderParagraph). That returns
+// the RenderObjects in the order of their associated RenderObjectWidgets. The
+// point of this test is to assert the children order in the render tree, though.
+List<String> _getChildOrder(RenderFlex flex) {
+ final List<String> childOrder = <String>[];
+ flex.visitChildren((RenderObject child) {
+ childOrder.add(((child as RenderParagraph).text as TextSpan).text);
+ });
+ return childOrder;
+}
