Merge pull request #1062 from Hixie/reposition

Handle Widget instances being moved as-is to different parts of the tree.

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

@@ -201,9 +201,15 @@ class MixedViewport extends RenderObjectWrapper {
           Widget widget = builder(index);
           assert(widget != null);
           assert(widget.key != null);
+          assert(widget.isFromOldGeneration);
           _Key key = new _Key.fromWidget(widget);
           Widget oldWidget = _childrenByKey[key];
           assert(oldWidget != null);
+          assert(() {
+            'One of the nodes that was in this MixedViewport was placed in another part of the tree, without the MixedViewport\'s token or builder being changed ' +
+            'and without the MixedViewport\'s MixedViewportLayoutState object being told about that any of the children were invalid.';
+            return oldWidget.isFromOldGeneration;
+          });
           assert(oldWidget.renderObject.parent == renderObject);
           widget = syncChild(widget, oldWidget, renderObject.childAfter(oldWidget.renderObject));
           assert(widget != null);
@@ -225,8 +231,11 @@ class MixedViewport extends RenderObjectWrapper {
     Widget newWidget = builder(index);
     assert(newWidget != null);
     assert(newWidget.key != null);
+    assert(newWidget.isFromOldGeneration);
     final _Key key = new _Key.fromWidget(newWidget);
     Widget oldWidget = _childrenByKey[key];
+    if (oldWidget != null && !oldWidget.isFromOldGeneration)
+      oldWidget = null;
     newWidget = syncChild(newWidget, oldWidget, _omit);
     assert(newWidget != null);
     // Update the offsets based on the newWidget's dimensions.
@@ -254,6 +263,8 @@ class MixedViewport extends RenderObjectWrapper {
     assert(widget.key != null); // items in lists must have keys
     final _Key key = new _Key.fromWidget(widget);
     Widget oldWidget = _childrenByKey[key];
+    if (oldWidget != null && !oldWidget.isFromOldGeneration)
+      oldWidget = null;
     widget = syncChild(widget, oldWidget, _omit);
     if (index >= offsets.length - 1) {
       assert(index == offsets.length - 1);
@@ -286,6 +297,18 @@ class MixedViewport extends RenderObjectWrapper {
     layoutState._notifyListeners();
   }
 
+  void _unsyncChild(Widget widget) {
+    assert(!widget.isFromOldGeneration);
+    // The following two lines are the equivalent of "syncChild(null,
+    // widget, null)", but actually doing that wouldn't work because
+    // widget is now from the new generation and so syncChild() would
+    // assume that that means someone else has already sync()ed with it
+    // and that it's wanted. But it's not wanted! We want to get rid of
+    // it. So we do it manually.
+    widget.detachRenderObject();
+    widget.remove();
+  }
+
   void _doLayout(BoxConstraints constraints) {
     Map<_Key, Widget> newChildren = new Map<_Key, Widget>();
     Map<int, Widget> builtChildren = new Map<int, Widget>();
@@ -334,7 +357,7 @@ class MixedViewport extends RenderObjectWrapper {
           builtChildren[index] = widget;
         } else {
           childrenByKey.remove(widgetKey);
-          syncChild(null, widget, null);
+          _unsyncChild(widget);
         }
       }
     }
@@ -378,7 +401,7 @@ class MixedViewport extends RenderObjectWrapper {
             break;
           }
           childrenByKey.remove(widgetKey);
-          syncChild(null, widget, null);
+          _unsyncChild(widget);
           startIndex += 1;
           assert(startIndex == offsets.length - 1);
         }

packages/unit/test/widget/stack_test.dart

@@ -43,4 +43,29 @@ void main() {
     expect(container.renderObject.parentData.bottom, isNull);
     expect(container.renderObject.parentData.left, isNull);
   });
+
+  test('Can remove parent data', () {
+    WidgetTester tester = new WidgetTester();
+    Container container;
+
+    tester.pumpFrame(() {
+      container = new Container(width: 10.0, height: 10.0);
+      return new Stack([ new Positioned(left: 10.0, child: container) ]);
+    });
+
+    expect(container.renderObject.parentData.top, isNull);
+    expect(container.renderObject.parentData.right, isNull);
+    expect(container.renderObject.parentData.bottom, isNull);
+    expect(container.renderObject.parentData.left, equals(10.0));
+
+    tester.pumpFrame(() {
+      return new Stack([ container ]);
+    });
+
+    expect(container.renderObject.parentData.top, isNull);
+    expect(container.renderObject.parentData.right, isNull);
+    expect(container.renderObject.parentData.bottom, isNull);
+    expect(container.renderObject.parentData.left, isNull);
+  });
+
 }

