Clean up how we remove nodes from the render tree, and make sure we reinsert them in the right plac…

sky/sdk/lib/widgets/widget.dart

 // Copyright 2015 The Chromium 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 'dart:async';
 import 'dart:collection';
 import 'dart:mirrors';
 import 'dart:sky' as sky;
 
 import '../app/view.dart';
@@ skipping 72 matching lines @@
   // This is called immediately before _sync().
   // Component._retainStatefulNodeIfPossible() calls syncFields().
   bool _retainStatefulNodeIfPossible(Widget old) => false;
 
   bool get interchangeable => false; // if true, then keys can be duplicated
 
   void _sync(Widget old, dynamic slot);
   // 'slot' is the identifier that the parent RenderObjectWrapper uses to know
   // where to put this descendant
 
-  void remove() {
-    _root = null;
-    setParent(null);
-  }
-
   Widget findAncestor(Type targetType) {
     var ancestor = _parent;
     while (ancestor != null && !reflectClass(ancestor.runtimeType).isSubtypeOf(reflectClass(targetType)))
       ancestor = ancestor._parent;
     return ancestor;
   }
 
+  void remove() {
+    _root = null;
+    setParent(null);
+  }
+
   void removeChild(Widget node) {
     node.remove();
   }
 
+  void detachRoot();
+
   // Returns the child which should be retained as the child of this node.
   Widget syncChild(Widget node, Widget oldNode, dynamic slot) {
 
     assert(oldNode is! Component || !oldNode._disqualifiedFromEverAppearingAgain);
 
     if (node == oldNode) {
       assert(node == null || node.mounted);
+      assert(node is! RenderObjectWrapper || node._ancestor != null);
       return node; // Nothing to do. Subtrees must be identical.
     }
 
     if (node == null) {
       // the child in this slot has gone away
       assert(oldNode.mounted);
+      oldNode.detachRoot();
       removeChild(oldNode);
       assert(!oldNode.mounted);
       return null;
     }
 
     if (oldNode != null &&
         oldNode.runtimeType == node.runtimeType &&
         oldNode.key == node.key &&
         node._retainStatefulNodeIfPossible(oldNode)) {
       assert(oldNode.mounted);
       assert(!node.mounted);
       oldNode._sync(node, slot);
       assert(oldNode.root is RenderObject);
       return oldNode;
     }
 
     if (oldNode != null &&
         (oldNode.runtimeType != node.runtimeType || oldNode.key != node.key)) {
       assert(oldNode.mounted);
+      oldNode.detachRoot();
       removeChild(oldNode);
       oldNode = null;
     }
 
     assert(!node.mounted);
     node.setParent(this);
     node._sync(oldNode, slot);
     assert(node.root is RenderObject);
     return node;
   }
+
+  String toString() => '$runtimeType($key)';
+
 }
 
 
 // Descendants of TagNode provide a way to tag RenderObjectWrapper and
 // Component nodes with annotations, such as event listeners,
 // stylistic information, etc.
 abstract class TagNode extends Widget {
 
   TagNode(Widget content, { String key })
     : this.content = content, super(key: key);
 
   Widget content;
 
   void _sync(Widget old, dynamic slot) {
     Widget oldContent = old == null ? null : (old as TagNode).content;
     content = syncChild(content, oldContent, slot);
     assert(content.root != null);
     _root = content.root;
     assert(_root == root); // in case a subclass reintroduces it
   }
 
   void remove() {
     if (content != null)
       removeChild(content);
     super.remove();
   }
 
+  void detachRoot() {
+    if (content != null)
+      content.detachRoot();
+  }
+
 }
 
 class ParentDataNode extends TagNode {
   ParentDataNode(Widget content, this.parentData, { String key })
     : super(content, key: key);
   final ParentData parentData;
 }
 
 typedef void GestureEventListener(sky.GestureEvent e);
 typedef void PointerEventListener(sky.PointerEvent e);
@@ skipping 110 matching lines @@
   }
 
   void remove() {
     assert(_built != null);
     assert(root != null);
     removeChild(_built);
     _built = null;
     super.remove();
   }
 
