diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index faea8b4fe7..8bf1794cb3 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart @@ -1536,10 +1536,14 @@ abstract class RenderObjectElement extends Buildab /// Attempts to update the given old children list using the given new /// widgets, removing obsolete elements and introducing new ones as necessary, /// and then returns the new child list. - List updateChildren(List oldChildren, List newWidgets) { + List updateChildren(List oldChildren, List newWidgets, { Set detachedChildren }) { assert(oldChildren != null); assert(newWidgets != null); + Element replaceWithNullIfDetached(Element child) { + return detachedChildren != null && detachedChildren.contains(child) ? null : child; + } + // This attempts to diff the new child list (this.children) with // the old child list (old.children), and update our renderObject // accordingly. @@ -1582,7 +1586,7 @@ abstract class RenderObjectElement extends Buildab // Update the top of the list. while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) { - Element oldChild = oldChildren[oldChildrenTop]; + Element oldChild = replaceWithNullIfDetached(oldChildren[oldChildrenTop]); Widget newWidget = newWidgets[newChildrenTop]; assert(oldChild == null || oldChild._debugLifecycleState == _ElementLifecycle.active); if (oldChild == null || !Widget.canUpdate(oldChild.widget, newWidget)) @@ -1597,7 +1601,7 @@ abstract class RenderObjectElement extends Buildab // Scan the bottom of the list. while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) { - Element oldChild = oldChildren[oldChildrenBottom]; + Element oldChild = replaceWithNullIfDetached(oldChildren[oldChildrenBottom]); Widget newWidget = newWidgets[newChildrenBottom]; assert(oldChild == null || oldChild._debugLifecycleState == _ElementLifecycle.active); if (oldChild == null || !Widget.canUpdate(oldChild.widget, newWidget)) @@ -1612,7 +1616,7 @@ abstract class RenderObjectElement extends Buildab if (haveOldChildren) { oldKeyedChildren = new Map(); while (oldChildrenTop <= oldChildrenBottom) { - Element oldChild = oldChildren[oldChildrenTop]; + Element oldChild = replaceWithNullIfDetached(oldChildren[oldChildrenTop]); assert(oldChild == null || oldChild._debugLifecycleState == _ElementLifecycle.active); if (oldChild != null) { if (oldChild.widget.key != null) @@ -1663,7 +1667,7 @@ abstract class RenderObjectElement extends Buildab // Update the bottom of the list. while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) { Element oldChild = oldChildren[oldChildrenTop]; - assert(oldChild != null); + assert(replaceWithNullIfDetached(oldChild) != null); assert(oldChild._debugLifecycleState == _ElementLifecycle.active); Widget newWidget = newWidgets[newChildrenTop]; assert(Widget.canUpdate(oldChild.widget, newWidget)); @@ -1678,8 +1682,10 @@ abstract class RenderObjectElement extends Buildab // clean up any of the remaining middle nodes from the old list if (haveOldChildren && oldKeyedChildren.isNotEmpty) { - for (Element oldChild in oldKeyedChildren.values) - _deactivateChild(oldChild); + for (Element oldChild in oldKeyedChildren.values) { + if (detachedChildren == null || !detachedChildren.contains(oldChild)) + _deactivateChild(oldChild); + } } return newChildren; @@ -1808,19 +1814,9 @@ class MultiChildRenderObjectElement exte } List _children; - // We null out detached children lazily to avoid O(n^2) work walking _children + // We keep a set of detached children to avoid O(n^2) work walking _children // repeatedly to remove children. - Set _detachedChildren; - - void _replaceDetachedChildrenWithNull() { - if (_detachedChildren != null && _detachedChildren.isNotEmpty) { - for (int i = 0; i < _children.length; ++i) { - if (_detachedChildren.contains(_children[i])) - _children[i] = null; - } - _detachedChildren.clear(); - } - } + final Set _detachedChildren = new HashSet(); void insertChildRenderObject(RenderObject child, Element slot) { final ContainerRenderObjectMixin renderObject = this.renderObject; @@ -1860,15 +1856,13 @@ class MultiChildRenderObjectElement exte } void visitChildren(ElementVisitor visitor) { - _replaceDetachedChildrenWithNull(); for (Element child in _children) { - if (child != null) + if (!_detachedChildren.contains(child)) visitor(child); } } bool detachChild(Element child) { - _detachedChildren ??= new Set(); _detachedChildren.add(child); _deactivateChild(child); return true; @@ -1888,8 +1882,8 @@ class MultiChildRenderObjectElement exte void update(T newWidget) { super.update(newWidget); assert(widget == newWidget); - _replaceDetachedChildrenWithNull(); - _children = updateChildren(_children, widget.children); + _children = updateChildren(_children, widget.children, detachedChildren: _detachedChildren); + _detachedChildren.clear(); } } diff --git a/packages/flutter/test/widget/reparent_state_test.dart b/packages/flutter/test/widget/reparent_state_test.dart index c344826711..99a84e01e6 100644 --- a/packages/flutter/test/widget/reparent_state_test.dart +++ b/packages/flutter/test/widget/reparent_state_test.dart @@ -196,4 +196,99 @@ void main() { expect(keyState.marker, equals("marked")); }); }); + + test('Reparent during update children', () { + testWidgets((WidgetTester tester) { + GlobalKey key = new GlobalKey(); + + tester.pumpWidget(new Stack( + children: [ + new StateMarker(key: key), + new Container(width: 100.0, height: 100.0), + ] + )); + + StateMarkerState keyState = key.currentState; + keyState.marker = "marked"; + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0), + new StateMarker(key: key), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + + tester.pumpWidget(new Stack( + children: [ + new StateMarker(key: key), + new Container(width: 100.0, height: 100.0), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + }); + }); + + test('Reparent to child during update children', () { + testWidgets((WidgetTester tester) { + GlobalKey key = new GlobalKey(); + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0), + new StateMarker(key: key), + new Container(width: 100.0, height: 100.0), + ] + )); + + StateMarkerState keyState = key.currentState; + keyState.marker = "marked"; + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0, child: new StateMarker(key: key)), + new Container(width: 100.0, height: 100.0), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0), + new StateMarker(key: key), + new Container(width: 100.0, height: 100.0), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0), + new Container(width: 100.0, height: 100.0, child: new StateMarker(key: key)), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + + tester.pumpWidget(new Stack( + children: [ + new Container(width: 100.0, height: 100.0), + new StateMarker(key: key), + new Container(width: 100.0, height: 100.0), + ] + )); + + expect(key.currentState, equals(keyState)); + expect(keyState.marker, equals("marked")); + }); + }); }