From 4d8934af485128fda1dad7cd2db5d10eefb2f3a8 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Thu, 20 Aug 2020 08:19:34 -0700 Subject: [PATCH] Re-apply "(insert|move|remove)ChildRenderObject Deprecation: Step 1 (#64189)" (#64254) This reverts commit ce40de69b7b4f89c66d19c8dbd3bd86ae30f1bc6. (this re-applies #64189 by reverting #64249) --- .../lib/src/cupertino/action_sheet.dart | 8 +- .../flutter/lib/src/cupertino/dialog.dart | 6 +- .../lib/src/cupertino/text_selection.dart | 15 +- packages/flutter/lib/src/material/chip.dart | 13 +- .../lib/src/material/input_decorator.dart | 13 +- .../flutter/lib/src/material/list_tile.dart | 13 +- packages/flutter/lib/src/widgets/binding.dart | 6 +- .../flutter/lib/src/widgets/framework.dart | 209 ++++++++-- .../lib/src/widgets/layout_builder.dart | 6 +- .../src/widgets/list_wheel_scroll_view.dart | 6 +- packages/flutter/lib/src/widgets/sliver.dart | 10 +- .../src/widgets/sliver_persistent_header.dart | 6 +- .../widgets/sliver_prototype_extent_list.dart | 14 +- packages/flutter/lib/src/widgets/table.dart | 6 +- .../widgets/render_object_element_test.dart | 383 ++++++++++++++++++ 15 files changed, 629 insertions(+), 85 deletions(-) create mode 100644 packages/flutter/test/widgets/render_object_element_test.dart diff --git a/packages/flutter/lib/src/cupertino/action_sheet.dart b/packages/flutter/lib/src/cupertino/action_sheet.dart index 8bc6e6436c..364d1489b7 100644 --- a/packages/flutter/lib/src/cupertino/action_sheet.dart +++ b/packages/flutter/lib/src/cupertino/action_sheet.dart @@ -448,13 +448,13 @@ class _CupertinoAlertRenderElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, _AlertSections slot) { + void insertRenderObjectChild(RenderObject child, _AlertSections slot) { _placeChildInSlot(child, slot); } @override - void moveChildRenderObject(RenderObject child, _AlertSections slot) { - _placeChildInSlot(child, slot); + void moveRenderObjectChild(RenderObject child, _AlertSections oldSlot, _AlertSections newSlot) { + _placeChildInSlot(child, newSlot); } @override @@ -478,7 +478,7 @@ class _CupertinoAlertRenderElement extends RenderObjectElement { } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, _AlertSections slot) { assert(child == renderObject.contentSection || child == renderObject.actionsSection); if (renderObject.contentSection == child) { renderObject.contentSection = null; diff --git a/packages/flutter/lib/src/cupertino/dialog.dart b/packages/flutter/lib/src/cupertino/dialog.dart index f8c536172c..b310a7169d 100644 --- a/packages/flutter/lib/src/cupertino/dialog.dart +++ b/packages/flutter/lib/src/cupertino/dialog.dart @@ -433,7 +433,7 @@ class _CupertinoDialogRenderElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, _AlertDialogSections slot) { + void insertRenderObjectChild(RenderObject child, _AlertDialogSections slot) { assert(slot != null); switch (slot) { case _AlertDialogSections.contentSection: @@ -446,7 +446,7 @@ class _CupertinoDialogRenderElement extends RenderObjectElement { } @override - void moveChildRenderObject(RenderObject child, _AlertDialogSections slot) { + void moveRenderObjectChild(RenderObject child, _AlertDialogSections oldSlot, _AlertDialogSections newSlot) { assert(false); } @@ -470,7 +470,7 @@ class _CupertinoDialogRenderElement extends RenderObjectElement { } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, _AlertDialogSections slot) { assert(child == renderObject.contentSection || child == renderObject.actionsSection); if (renderObject.contentSection == child) { renderObject.contentSection = null; diff --git a/packages/flutter/lib/src/cupertino/text_selection.dart b/packages/flutter/lib/src/cupertino/text_selection.dart index d3bd9e083e..fad1d674fa 100644 --- a/packages/flutter/lib/src/cupertino/text_selection.dart +++ b/packages/flutter/lib/src/cupertino/text_selection.dart @@ -775,10 +775,9 @@ class _CupertinoTextSelectionToolbarItemsElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderObject child, dynamic slot) { if (slot is _CupertinoTextSelectionToolbarItemsSlot) { assert(child is RenderBox); - assert(slot is _CupertinoTextSelectionToolbarItemsSlot); _updateRenderObject(child as RenderBox, slot); assert(renderObject.childToSlot.containsKey(child)); assert(renderObject.slotToChild.containsKey(slot)); @@ -794,9 +793,9 @@ class _CupertinoTextSelectionToolbarItemsElement extends RenderObjectElement { // This is not reachable for children that don't have an IndexedSlot. @override - void moveChildRenderObject(RenderObject child, IndexedSlot slot) { + void moveRenderObjectChild(RenderObject child, IndexedSlot oldSlot, IndexedSlot newSlot) { assert(child.parent == renderObject); - renderObject.move(child as RenderBox, after: slot?.value?.renderObject as RenderBox); + renderObject.move(child as RenderBox, after: newSlot?.value?.renderObject as RenderBox); } static bool _shouldPaint(Element child) { @@ -804,18 +803,20 @@ class _CupertinoTextSelectionToolbarItemsElement extends RenderObjectElement { } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { // Check if the child is in a slot. - if (renderObject.childToSlot.containsKey(child)) { + if (slot is _CupertinoTextSelectionToolbarItemsSlot) { assert(child is RenderBox); + assert(renderObject.slotToChild.containsKey(slot)); assert(renderObject.childToSlot.containsKey(child)); - _updateRenderObject(null, renderObject.childToSlot[child]); + _updateRenderObject(null, slot); assert(!renderObject.childToSlot.containsKey(child)); assert(!renderObject.slotToChild.containsKey(slot)); return; } // Otherwise look for it in the list of children. + assert(slot is IndexedSlot); assert(child.parent == renderObject); renderObject.remove(child as RenderBox); } diff --git a/packages/flutter/lib/src/material/chip.dart b/packages/flutter/lib/src/material/chip.dart index 6a8900d08b..f0f6c90b0f 100644 --- a/packages/flutter/lib/src/material/chip.dart +++ b/packages/flutter/lib/src/material/chip.dart @@ -2148,26 +2148,25 @@ class _RenderChipElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slotValue) { + void insertRenderObjectChild(RenderObject child, _ChipSlot slot) { assert(child is RenderBox); - assert(slotValue is _ChipSlot); - final _ChipSlot slot = slotValue as _ChipSlot; _updateRenderObject(child, slot); assert(renderObject.childToSlot.keys.contains(child)); assert(renderObject.slotToChild.keys.contains(slot)); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, _ChipSlot slot) { assert(child is RenderBox); - assert(renderObject.childToSlot.keys.contains(child)); - _updateRenderObject(null, renderObject.childToSlot[child]); + assert(renderObject.childToSlot[child] == slot); + assert(renderObject.slotToChild[slot] == child); + _updateRenderObject(null, slot); assert(!renderObject.childToSlot.keys.contains(child)); assert(!renderObject.slotToChild.keys.contains(slot)); } @override - void moveChildRenderObject(RenderObject child, dynamic slotValue) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false, 'not reachable'); } } diff --git a/packages/flutter/lib/src/material/input_decorator.dart b/packages/flutter/lib/src/material/input_decorator.dart index 76d3962621..520a32e86f 100644 --- a/packages/flutter/lib/src/material/input_decorator.dart +++ b/packages/flutter/lib/src/material/input_decorator.dart @@ -1654,26 +1654,25 @@ class _RenderDecorationElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slotValue) { + void insertRenderObjectChild(RenderObject child, _DecorationSlot slot) { assert(child is RenderBox); - assert(slotValue is _DecorationSlot); - final _DecorationSlot slot = slotValue as _DecorationSlot; _updateRenderObject(child as RenderBox, slot); assert(renderObject.childToSlot.keys.contains(child)); assert(renderObject.slotToChild.keys.contains(slot)); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, _DecorationSlot slot) { assert(child is RenderBox); - assert(renderObject.childToSlot.keys.contains(child)); - _updateRenderObject(null, renderObject.childToSlot[child]); + assert(renderObject.childToSlot[child] == slot); + assert(renderObject.slotToChild[slot] == child); + _updateRenderObject(null, slot); assert(!renderObject.childToSlot.keys.contains(child)); assert(!renderObject.slotToChild.keys.contains(slot)); } @override - void moveChildRenderObject(RenderObject child, dynamic slotValue) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false, 'not reachable'); } } diff --git a/packages/flutter/lib/src/material/list_tile.dart b/packages/flutter/lib/src/material/list_tile.dart index 97929f9f09..d1ecc8a0bd 100644 --- a/packages/flutter/lib/src/material/list_tile.dart +++ b/packages/flutter/lib/src/material/list_tile.dart @@ -1250,26 +1250,25 @@ class _ListTileElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slotValue) { + void insertRenderObjectChild(RenderObject child, _ListTileSlot slot) { assert(child is RenderBox); - assert(slotValue is _ListTileSlot); - final _ListTileSlot slot = slotValue as _ListTileSlot; _updateRenderObject(child as RenderBox, slot); assert(renderObject.childToSlot.keys.contains(child)); assert(renderObject.slotToChild.keys.contains(slot)); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, _ListTileSlot slot) { assert(child is RenderBox); - assert(renderObject.childToSlot.keys.contains(child)); - _updateRenderObject(null, renderObject.childToSlot[child]); + assert(renderObject.childToSlot[child] == slot); + assert(renderObject.slotToChild[slot] == child); + _updateRenderObject(null, slot); assert(!renderObject.childToSlot.keys.contains(child)); assert(!renderObject.slotToChild.keys.contains(slot)); } @override - void moveChildRenderObject(RenderObject child, dynamic slotValue) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false, 'not reachable'); } } diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index 71296d8b7a..ea6d60df52 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -1268,19 +1268,19 @@ class RenderObjectToWidgetElement extends RootRenderObje RenderObjectWithChildMixin get renderObject => super.renderObject as RenderObjectWithChildMixin; @override - void insertChildRenderObject(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderObject child, dynamic slot) { assert(slot == _rootChildSlot); assert(renderObject.debugValidateChild(child)); renderObject.child = child as T; } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { assert(renderObject.child == child); renderObject.child = null; } diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 1de80e4771..81991079c9 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -3290,9 +3290,9 @@ abstract class Element extends DiagnosticableTree implements BuildContext { /// the "active" lifecycle state. /// /// Subclasses that override this method are likely to want to also override - /// [update], [visitChildren], [RenderObjectElement.insertChildRenderObject], - /// [RenderObjectElement.moveChildRenderObject], and - /// [RenderObjectElement.removeChildRenderObject]. + /// [update], [visitChildren], [RenderObjectElement.insertRenderObjectChild], + /// [RenderObjectElement.moveRenderObjectChild], and + /// [RenderObjectElement.removeRenderObjectChild]. @mustCallSuper void mount(Element parent, dynamic newSlot) { assert(_debugLifecycleState == _ElementLifecycle.initial); @@ -5335,9 +5335,9 @@ class InheritedElement extends ProxyElement { /// ### Maintaining the render object tree /// /// Once a descendant produces a render object, it will call -/// [insertChildRenderObject]. If the descendant's slot changes identity, it -/// will call [moveChildRenderObject]. If a descendant goes away, it will call -/// [removeChildRenderObject]. +/// [insertRenderObjectChild]. If the descendant's slot changes identity, it +/// will call [moveRenderObjectChild]. If a descendant goes away, it will call +/// [removeRenderObjectChild]. /// /// These three methods should update the render tree accordingly, attaching, /// moving, and detaching the given child render object from this element's own @@ -5727,10 +5727,11 @@ abstract class RenderObjectElement extends Element { @override void _updateSlot(dynamic newSlot) { - assert(slot != newSlot); + final dynamic oldSlot = slot; + assert(oldSlot != newSlot); super._updateSlot(newSlot); assert(slot == newSlot); - _ancestorRenderObjectElement.moveChildRenderObject(renderObject, slot); + _ancestorRenderObjectElement.moveRenderObjectChild(renderObject, oldSlot, slot); } @override @@ -5738,7 +5739,7 @@ abstract class RenderObjectElement extends Element { assert(_ancestorRenderObjectElement == null); _slot = newSlot; _ancestorRenderObjectElement = _findAncestorRenderObjectElement(); - _ancestorRenderObjectElement?.insertChildRenderObject(renderObject, newSlot); + _ancestorRenderObjectElement?.insertRenderObjectChild(renderObject, newSlot); final ParentDataElement parentDataElement = _findAncestorParentDataElement(); if (parentDataElement != null) _updateParentData(parentDataElement.widget); @@ -5747,12 +5748,59 @@ abstract class RenderObjectElement extends Element { @override void detachRenderObject() { if (_ancestorRenderObjectElement != null) { - _ancestorRenderObjectElement.removeChildRenderObject(renderObject); + _ancestorRenderObjectElement.removeRenderObjectChild(renderObject, slot); _ancestorRenderObjectElement = null; } _slot = null; } + /// Insert the given child into [renderObject] at the given slot. + /// + /// {@macro flutter.widgets.slots} + /// + /// ## Deprecation + /// + /// This method has been deprecated in favor of [insertRenderObjectChild]. + /// + /// The reason for the deprecation is to provide the `oldSlot` argument to + /// the [moveRenderObjectChild] method (such an argument was missing from + /// the now-deprecated [moveChildRenderObject] method) and the `slot` + /// argument to the [removeRenderObjectChild] method (such an argument was + /// missing from the now-deprecated [removeChildRenderObject] method). While + /// no argument was added to [insertRenderObjectChild], the name change (and + /// corresponding deprecation) was made to maintain naming parity with the + /// other two methods. + /// + /// To migrate, simply override [insertRenderObjectChild] instead of + /// [insertChildRenderObject]. The arguments stay the same. Subclasses should + /// _not_ call `super.insertRenderObjectChild(...)`. + @protected + @mustCallSuper + @Deprecated( + 'Override insertRenderObjectChild instead. ' + 'This feature was deprecated after v1.21.0-9.0.pre.' + ) + void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot) { + assert(() { + throw FlutterError.fromParts([ + ErrorSummary('RenderObjectElement.insertChildRenderObject() is deprecated.'), + toDiagnosticsNode( + name: 'insertChildRenderObject() was called on this Element', + style: DiagnosticsTreeStyle.shallow, + ), + ErrorDescription('insertChildRenderObject() has been deprecated in favor of ' + 'insertRenderObjectChild(). See https://github.com/flutter/flutter/issues/63269 ' + 'for details.'), + ErrorHint('Rather than overriding insertChildRenderObject() in your ' + 'RenderObjectElement subclass, override insertRenderObjectChild() instead, ' + "and DON'T call super.insertRenderObjectChild(). If you're implementing a " + 'new RenderObjectElement, you should override/implement ' + 'insertRenderObjectChild(), moveRenderObjectChild(), and ' + 'removeRenderObjectChild().'), + ]); + }()); + } + /// Insert the given child into [renderObject] at the given slot. /// /// {@template flutter.widgets.slots} @@ -5762,7 +5810,9 @@ abstract class RenderObjectElement extends Element { /// [IndexedSlot] is a convenient value for the slot. /// {@endtemplate} @protected - void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot); + void insertRenderObjectChild(covariant RenderObject child, covariant dynamic slot) { + insertChildRenderObject(child, slot); + } /// Move the given child to the given slot. /// @@ -5778,14 +5828,127 @@ abstract class RenderObjectElement extends Element { /// always having the same slot (and where children in different slots are never /// compared against each other for the purposes of updating one slot with the /// element from another slot) would never call this. + /// + /// ## Deprecation + /// + /// This method has been deprecated in favor of [moveRenderObjectChild]. + /// + /// The reason for the deprecation is to provide the `oldSlot` argument to + /// the [moveRenderObjectChild] method (such an argument was missing from + /// the now-deprecated [moveChildRenderObject] method) and the `slot` + /// argument to the [removeRenderObjectChild] method (such an argument was + /// missing from the now-deprecated [removeChildRenderObject] method). While + /// no argument was added to [insertRenderObjectChild], the name change (and + /// corresponding deprecation) was made to maintain naming parity with the + /// other two methods. + /// + /// To migrate, simply override [moveRenderObjectChild] instead of + /// [moveChildRenderObject]. The `slot` argument becomes the `newSlot` + /// argument, and the method will now take a new `oldSlot` argument that + /// subclasses may find useful. Subclasses should _not_ call + /// `super.moveRenderObjectChild(...)`. @protected - void moveChildRenderObject(covariant RenderObject child, covariant dynamic slot); + @mustCallSuper + @Deprecated( + 'Override moveRenderObjectChild instead. ' + 'This feature was deprecated after v1.21.0-9.0.pre.' + ) + void moveChildRenderObject(covariant RenderObject child, covariant dynamic slot) { + assert(() { + throw FlutterError.fromParts([ + ErrorSummary('RenderObjectElement.moveChildRenderObject() is deprecated.'), + toDiagnosticsNode( + name: 'super.moveChildRenderObject() was called on this Element', + style: DiagnosticsTreeStyle.shallow, + ), + ErrorDescription('moveChildRenderObject() has been deprecated in favor of ' + 'moveRenderObjectChild(). See https://github.com/flutter/flutter/issues/63269 ' + 'for details.'), + ErrorHint('Rather than overriding moveChildRenderObject() in your ' + 'RenderObjectElement subclass, override moveRenderObjectChild() instead, ' + "and DON'T call super.moveRenderObjectChild(). If you're implementing a " + 'new RenderObjectElement, you should override/implement ' + 'insertRenderObjectChild(), moveRenderObjectChild(), and ' + 'removeRenderObjectChild().'), + ]); + }()); + } + + /// Move the given child from the given old slot to the given new slot. + /// + /// The given child is guaranteed to have [renderObject] as its parent. + /// + /// {@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 + /// that element was previously given. [MultiChildRenderObjectElement] does this, + /// for example. [SingleChildRenderObjectElement] does not (since the `slot` is + /// always null). An [Element] that has a specific set of slots with each child + /// always having the same slot (and where children in different slots are never + /// compared against each other for the purposes of updating one slot with the + /// element from another slot) would never call this. + @protected + void moveRenderObjectChild(covariant RenderObject child, covariant dynamic oldSlot, covariant dynamic newSlot) { + moveChildRenderObject(child, newSlot); + } /// Remove the given child from [renderObject]. /// /// The given child is guaranteed to have [renderObject] as its parent. + /// + /// ## Deprecation + /// + /// This method has been deprecated in favor of [removeRenderObjectChild]. + /// + /// The reason for the deprecation is to provide the `oldSlot` argument to + /// the [moveRenderObjectChild] method (such an argument was missing from + /// the now-deprecated [moveChildRenderObject] method) and the `slot` + /// argument to the [removeRenderObjectChild] method (such an argument was + /// missing from the now-deprecated [removeChildRenderObject] method). While + /// no argument was added to [insertRenderObjectChild], the name change (and + /// corresponding deprecation) was made to maintain naming parity with the + /// other two methods. + /// + /// To migrate, simply override [removeRenderObjectChild] instead of + /// [removeChildRenderObject]. The method will now take a new `slot` argument + /// that subclasses may find useful. Subclasses should _not_ call + /// `super.removeRenderObjectChild(...)`. @protected - void removeChildRenderObject(covariant RenderObject child); + @mustCallSuper + @Deprecated( + 'Override removeRenderObjectChild instead. ' + 'This feature was deprecated after v1.21.0-9.0.pre.' + ) + void removeChildRenderObject(covariant RenderObject child) { + assert(() { + throw FlutterError.fromParts([ + ErrorSummary('RenderObjectElement.removeChildRenderObject() is deprecated.'), + toDiagnosticsNode( + name: 'super.removeChildRenderObject() was called on this Element', + style: DiagnosticsTreeStyle.shallow, + ), + ErrorDescription('removeChildRenderObject() has been deprecated in favor of ' + 'removeRenderObjectChild(). See https://github.com/flutter/flutter/issues/63269 ' + 'for details.'), + ErrorHint('Rather than overriding removeChildRenderObject() in your ' + 'RenderObjectElement subclass, override removeRenderObjectChild() instead, ' + "and DON'T call super.removeRenderObjectChild(). If you're implementing a " + 'new RenderObjectElement, you should override/implement ' + 'insertRenderObjectChild(), moveRenderObjectChild(), and ' + 'removeRenderObjectChild().'), + ]); + }()); + } + + /// Remove the given child from [renderObject]. + /// + /// The given child is guaranteed to have been inserted at the given `slot` + /// and have [renderObject] as its parent. + @protected + void removeRenderObjectChild(covariant RenderObject child, covariant dynamic slot) { + removeChildRenderObject(child); + } @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { @@ -5837,17 +6000,17 @@ class LeafRenderObjectElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderObject child, dynamic slot) { assert(false); } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { assert(false); } @@ -5900,7 +6063,7 @@ class SingleChildRenderObjectElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderObject child, dynamic slot) { final RenderObjectWithChildMixin renderObject = this.renderObject as RenderObjectWithChildMixin; assert(slot == null); assert(renderObject.debugValidateChild(child)); @@ -5909,12 +6072,12 @@ class SingleChildRenderObjectElement extends RenderObjectElement { } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { final RenderObjectWithChildMixin renderObject = this.renderObject as RenderObjectWithChildMixin; assert(renderObject.child == child); renderObject.child = null; @@ -5958,7 +6121,7 @@ class MultiChildRenderObjectElement extends RenderObjectElement { final Set _forgottenChildren = HashSet(); @override - void insertChildRenderObject(RenderObject child, IndexedSlot slot) { + void insertRenderObjectChild(RenderObject child, IndexedSlot slot) { final ContainerRenderObjectMixin> renderObject = this.renderObject as ContainerRenderObjectMixin>; assert(renderObject.debugValidateChild(child)); @@ -5967,16 +6130,16 @@ class MultiChildRenderObjectElement extends RenderObjectElement { } @override - void moveChildRenderObject(RenderObject child, IndexedSlot slot) { + void moveRenderObjectChild(RenderObject child, IndexedSlot oldSlot, IndexedSlot newSlot) { final ContainerRenderObjectMixin> renderObject = this.renderObject as ContainerRenderObjectMixin>; assert(child.parent == renderObject); - renderObject.move(child, after: slot?.value?.renderObject); + renderObject.move(child, after: newSlot?.value?.renderObject); assert(renderObject == this.renderObject); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { final ContainerRenderObjectMixin> renderObject = this.renderObject as ContainerRenderObjectMixin>; assert(child.parent == renderObject); diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index f5751e62be..558a9a3918 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -156,7 +156,7 @@ class _LayoutBuilderElement extends RenderOb } @override - void insertChildRenderObject(RenderObject child, dynamic slot) { + void insertRenderObjectChild(RenderObject child, dynamic slot) { final RenderObjectWithChildMixin renderObject = this.renderObject; assert(slot == null); assert(renderObject.debugValidateChild(child)); @@ -165,12 +165,12 @@ class _LayoutBuilderElement extends RenderOb } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false); } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, dynamic slot) { final RenderConstrainedLayoutBuilder renderObject = this.renderObject; assert(renderObject.child == child); renderObject.child = null; diff --git a/packages/flutter/lib/src/widgets/list_wheel_scroll_view.dart b/packages/flutter/lib/src/widgets/list_wheel_scroll_view.dart index 26e32c3413..1b6fc21deb 100644 --- a/packages/flutter/lib/src/widgets/list_wheel_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/list_wheel_scroll_view.dart @@ -907,7 +907,7 @@ class ListWheelElement extends RenderObjectElement implements ListWheelChildMana } @override - void insertChildRenderObject(RenderObject child, int slot) { + void insertRenderObjectChild(RenderObject child, int slot) { final RenderListWheelViewport renderObject = this.renderObject; assert(renderObject.debugValidateChild(child)); renderObject.insert(child as RenderBox, after: _childElements[slot - 1]?.renderObject as RenderBox); @@ -915,7 +915,7 @@ class ListWheelElement extends RenderObjectElement implements ListWheelChildMana } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, int oldSlot, int newSlot) { const String moveChildRenderObjectErrorMessage = 'Currently we maintain the list in contiguous increasing order, so ' 'moving children around is not allowed.'; @@ -923,7 +923,7 @@ class ListWheelElement extends RenderObjectElement implements ListWheelChildMana } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, int slot) { assert(child.parent == renderObject); renderObject.remove(child as RenderBox); } diff --git a/packages/flutter/lib/src/widgets/sliver.dart b/packages/flutter/lib/src/widgets/sliver.dart index 1fde2ef30e..c040a056e0 100644 --- a/packages/flutter/lib/src/widgets/sliver.dart +++ b/packages/flutter/lib/src/widgets/sliver.dart @@ -1273,7 +1273,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render } @override - void insertChildRenderObject(covariant RenderObject child, int slot) { + void insertRenderObjectChild(covariant RenderObject child, int slot) { assert(slot != null); assert(_currentlyUpdatingChildIndex == slot); assert(renderObject.debugValidateChild(child)); @@ -1286,14 +1286,14 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render } @override - void moveChildRenderObject(covariant RenderObject child, int slot) { - assert(slot != null); - assert(_currentlyUpdatingChildIndex == slot); + void moveRenderObjectChild(covariant RenderObject child, int oldSlot, int newSlot) { + assert(newSlot != null); + assert(_currentlyUpdatingChildIndex == newSlot); renderObject.move(child as RenderBox, after: _currentBeforeChild); } @override - void removeChildRenderObject(covariant RenderObject child) { + void removeRenderObjectChild(covariant RenderObject child, int slot) { assert(_currentlyUpdatingChildIndex != null); renderObject.remove(child as RenderBox); } diff --git a/packages/flutter/lib/src/widgets/sliver_persistent_header.dart b/packages/flutter/lib/src/widgets/sliver_persistent_header.dart index b26cc4bd1b..673e1cb7bd 100644 --- a/packages/flutter/lib/src/widgets/sliver_persistent_header.dart +++ b/packages/flutter/lib/src/widgets/sliver_persistent_header.dart @@ -236,18 +236,18 @@ class _SliverPersistentHeaderElement extends RenderObjectElement { } @override - void insertChildRenderObject(covariant RenderBox child, dynamic slot) { + void insertRenderObjectChild(covariant RenderBox child, dynamic slot) { assert(renderObject.debugValidateChild(child)); renderObject.child = child; } @override - void moveChildRenderObject(covariant RenderObject child, dynamic slot) { + void moveRenderObjectChild(covariant RenderObject child, dynamic oldSlot, dynamic newSlot) { assert(false); } @override - void removeChildRenderObject(covariant RenderObject child) { + void removeRenderObjectChild(covariant RenderObject child, dynamic slot) { renderObject.child = null; } diff --git a/packages/flutter/lib/src/widgets/sliver_prototype_extent_list.dart b/packages/flutter/lib/src/widgets/sliver_prototype_extent_list.dart index b6e6c2aa8d..fab01fafc4 100644 --- a/packages/flutter/lib/src/widgets/sliver_prototype_extent_list.dart +++ b/packages/flutter/lib/src/widgets/sliver_prototype_extent_list.dart @@ -75,12 +75,12 @@ class _SliverPrototypeExtentListElement extends SliverMultiBoxAdaptorElement { static final Object _prototypeSlot = Object(); @override - void insertChildRenderObject(covariant RenderObject child, covariant dynamic slot) { + void insertRenderObjectChild(covariant RenderObject child, covariant dynamic slot) { if (slot == _prototypeSlot) { assert(child is RenderBox); renderObject.child = child as RenderBox; } else { - super.insertChildRenderObject(child, slot as int); + super.insertRenderObjectChild(child, slot as int); } } @@ -91,19 +91,19 @@ class _SliverPrototypeExtentListElement extends SliverMultiBoxAdaptorElement { } @override - void moveChildRenderObject(RenderBox child, dynamic slot) { - if (slot == _prototypeSlot) + void moveRenderObjectChild(RenderBox child, dynamic oldSlot, dynamic newSlot) { + if (newSlot == _prototypeSlot) assert(false); // There's only one prototype child so it cannot be moved. else - super.moveChildRenderObject(child, slot as int); + super.moveRenderObjectChild(child, oldSlot as int, newSlot as int); } @override - void removeChildRenderObject(RenderBox child) { + void removeRenderObjectChild(RenderBox child, dynamic slot) { if (renderObject.child == child) renderObject.child = null; else - super.removeChildRenderObject(child); + super.removeRenderObjectChild(child, slot as int); } @override diff --git a/packages/flutter/lib/src/widgets/table.dart b/packages/flutter/lib/src/widgets/table.dart index 70c4a99e9a..afbfb40d53 100644 --- a/packages/flutter/lib/src/widgets/table.dart +++ b/packages/flutter/lib/src/widgets/table.dart @@ -303,16 +303,16 @@ class _TableElement extends RenderObjectElement { } @override - void insertChildRenderObject(RenderObject child, IndexedSlot slot) { + void insertRenderObjectChild(RenderObject child, IndexedSlot slot) { renderObject.setupParentData(child); } @override - void moveChildRenderObject(RenderObject child, dynamic slot) { + void moveRenderObjectChild(RenderObject child, IndexedSlot oldSlot, IndexedSlot newSlot) { } @override - void removeChildRenderObject(RenderObject child) { + void removeRenderObjectChild(RenderObject child, IndexedSlot slot) { final TableCellParentData childParentData = child.parentData as TableCellParentData; renderObject.setChild(childParentData.x, childParentData.y, null); } diff --git a/packages/flutter/test/widgets/render_object_element_test.dart b/packages/flutter/test/widgets/render_object_element_test.dart new file mode 100644 index 0000000000..fbbbe0f897 --- /dev/null +++ b/packages/flutter/test/widgets/render_object_element_test.dart @@ -0,0 +1,383 @@ +// 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. + +// @dart = 2.8 + +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/widgets.dart'; + +@immutable +class Pair { + const Pair(this.first, this.second); + final T first; + final T second; + + @override + bool operator ==(Object other) { + return other is Pair && other.first == first && other.second == second; + } + + @override + int get hashCode => hashValues(first, second); + + @override + String toString() => '($first,$second)'; +} + +/// Widget that will layout one child in the top half of this widget's size +/// and the other child in the bottom half. It will swap which child is on top +/// and which is on bottom every time the widget is rendered. +abstract class Swapper extends RenderObjectWidget { + const Swapper({this.stable, this.swapper}); + + final Widget stable; + final Widget swapper; + + @override + SwapperElement createElement(); + + @override + RenderObject createRenderObject(BuildContext context) => RenderSwapper(); +} + +class SwapperWithProperOverrides extends Swapper { + const SwapperWithProperOverrides({ + Widget stable, + Widget swapper, + }) : super(stable: stable, swapper: swapper); + + @override + SwapperElement createElement() => SwapperElementWithProperOverrides(this); +} + +class SwapperWithNoOverrides extends Swapper { + const SwapperWithNoOverrides({ + Widget stable, + Widget swapper, + }) : super(stable: stable, swapper: swapper); + + @override + SwapperElement createElement() => SwapperElementWithNoOverrides(this); +} + +class SwapperWithDeprecatedOverrides extends Swapper { + const SwapperWithDeprecatedOverrides({ + Widget stable, + Widget swapper, + }) : super(stable: stable, swapper: swapper); + + @override + SwapperElement createElement() => SwapperElementWithDeprecatedOverrides(this); +} + +abstract class SwapperElement extends RenderObjectElement { + SwapperElement(Swapper widget) : super(widget); + + Element stable; + Element swapper; + bool swapperIsOnTop = true; + List insertSlots = []; + List> moveSlots = >[]; + List removeSlots = []; + + @override + Swapper get widget => super.widget as Swapper; + + @override + RenderSwapper get renderObject => super.renderObject as RenderSwapper; + + @override + void visitChildren(ElementVisitor visitor) { + if (stable != null) + visitor(stable); + if (swapper != null) + visitor(swapper); + } + + @override + void update(Swapper newWidget) { + super.update(newWidget); + _updateChildren(newWidget); + } + + @override + void mount(Element parent, dynamic newSlot) { + super.mount(parent, newSlot); + _updateChildren(widget); + } + + void _updateChildren(Swapper widget) { + stable = updateChild(stable, widget.stable, 'stable'); + swapper = updateChild(swapper, widget.swapper, swapperIsOnTop); + swapperIsOnTop = !swapperIsOnTop; + } +} + +class SwapperElementWithProperOverrides extends SwapperElement { + SwapperElementWithProperOverrides(Swapper widget) : super(widget); + + @override + void insertRenderObjectChild(RenderBox child, dynamic slot) { + insertSlots.add(slot); + assert(child != null); + if (slot == 'stable') + renderObject.stable = child; + else + renderObject.setSwapper(child, slot as bool); + } + + @override + void moveRenderObjectChild(RenderBox child, bool oldIsOnTop, bool newIsOnTop) { + moveSlots.add(Pair(oldIsOnTop, newIsOnTop)); + assert(oldIsOnTop == !newIsOnTop); + renderObject.setSwapper(child, newIsOnTop); + } + + @override + void removeRenderObjectChild(RenderBox child, dynamic slot) { + removeSlots.add(slot); + if (slot == 'stable') + renderObject.stable = null; + else + renderObject.setSwapper(null, slot as bool); + } +} + +class SwapperElementWithNoOverrides extends SwapperElement { + SwapperElementWithNoOverrides(Swapper widget) : super(widget); +} + +class SwapperElementWithDeprecatedOverrides extends SwapperElement { + SwapperElementWithDeprecatedOverrides(Swapper widget) : super(widget); + + @override + // ignore: must_call_super + void insertChildRenderObject(RenderBox child, dynamic slot) { + insertSlots.add(slot); + assert(child != null); + if (slot == 'stable') + renderObject.stable = child; + else + renderObject.setSwapper(child, slot as bool); + } + + @override + // ignore: must_call_super + void moveChildRenderObject(RenderBox child, bool isOnTop) { + moveSlots.add(Pair(null, isOnTop)); + renderObject.setSwapper(child, isOnTop); + } + + @override + // ignore: must_call_super + void removeChildRenderObject(RenderBox child) { + removeSlots.add(null); + if (child == renderObject._stable) + renderObject.stable = null; + else + renderObject.setSwapper(null, swapperIsOnTop); + } +} + +class RenderSwapper extends RenderBox { + RenderBox _stable; + RenderBox get stable => _stable; + set stable(RenderBox child) { + if (child == _stable) + return; + if (_stable != null) + dropChild(_stable); + _stable = child; + if (child != null) + adoptChild(child); + } + + bool _swapperIsOnTop; + RenderBox _swapper; + RenderBox get swapper => _swapper; + void setSwapper(RenderBox child, bool isOnTop) { + if (isOnTop != _swapperIsOnTop) { + _swapperIsOnTop = isOnTop; + markNeedsLayout(); + } + if (child == _swapper) + return; + if (_swapper != null) + dropChild(_swapper); + _swapper = child; + if (child != null) + adoptChild(child); + } + + @override + void visitChildren(RenderObjectVisitor visitor) { + if (_stable != null) + visitor(_stable); + if (_swapper != null) + visitor(_swapper); + } + + @override + void attach(PipelineOwner owner) { + super.attach(owner); + visitChildren((RenderObject child) => child.attach(owner)); + } + + @override + void detach() { + super.detach(); + visitChildren((RenderObject child) => child.detach()); + } + + @override + void performLayout() { + assert(constraints.hasBoundedWidth); + assert(constraints.hasTightHeight); + size = constraints.biggest; + const Offset topOffset = Offset.zero; + final Offset bottomOffset = Offset(0, size.height / 2); + final BoxConstraints childConstraints = constraints.copyWith( + minHeight: constraints.minHeight / 2, + maxHeight: constraints.maxHeight / 2, + ); + if (_stable != null) { + final BoxParentData stableParentData = _stable.parentData as BoxParentData; + _stable.layout(childConstraints); + stableParentData.offset = _swapperIsOnTop ? bottomOffset : topOffset; + } + if (_swapper != null) { + final BoxParentData swapperParentData = _swapper.parentData as BoxParentData; + _swapper.layout(childConstraints); + swapperParentData.offset = _swapperIsOnTop ? topOffset : bottomOffset; + } + } + + @override + void paint(PaintingContext context, Offset offset) { + visitChildren((RenderObject child) { + final BoxParentData childParentData = child.parentData as BoxParentData; + context.paintChild(child, offset + childParentData.offset); + }); + } + + @override + void redepthChildren() { + visitChildren((RenderObject child) => redepthChild(child)); + } +} + +BoxParentData parentDataFor(RenderObject renderObject) => renderObject.parentData as BoxParentData; + +void main() { + testWidgets('RenderObjectElement *RenderObjectChild methods get called with correct arguments', (WidgetTester tester) async { + const Key redKey = ValueKey('red'); + const Key blueKey = ValueKey('blue'); + Widget widget() { + return SwapperWithProperOverrides( + stable: ColoredBox( + key: redKey, + color: Color(nonconst(0xffff0000)), + ), + swapper: ColoredBox( + key: blueKey, + color: Color(nonconst(0xff0000ff)), + ), + ); + } + + await tester.pumpWidget(widget()); + final SwapperElement swapper = tester.element(find.byType(SwapperWithProperOverrides)); + final RenderBox redBox = tester.renderObject(find.byKey(redKey)); + final RenderBox blueBox = tester.renderObject(find.byKey(blueKey)); + expect(swapper.insertSlots.length, 2); + expect(swapper.insertSlots, contains('stable')); + expect(swapper.insertSlots, contains(true)); + expect(swapper.moveSlots, isEmpty); + expect(swapper.removeSlots, isEmpty); + expect(parentDataFor(redBox).offset, const Offset(0, 300)); + expect(parentDataFor(blueBox).offset, Offset.zero); + await tester.pumpWidget(widget()); + expect(swapper.insertSlots.length, 2); + expect(swapper.moveSlots.length, 1); + expect(swapper.moveSlots, contains(const Pair(true, false))); + expect(swapper.removeSlots, isEmpty); + expect(parentDataFor(redBox).offset, Offset.zero); + expect(parentDataFor(blueBox).offset, const Offset(0, 300)); + await tester.pumpWidget(const SwapperWithProperOverrides()); + expect(redBox.attached, false); + expect(blueBox.attached, false); + expect(swapper.insertSlots.length, 2); + expect(swapper.moveSlots.length, 1); + expect(swapper.removeSlots.length, 2); + expect(swapper.removeSlots, contains('stable')); + expect(swapper.removeSlots, contains(false)); + }); + + testWidgets('RenderObjectElement *RenderObjectChild methods delegate to deprecated methods', (WidgetTester tester) async { + const Key redKey = ValueKey('red'); + const Key blueKey = ValueKey('blue'); + Widget widget() { + return SwapperWithDeprecatedOverrides( + stable: ColoredBox( + key: redKey, + color: Color(nonconst(0xffff0000)), + ), + swapper: ColoredBox( + key: blueKey, + color: Color(nonconst(0xff0000ff)), + ), + ); + } + + await tester.pumpWidget(widget()); + final SwapperElement swapper = tester.element(find.byType(SwapperWithDeprecatedOverrides)); + final RenderBox redBox = tester.renderObject(find.byKey(redKey)); + final RenderBox blueBox = tester.renderObject(find.byKey(blueKey)); + expect(swapper.insertSlots.length, 2); + expect(swapper.insertSlots, contains('stable')); + expect(swapper.insertSlots, contains(true)); + expect(swapper.moveSlots, isEmpty); + expect(swapper.removeSlots, isEmpty); + expect(parentDataFor(redBox).offset, const Offset(0, 300)); + expect(parentDataFor(blueBox).offset, Offset.zero); + await tester.pumpWidget(widget()); + expect(swapper.insertSlots.length, 2); + expect(swapper.moveSlots.length, 1); + expect(swapper.moveSlots, contains(const Pair(null, false))); + expect(swapper.removeSlots, isEmpty); + expect(parentDataFor(redBox).offset, Offset.zero); + expect(parentDataFor(blueBox).offset, const Offset(0, 300)); + await tester.pumpWidget(const SwapperWithDeprecatedOverrides()); + expect(redBox.attached, false); + expect(blueBox.attached, false); + expect(swapper.insertSlots.length, 2); + expect(swapper.moveSlots.length, 1); + expect(swapper.removeSlots.length, 2); + expect(swapper.removeSlots, [null,null]); + }); + + testWidgets('RenderObjectElement *ChildRenderObject methods fail with deprecation message', (WidgetTester tester) async { + const Key redKey = ValueKey('red'); + const Key blueKey = ValueKey('blue'); + Widget widget() { + return SwapperWithNoOverrides( + stable: ColoredBox( + key: redKey, + color: Color(nonconst(0xffff0000)), + ), + swapper: ColoredBox( + key: blueKey, + color: Color(nonconst(0xff0000ff)), + ), + ); + } + + await tester.pumpWidget(widget()); + final FlutterError error = tester.takeException() as FlutterError; + final ErrorSummary summary = error.diagnostics.first as ErrorSummary; + expect(summary.toString(), contains('deprecated')); + }); +}