updateChildren() needs to walk the list forward

This patch changes the framework to walk the child list forwards so that build
functions with global side effects do sensible things. Specifically, if you
have a number of autofocusable children, the first one the list will acquire
the focus because it gets built first now.

Fixes #182

packages/flutter/lib/src/rendering/object.dart

@@ -1294,59 +1294,62 @@ abstract class ContainerRenderObjectMixin<ChildType extends RenderObject, Parent
 
   ChildType _firstChild;
   ChildType _lastChild;
-  void _addToChildList(ChildType child, { ChildType before }) {
+  void _insertIntoChildList(ChildType child, { ChildType after }) {
     final ParentDataType childParentData = child.parentData;
     assert(childParentData.nextSibling == null);
     assert(childParentData.previousSibling == null);
     _childCount += 1;
     assert(_childCount > 0);
-    if (before == null) {
-      // append at the end (_lastChild)
-      childParentData.previousSibling = _lastChild;
-      if (_lastChild != null) {
-        final ParentDataType _lastChildParentData = _lastChild.parentData;
-        _lastChildParentData.nextSibling = child;
+    if (after == null) {
+      // insert at the start (_firstChild)
+      childParentData.nextSibling = _firstChild;
+      if (_firstChild != null) {
+        final ParentDataType _firstChildParentData = _firstChild.parentData;
+        _firstChildParentData.previousSibling = child;
       }
-      _lastChild = child;
-      if (_firstChild == null)
-        _firstChild = child;
+      _firstChild = child;
+      if (_lastChild == null)
+        _lastChild = child;
     } else {
       assert(_firstChild != null);
       assert(_lastChild != null);
-      assert(_debugUltimatePreviousSiblingOf(before, equals: _firstChild));
-      assert(_debugUltimateNextSiblingOf(before, equals: _lastChild));
-      final ParentDataType beforeParentData = before.parentData;
-      if (beforeParentData.previousSibling == null) {
-        // insert at the start (_firstChild); we'll end up with two or more children
-        assert(before == _firstChild);
-        childParentData.nextSibling = before;
-        beforeParentData.previousSibling = child;
-        _firstChild = child;
+      assert(_debugUltimatePreviousSiblingOf(after, equals: _firstChild));
+      assert(_debugUltimateNextSiblingOf(after, equals: _lastChild));
+      final ParentDataType afterParentData = after.parentData;
+      if (afterParentData.nextSibling == null) {
+        // insert at the end (_lastChild); we'll end up with two or more children
+        assert(after == _lastChild);
+        childParentData.previousSibling = after;
+        afterParentData.nextSibling = child;
+        _lastChild = child;
       } else {
         // insert in the middle; we'll end up with three or more children
         // set up links from child to siblings
-        childParentData.previousSibling = beforeParentData.previousSibling;
-        childParentData.nextSibling = before;
+        childParentData.nextSibling = afterParentData.nextSibling;
+        childParentData.previousSibling = after;
         // set up links from siblings to child
         final ParentDataType childPreviousSiblingParentData = childParentData.previousSibling.parentData;
         final ParentDataType childNextSiblingParentData = childParentData.nextSibling.parentData;
         childPreviousSiblingParentData.nextSibling = child;
         childNextSiblingParentData.previousSibling = child;
-        assert(beforeParentData.previousSibling == child);
+        assert(afterParentData.nextSibling == child);
       }
     }
   }
-  /// Insert child into this render object's child list before the given child.
-  ///
-  /// To insert a child at the end of the child list, omit the before parameter.
-  void add(ChildType child, { ChildType before }) {
+  /// Insert child into this render object's child list after the given child.
+  void insert(ChildType child, { ChildType after }) {
     assert(child != this);
-    assert(before != this);
-    assert(child != before);
+    assert(after != this);
+    assert(child != after);
     assert(child != _firstChild);
     assert(child != _lastChild);
     adoptChild(child);
-    _addToChildList(child, before: before);
+    _insertIntoChildList(child, after: after);
+  }
+
+  /// Append child to the end of this render object's child list.
+  void add(ChildType child) {
+    insert(child, after: _lastChild);
   }
 
   /// Add all the children to the end of this render object's child list.
@@ -1411,16 +1414,16 @@ abstract class ContainerRenderObjectMixin<ChildType extends RenderObject, Parent
   /// More efficient than removing and re-adding the child. Requires the child
   /// to already be in the child list at some position. Pass null for before to
   /// move the child to the end of the child list.
-  void move(ChildType child, { ChildType before }) {
+  void move(ChildType child, { ChildType after }) {
     assert(child != this);
-    assert(before != this);
-    assert(child != before);
+    assert(after != this);
+    assert(child != after);
     assert(child.parent == this);
     final ParentDataType childParentData = child.parentData;
-    if (childParentData.nextSibling == before)
+    if (childParentData.previousSibling == after)
       return;
     _removeFromChildList(child);
-    _addToChildList(child, before: before);
+    _insertIntoChildList(child, after: after);
   }
 
   void attach() {

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

@@ -1507,79 +1507,80 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> extends Buildab
     // Widgets without keys might be synced but there is no guarantee.
 
     // The general approach is to sync the entire new list backwards, as follows:
-    // 1. Walk the lists from the top until you no longer have
-    //    matching nodes. We don't sync these yet, but we now know to
-    //    skip them below. We do this because at each sync we need to
-    //    pass the pointer to the new next widget as the slot, which
-    //    we can't do until we've synced the next child.
-    // 2. Walk the lists from the bottom, syncing nodes, until you no
-    //    longer have matching nodes.
+    // 1. Walk the lists from the top, syncing nodes, until you no longer have
+    //    matching nodes.
+    // 2. Walk the lists from the bottom, without syncing nodes, until you no
+    //    longer have matching nodes. We'll sync these nodes at the end. We
+    //    don't sync them now because we want to sync all the nodes in order
+    //    from beginning ot end.
     // At this point we narrowed the old and new lists to the point
     // where the nodes no longer match.
     // 3. Walk the narrowed part of the old list to get the list of
     //    keys and sync null with non-keyed items.
-    // 4. Walk the narrowed part of the new list backwards:
+    // 4. Walk the narrowed part of the new list forwards:
     //     * Sync unkeyed items with null
     //     * Sync keyed items with the source if it exists, else with null.
-    // 5. Walk the top list again but backwards, syncing the nodes.
+    // 5. Walk the bottom of the list again, syncing the nodes.
     // 6. Sync null with any items in the list of keys that are still
     //    mounted.
 
-    int childrenTop = 0;
+    int newChildrenTop = 0;
+    int oldChildrenTop = 0;
     int newChildrenBottom = newWidgets.length - 1;
     int oldChildrenBottom = oldChildren.length - 1;
 
-    // top of the lists
-    while ((childrenTop <= oldChildrenBottom) && (childrenTop <= newChildrenBottom)) {
-      Element oldChild = oldChildren[childrenTop];
-      Widget newWidget = newWidgets[childrenTop];
+    List<Element> newChildren = oldChildren.length == newWidgets.length ?
+        oldChildren : new List<Element>(newWidgets.length);
+
+    Element previousChild;
+
+    // Update the top of the list.
+    while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) {
+      Element oldChild = oldChildren[oldChildrenTop];
+      Widget newWidget = newWidgets[newChildrenTop];
       assert(oldChild._debugLifecycleState == _ElementLifecycle.active);
       if (!Widget.canUpdate(oldChild.widget, newWidget))
         break;
-      childrenTop += 1;
+      Element newChild = updateChild(oldChild, newWidget, previousChild);
+      assert(newChild._debugLifecycleState == _ElementLifecycle.active);
+      newChildren[newChildrenTop] = newChild;
+      previousChild = newChild;
+      newChildrenTop += 1;
+      oldChildrenTop += 1;
     }
 
-    List<Element> newChildren = oldChildren.length == newWidgets.length ?
-        oldChildren : new List<Element>(newWidgets.length);
-
-    Element nextSibling;
-
-    // bottom of the lists
-    while ((childrenTop <= oldChildrenBottom) && (childrenTop <= newChildrenBottom)) {
+    // Scan the bottom of the list.
+    while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) {
       Element oldChild = oldChildren[oldChildrenBottom];
       Widget newWidget = newWidgets[newChildrenBottom];
       assert(oldChild._debugLifecycleState == _ElementLifecycle.active);
       if (!Widget.canUpdate(oldChild.widget, newWidget))
         break;
-      Element newChild = updateChild(oldChild, newWidget, nextSibling);
-      assert(newChild._debugLifecycleState == _ElementLifecycle.active);
-      newChildren[newChildrenBottom] = newChild;
-      nextSibling = newChild;
       oldChildrenBottom -= 1;
       newChildrenBottom -= 1;
     }
 
-    // middle of the lists - old list
-    bool haveOldNodes = childrenTop <= oldChildrenBottom;
+    // Scan the old children in the middle of the list.
+    bool haveOldChildren = oldChildrenTop <= oldChildrenBottom;
     Map<Key, Element> oldKeyedChildren;
-    if (haveOldNodes) {
+    if (haveOldChildren) {
       oldKeyedChildren = new Map<Key, Element>();
-      while (childrenTop <= oldChildrenBottom) {
-        Element oldChild = oldChildren[oldChildrenBottom];
+      while (oldChildrenTop <= oldChildrenBottom) {
+        Element oldChild = oldChildren[oldChildrenTop];
         assert(oldChild._debugLifecycleState == _ElementLifecycle.active);
         if (oldChild.widget.key != null)
           oldKeyedChildren[oldChild.widget.key] = oldChild;
         else
           _deactivateChild(oldChild);
-        oldChildrenBottom -= 1;
+        oldChildrenTop += 1;
       }
     }
 
-    // middle of the lists - new list
-    while (childrenTop <= newChildrenBottom) {
+    // Update the middle of the list.
+    while (newChildrenTop <= newChildrenBottom) {
       Element oldChild;
-      Widget newWidget = newWidgets[newChildrenBottom];
-      if (haveOldNodes) {
+      Widget newWidget = newWidgets[newChildrenTop];
+      if (haveOldChildren) {
         Key key = newWidget.key;
         if (key != null) {
           oldChild = oldKeyedChildren[newWidget.key];
@@ -1596,32 +1597,38 @@ abstract class RenderObjectElement<T extends RenderObjectWidget> extends Buildab
         }
       }
       assert(oldChild == null || Widget.canUpdate(oldChild.widget, newWidget));
-      Element newChild = updateChild(oldChild, newWidget, nextSibling);
+      Element newChild = updateChild(oldChild, newWidget, previousChild);
       assert(newChild._debugLifecycleState == _ElementLifecycle.active);
       assert(oldChild == newChild || oldChild == null || oldChild._debugLifecycleState != _ElementLifecycle.active);
-      newChildren[newChildrenBottom] = newChild;
-      nextSibling = newChild;
-      newChildrenBottom -= 1;
+      newChildren[newChildrenTop] = newChild;
+      previousChild = newChild;
+      newChildrenTop += 1;
     }
-    assert(oldChildrenBottom == newChildrenBottom);
-    assert(childrenTop == newChildrenBottom + 1);
 
-    // now sync the top of the list
-    while (childrenTop > 0) {
-      childrenTop -= 1;
-      Element oldChild = oldChildren[childrenTop];
+    // We've scaned the whole list.
+    assert(oldChildrenTop == oldChildrenBottom + 1);
+    assert(newChildrenTop == newChildrenBottom + 1);
+    assert(newWidgets.length - newChildrenTop == oldChildren.length - oldChildrenTop);
+    newChildrenBottom = newWidgets.length - 1;
+    oldChildrenBottom = oldChildren.length - 1;
+
+    // Update the bottom of the list.
+    while ((oldChildrenTop <= oldChildrenBottom) && (newChildrenTop <= newChildrenBottom)) {
+      Element oldChild = oldChildren[oldChildrenTop];
       assert(oldChild._debugLifecycleState == _ElementLifecycle.active);
-      Widget newWidget = newWidgets[childrenTop];
+      Widget newWidget = newWidgets[newChildrenTop];
       assert(Widget.canUpdate(oldChild.widget, newWidget));
-      Element newChild = updateChild(oldChild, newWidget, nextSibling);
+      Element newChild = updateChild(oldChild, newWidget, previousChild);
       assert(newChild._debugLifecycleState == _ElementLifecycle.active);
       assert(oldChild == newChild || oldChild == null || oldChild._debugLifecycleState != _ElementLifecycle.active);
-      newChildren[childrenTop] = newChild;
-      nextSibling = newChild;
+      newChildren[newChildrenTop] = newChild;
+      previousChild = newChild;
+      newChildrenTop += 1;
+      oldChildrenTop += 1;
     }
 
     // clean up any of the remaining middle nodes from the old list
-    if (haveOldNodes && !oldKeyedChildren.isEmpty) {
+    if (haveOldChildren && !oldKeyedChildren.isEmpty) {
       for (Element oldChild in oldKeyedChildren.values)
         _deactivateChild(oldChild);
     }
@@ -1755,15 +1762,13 @@ class MultiChildRenderObjectElement<T extends MultiChildRenderObjectWidget> exte
 
   void insertChildRenderObject(RenderObject child, Element slot) {
     final ContainerRenderObjectMixin renderObject = this.renderObject;
-    final RenderObject nextSibling = slot?.renderObject;
-    renderObject.add(child, before: nextSibling);
+    renderObject.insert(child, after: slot?.renderObject);
     assert(renderObject == this.renderObject);
   }
 
   void moveChildRenderObject(RenderObject child, dynamic slot) {
     final ContainerRenderObjectMixin renderObject = this.renderObject;
-    final RenderObject nextSibling = slot?.renderObject;
-    renderObject.move(child, before: nextSibling);
+    renderObject.move(child, after: slot?.renderObject);
     assert(renderObject == this.renderObject);
   }
 
@@ -1797,7 +1802,7 @@ class MultiChildRenderObjectElement<T extends MultiChildRenderObjectWidget> exte
     super.mount(parent, newSlot);
     _children = new List<Element>(widget.children.length);
     Element previousChild;
-    for (int i = _children.length - 1; i >= 0; --i) {
+    for (int i = 0; i < _children.length; ++i) {
       Element newChild = _inflateWidget(widget.children[i], previousChild);
       _children[i] = newChild;
       previousChild = newChild;

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

@@ -118,14 +118,12 @@ abstract class _ViewportBaseElement<T extends _ViewportBase> extends RenderObjec
   }
 
   void insertChildRenderObject(RenderObject child, Element slot) {
-    RenderObject nextSibling = slot?.renderObject;
-    renderObject.add(child, before: nextSibling);
+    renderObject.insert(child, after: slot?.renderObject);
   }
 
   void moveChildRenderObject(RenderObject child, Element slot) {
     assert(child.parent == renderObject);
-    RenderObject nextSibling = slot?.renderObject;
-    renderObject.move(child, before: nextSibling);
+    renderObject.move(child, after: slot?.renderObject);
   }
 
   void removeChildRenderObject(RenderObject child) {

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

@@ -552,14 +552,12 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
     if (haveChildren) {
       // Place all our children in our RenderObject.
       // All the children we are placing are in builtChildren and newChildren.
-      // We will walk them backwards so we can set the slots at the same time.
-      Element nextSibling = null;
-      while (index > startIndex) {
-        index -= 1;
-        final Element element = builtChildren[index];
-        if (element.slot != nextSibling)
-          updateSlotForChild(element, nextSibling);
-        nextSibling = element;
+      Element previousChild = null;
+      for (int i = startIndex; i < index; ++i) {
+        final Element element = builtChildren[i];
+        if (element.slot != previousChild)
+          updateSlotForChild(element, previousChild);
+        previousChild = element;
       }
     }
 
@@ -577,20 +575,19 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
     if (slot == _omit)
       return;
     assert(slot == null || slot is Element);
-    RenderObject nextSibling = slot?.renderObject;
-    renderObject.add(child, before: nextSibling);
+    renderObject.insert(child, after: slot?.renderObject);
   }
 
   void moveChildRenderObject(RenderObject child, dynamic slot) {
     if (slot == _omit)
       return;
     assert(slot == null || slot is Element);
-    RenderObject nextSibling = slot?.renderObject;
-    assert(nextSibling == null || nextSibling.parent == renderObject);
+    RenderObject previousSibling = slot?.renderObject;
+    assert(previousSibling == null || previousSibling.parent == renderObject);
     if (child.parent == renderObject)
-      renderObject.move(child, before: nextSibling);
+      renderObject.move(child, after: previousSibling);
     else
-      renderObject.add(child, before: nextSibling);
+      renderObject.insert(child, after: previousSibling);
   }
 
   void removeChildRenderObject(RenderObject child) {

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

@@ -149,14 +149,12 @@ abstract class VirtualViewportElement<T extends VirtualViewport> extends RenderO
   }
 
   void insertChildRenderObject(RenderObject child, Element slot) {
-    RenderObject nextSibling = slot?.renderObject;
-    renderObject.add(child, before: nextSibling);
+    renderObject.insert(child, after: slot?.renderObject);
   }
 
   void moveChildRenderObject(RenderObject child, Element slot) {
     assert(child.parent == renderObject);
-    RenderObject nextSibling = slot?.renderObject;
-    renderObject.move(child, before: nextSibling);
+    renderObject.move(child, after: slot?.renderObject);
   }
 
   void removeChildRenderObject(RenderObject child) {

packages/flutter/test/widget/focus_test.dart

@@ -36,17 +36,16 @@ void main() {
         new Focus(
           child: new Column(
             children: <Widget>[
-              // reverse these when you fix https://github.com/flutter/engine/issues/1495
-              new TestFocusable(
-                key: keyB,
-                no: 'b',
-                yes: 'B FOCUSED'
-              ),
               new TestFocusable(
                 key: keyA,
                 no: 'a',
                 yes: 'A FOCUSED'
               ),
+              new TestFocusable(
+                key: keyB,
+                no: 'b',
+                yes: 'B FOCUSED'
+              ),
             ]
           )
         )