Handle the case of reparenting while updating children

Instead of trying to flush the detached children from the child list, we keep
the set of detached children up to date and query on every read.

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

@@ -1536,10 +1536,14 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> 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<Element> updateChildren(List<Element> oldChildren, List<Widget> newWidgets) {
+  List<Element> updateChildren(List<Element> oldChildren, List<Widget> newWidgets, { Set<Element> 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<T extends RenderObjectWidget> 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<T extends RenderObjectWidget> 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<T extends RenderObjectWidget> extends Buildab
     if (haveOldChildren) {
       oldKeyedChildren = new Map<Key, Element>();
       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<T extends RenderObjectWidget> 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,9 +1682,11 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> extends Buildab
 
     // clean up any of the remaining middle nodes from the old list
     if (haveOldChildren && oldKeyedChildren.isNotEmpty) {
-      for (Element oldChild in oldKeyedChildren.values)
+      for (Element oldChild in oldKeyedChildren.values) {
+        if (detachedChildren == null || !detachedChildren.contains(oldChild))
           _deactivateChild(oldChild);
       }
+    }
 
     return newChildren;
   }
@@ -1808,19 +1814,9 @@ class MultiChildRenderObjectElement<T extends MultiChildRenderObjectWidget> exte
   }
 
   List<Element> _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<Element> _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<Element> _detachedChildren = new HashSet<Element>();
 
   void insertChildRenderObject(RenderObject child, Element slot) {
     final ContainerRenderObjectMixin renderObject = this.renderObject;
@@ -1860,15 +1856,13 @@ class MultiChildRenderObjectElement<T extends MultiChildRenderObjectWidget> 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<Element>();
     _detachedChildren.add(child);
     _deactivateChild(child);
     return true;
@@ -1888,8 +1882,8 @@ class MultiChildRenderObjectElement<T extends MultiChildRenderObjectWidget> 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();
   }
 }
 

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: <Widget>[
+          new StateMarker(key: key),
+          new Container(width: 100.0, height: 100.0),
+        ]
+      ));
+
+      StateMarkerState keyState = key.currentState;
+      keyState.marker = "marked";
+
+      tester.pumpWidget(new Stack(
+        children: <Widget>[
+          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: <Widget>[
+          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: <Widget>[
+          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: <Widget>[
+          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: <Widget>[
+          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: <Widget>[
+          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: <Widget>[
+          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"));
+    });
+  });
 }