OneChildRenderObjectWrapper and Scaffold were ignoring the return value of syncChild().

This would cause them to forget what the most recent child they'd been given was, with disastrous effects later.

R=abarth@chromium.org

Review URL: https://codereview.chromium.org/1173683003

examples/stocks2/lib/stock_app.dart

@@ -158,7 +158,7 @@ class StocksApp extends App {
     return new ToolBar(
         left: new IconButton(
           icon: 'navigation/menu_white',
-          onGestureTap: _drawerController.toggle),
+          onGestureTap: (_) => _drawerController.toggle()),
         center: new Text('Stocks'),
         right: [
           new IconButton(

sdk/lib/framework/components2/drawer.dart

@@ -40,7 +40,7 @@ class DrawerController {
 
   bool get isClosed => position.value == -_kWidth;
   bool get _isMostlyClosed => position.value <= -_kWidth / 2;
-  void toggle(_) => _isMostlyClosed ? _open() : _close();
+  void toggle() => _isMostlyClosed ? _open() : _close();
 
   void handleMaskTap(_) => _close();
   void handlePointerDown(_) => position.stop();

sdk/lib/framework/components2/scaffold.dart

@@ -158,20 +158,23 @@ class Scaffold extends RenderObjectWrapper {
 
   Scaffold({
     Object key,
-    this.toolbar,
-    this.body,
-    this.statusbar,
-    this.drawer,
-    this.floatingActionButton
-  }) : super(
-    key: key
-  );
-
-  final UINode toolbar;
-  final UINode body;
-  final UINode statusbar;
-  final UINode drawer;
-  final UINode floatingActionButton;
+    UINode toolbar,
+    UINode body,
+    UINode statusbar,
+    UINode drawer,
+    UINode floatingActionButton
+  }) : _toolbar = toolbar,
+       _body = body,
+       _statusbar = statusbar,
+       _drawer = drawer,
+       _floatingActionButton = floatingActionButton,
+       super(key: key);
+
+  UINode _toolbar;
+  UINode _body;
+  UINode _statusbar;
+  UINode _drawer;
+  UINode _floatingActionButton;
 
   RenderScaffold root;
   RenderScaffold createNode() => new RenderScaffold();
@@ -187,26 +190,26 @@ class Scaffold extends RenderObjectWrapper {
   }
 
   void remove() {
-    if (toolbar != null)
-      removeChild(toolbar);
-    if (body != null)
-      removeChild(body);
-    if (statusbar != null)
-      removeChild(statusbar);
-    if (drawer != null)
-      removeChild(drawer);
-    if (floatingActionButton != null)
-      removeChild(floatingActionButton);
+    if (_toolbar != null)
+      removeChild(_toolbar);
+    if (_body != null)
+      removeChild(_body);
+    if (_statusbar != null)
+      removeChild(_statusbar);
+    if (_drawer != null)
+      removeChild(_drawer);
+    if (_floatingActionButton != null)
+      removeChild(_floatingActionButton);
     super.remove();
   }
 
   void syncRenderObject(UINode old) {
     super.syncRenderObject(old);
-    syncChild(toolbar, old is Scaffold ? old.toolbar : null, ScaffoldSlots.toolbar);
-    syncChild(body, old is Scaffold ? old.body : null, ScaffoldSlots.body);
-    syncChild(statusbar, old is Scaffold ? old.statusbar : null, ScaffoldSlots.statusBar);
-    syncChild(drawer, old is Scaffold ? old.drawer : null, ScaffoldSlots.drawer);
-    syncChild(floatingActionButton, old is Scaffold ? old.floatingActionButton : null, ScaffoldSlots.floatingActionButton);
+    _toolbar = syncChild(_toolbar, old is Scaffold ? old._toolbar : null, ScaffoldSlots.toolbar);
+    _body = syncChild(_body, old is Scaffold ? old._body : null, ScaffoldSlots.body);
+    _statusbar = syncChild(_statusbar, old is Scaffold ? old._statusbar : null, ScaffoldSlots.statusBar);
+    _drawer = syncChild(_drawer, old is Scaffold ? old._drawer : null, ScaffoldSlots.drawer);
+    _floatingActionButton = syncChild(_floatingActionButton, old is Scaffold ? old._floatingActionButton : null, ScaffoldSlots.floatingActionButton);
   }
 
 }

sdk/lib/framework/fn2.dart

@@ -116,6 +116,8 @@ abstract class UINode {
   // Returns the child which should be retained as the child of this node.
   UINode syncChild(UINode node, UINode oldNode, dynamic slot) {
 
+    assert(oldNode is! Component || !oldNode._disqualifiedFromEverAppearingAgain);
+
     if (node == oldNode) {
       assert(node == null || node.mounted);
       return node; // Nothing to do. Subtrees must be identical.
@@ -331,14 +333,15 @@ abstract class RenderObjectWrapper extends UINode {
 }
 
 abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper {
-  final UINode child;
+  UINode _child;
+  UINode get child => _child;
 
-  OneChildRenderObjectWrapper({ this.child, Object key }) : super(key: key);
+  OneChildRenderObjectWrapper({ UINode child, Object key }) : _child = child, super(key: key);
 
   void syncRenderObject(RenderObjectWrapper old) {
     super.syncRenderObject(old);
     UINode oldChild = old == null ? null : (old as OneChildRenderObjectWrapper).child;
-    syncChild(child, oldChild, null);
+    _child = syncChild(child, oldChild, null);
   }
 
   void insert(RenderObjectWrapper child, dynamic slot) {
@@ -531,6 +534,7 @@ abstract class MultiChildRenderObjectWrapper extends RenderObjectWrapper {
   void removeChild(UINode node) {
     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(root == this.root); // TODO(ianh): Remove this once the analyzer is cleverer
@@ -783,7 +787,7 @@ class Image extends RenderObjectWrapper {
   }
 }
 
-List<Component> _dirtyComponents = new List<Component>();
+Set<Component> _dirtyComponents = new Set<Component>();
 bool _buildScheduled = false;
 bool _inRenderDirtyComponents = false;
 
@@ -797,8 +801,9 @@ void _buildDirtyComponents() {
   try {
     _inRenderDirtyComponents = true;
 
-    _dirtyComponents.sort((a, b) => a._order - b._order);
-    for (var comp in _dirtyComponents) {
+    List<Component> sortedDirtyComponents = _dirtyComponents.toList();
+    sortedDirtyComponents.sort((Component a, Component b) => a._order - b._order);
+    for (var comp in sortedDirtyComponents) {
       comp._buildIfDirty();
     }
 
@@ -832,6 +837,8 @@ abstract class Component extends UINode {
   bool get _isBuilding => _currentlyBuilding == this;
   bool _dirty = true;
 
+  bool _disqualifiedFromEverAppearingAgain = false;
+
   UINode _built;
   final int _order;
   static int _currentOrder = 0;
@@ -864,6 +871,7 @@ abstract class Component extends UINode {
       : this(key: key, stateful: stateful);
 
   void _didMount() {
+    assert(!_disqualifiedFromEverAppearingAgain);
     super._didMount();
     if (_mountCallbacks != null)
       for (Function fn in _mountCallbacks)
@@ -890,6 +898,8 @@ abstract class Component extends UINode {
   }
 
   bool _willSync(UINode old) {
+    assert(!_disqualifiedFromEverAppearingAgain);
+
     Component oldComponent = old as Component;
     if (oldComponent == null || !oldComponent._stateful)
       return false;
@@ -898,6 +908,7 @@ abstract class Component extends UINode {
     _stateful = false;
     _built = oldComponent._built;
     assert(_built != null);
+    _disqualifiedFromEverAppearingAgain = true;
 
     // Make |oldComponent| the "new" component
     reflect.copyPublicFields(this, oldComponent);
@@ -916,6 +927,7 @@ abstract class Component extends UINode {
    */
   void _sync(UINode old, dynamic slot) {
     assert(_built == null || old == null);
+    assert(!_disqualifiedFromEverAppearingAgain);
 
     Component oldComponent = old as Component;
 
@@ -945,6 +957,7 @@ abstract class Component extends UINode {
   }
 
   void _buildIfDirty() {
+    assert(!_disqualifiedFromEverAppearingAgain);
     if (!_dirty || !_mounted)
       return;
 
@@ -957,6 +970,7 @@ abstract class Component extends UINode {
   }
 
   void setState(Function fn()) {
+    assert(!_disqualifiedFromEverAppearingAgain);
     _stateful = true;
     fn();
     if (_isBuilding || _dirty || !_mounted)