From 2c379a393ee73a80eda4ad3b38f6ec6a064c89c9 Mon Sep 17 00:00:00 2001
From: Rafael Weinstein <rafaelw@chromium.org>
Date: Thu, 26 Dec 2013 15:55:00 -0800
Subject: [PATCH] Produce sub-templates synchronously.

This patch make handling of bindings for template bind/if/repeat special, in that they are processed privately as a group (and are not available as public binding names).

Also, the processing order for binding recursive is now post-order, this means that all children elements will be bound and templates produced before a given node binds.

R=arv
BUG=

Review URL: https://codereview.appspot.com/45890043
---
 benchmark/benchmark.js |   2 +-
 src/TemplateBinding.js | 375 ++++++++++++++++++-----------------------
 tests/tests.js         |  99 +----------
 3 files changed, 175 insertions(+), 301 deletions(-)

diff --git a/benchmark/benchmark.js b/benchmark/benchmark.js
index f9e2d30..c08705a 100644
--- a/benchmark/benchmark.js
+++ b/benchmark/benchmark.js
@@ -225,7 +225,7 @@
     },
 
     teardownMDVVariant: function() {
-      this.template.bindings.repeat.close();
+      this.template.bindings.iterator.close();
       this.template = undefined;
     },
 
diff --git a/src/TemplateBinding.js b/src/TemplateBinding.js
index 9a42a13..78bfa6a 100644
--- a/src/TemplateBinding.js
+++ b/src/TemplateBinding.js
@@ -23,13 +23,22 @@
   var forEach = Array.prototype.forEach.call.bind(Array.prototype.forEach);
 
   function getTreeScope(node) {
-    while (node.parentNode) {
-      node = node.parentNode;
+    while (node.parentNode || node.templateCreator_) {
+      if (!node.parentNode && node.templateCreator_)
+        node = node.templateCreator_;
+      else
+        node = node.parentNode;
     }
 
     return typeof node.getElementById === 'function' ? node : null;
   }
 
+  function getInstanceRoot(node) {
+    while (node.parentNode)
+      node = node.parentNode;
+    return node.templateCreator_ ? node : null;
+  }
+
   var Map;
   if (global.Map && typeof global.Map.prototype.forEach === 'function') {
     Map = global.Map;
@@ -441,7 +450,9 @@
   function ensureSetModelScheduled(template) {
     if (!template.setModelFn_) {
       template.setModelFn_ = function() {
-        addBindings(template, template.model, template.prepareBindingFn_);
+        processBindings(template,
+                        getBindings(template, template.prepareBindingFn_),
+                        template.model_);
       };
     }
 
@@ -449,101 +460,34 @@
   }
 
   mixin(HTMLTemplateElement.prototype, {
-    bind: function(name, value, oneTime) {
-      if (!this.iterator_)
-        this.iterator_ = new TemplateIterator(this);
-
-      this.bindings = this.bindings || {};
-      if (name === 'bind') {
-        this.unbind(name);
-        this.iterator_.hasBind = true;
-        if (!oneTime) {
-          value.open(this.iterator_.depsChanged, this.iterator_);
-          this.iterator_.bindObserver = value;
-        } else {
-          this.iterator_.bindValue = value;
+    processBindingDirectives_: function(directives) {
+      if (this.iterator_) {
+        if (this.iterator_.deps) {
+          if (this.iterator_.deps.ifOneTime === false)
+            this.iterator_.deps.ifValue.close();
+          if (this.iterator_.deps.oneTime === false)
+            this.iterator_.deps.value.close();
         }
-
-        this.iterator_.depsChanged();
-        return this.bindings.bind = this.iterator_;
       }
 
-      if (name === 'repeat') {
-        this.unbind(name);
-        this.iterator_.hasRepeat = true;
-        if (!oneTime) {
-          value.open(this.iterator_.depsChanged, this.iterator_);
-          this.iterator_.repeatObserver = value;
-        } else {
-          this.iterator_.repeatValue = value;
+      if (!directives.if && !directives.bind &&!directives.repeat) {
+        if (this.iterator_) {
+          this.iterator_.close();
+          this.iterator_ = undefined;
+          this.bindings.iterator = undefined;
         }
 
-        this.iterator_.depsChanged();
-        return this.bindings.repeat = this.iterator_;
-      }
-
-      if (name === 'if') {
-        this.unbind(name);
-        this.iterator_.hasIf = true;
-        if (!oneTime) {
-          value.open(this.iterator_.depsChanged, this.iterator_);
-          this.iterator_.ifObserver = value;
-        } else {
-          this.iterator_.ifValue = value;
-        }
-
-        this.iterator_.depsChanged();
-        return this.bindings.if = this.iterator_;
-      }
-
-      return HTMLElement.prototype.bind.call(this, name, value, oneTime);
-    },
-
-    unbind: function(name) {
-      if (name === 'bind') {
-        if (!this.iterator_)
-          return;
-
-        this.iterator_.hasBind = false;
-        this.iterator_.bindValue = undefined;
-        if (this.iterator_.bindObserver)
-          this.iterator_.bindObserver.close();
-        this.iterator_.bindObserver = undefined;
-
-        this.iterator_.depsChanged();
-        return this.bindings.bind = undefined;
-      }
-
-      if (name === 'repeat') {
-        if (!this.iterator_)
-          return;
-
-        this.iterator_.hasRepeat = false;
-        this.iterator_.repeatValue = undefined;
-        if (this.iterator_.repeatObserver)
-          this.iterator_.repeatObserver.close();
-        this.iterator_.repeatObserver = undefined;
-
-        this.iterator_.depsChanged();
-        return this.bindings.repeat = undefined;
+        return;
       }
 
-      if (name === 'if') {
-        if (!this.iterator_)
-          return;
-
-        this.iterator_.hasIf = false;
-        this.iterator_.ifValue = undefined;
-
-        if (this.iterator_.ifObserver)
-          this.iterator_.ifObserver.close();
-        this.iterator_.ifObserver = undefined;
-
-        this.iterator_.depsChanged();
-        return this.bindings.if = undefined;
+      if (!this.iterator_) {
+        this.iterator_ = new TemplateIterator(this);
+        this.bindings = this.bindings || {};
+        this.bindings.iterator = this.iterator_;
       }
 
-      return HTMLElement.prototype.unbind.call(this, name);
+      this.iterator_.updateDependencies(directives, this.model_);
+      return this.iterator_;
     },
 
     createInstance: function(model, instanceBindings) {
@@ -557,15 +501,29 @@
       }
 
       var stagingDocument = getTemplateStagingDocument(this);
-      var instance = createAndBindInstance(content, null, stagingDocument, map,
-                                           model,
-                                           this.bindingDelegate_,
-                                           instanceBindings);
-
-      // TODO(rafaelw): We can do this more lazily, but setting a sentinel
-      // in the parent of the template element, and creating it when it's
-      // asked for by walking back to find the iterating template.
-      addTemplateInstanceRecord(instance, model);
+      var instance = stagingDocument.createDocumentFragment();
+      instance.templateCreator_ = this;
+
+      var instanceRecord = {
+        firstNode: null,
+        lastNode: null,
+        model: model
+      };
+
+      var i = 0;
+      for (var child = content.firstChild; child; child = child.nextSibling) {
+        var clone = cloneAndBindingInstance(child, instance, stagingDocument,
+                                            map.children[i++],
+                                            model,
+                                            this.bindingDelegate_,
+                                            instanceBindings);
+        clone.templateInstance_ = instanceRecord;
+      }
+
+      instanceRecord.firstNode = instance.firstChild;
+      // TODO(rafaelw): This could be wrong if the last node is <template>
+      // which has produced instances
+      instanceRecord.lastNode = instance.lastChild;
       return instance;
     },
 
@@ -613,6 +571,10 @@
         var treeScope = getTreeScope(this);
         if (treeScope)
           ref = treeScope.getElementById(refId);
+        if (!ref) {
+          var instanceRoot = getInstanceRoot(this);
+          ref = instanceRoot.querySelector('#' + refId);
+        }
       }
 
       if (!ref)
@@ -725,6 +687,12 @@
   }
 
   function processBinding(name, tokens, node, model) {
+    if (tokens.onlyOneTime)
+      return processOneTimeBinding(name, tokens, node, model);
+
+    if (tokens.hasOnePath)
+      return processSinglePathBinding(name, tokens, node, model);
+
     var observer = new CompoundObserver();
 
     for (var i = 1; i < tokens.length; i += 4) {
@@ -754,19 +722,24 @@
     for (var i = 0; i < bindings.length; i += 2) {
       var name = bindings[i]
       var tokens = bindings[i + 1];
-      var value;
-
-      if (tokens.onlyOneTime)
-        value = processOneTimeBinding(name, tokens, node, model);
-      else if (tokens.hasOnePath)
-        value = processSinglePathBinding(name, tokens, node, model);
-      else
-        value = processBinding(name, tokens, node, model);
-
+      var value = processBinding(name, tokens, node, model);
       var binding = node.bind(name, value, tokens.onlyOneTime);
       if (binding && instanceBindings)
         instanceBindings.push(binding);
     }
+
+    if (!bindings.isTemplate)
+      return;
+
+    node.model_ = model;
+    var iter = node.processBindingDirectives_(bindings);
+    if (instanceBindings && iter)
+      instanceBindings.push(iter);
+  }
+
+  function parseWithDefault(el, name, prepareBindingFn) {
+    var v = el.getAttribute(name);
+    return parseMustaches(v == '' ? '{{}}' : v, name, el, prepareBindingFn);
   }
 
   function parseAttributeBindings(element, prepareBindingFn) {
@@ -790,14 +763,9 @@
         name = name.substring(1);
       }
 
-      if (isTemplate(element)) {
-        if (name === IF) {
-          ifFound = true;
-          value = value || '{{}}';  // Accept 'naked' if.
-        } else if (name === BIND || name === REPEAT) {
-          bindFound = true;
-          value = value || '{{}}';  // Accept 'naked' bind & repeat.
-        }
+      if (isTemplate(element) &&
+          (name === IF || name === BIND || name === REPEAT)) {
+        continue;
       }
 
       var tokens = parseMustaches(value, name, element,
@@ -808,10 +776,14 @@
       bindings.push(name, tokens);
     }
 
-    // Treat <template if> as <template bind if>
-    if (ifFound && !bindFound) {
-      bindings.push(BIND, parseMustaches('{{}}', BIND, element,
-                                         prepareBindingFn));
+    if (isTemplate(element)) {
+      bindings.isTemplate = true;
+      bindings.if = parseWithDefault(element, IF, prepareBindingFn);
+      bindings.bind = parseWithDefault(element, BIND, prepareBindingFn);
+      bindings.repeat = parseWithDefault(element, REPEAT, prepareBindingFn);
+
+      if (bindings.if && !bindings.bind && !bindings.repeat)
+        bindings.bind = parseMustaches('{{}}', BIND, element, prepareBindingFn);
     }
 
     return bindings;
@@ -831,42 +803,30 @@
     return [];
   }
 
-  function createAndBindInstance(node, parent, stagingDocument, bindings, model,
+  function cloneAndBindingInstance(node, parent, stagingDocument, bindings, model,
                                  delegate,
-                                 instanceBindings) {
-    var clone = stagingDocument.importNode(node, false);
-    if (parent)
-      parent.appendChild(clone);
+                                 instanceBindings, instanceRecord) {
+    var clone = parent.appendChild(stagingDocument.importNode(node, false));
+
+    var i = 0;
+    for (var child = node.firstChild; child; child = child.nextSibling) {
+      cloneAndBindingInstance(child, clone, stagingDocument,
+                            bindings.children[i++],
+                            model,
+                            delegate,
+                            instanceBindings);
+    }
 
     if (bindings.isTemplate) {
       HTMLTemplateElement.decorate(clone, node);
       if (delegate)
         clone.setBindingDelegate_(delegate);
-    } else {
-      var i = 0;
-      for (var child = node.firstChild; child; child = child.nextSibling) {
-        createAndBindInstance(child, clone, stagingDocument,
-                              bindings.children[i++],
-                              model,
-                              delegate,
-                              instanceBindings);
-      }
     }
 
     processBindings(clone, bindings, model, instanceBindings);
     return clone;
   }
 
-  function addBindings(node, model, prepareBindingFn) {
-    assert(node);
-
-    var bindings = getBindings(node, prepareBindingFn);
-    processBindings(node, bindings, model);
-
-    for (var child = node.firstChild; child ; child = child.nextSibling)
-      addBindings(child, model, prepareBindingFn);
-  }
-
   function createInstanceBindingMap(node, prepareBindingFn) {
     var map = getBindings(node, prepareBindingFn);
     map.children = {};
@@ -875,34 +835,9 @@
       map.children[index++] = createInstanceBindingMap(child, prepareBindingFn);
     }
 
-    if (isTemplate(node))
-      map.isTemplate = true;
-
     return map;
   }
 
-  function TemplateInstance(firstNode, lastNode, model) {
-    // TODO(rafaelw): firstNode & lastNode should be read-synchronous
-    // in cases where script has modified the template instance boundary.
-    // All should be read-only.
-    this.firstNode = firstNode;
-    this.lastNode = lastNode;
-    this.model = model;
-  }
-
-  function addTemplateInstanceRecord(fragment, model) {
-    if (!fragment.firstChild)
-      return;
-
-    var instanceRecord = new TemplateInstance(fragment.firstChild,
-                                              fragment.lastChild, model);
-    var node = instanceRecord.firstNode;
-    while (node) {
-      node.templateInstance_ = instanceRecord;
-      node = node.nextSibling;
-    }
-  }
-
   Object.defineProperty(Node.prototype, 'templateInstance', {
     get: function() {
       var instance = this.templateInstance_;
@@ -919,51 +854,73 @@
     //   <instanceTerminatorNode, [bindingsSetupByInstance]>
     this.terminators = [];
 
+    this.deps = undefined;
     this.iteratedValue = [];
     this.presentValue = undefined;
     this.arrayObserver = undefined;
+  }
 
-    this.hasRepeat = false;
-    this.repeatValue = undefined;
-    this.repeatObserver = undefined;
+  TemplateIterator.prototype = {
+    updateDependencies: function(directives, model) {
+      var deps = this.deps;
+      if (this.deps) {
+        if (this.deps.ifOneTime === false)
+          this.deps.ifValue.close();
+        if (this.deps.oneTime === false)
+          this.deps.value.close();
+      }
 
-    this.hasBind = false;
-    this.bindValue = undefined;
-    this.bindObserver = undefined;
+      var deps = this.deps = {};
+      var template = this.templateElement_;
 
-    this.hasIf = false;
-    this.ifValue = undefined;
-    this.ifObserver = undefined;
-  }
+      if (directives.if) {
+        deps.hasIf = true;
+        deps.ifOneTime = directives.if.onlyOneTime;
+        deps.ifValue = processBinding(IF, directives.if, template, model);
 
-  TemplateIterator.prototype = {
-    depsChanged: function() {
-      ensureScheduled(this);
-    },
+        // oneTime if & predicate is false. nothing else to do.
+        if (deps.ifOneTime && !deps.ifValue)
+          this.updateIteratedValue();
 
-    // Called as a result ensureScheduled (above).
-    resolve: function() {
-      if (this.hasIf) {
-        var ifValue = this.ifObserver ? this.ifObserver.discardChanges()
-                                      : this.ifValue;
+        if (!deps.ifOneTime)
+          deps.ifValue.open(this.updateIteratedValue, this);
+      }
 
-        if (!ifValue)
-          return this.valueChanged(); // remove any instances
+      if (directives.repeat) {
+        deps.repeat = true;
+        deps.oneTime = directives.repeat.onlyOneTime;
+        deps.value = processBinding(REPEAT, directives.repeat, template, model);
+      } else {
+        deps.repeat = false;
+        deps.oneTime = directives.bind.onlyOneTime;
+        deps.value = processBinding(BIND, directives.bind, template, model);
       }
 
-      var iterateValue;
-      if (this.hasRepeat) {
-        iterateValue = this.repeatObserver ?
-            this.repeatObserver.discardChanges() : this.repeatValue;
-      } else if (this.hasBind) {
-        iterateValue = this.bindObserver ?
-            [this.bindObserver.discardChanges()] : [this.bindValue];
+      if (!deps.oneTime)
+        deps.value.open(this.updateIteratedValue, this);
+
+      this.updateIteratedValue();
+    },
+
+    // Called as a result ensureScheduled (above).
+    updateIteratedValue: function() {
+      if (this.deps.hasIf) {
+        var ifValue = this.deps.ifValue;
+        if (!this.deps.ifOneTime)
+          ifValue = ifValue.discardChanges();
+        if (!ifValue)
+          return this.valueChanged();
       }
 
-      // Only need to observe iterated array if there's a repeatObserver
-      // whose value is actually an Array.
-      this.valueChanged(iterateValue,
-                        this.repeatObserver && Array.isArray(iterateValue));
+      var value = this.deps.value;
+      if (!this.deps.oneTime)
+        value = value.discardChanges();
+      if (!this.deps.repeat)
+        value = [value];
+      var observe = this.deps.repeat &&
+                    !this.deps.oneTime &&
+                    Array.isArray(value);
+      this.valueChanged(value, observe);
     },
 
     valueChanged: function(value, observeValue) {
@@ -1054,7 +1011,7 @@
 
       var template = this.templateElement_;
 
-      if (!template.parentNode || !template.ownerDocument.defaultView) {
+      if (!template.parentNode) {
         this.close();
         return;
       }
@@ -1187,15 +1144,13 @@
       }
 
       this.terminators.length = 0;
-      if (this.bindObserver)
-        this.bindObserver.close();
-      this.bindObserver = undefined;
-      if (this.repeatObserver)
-        this.repeatObserver.close();
-      this.repeatObserver = undefined;
-      if (this.ifObserver)
-        this.ifObserver.close();
-      this.ifObserver = undefined;
+
+      if (this.deps) {
+        if (this.deps.ifOneTime === false)
+          this.deps.ifValue.close();
+        if (this.deps.oneTime === false)
+          this.deps.value.close();
+      }
 
       this.templateElement_.iterator_ = undefined;
       this.closed = true;
diff --git a/tests/tests.js b/tests/tests.js
index 4819fbe..f9ae679 100644
--- a/tests/tests.js
+++ b/tests/tests.js
@@ -30,7 +30,7 @@ function doTeardown() {
   document.body.removeChild(testDiv);
   unbindAll(testDiv);
   Platform.performMicrotaskCheckpoint();
-  assert.strictEqual(4, Observer._allObserversCount);
+  assert.strictEqual(2, Observer._allObserversCount);
 }
 
 function createTestHtml(s) {
@@ -133,8 +133,9 @@ suite('Template Instantiation', function() {
     doc.adoptNode(div);
     recursivelySetTemplateModel(template, {});
     Platform.performMicrotaskCheckpoint();
-    assert.strictEqual(1, div.childNodes.length);
+    assert.strictEqual(2, div.childNodes.length);
     assert.isFalse(!!Observer._errorThrownDuringCallback);
+    unbindAll(div);
   });
 
   test('Empty Bind', function() {
@@ -1111,20 +1112,6 @@ suite('Template Instantiation', function() {
     assert.strictEqual('Hi Leela', div.childNodes[1].textContent);
   });
 
-  test('BindImperative', function() {
-    var div = createTestHtml(
-        '<template>' +
-          'Hi {{ name }}' +
-        '</template>');
-    var t = div.firstChild;
-
-    var model = {name: 'Leela'};
-    t.bind('bind', new PathObserver(model, ''));
-
-    Platform.performMicrotaskCheckpoint();
-    assert.strictEqual('Hi Leela', div.childNodes[1].textContent);
-  });
-
   test('BindPlaceHolderHasNewLine', function() {
     var div = createTestHtml('<template bind="{{}}">Hi {{\nname\n}}</template>');
     var model = {name: 'Leela'};
@@ -1171,31 +1158,6 @@ suite('Template Instantiation', function() {
     assert.strictEqual('Hi Fry', t2.nextSibling.textContent);
   });
 
-  test('BindChanged', function() {
-    var model = {
-      XX: {name: 'Leela', title: 'Captain'},
-      XY: {name: 'Fry', title: 'Delivery boy'},
-      XZ: {name: 'Zoidberg', title: 'Doctor'}
-    };
-
-    var div = createTestHtml(
-        '<template bind="{{ XX }}">Hi {{ name }}</template>');
-
-    recursivelySetTemplateModel(div, model);
-
-    var t = div.firstChild;
-    Platform.performMicrotaskCheckpoint();
-
-    assert.strictEqual(2, div.childNodes.length);
-    assert.strictEqual('Hi Leela', t.nextSibling.textContent);
-
-    t.bind('bind', new PathObserver(model, 'XZ'));
-    Platform.performMicrotaskCheckpoint();
-
-    assert.strictEqual(2, div.childNodes.length);
-    assert.strictEqual('Hi Zoidberg', t.nextSibling.textContent);
-  });
-
   function assertNodesAre() {
     var expectedLength = arguments.length;
     assert.strictEqual(expectedLength + 1, div.childNodes.length);
@@ -1378,14 +1340,13 @@ suite('Template Instantiation', function() {
 
   test('Checked', function() {
     var div = createTestHtml(
-        '<template>' +
+        '<template bind>' +
           '<input type="checkbox" checked="{{a}}">' +
         '</template>');
     var t = div.firstChild;
-    var m = {
+    t.model = {
       a: true
     };
-    t.bind('bind', new PathObserver(m, ''));
     Platform.performMicrotaskCheckpoint();
 
     var instanceInput = t.nextSibling;
@@ -2007,46 +1968,6 @@ suite('Template Instantiation', function() {
     assert.strictEqual(0, records.length);
   });
 
-
-  test('ChangeFromBindToRepeat', function() {
-    var div = createTestHtml(
-        '<template bind="{{a}}">' +
-          '{{ length }}' +
-        '</template>');
-    var template = div.firstChild;
-
-    var m = {
-      a: [
-        {length: 0},
-        {
-          length: 1,
-          b: {length: 4}
-        },
-        {length: 2}
-      ]
-    };
-    recursivelySetTemplateModel(div, m);
-    Platform.performMicrotaskCheckpoint();
-
-    assert.strictEqual(2, div.childNodes.length);
-    assert.strictEqual('3', div.childNodes[1].textContent);
-
-    template.unbind('bind');
-    template.bind('repeat', new PathObserver(m, 'a'));
-    Platform.performMicrotaskCheckpoint();
-    assert.strictEqual(4, div.childNodes.length);
-    assert.strictEqual('0', div.childNodes[1].textContent);
-    assert.strictEqual('1', div.childNodes[2].textContent);
-    assert.strictEqual('2', div.childNodes[3].textContent);
-
-    template.unbind('repeat');
-    template.bind('bind', new PathObserver(m, 'a.1.b'));
-
-    Platform.performMicrotaskCheckpoint();
-    assert.strictEqual(2, div.childNodes.length);
-    assert.strictEqual('4', div.childNodes[1].textContent);
-  });
-
   test('ChangeRefId', function() {
     var div = createTestHtml(
         '<template id="a">a:{{ }}</template>' +
@@ -2212,7 +2133,7 @@ suite('Template Instantiation', function() {
     };
 
     var div = createTestHtml(
-      '<template bind="{{a}}">' +
+      '<template>' +
         '<template bind="{{b}}">' +
           '{{ foo }}:{{ replaceme }}' +
         '</template>' +
@@ -2224,14 +2145,12 @@ suite('Template Instantiation', function() {
       }
     };
 
-    var host = testDiv.appendChild(document.createElement('div'));
     outer.bindingDelegate = delegate;
     var instance = outer.createInstance(model);
     assert.strictEqual(instance.firstChild.ref, outer.content.firstChild);
-
-    host.appendChild(instance);
-    Platform.performMicrotaskCheckpoint();
-    assert.strictEqual('bar:replaced', host.firstChild.nextSibling.textContent);
+    assert.strictEqual('bar:replaced',
+                       instance.firstChild.nextSibling.textContent);
+    unbindAll(instance);
   });
 
   test('Bootstrap', function() {