work around for https://github.com/Polymer/TemplateBinding/issues/150

pkg/template_binding/lib/src/instance_binding_map.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of template_binding;
 
 class _InstanceBindingMap {
   final List bindings;
-  final List<_InstanceBindingMap> children;
+  final Map<int, _InstanceBindingMap> children;

[Jennifer Messerly, 2013/12/09 23:49:22]

I noticed the JavaScript version uses a Map too. I didn't notice this change
during porting, but it's now `{}` in JavaScript instead of `[]`.

overall I think it's probably an okay change, it makes the Map more sparse,
though we could likely use an even more optimal data structure since typically
these maps are small and always have integer keys...

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

It seemed to me that they still used a [] for the top-level "map", but {} for
the children, but this should work ok here.

[Jennifer Messerly, 2013/12/10 02:15:43]

yup. I like using a class for InstanceBindingMap though :). I find the untyped
JS code that uses heterogeneous arrays very hard to follow.

   final Node templateRef;
 
-  _InstanceBindingMap._(this.bindings, this.children, this.templateRef);
+  // Workaround for:
+  // https://github.com/Polymer/TemplateBinding/issues/150
+  final int numChildren;
 
-  factory _InstanceBindingMap(Node node, BindingDelegate delegate) {
-    var bindings = _getBindings(node, delegate);
-    Node templateRef = null;
-
-    if (isSemanticTemplate(node)) templateRef = node;
-
-    List children = null;
-    for (var c = node.firstChild, i = 0; c != null; c = c.nextNode, i++) {
-      var childMap = new _InstanceBindingMap(c, delegate);
-      if (childMap == null) continue;
-
-      if (children == null) children = new List(node.nodes.length);
-      children[i] = childMap;
-    }
-
-    return new _InstanceBindingMap._(bindings, children, templateRef);
-  }
+  _InstanceBindingMap._(this.bindings, this.children, this.templateRef,
+      this.numChildren);
 }
 
+_InstanceBindingMap _createInstanceBindingMap(Node node,
+    BindingDelegate delegate) {
+
+  var bindings = _getBindings(node, delegate);
+  Node templateRef = null;
+
+  if (isSemanticTemplate(node)) templateRef = node;
+
+  Map children = null;
+  int i = 0;
+  for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
+    var childMap = _createInstanceBindingMap(c, delegate);
+    if (childMap == null) continue;
+
+    if (children == null) children = new HashMap();
+    children[i] = childMap;
+  }
+
+  if (bindings == null && children == null && templateRef == null) return null;

[Jennifer Messerly, 2013/12/09 23:49:22]

we were missing this important optimization.

+
+  return new _InstanceBindingMap._(bindings, children, templateRef, i);
+}
 
 void _addMapBindings(Node node, _InstanceBindingMap map, model,
     BindingDelegate delegate, List bound) {
   if (map == null) return;
 
   if (map.templateRef != null) {
     TemplateBindExtension.decorate(node, map.templateRef);
     if (delegate != null) {
       templateBindFallback(node)._bindingDelegate = delegate;
     }
   }
 
   if (map.bindings != null) {
     _processBindings(map.bindings, node, model, bound);
   }
 
   if (map.children == null) return;
 
-  int i = 0;
-  for (var c = node.firstChild; c != null; c = c.nextNode) {
-    _addMapBindings(c, map.children[i++], model, delegate, bound);
+  // To workaround https://github.com/Polymer/TemplateBinding/issues/150,

[Jennifer Messerly, 2013/12/09 23:49:22]

I'm not sure how robust this workaround is, but it seems safer than what the
code was doing.

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

what would be more robust? a Map<Node, _InstanceBindingMap> with node instead of
int keys? 

[Jennifer Messerly, 2013/12/10 02:15:43]

the problem is, they aren't the same nodes. We're gather data from the
<template> and trying to apply it the instantiation.

Perhaps if the bindings were applied while the tree is being cloned, that would
be more robust. Then it wouldn't need to rely on child offset.

+  // we try and detect cases where creating a custom element resulted in extra
+  // children compared to what we expected. We assume these new children are all
+  // at the beginning, because _deepCloneIgnoreTemplateContent creates the
+  // element then appends the template content's children to the end.
+
+  int i = map.numChildren - node.nodes.length;
+  for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
+    if (i < 0) continue;
+    _addMapBindings(c, map.children[i], model, delegate, bound);

[Siggi Cherem (dart-lang), 2013/12/10 00:42:23]

I'm not sure I follow this yet. Doesn't this 'i' need to be adjusted by some
offset here so that we match the original offset used by
createInstanceBindingMap?

For example,

int offset = map.numChildren - node.nodes.length;
int i = 0;
for (var c = node.firstChild; c != null; c = c.nextNode, i++) {
  if (i < offset) continue;
  _addMapBindings(c, map.children[i - offset], model, delegate, bound);

[Jennifer Messerly, 2013/12/10 02:15:43]

we're adjusting because it starts negative. Let's say map.numChildren is 3 (the
template had 3 children):
<template>
  <node><A><B><C>

 and node.nodes.length is 5 (the node's created must've added 2 children to
itself):

  <node><X><Y><A><B><C>

so we compute:

int i =  map.numChildren - node.nodes.length;
// so: i = 3 - 5; i = -2;

for (var c = node.firstChild; c != null; c = c.nextNode, i++) {

  // first iteration: i = -2, c = <X>
  if (i < 0) continue; // -2 < 0 so continue to next iteration

  // second iteration: i = -1, c = <Y>
  if (i < 0) continue; // -1 < 0 so continue to next iteration

  // third iteration: i = 0, c = <A>
  if (i < 0) continue; // 0 is not less than 0 so fall through

  _addMapBindings(<A>, map.children[0], model, delegate, bound);
  // ... etc ...

If it is more clear I could use two variables, I'm just folding "i" and "offset"
together. Your example code would work though, if the first line is changed to
be `int offset = node.nodes.length - map.numChildren;`

   }
 }