Key refactor, part 1: remove the dependence on the runtimeType in keys, otherwise they'll get overl…

sky/sdk/lib/widgets/widget.dart

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 import 'dart:async';
 import 'dart:collection';
 import 'dart:mirrors';
 import 'dart:sky' as sky;
 
 import '../app/view.dart';
 import '../rendering/box.dart';
 import '../rendering/object.dart';
 
 export '../rendering/box.dart' show BoxConstraints, BoxDecoration, Border, BorderSide, EdgeDims;
 export '../rendering/flex.dart' show FlexDirection;
 export '../rendering/object.dart' show Point, Size, Rect, Color, Paint, Path;
 
 final bool _shouldLogRenderDuration = false;
 
 // All Effen nodes derive from Widget. All nodes have a _parent, a _key and
 // can be sync'd.
 abstract class Widget {
 
   Widget({ String key }) {
-    _key = key == null ? "$runtimeType" : "$runtimeType-$key";
+    _key = key != null ? key : runtimeType.toString();
     assert(this is AbstractWidgetRoot || _inRenderDirtyComponents); // you should not build the UI tree ahead of time, build it only during build()
   }
 
   String _key;
   String get key => _key;
 
   Widget _parent;
   Widget get parent => _parent;
 
   bool _mounted = false;
@@ skipping 81 matching lines @@
     }
 
     if (node == null) {
       // the child in this slot has gone away
       assert(oldNode.mounted);
       removeChild(oldNode);
       assert(!oldNode.mounted);
       return null;
     }
 
