From b1e64571e66fc4e122ddd8a1a05ae11df0471d63 Mon Sep 17 00:00:00 2001
From: Hixie <github.com@spam.hixie.ch>
Date: Thu, 17 Sep 2015 13:24:45 -0700
Subject: [PATCH] Fix removal logic in widgets

We were not removing children if they were more recently synced than we
were. This makes no sense. We should remove all children unless they
were synced this very generation already (in which case they'll be
somewhere else in the tree by now).
---
 .../flutter/lib/src/widgets/framework.dart    |  7 ++-
 .../unit/test/widget/duplicate_key_test.dart  | 51 +++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 packages/unit/test/widget/duplicate_key_test.dart

diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart
index 8638bad53b..4f7b2d9361 100644
--- a/packages/flutter/lib/src/widgets/framework.dart
+++ b/packages/flutter/lib/src/widgets/framework.dart
@@ -377,8 +377,13 @@ abstract class Widget {
 
   void remove() {
     walkChildren((Widget child) {
-      if (child._generation <= _generation)
+      if (child.isFromOldGeneration || !isFromOldGeneration) {
+        assert(child.parent == this);
         child.remove();
+      } else {
+        // if it's from the current generation and we're not, it means it's been moved somewhere else in the tree already and isn't really our child anymore
+        assert(child.parent != this);
+      }
     });
     _renderObject = null;
     setParent(null);
diff --git a/packages/unit/test/widget/duplicate_key_test.dart b/packages/unit/test/widget/duplicate_key_test.dart
new file mode 100644
index 0000000000..a3a8e75a01
--- /dev/null
+++ b/packages/unit/test/widget/duplicate_key_test.dart
@@ -0,0 +1,51 @@
+import 'package:sky/src/widgets/framework.dart';
+import 'package:sky/src/widgets/basic.dart';
+import 'package:test/test.dart';
+import 'widget_tester.dart';
+
+class Item {
+  GlobalKey key1 = new GlobalKey();
+  GlobalKey key2 = new GlobalKey();
+  String toString() => "Item($key1, $key2)";
+}
+List<Item> items = [new Item(), new Item()];
+
+class StatefulLeaf extends StatefulComponent {
+  StatefulLeaf({ GlobalKey key }) : super(key: key);
+  void syncConstructorArguments(StatefulLeaf source) { }
+  void test() { setState(() { }); }
+  Widget build() => new Text('leaf');
+}
+
+class KeyedWrapper extends Component {
+  KeyedWrapper(this.key1, this.key2);
+  Key key1, key2;
+  Widget build() {
+    return new Container(
+      key: key1,
+      child: new StatefulLeaf(
+        key: key2
+      )
+    );
+  }
+}
+
+Widget builder() {
+  return new Column([
+    new KeyedWrapper(items[1].key1, items[1].key2),
+    new KeyedWrapper(items[0].key1, items[0].key2)
+  ]);
+}
+
+void main() {
+  test('duplicate key smoke test', () {
+    WidgetTester tester = new WidgetTester();
+    tester.pumpFrame(builder);
+    tester.findWidget((widget) => widget is StatefulLeaf).test();
+    tester.pumpFrameWithoutChange();
+    Item lastItem = items[1];
+    items.remove(lastItem);
+    items.insert(0, lastItem);
+    tester.pumpFrame(builder); // this marks the app dirty and rebuilds it
+  });
+}