Ignore generation of child if child is unchanged

Also:

 - don't mark a node as from the new generation if it is dirty, since we
   know it still has to be built.

 - establish the rule that you can't call setState() during initState()
   or build().

 - make syncChild() return early for unchanged children.

 - update the tests, including adding a new one.

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

@@ -389,6 +389,19 @@ abstract class Widget {
   // Returns the child which should be retained as the child of this node.
   Widget syncChild(Widget newNode, Widget oldNode, dynamic slot) {
 
+    if (newNode == oldNode) {
+      // TODO(ianh): Simplify next few asserts once the analyzer is cleverer
+      assert(newNode is! RenderObjectWrapper ||
+             (newNode is RenderObjectWrapper && newNode._ancestor != null)); // if the child didn't change, it had better be configured
+      assert(newNode is! StatefulComponent ||
+             (newNode is StatefulComponent && newNode._isStateInitialized)); // if the child didn't change, it had better be configured
+      if (newNode != null) {
+        newNode.setParent(this);
+        newNode._markAsFromCurrentGeneration();
+      }
+      return newNode; // Nothing to do. Subtrees must be identical.
+    }
+
     assert(() {
       'You have probably used a single instance of a Widget in two different places in the widget tree. Widgets can only be used in one place at a time.';
       return newNode == null || newNode.isFromOldGeneration;
@@ -397,27 +410,19 @@ abstract class Widget {
     if (oldNode != null && !oldNode.isFromOldGeneration)
       oldNode = null;
 
-    if (newNode == oldNode) {
-      assert(newNode == null || newNode.isFromOldGeneration);
-      assert(newNode is! RenderObjectWrapper ||
-             (newNode is RenderObjectWrapper && newNode._ancestor != null)); // TODO(ianh): Simplify this once the analyzer is cleverer
-      if (newNode != null) {
-        newNode.setParent(this);
-        newNode._markAsFromCurrentGeneration();
-      }
-      return newNode; // Nothing to do. Subtrees must be identical.
-    }
-
     if (newNode == null) {
-      // the child in this slot has gone away (we know oldNode != null)
-      assert(oldNode != null);
-      assert(oldNode.isFromOldGeneration);
-      assert(oldNode.mounted);
-      oldNode.detachRenderObject();
-      oldNode.remove();
-      assert(!oldNode.mounted);
-      // we don't update the generation of oldNode, because there's
-      // still a chance it could be reused as-is later in the tree.
+      // the child in this slot has gone away
+      // remove it if they old one is still here
+      if (oldNode != null) {
+        assert(oldNode != null);
+        assert(oldNode.isFromOldGeneration);
+        assert(oldNode.mounted);
+        oldNode.detachRenderObject();
+        oldNode.remove();
+        assert(!oldNode.mounted);
+        // we don't update the generation of oldNode, because there's
+        // still a chance it could be reused as-is later in the tree.
+      }
       return null;
     }
 
@@ -868,6 +873,12 @@ abstract class Component extends Widget {
     super._sync(old, slot);
   }
 
+  void _markAsFromCurrentGeneration() {
+    if (_dirty)
+      return;
+    super._markAsFromCurrentGeneration();
+  }
+
   void _buildIfDirty() {
     if (!_dirty || !_mounted)
       return;
@@ -947,6 +958,8 @@ abstract class StatefulComponent extends Component {
   // Calls function fn immediately and then schedules another build
   // for this Component.
   void setState(void fn()) {
+    assert(!_debugIsBuilding);
+    assert(_isStateInitialized);
     fn();
     _scheduleBuild();
   }
@@ -985,12 +998,14 @@ void exitLayoutCallbackBuilder(LayoutCallbackBuilderHandle handle) {
 
 List<int> _debugFrameTimes = <int>[];
 
+// TODO(ianh): Move this to Component
 void _absorbDirtyComponents(List<Component> list) {
   list.addAll(_dirtyComponents);
   _dirtyComponents.clear();
   list.sort((Component a, Component b) => a._order - b._order);
 }
 
+// TODO(ianh): Move this to Component
 void _buildDirtyComponents() {
   assert(!_dirtyComponents.isEmpty);
 
@@ -1038,11 +1053,13 @@ void _buildDirtyComponents() {
 
 }
 
+// TODO(ianh): Move this to Widget
 void _endSyncPhase() {
   Widget._currentGeneration += 1;
   Widget._notifyMountStatusChanged();
 }
 
+// TODO(ianh): Move this to Component
 void _scheduleComponentForRender(Component component) {
   _dirtyComponents.add(component);
   if (!_buildScheduled) {

packages/unit/test/widget/navigator_test.dart

@@ -4,6 +4,26 @@ import 'package:test/test.dart';
 
 import 'widget_tester.dart';
 
+class FirstComponent extends Component {
+  FirstComponent(this.navigator);
+
+  final Navigator navigator;
+
+  Widget build() {
+    return new GestureDetector(
+      onTap: () {
+        navigator.pushNamed('/second');
+      },
+      child: new Container(
+        decoration: new BoxDecoration(
+          backgroundColor: new Color(0xFFFFFF00)
+        ),
+        child: new Text('X')
+      )
+    );
+  }
+}
+
 class SecondComponent extends StatefulComponent {
   SecondComponent(this.navigator);
 
@@ -26,28 +46,8 @@ class SecondComponent extends StatefulComponent {
   }
 }
 
-class FirstComponent extends Component {
-  FirstComponent(this.navigator);
-
-  final Navigator navigator;
-
-  Widget build() {
-    return new GestureDetector(
-      onTap: () {
-        navigator.pushNamed('/second');
-      },
-      child: new Container(
-        decoration: new BoxDecoration(
-          backgroundColor: new Color(0xFFFFFF00)
-        ),
-        child: new Text('X')
-      )
-    );
-  }
-}
-
 void main() {
-  test('Can navigator to and from a stateful component', () {
+  test('Can navigator navigate to and from a stateful component', () {
     WidgetTester tester = new WidgetTester();
 
     final NavigationState routes = new NavigationState([
@@ -65,8 +65,15 @@ void main() {
       return new Navigator(routes);
     });
 
+    expect(tester.findText('X'), isNotNull);
+    expect(tester.findText('Y'), isNull);
+
     tester.tap(tester.findText('X'));
     scheduler.beginFrame(10.0);
+
+    expect(tester.findText('X'), isNotNull);
+    expect(tester.findText('Y'), isNotNull);
+
     scheduler.beginFrame(20.0);
     scheduler.beginFrame(30.0);
     scheduler.beginFrame(1000.0);
@@ -77,5 +84,8 @@ void main() {
     scheduler.beginFrame(1030.0);
     scheduler.beginFrame(2000.0);
 
+    expect(tester.findText('X'), isNotNull);
+    expect(tester.findText('Y'), isNull);
+
   });
 }

packages/unit/test/widget/set_state_test.dart

@@ -0,0 +1,75 @@
+import 'package:sky/gestures/arena.dart';
+import 'package:sky/gestures/pointer_router.dart';
+import 'package:sky/gestures/tap.dart';
+import 'package:sky/widgets.dart';
+import 'package:test/test.dart';
+
+import '../engine/mock_events.dart';
+import 'widget_tester.dart';
+
+class Inside extends StatefulComponent {
+  void syncConstructorArguments(Inside source) {
+  }
+
+  Widget build() {
+    return new Listener(
+      onPointerDown: _handlePointerDown,
+      child: new Text('INSIDE')
+    );
+  }
+
+  EventDisposition _handlePointerDown(_) {
+    setState(() { });
+    return EventDisposition.processed;
+  }
+}
+
+class Middle extends StatefulComponent {
+  Inside child;
+
+  Middle({ this.child });
+
+  void syncConstructorArguments(Middle source) {
+    child = source.child;
+  }
+
+  Widget build() {
+    return new Listener(
+      onPointerDown: _handlePointerDown,
+      child: child
+    );
+  }
+
+  EventDisposition _handlePointerDown(_) {
+    setState(() { });
+    return EventDisposition.processed;
+  }
+
+}
+
+class Outside extends App {
+  Widget build() {
+    return new Middle(child: new Inside());
+  }
+}
+
+void main() {
+  test('setState() smoke test', () {
+    WidgetTester tester = new WidgetTester();
+
+    tester.pumpFrame(() {
+      return new Outside();
+    });
+
+    TestPointer pointer = new TestPointer(1);
+    Point location = tester.getCenter(tester.findText('INSIDE'));
+    tester.dispatchEvent(pointer.down(location), location);
+
+    tester.pumpFrameWithoutChange();
+
+    tester.dispatchEvent(pointer.up(), location);
+   
+    tester.pumpFrameWithoutChange();
+
+  });
+}

packages/unit/test/widget/stateful_components_test.dart

@@ -40,27 +40,36 @@ void main() {
 
     WidgetTester tester = new WidgetTester();
 
-    InnerComponent inner;
+    InnerComponent inner1;
+    InnerComponent inner2;
     OuterContainer outer;
 
     tester.pumpFrame(() {
-      return new OuterContainer(child: new InnerComponent());
-    });
-
-    tester.pumpFrame(() {
-      inner = new InnerComponent();
-      outer = new OuterContainer(child: inner);
+      inner1 = new InnerComponent();
+      outer = new OuterContainer(child: inner1);
       return outer;
     });
 
-    expect(inner._didInitState, isFalse);
-    expect(inner.parent, isNull);
+    expect(inner1._didInitState, isTrue);
+    expect(inner1.parent, isNotNull);
+
+    tester.pumpFrame(() {
+      inner2 = new InnerComponent();
+      return new OuterContainer(child: inner2);
+    });
+
+    expect(inner1._didInitState, isTrue);
+    expect(inner1.parent, isNotNull);
+    expect(inner2._didInitState, isFalse);
+    expect(inner2.parent, isNull);
 
     outer.setState(() {});
-    scheduler.beginFrame(0.0);
+    tester.pumpFrameWithoutChange(0.0);
 
-    expect(inner._didInitState, isFalse);
-    expect(inner.parent, isNull);
+    expect(inner1._didInitState, isTrue);
+    expect(inner1.parent, isNotNull);
+    expect(inner2._didInitState, isFalse);
+    expect(inner2.parent, isNull);
 
   });
 }

packages/unit/test/widget/widget_tester.dart

@@ -9,7 +9,6 @@ import '../engine/mock_events.dart';
 typedef Widget WidgetBuilder();
 
 class TestApp extends App {
-  TestApp();
 
   WidgetBuilder _builder;
   void set builder (WidgetBuilder value) {
@@ -29,6 +28,7 @@ class WidgetTester {
   WidgetTester() {
     _app = new TestApp();
     runApp(_app);
+    scheduler.beginFrame(0.0); // to initialise the app
   }
 
   TestApp _app;