+  void detachRoot() {
+    assert(_built != null);
+    assert(root != null);
+    _built.detachRoot();
+  }
+
   bool _retainStatefulNodeIfPossible(Widget old) {
     assert(!_disqualifiedFromEverAppearingAgain);
 
     Component oldComponent = old as Component;
     if (oldComponent == null || !oldComponent._stateful)
       return false;
 
     assert(runtimeType == oldComponent.runtimeType);
     assert(key == oldComponent.key);
 
@@ skipping 141 matching lines @@
 // They are fully immutable, with one exception: A Widget which is a
 // Component which lives within an MultiChildRenderObjectWrapper's
 // children list, may be replaced with the "old" instance if it has
 // become stateful.
 abstract class RenderObjectWrapper extends Widget {
 
   RenderObjectWrapper({ String key }) : super(key: key);
 
   RenderObject createNode();
 
-  void insert(RenderObjectWrapper child, dynamic slot);
-
   static final Map<RenderObject, RenderObjectWrapper> _nodeMap =
       new HashMap<RenderObject, RenderObjectWrapper>();
+  static RenderObjectWrapper _getMounted(RenderObject node) => _nodeMap[node];
 
-  static RenderObjectWrapper _getMounted(RenderObject node) => _nodeMap[node];
+  RenderObjectWrapper _ancestor;
+  void insert(RenderObjectWrapper child, dynamic slot);
+  void detachChildRoot(RenderObjectWrapper child);
 
   void _sync(Widget old, dynamic slot) {
     // TODO(abarth): We should split RenderObjectWrapper into two pieces so that
     //               RenderViewObject doesn't need to inherit all this code it
     //               doesn't need.
     assert(parent != null || this is RenderViewWrapper);
     if (old == null) {
       _root = createNode();
-      var ancestor = findAncestor(RenderObjectWrapper);
-      if (ancestor is RenderObjectWrapper)
-        ancestor.insert(this, slot);
+      _ancestor = findAncestor(RenderObjectWrapper);
+      if (_ancestor is RenderObjectWrapper)
+        _ancestor.insert(this, slot);
+      else
+        print("$this has no ancestor");
     } else {
       _root = old.root;
+      _ancestor = old._ancestor;
     }
     assert(_root == root); // in case a subclass reintroduces it
     assert(root != null);
     assert(mounted);
     _nodeMap[root] = this;
     syncRenderObject(old);
   }
 
   void syncRenderObject(RenderObjectWrapper old) {
     ParentData parentData = null;
@@ skipping 13 matching lines @@
       if (parent.root != null)
         parent.root.markNeedsLayout();
     }
   }
 
   void remove() {
     assert(root != null);
     _nodeMap.remove(root);
     super.remove();
   }
+
+  void detachRoot() {
+    assert(_ancestor != null);
+    assert(root != null);
+    _ancestor.detachChildRoot(this);
+  }
+
 }
 
 abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper {
 
   OneChildRenderObjectWrapper({ String key, Widget child })
     : _child = child, super(key: key);
 
   Widget _child;
   Widget get child => _child;
 
   void syncRenderObject(RenderObjectWrapper old) {
     super.syncRenderObject(old);
     Widget oldChild = old == null ? null : (old as OneChildRenderObjectWrapper).child;
     _child = syncChild(child, oldChild, null);
   }
 
   void insert(RenderObjectWrapper child, dynamic slot) {
     final root = this.root; // TODO(ianh): Remove this once the analyzer is cleverer
+    assert(root is RenderObjectWithChildMixin);
     assert(slot == null);
-    assert(root is RenderObjectWithChildMixin);
     root.child = child.root;
     assert(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
   }
 
-  void removeChild(Widget node) {
+  void detachChildRoot(RenderObjectWrapper child) {
     final root = this.root; // TODO(ianh): Remove this once the analyzer is cleverer
     assert(root is RenderObjectWithChildMixin);
+    assert(root.child == child.root);
     root.child = null;
-    super.removeChild(node);
     assert(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
   }
 
   void remove() {
     if (child != null)
       removeChild(child);
     super.remove();
   }
 
 }
@@ skipping 12 matching lines @@
   final List<Widget> children;
 
   void insert(RenderObjectWrapper child, dynamic slot) {
     final root = this.root; // TODO(ianh): Remove this once the analyzer is cleverer
     assert(slot == null || slot is RenderObject);
     assert(root is ContainerRenderObjectMixin);
     root.add(child.root, before: slot);
     assert(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
   }
 
-  void removeChild(Widget node) {
+  void detachChildRoot(RenderObjectWrapper child) {
     final root = this.root; // TODO(ianh): Remove this once the analyzer is cleverer
     assert(root is ContainerRenderObjectMixin);
-    assert(node.root.parent == root);
-    root.remove(node.root);
-    super.removeChild(node);
+    assert(child.root.parent == root);
+    root.remove(child.root);
     assert(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
   }
 
   void remove() {
     assert(children != null);
     for (var child in children) {
       assert(child != null);
       removeChild(child);
     }
     super.remove();
@@ skipping 42 matching lines @@
       currentNode = children[endIndex - 1];
       oldNode = oldChildren[oldEndIndex - 1];
 
       if (currentNode.runtimeType != oldNode.runtimeType || currentNode.key != oldNode.key) {
         break;
       }
 
       endIndex--;
       oldEndIndex--;
       sync(endIndex);
+      nextSibling = children[endIndex].root;
     }
 
     HashMap<String, Widget> oldNodeIdMap = null;
 
     bool oldNodeReordered(String key) {
       return oldNodeIdMap != null &&
              oldNodeIdMap.containsKey(key) &&
              oldNodeIdMap[key] == null;
     }
 
@@ skipping 62 matching lines @@
     while (startIndex < endIndex) {
       currentNode = children[startIndex];
       sync(startIndex);
       startIndex++;
     }
 
     // Removals
     currentNode = null;
     while (oldStartIndex < oldEndIndex) {
       oldNode = oldChildren[oldStartIndex];
-      removeChild(oldNode);
+      syncChild(null, oldNode, null);
       advanceOldStartIndex();
     }
 
     assert(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
   }
 
 }
 
 class WidgetAppView extends AppView {
 
@@ skipping 108 matching lines @@
     if (root.parent == null) {
       // we haven't attached it yet
       assert(_container.child == null);
       _container.child = root;
     }
     assert(root.parent == _container);
   }
 
   Widget build() => builder();
 }