diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 51d8876d05..e3ce722f74 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -5495,12 +5495,15 @@ abstract class RenderObjectElement extends Element { /// 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 an - /// [IndexedSlot] 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). + /// each individual child. If `slots` is non-null, the value for the `newSlot` + /// argument of [updateChild] is retrieved from that list using the index that + /// the currently processed `child` corresponds to in the `newWidgets` list + /// (`newWidgets` and `slots` must have the same length). If `slots` is null, + /// an [IndexedSlot] is used as the value for the `newSlot` argument. + /// In that case, [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 @@ -5525,14 +5528,21 @@ abstract class RenderObjectElement extends Element { /// knows where a child needs to move to in a linked list by providing its new /// previous sibling. @protected - List updateChildren(List oldChildren, List newWidgets, { Set? forgottenChildren }) { + List updateChildren(List oldChildren, List newWidgets, { Set? forgottenChildren, List? slots }) { assert(oldChildren != null); assert(newWidgets != null); + assert(slots == null || newWidgets.length == slots.length); Element? replaceWithNullIfForgotten(Element child) { return forgottenChildren != null && forgottenChildren.contains(child) ? null : child; } + Object? slotFor(int newChildIndex, Element? previousChild) { + return slots != null + ? slots[newChildIndex] + : IndexedSlot(newChildIndex, previousChild); + } + // This attempts to diff the new child list (newWidgets) with // the old child list (oldChildren), and produce a new list of elements to // be the new list of child elements of this element. The called of this @@ -5581,7 +5591,7 @@ abstract class RenderObjectElement extends Element { assert(oldChild == null || oldChild._lifecycleState == _ElementLifecycle.active); if (oldChild == null || !Widget.canUpdate(oldChild.widget, newWidget)) break; - final Element newChild = updateChild(oldChild, newWidget, IndexedSlot(newChildrenTop, previousChild))!; + final Element newChild = updateChild(oldChild, newWidget, slotFor(newChildrenTop, previousChild))!; assert(newChild._lifecycleState == _ElementLifecycle.active); newChildren[newChildrenTop] = newChild; previousChild = newChild; @@ -5639,7 +5649,7 @@ abstract class RenderObjectElement extends Element { } } assert(oldChild == null || Widget.canUpdate(oldChild.widget, newWidget)); - final Element newChild = updateChild(oldChild, newWidget, IndexedSlot(newChildrenTop, previousChild))!; + final Element newChild = updateChild(oldChild, newWidget, slotFor(newChildrenTop, previousChild))!; assert(newChild._lifecycleState == _ElementLifecycle.active); assert(oldChild == newChild || oldChild == null || oldChild._lifecycleState != _ElementLifecycle.active); newChildren[newChildrenTop] = newChild; @@ -5661,7 +5671,7 @@ abstract class RenderObjectElement extends Element { assert(oldChild._lifecycleState == _ElementLifecycle.active); final Widget newWidget = newWidgets[newChildrenTop]; assert(Widget.canUpdate(oldChild.widget, newWidget)); - final Element newChild = updateChild(oldChild, newWidget, IndexedSlot(newChildrenTop, previousChild))!; + final Element newChild = updateChild(oldChild, newWidget, slotFor(newChildrenTop, previousChild))!; assert(newChild._lifecycleState == _ElementLifecycle.active); assert(oldChild == newChild || oldChild == null || oldChild._lifecycleState != _ElementLifecycle.active); newChildren[newChildrenTop] = newChild; diff --git a/packages/flutter/lib/src/widgets/table.dart b/packages/flutter/lib/src/widgets/table.dart index 110430841b..29d8fa47c6 100644 --- a/packages/flutter/lib/src/widgets/table.dart +++ b/packages/flutter/lib/src/widgets/table.dart @@ -348,37 +348,50 @@ class _TableElement extends RenderObjectElement { @override RenderTable get renderObject => super.renderObject as RenderTable; - // This class ignores the child's slot entirely. - // Instead of doing incremental updates to the child list, it replaces the entire list each frame. - List<_TableElementRow> _children = const<_TableElementRow>[]; + bool _doingMountOrUpdate = false; + @override - void mount(Element? parent, dynamic newSlot) { + void mount(Element? parent, Object? newSlot) { + assert(!_doingMountOrUpdate); + _doingMountOrUpdate = true; super.mount(parent, newSlot); + int rowIndex = -1; _children = widget.children.map<_TableElementRow>((TableRow row) { + int columnIndex = 0; + rowIndex += 1; return _TableElementRow( key: row.key, children: row.children!.map((Widget child) { assert(child != null); - return inflateWidget(child, null); + return inflateWidget(child, _TableSlot(columnIndex++, rowIndex)); }).toList(growable: false), ); }).toList(growable: false); _updateRenderObjectChildren(); + assert(_doingMountOrUpdate); + _doingMountOrUpdate = false; } @override - void insertRenderObjectChild(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderBox child, _TableSlot slot) { renderObject.setupParentData(child); + // Once [mount]/[update] are done, the children are getting set all at once + // in [_updateRenderObjectChildren]. + if (!_doingMountOrUpdate) { + renderObject.setChild(slot.column, slot.row, child); + } } @override - void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { + void moveRenderObjectChild(RenderBox child, _TableSlot oldSlot, _TableSlot newSlot) { + assert(_doingMountOrUpdate); + // Child gets moved at the end of [update] in [_updateRenderObjectChildren]. } @override - void removeRenderObjectChild(RenderObject child, dynamic slot) { + void removeRenderObjectChild(RenderBox child, _TableSlot slot) { final TableCellParentData childParentData = child.parentData! as TableCellParentData; renderObject.setChild(childParentData.x!, childParentData.y!, null); } @@ -387,6 +400,8 @@ class _TableElement extends RenderObjectElement { @override void update(Table newWidget) { + assert(!_doingMountOrUpdate); + _doingMountOrUpdate = true; final Map> oldKeyedRows = >{}; for (final _TableElementRow row in _children) { if (row.key != null) { @@ -396,7 +411,8 @@ class _TableElement extends RenderObjectElement { final Iterator<_TableElementRow> oldUnkeyedRows = _children.where((_TableElementRow row) => row.key == null).iterator; final List<_TableElementRow> newChildren = <_TableElementRow>[]; final Set> taken = >{}; - for (final TableRow row in newWidget.children) { + for (int rowIndex = 0; rowIndex < newWidget.children.length; rowIndex++) { + final TableRow row = newWidget.children[rowIndex]; List oldChildren; if (row.key != null && oldKeyedRows.containsKey(row.key)) { oldChildren = oldKeyedRows[row.key]!; @@ -406,9 +422,13 @@ class _TableElement extends RenderObjectElement { } else { oldChildren = const []; } + final List<_TableSlot> slots = List<_TableSlot>.generate( + row.children!.length, + (int columnIndex) => _TableSlot(columnIndex, rowIndex), + ); newChildren.add(_TableElementRow( key: row.key, - children: updateChildren(oldChildren, row.children!, forgottenChildren: _forgottenChildren), + children: updateChildren(oldChildren, row.children!, forgottenChildren: _forgottenChildren, slots: slots), )); } while (oldUnkeyedRows.moveNext()) @@ -421,6 +441,8 @@ class _TableElement extends RenderObjectElement { _forgottenChildren.clear(); super.update(newWidget); assert(widget == newWidget); + assert(_doingMountOrUpdate); + _doingMountOrUpdate = false; } void _updateRenderObjectChildren() { @@ -489,3 +511,30 @@ class TableCell extends ParentDataWidget { properties.add(EnumProperty('verticalAlignment', verticalAlignment)); } } + +@immutable +class _TableSlot with Diagnosticable { + const _TableSlot(this.column, this.row); + + final int column; + final int row; + + @override + bool operator ==(Object other) { + if (other.runtimeType != runtimeType) + return false; + return other is _TableSlot + && column == other.column + && row == other.row; + } + + @override + int get hashCode => hashValues(column, row); + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(IntProperty('x', column)); + properties.add(IntProperty('y', row)); + } +} diff --git a/packages/flutter/test/widgets/table_test.dart b/packages/flutter/test/widgets/table_test.dart index 413b509622..8fc1a24326 100644 --- a/packages/flutter/test/widgets/table_test.dart +++ b/packages/flutter/test/widgets/table_test.dart @@ -964,5 +964,41 @@ void main() { } }); + testWidgets('Can replace child with a different RenderObject type', (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/69395. + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Table(children: const [ + TableRow(children: [ + TestChildWidget(), + TestChildWidget(), + TestChildWidget(), + ]), + TableRow(children: [ + TestChildWidget(), + TestChildWidget(), + TestChildWidget(), + ]), + ]), + ), + ); + final RenderTable table = tester.renderObject(find.byType(Table)); + + expect(find.text('CRASHHH'), findsNothing); + expect(find.byType(SizedBox), findsNWidgets(3 * 2)); + final Type toBeReplaced = table.column(2).last.runtimeType; + + final TestChildState state = tester.state(find.byType(TestChildWidget).last); + state.toggleMe(); + await tester.pump(); + + expect(find.byType(SizedBox), findsNWidgets(5)); + expect(find.text('CRASHHH'), findsOneWidget); + + // The RenderObject got replaced by a different type. + expect(table.column(2).last.runtimeType, isNot(toBeReplaced)); + }); + // TODO(ianh): Test handling of TableCell object }