packages/unit/test/widget/syncing_test.dart

@@ -4,13 +4,15 @@ import 'package:test/test.dart';
 import 'widget_tester.dart';
 
 class TestState extends StatefulComponent {
-  TestState({this.child, this.state});
+  TestState({ this.child, this.persistentState, this.syncedState });
   Widget child;
-  int state;
+  int persistentState;
+  int syncedState;
   int syncs = 0;
   void syncConstructorArguments(TestState source) {
     child = source.child;
-    // we explicitly do NOT sync the state from the new instance
+    syncedState = source.syncedState;
+    // we explicitly do NOT sync the persistentState from the new instance
     // because we're using that to track whether we got recreated
     syncs += 1;
   }
@@ -29,7 +31,7 @@ void main() {
       return new Container(
         child: new Container(
           child: new TestState(
-            state: 1,
+            persistentState: 1,
             child: new Container()
           )
         )
@@ -38,21 +40,21 @@ void main() {
 
     TestState stateWidget = tester.findWidget((widget) => widget is TestState);
 
-    expect(stateWidget.state, equals(1));
+    expect(stateWidget.persistentState, equals(1));
     expect(stateWidget.syncs, equals(0));
 
     tester.pumpFrame(() {
       return new Container(
         child: new Container(
           child: new TestState(
-            state: 2,
+            persistentState: 2,
             child: new Container()
           )
         )
       );
     });
 
-    expect(stateWidget.state, equals(1));
+    expect(stateWidget.persistentState, equals(1));
     expect(stateWidget.syncs, equals(1));
 
   });
@@ -92,4 +94,94 @@ void main() {
   //
   // });
 
+  test('swap instances around', () {
+
+    WidgetTester tester = new WidgetTester();
+
+    Widget a, b;
+    tester.pumpFrame(() {
+      a = new TestState(persistentState: 0x61, syncedState: 0x41, child: new Text('apple'));
+      b = new TestState(persistentState: 0x62, syncedState: 0x42, child: new Text('banana'));
+      return new Column([]);
+    });
+    GlobalKey keyA = new GlobalKey();
+    GlobalKey keyB = new GlobalKey();
+
+    TestState foundA, foundB;
+
+    tester.pumpFrame(() {
+      return new Column([
+        new Container(
+          key: keyA,
+          child: a
+        ),
+        new Container(
+          key: keyB,
+          child: b
+        )
+      ]);
+    });
+
+    foundA = (tester.findWidget((widget) => widget.key == keyA) as Container).child as TestState;
+    foundB = (tester.findWidget((widget) => widget.key == keyB) as Container).child as TestState;
+
+    expect(foundA, equals(a));
+    expect(foundA.persistentState, equals(0x61));
+    expect(foundA.syncedState, equals(0x41));
+    expect(foundB, equals(b));
+    expect(foundB.persistentState, equals(0x62));
+    expect(foundB.syncedState, equals(0x42));
+
+    tester.pumpFrame(() {
+      return new Column([
+        new Container(
+          key: keyA,
+          child: a
+        ),
+        new Container(
+          key: keyB,
+          child: b
+        )
+      ]);
+    });
+
+    foundA = (tester.findWidget((widget) => widget.key == keyA) as Container).child as TestState;
+    foundB = (tester.findWidget((widget) => widget.key == keyB) as Container).child as TestState;
+
+    // same as before
+    expect(foundA, equals(a));
+    expect(foundA.persistentState, equals(0x61));
+    expect(foundA.syncedState, equals(0x41));
+    expect(foundB, equals(b));
+    expect(foundB.persistentState, equals(0x62));
+    expect(foundB.syncedState, equals(0x42));
+
+    // now we swap the nodes over
+    // since they are both "old" nodes, they shouldn't sync with each other even though they look alike
+
+    tester.pumpFrame(() {
+      return new Column([
+        new Container(
+          key: keyA,
+          child: b
+        ),
+        new Container(
+          key: keyB,
+          child: a
+        )
+      ]);
+    });
+
+    foundA = (tester.findWidget((widget) => widget.key == keyA) as Container).child as TestState;
+    foundB = (tester.findWidget((widget) => widget.key == keyB) as Container).child as TestState;
+
+    expect(foundA, equals(b));
+    expect(foundA.persistentState, equals(0x62));
+    expect(foundA.syncedState, equals(0x42));
+    expect(foundB, equals(a));
+    expect(foundB.persistentState, equals(0x61));
+    expect(foundB.syncedState, equals(0x41));
+
+  });
+
 }