-    if (oldNode != null && node._key == oldNode._key && node._retainStatefulNodeIfPossible(oldNode)) {
+    if (oldNode != null && oldNode.runtimeType == node.runtimeType && node.key == oldNode.key && node._retainStatefulNodeIfPossible(oldNode)) {
       assert(oldNode.mounted);
       assert(!node.mounted);
       oldNode._sync(node, slot);
       assert(oldNode.root is RenderObject);
       return oldNode;
     }
 
-    if (oldNode != null && node._key != oldNode._key) {
+    if (oldNode != null && (oldNode.runtimeType != node.runtimeType || node.key != oldNode.key)) {
       assert(oldNode.mounted);
       removeChild(oldNode);
       oldNode = null;
     }
 
     assert(!node.mounted);
     node.setParent(this);
     node._sync(oldNode, slot);
     assert(node.root is RenderObject);
     return node;
@@ skipping 154 matching lines @@
     super.remove();
   }
 
   bool _retainStatefulNodeIfPossible(Widget old) {
     assert(!_disqualifiedFromEverAppearingAgain);
 
     Component oldComponent = old as Component;
     if (oldComponent == null || !oldComponent._stateful)
       return false;
 
+    assert(runtimeType == oldComponent.runtimeType);
     assert(key == oldComponent.key);
 
     // Make |this|, the newly-created object, into the "old" Component, and kill it
     _stateful = false;
     _built = oldComponent._built;
     assert(_built != null);
     _disqualifiedFromEverAppearingAgain = true;
 
     // Make |oldComponent| the "new" component
     oldComponent._built = null;
@@ skipping 186 matching lines @@
 
   void remove() {
     assert(root != null);
     _nodeMap.remove(root);
     super.remove();
   }
 }
 
 abstract class OneChildRenderObjectWrapper extends RenderObjectWrapper {
 
-  OneChildRenderObjectWrapper({ Widget child, String key })
+  OneChildRenderObjectWrapper({ String key, Widget child })
     : _child = child, super(key: key);
 
   Widget _child;
   Widget get child => _child;
 
   void syncRenderObject(RenderObjectWrapper old) {
     super.syncRenderObject(old);
     Widget oldChild = old == null ? null : (old as OneChildRenderObjectWrapper).child;
     _child = syncChild(child, oldChild, null);
   }
@@ skipping 61 matching lines @@
     super.remove();
   }
 
   bool _debugHasDuplicateIds() {
     var idSet = new HashSet<String>();
     for (var child in children) {
       assert(child != null);
       if (child.interchangeable)
         continue; // when these nodes are reordered, we just reassign the data
 
-      if (!idSet.add(child._key)) {
-        throw '''If multiple non-interchangeable nodes of the same type exist as children
-                of another node, they must have unique keys.
-                Duplicate: "${child._key}"''';
+      if (!idSet.add(child.key)) {
+        throw '''If multiple non-interchangeable nodes exist as children of another node, they must have unique keys. Duplicate: "${child.key}"''';
       }
     }
     return false;
   }
 
   void syncRenderObject(MultiChildRenderObjectWrapper old) {
     super.syncRenderObject(old);
 
     final root = this.root; // TODO(ianh): Remove this once the analyzer is cleverer
     if (root is! ContainerRenderObjectMixin)
@@ skipping 14 matching lines @@
       children[atIndex] = syncChild(currentNode, oldNode, nextSibling);
       assert(children[atIndex] != null);
     }
 
     // Scan backwards from end of list while nodes can be directly synced
     // without reordering.
     while (endIndex > startIndex && oldEndIndex > oldStartIndex) {
       currentNode = children[endIndex - 1];
       oldNode = oldChildren[oldEndIndex - 1];
 
-      if (currentNode._key != oldNode._key) {
+      if (currentNode.runtimeType != oldNode.runtimeType || currentNode.key != oldNode.key) {
         break;
       }
 
       endIndex--;
       oldEndIndex--;
       sync(endIndex);
     }
 
     HashMap<String, Widget> oldNodeIdMap = null;
 
     bool oldNodeReordered(String key) {
       return oldNodeIdMap != null &&
              oldNodeIdMap.containsKey(key) &&
              oldNodeIdMap[key] == null;
     }
 
     void advanceOldStartIndex() {
       oldStartIndex++;
       while (oldStartIndex < oldEndIndex &&
-             oldNodeReordered(oldChildren[oldStartIndex]._key)) {
+             oldNodeReordered(oldChildren[oldStartIndex].key)) {
         oldStartIndex++;
       }
     }
 
     void ensureOldIdMap() {
       if (oldNodeIdMap != null)
         return;
 
       oldNodeIdMap = new HashMap<String, Widget>();
       for (int i = oldStartIndex; i < oldEndIndex; i++) {
         var node = oldChildren[i];
         if (!node.interchangeable)
-          oldNodeIdMap.putIfAbsent(node._key, () => node);
+          oldNodeIdMap.putIfAbsent(node.key, () => node);
       }
     }
 
     bool searchForOldNode() {
       if (currentNode.interchangeable)
         return false; // never re-order these nodes
 
       ensureOldIdMap();
-      oldNode = oldNodeIdMap[currentNode._key];
+      oldNode = oldNodeIdMap[currentNode.key];
       if (oldNode == null)
         return false;
 
-      oldNodeIdMap[currentNode._key] = null; // mark it reordered
+      oldNodeIdMap[currentNode.key] = null; // mark it reordered
       assert(root is ContainerRenderObjectMixin);
       assert(old.root is ContainerRenderObjectMixin);
       assert(oldNode.root != null);
 
       (old.root as ContainerRenderObjectMixin).remove(oldNode.root); // TODO(ianh): Remove cast once the analyzer is cleverer
       root.add(oldNode.root, before: nextSibling);
 
       return true;
     }
 
     // Scan forwards, this time we may re-order;
     nextSibling = root.firstChild;
     while (startIndex < endIndex && oldStartIndex < oldEndIndex) {
       currentNode = children[startIndex];
       oldNode = oldChildren[oldStartIndex];
 
-      if (currentNode._key == oldNode._key) {
-        assert(currentNode.runtimeType == oldNode.runtimeType);
+      if (currentNode.runtimeType == oldNode.runtimeType && currentNode.key == oldNode.key) {
         nextSibling = root.childAfter(nextSibling);
         sync(startIndex);
         startIndex++;
         advanceOldStartIndex();
         continue;
       }
 
       oldNode = null;
       searchForOldNode();
       sync(startIndex);
@@ skipping 126 matching lines @@
     if (root.parent == null) {
       // we haven't attached it yet
       assert(_container.child == null);
       _container.child = root;
     }
     assert(root.parent == _container);
   }
 
   Widget build() => builder();
 }