From 6cebeace91bcbede2fe3559389c453f5fd5188ee Mon Sep 17 00:00:00 2001
From: Kevin Schaaf <kschaaf@google.com>
Date: Thu, 2 Mar 2017 02:28:46 -0800
Subject: [PATCH 1/3] Adds `restamp` mode to dom-repeat.

---
 lib/elements/dom-repeat.html | 196 +++++++++++++++++++++++------------
 test/runner.html             |   1 +
 test/unit/dom-repeat.html    |  12 ++-
 3 files changed, 138 insertions(+), 71 deletions(-)

diff --git a/lib/elements/dom-repeat.html b/lib/elements/dom-repeat.html
index 5efa30a43d..694a29beb9 100644
--- a/lib/elements/dom-repeat.html
+++ b/lib/elements/dom-repeat.html
@@ -262,6 +262,32 @@
         _targetFrameTime: {
           type: Number,
           computed: '__computeFrameTime(targetFramerate)'
+        },
+
+        /**
+         * When `restamp` is true, template instances are never reused with
+         * different items.  Instances mapping to current items are moved
+         * to their new location, new instances are created for any new
+         * items, and instances for removed items are discarded.  This mode is
+         * generally more expensive than `restamp: false` as it results in
+         * more instances to be created and discarded during updates and
+         * disconnection/reconnection churn.  By default, object identity is
+         * used for mapping instances to items.  Set `restampKey` to provide
+         * key based identity.
+         */
+        restamp: {
+          type: Boolean
+        },
+
+        /**
+         * When `restamp: true` is used, `restampKey` can be used to provide
+         * a path into the item object that serves as a unique key for the
+         * item, for purposes of mapping instances to items.  Instances for
+         * items with the same key will be reused, while all others will be
+         * either restamped or discarded.
+         */
+        restampKey: {
+          type: String
         }
 
       }
@@ -276,7 +302,6 @@
       super();
       this.__instances = [];
       this.__limit = Infinity;
-      this.__pool = [];
       this.__renderDebouncer = null;
       this.__itemsIdxToInstIdx = {};
       this.__chunkCount = null;
@@ -291,8 +316,9 @@
     disconnectedCallback() {
       super.disconnectedCallback();
       this.__isDetached = true;
-      for (let i=0; i<this.__instances.length; i++) {
-        this.__detachInstance(i);
+      let instances = this.__instances;
+      for (let i=0; i<instances.length; i++) {
+        this.__detachInstance(instances[i]);
       }
     }
 
@@ -301,9 +327,9 @@
       // only perform attachment if the element was previously detached.
       if (this.__isDetached) {
         this.__isDetached = false;
-        let parent = this.parentNode;
-        for (let i=0; i<this.__instances.length; i++) {
-          this.__attachInstance(i, parent);
+        let instances = this.__instances;
+        for (let i=0; i<instances.length; i++) {
+          this.__attachInstance(instances[i]);
         }
       }
     }
@@ -487,13 +513,34 @@
         // No template found yet
         return;
       }
-      this.__applyFullRefresh();
-      // Reset the pool
-      // TODO(kschaaf): Reuse pool across turns and nested templates
-      // Now that objects/arrays are re-evaluated when set, we can safely
-      // reuse pooled instances across turns, however we still need to decide
-      // semantics regarding how long to hold, how many to hold, etc.
-      this.__pool.length = 0;
+      const items = this.items || [];
+      let inst2items = new Array(items.length);
+      for (let i=0; i<items.length; i++) {
+        inst2items[i] = i;
+      }
+      // Apply user filter
+      if (this.__filterFn) {
+        inst2items = inst2items.filter((i, idx) =>
+          this.__filterFn(items[i], idx, items));
+      }
+      // Apply user sort
+      if (this.__sortFn) {
+        inst2items.sort((a, b) => this.__sortFn(items[a], items[b]));
+      }
+      // items->inst map kept for item path forwarding
+      const items2inst = this.__itemsIdxToInstIdx = {};
+      const instances = this.__instances;
+      const limit = Math.max(Math.min(inst2items.length, this.__limit), 0);
+      // Generate instances and assign items
+      if (this.restamp) {
+        this.__renderRestamp(items, instances, limit, inst2items, items2inst);
+      } else {
+        this.__renderRefresh(items, instances, limit, inst2items, items2inst);
+      }
+      // Remove any extra instances from previous state
+      while (this.__instances.length > limit) {
+        this.__detachAndRemoveInstanceAt(limit);
+      }
       // Set rendered item count
       this._setRenderedItemCount(this.__instances.length);
       // Notify users
@@ -505,65 +552,90 @@
       this.__tryRenderChunk();
     }
 
-    __applyFullRefresh() {
-      const items = this.items || [];
-      let isntIdxToItemsIdx = new Array(items.length);
-      for (let i=0; i<items.length; i++) {
-        isntIdxToItemsIdx[i] = i;
-      }
-      // Apply user filter
-      if (this.__filterFn) {
-        isntIdxToItemsIdx = isntIdxToItemsIdx.filter((i, idx, array) =>
-          this.__filterFn(items[i], idx, array));
-      }
-      // Apply user sort
-      if (this.__sortFn) {
-        isntIdxToItemsIdx.sort((a, b) => this.__sortFn(items[a], items[b]));
+    __renderRestamp(items, instances, limit, inst2items, items2inst) {
+      let keyPath = this.restampKey;
+      const instCache = new Map();
+      nextItem: for (let i=0; i<limit; i++) {
+        let itemIdx = inst2items[i];
+        let item = items[itemIdx];
+        let key = keyPath && Polymer.Path.get(item, keyPath) || item;
+        let inst, instItem;
+        while ((inst = instances[i])) {
+          instItem = inst[this.as];
+          let instKey = keyPath && Polymer.Path.get(instItem, keyPath) || instItem;
+          if (key === instKey) {
+            inst._setPendingProperty(this.as, item);
+            inst._setPendingProperty(this.indexAs, i);
+            inst._setPendingProperty(this.itemsIndexAs, itemIdx);
+            inst._flushProperties();
+            items2inst[itemIdx] = i;
+            continue nextItem;
+          }
+          let cache = instCache.get(instKey);
+          if (cache) {
+            cache.push(inst);
+          } else {
+            instCache.set(instKey, [inst]);
+          }
+          instances.splice(i, 1);
+        }
+        let cache = instCache.get(key);
+        if (cache && (inst = cache.shift())) {
+          this.__reuseInstance(inst, item, i, itemIdx);
+          if (!cache.length) {
+            instCache.delete(key);
+          }
+        } else {
+          this.__insertInstance(item, i, itemIdx);
+        }
+        items2inst[itemIdx] = i;
       }
-      // items->inst map kept for item path forwarding
-      const itemsIdxToInstIdx = this.__itemsIdxToInstIdx = {};
-      let instIdx = 0;
-      // Generate instances and assign items
-      const limit = Math.min(isntIdxToItemsIdx.length, this.__limit);
-      for (; instIdx<limit; instIdx++) {
-        let inst = this.__instances[instIdx];
-        let itemIdx = isntIdxToItemsIdx[instIdx];
+      instCache.forEach(cache => cache.forEach(inst => this.__detachInstance(inst)));
+    }
+
+    __renderRefresh(items, instances, limit, inst2items, items2inst) {
+      for (let i=0; i<limit; i++) {
+        let inst = instances[i];
+        let itemIdx = inst2items[i];
         let item = items[itemIdx];
-        itemsIdxToInstIdx[itemIdx] = instIdx;
-        if (inst && instIdx < this.__limit) {
+        if (inst) {
           inst._setPendingProperty(this.as, item);
-          inst._setPendingProperty(this.indexAs, instIdx);
+          inst._setPendingProperty(this.indexAs, i);
           inst._setPendingProperty(this.itemsIndexAs, itemIdx);
           inst._flushProperties();
         } else {
-          this.__insertInstance(item, instIdx, itemIdx);
+          this.__insertInstance(item, i, itemIdx);
         }
-      }
-      // Remove any extra instances from previous state
-      for (let i=this.__instances.length-1; i>=instIdx; i--) {
-        this.__detachAndRemoveInstance(i);
+        items2inst[itemIdx] = i;
       }
     }
 
-    __detachInstance(idx) {
-      let inst = this.__instances[idx];
+    __detachInstance(inst) {
       for (let i=0; i<inst.children.length; i++) {
         let el = inst.children[i];
         inst.root.appendChild(el);
       }
-      return inst;
     }
 
-    __attachInstance(idx, parent) {
-      let inst = this.__instances[idx];
-      parent.insertBefore(inst.root, this);
+    __reuseInstance(inst, item, idx, itemIdx) {
+      let beforeInst = this.__instances[idx];
+      let beforeNode = beforeInst ? beforeInst.children[0] : this;
+      for (let i=0; i<inst.children.length; i++) {
+        this.parentNode.insertBefore(inst.children[i], beforeNode);
+      }
+      this.__instances.splice(idx, 0, inst);
+      inst._setPendingProperty(this.as, item);
+      inst._setPendingProperty(this.indexAs, idx);
+      inst._setPendingProperty(this.itemsIndexAs, itemIdx);
+      inst._flushProperties();
     }
 
-    __detachAndRemoveInstance(idx) {
-      let inst = this.__detachInstance(idx);
-      if (inst) {
-        this.__pool.push(inst);
-      }
+    __attachInstance(inst) {
+      this.parentNode.insertBefore(inst.root, this);
+    }
+
+    __detachAndRemoveInstanceAt(idx) {
+      this.__detachInstance(this.__instances[idx]);
       this.__instances.splice(idx, 1);
     }
 
@@ -576,21 +648,11 @@
     }
 
     __insertInstance(item, instIdx, itemIdx) {
-      let inst = this.__pool.pop();
-      if (inst) {
-        // TODO(kschaaf): If the pool is shared across turns, hostProps
-        // need to be re-set to reused instances in addition to item
-        inst._setPendingProperty(this.as, item);
-        inst._setPendingProperty(this.indexAs, instIdx);
-        inst._setPendingProperty(this.itemsIndexAs, itemIdx);
-        inst._flushProperties();
-      } else {
-        inst = this.__stampInstance(item, instIdx, itemIdx);
-      }
-      let beforeRow = this.__instances[instIdx + 1];
+      let inst = this.__stampInstance(item, instIdx, itemIdx);
+      let beforeRow = this.__instances[instIdx];
       let beforeNode = beforeRow ? beforeRow.children[0] : this;
       this.parentNode.insertBefore(inst.root, beforeNode);
-      this.__instances[instIdx] = inst;
+      this.__instances.splice(instIdx, 0, inst);
       return inst;
     }
 
diff --git a/test/runner.html b/test/runner.html
index e15a8de0a5..e2f6a527ad 100644
--- a/test/runner.html
+++ b/test/runner.html
@@ -59,6 +59,7 @@
       'unit/path.html',
       'unit/templatize.html',
       'unit/dom-repeat.html',
+      'unit/dom-repeat.html?restamp',
       'unit/dom-if.html',
       'unit/dom-bind.html',
       'unit/array-selector.html',
diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html
index 79176432f9..ede5fa6bae 100644
--- a/test/unit/dom-repeat.html
+++ b/test/unit/dom-repeat.html
@@ -15,6 +15,11 @@
   <script src="../../../web-component-tester/browser.js"></script>
   <link rel="import" href="../../polymer.html">
   <link rel="import" href="dom-repeat-elements.html">
+  <script>
+  if (document.location.search.indexOf('restamp') >= 0) {
+    customElements.get('dom-repeat').prototype.restamp = true;
+  }
+  </script>
   <style>
     /* makes painting faster when testing on IOS simulator */
     x-repeat-chunked {
@@ -301,6 +306,7 @@ <h4>x-repeat-chunked</h4>
         // reset sort fn
         configured.$.repeater.sort = null;
         configured.$.repeater.render();
+        stamped = configured.root.querySelectorAll('*:not(template):not(dom-repeat)');
         assert.equal(stamped.length, 3 + 3*3 + 3*3*3, 'total stamped count incorrect');
         assert.equal(stamped[0].itemaProp, 'prop-1');
         assert.equal(stamped[0].indexa, 0);
@@ -327,6 +333,7 @@ <h4>x-repeat-chunked</h4>
         // reset sort fn
         configured.$.repeater.sort = null;
         configured.$.repeater.render();
+        stamped = configured.root.querySelectorAll('*:not(template):not(dom-repeat)');
         assert.equal(stamped.length, 3 + 3*3 + 3*3*3, 'total stamped count incorrect');
         assert.equal(stamped[0].itemaProp, 'prop-1');
         assert.equal(stamped[0].indexa, 0);
@@ -3350,10 +3357,7 @@ <h4>x-repeat-chunked</h4>
           var prev = items.slice();
           items.sort();
           var splices = Polymer.ArraySplice.calculateSplices(items, prev);
-          var change = {
-            indexSplices: splices
-          };
-          primitive.set('items.splices', change);
+          primitive.notifySplices('items', splices);
           setTimeout(function() {
             var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
             for (var i=0; i<items.length; i++) {

From 6335b24f75cf4940c604082c35810d94763737af Mon Sep 17 00:00:00 2001
From: Kevin Schaaf <kschaaf@google.com>
Date: Fri, 21 Apr 2017 12:38:45 -0700
Subject: [PATCH 2/3] Disable closure linting until the count is driven down to
 a reasonable level

---
 .travis.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 6725fc9f5e..cdb0129741 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -19,7 +19,6 @@ before_script:
 - npm install -g bower gulp-cli@1
 - bower install
 - gulp lint
-- gulp lint-closure
 script:
 - xvfb-run wct
 - if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then wct -s 'windows 10/microsoftedge@14' -s 'windows 8.1/internet explorer@11' -s 'os x 10.11/safari@9' -s 'macos 10.12/safari@10' -s 'Linux/chrome@41'; fi

From 5a54c32b92d3b897a50ce0998f45a5ff91e76b3f Mon Sep 17 00:00:00 2001
From: Tim van der Lippe <tvanderlippe@google.com>
Date: Mon, 31 Jul 2017 20:18:22 -0700
Subject: [PATCH 3/3] Fix tests in non-Chrome browsers

The customElements.get function was undefined until
the polyfill resolved the HTMLImports.
Instead of waiting for the imports to resolve and
modifying the prototype, it was easier to extract
a function that does the logic, as the prototype
was not properly propagated in non-Chrome browsers
---
 test/unit/dom-repeat.html | 61 ++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html
index 1cba2a59d0..7ab64294b9 100644
--- a/test/unit/dom-repeat.html
+++ b/test/unit/dom-repeat.html
@@ -15,11 +15,6 @@
   <script src="../../../web-component-tester/browser.js"></script>
   <link rel="import" href="../../polymer.html">
   <link rel="import" href="dom-repeat-elements.html">
-  <script>
-  if (document.location.search.indexOf('restamp') >= 0) {
-    customElements.get('dom-repeat').prototype.restamp = true;
-  }
-  </script>
   <style>
     /* makes painting faster when testing on IOS simulator */
     x-repeat-chunked {
@@ -151,6 +146,14 @@ <h4>x-repeat-chunked</h4>
       stamped[38] .. 3-3-3
     */
 
+    function setupFixture(fixtureName) {
+      var el = fixture(fixtureName);
+      if (document.location.search.indexOf('restamp') >= 0) {
+        el.restamp = true;
+      }
+      return el;
+    }
+
     function arrayDelete(el, path, item) {
       var array = el.get(path);
       var idx = array.indexOf(item);
@@ -174,7 +177,7 @@ <h4>x-repeat-chunked</h4>
       let configured;
 
       setup(function() {
-        configured = fixture('configured');
+        configured = setupFixture('configured');
         Polymer.flush();
       });
 
@@ -650,7 +653,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.prop = 'outer';
         unconfigured.$.el1.item = {prop: 'outerItem'};
@@ -783,7 +786,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -871,7 +874,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         unconfigured.$.el1.domUpdateHandlerCount = 0;
@@ -1124,7 +1127,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.filter = function(o) {
           return o.prop.indexOf('2') < 0;
@@ -1334,7 +1337,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -1543,7 +1546,7 @@ <h4>x-repeat-chunked</h4>
       var unconfigured
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -1742,7 +1745,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured-mutable');
+        unconfigured = setupFixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -2042,7 +2045,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured-mutable');
+        unconfigured = setupFixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.filter = function(o) {
           return o.prop.indexOf('2') < 0;
@@ -2259,7 +2262,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured-mutable');
+        unconfigured = setupFixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -2475,7 +2478,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured-mutable');
+        unconfigured = setupFixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -2681,7 +2684,7 @@ <h4>x-repeat-chunked</h4>
       let primitive;
 
       setup(function() {
-        primitive = fixture('primitive');
+        primitive = setupFixture('primitive');
         Polymer.flush();
       });
 
@@ -3227,7 +3230,7 @@ <h4>x-repeat-chunked</h4>
       let primitive;
 
       setup(function() {
-        primitive = fixture('primitive');
+        primitive = setupFixture('primitive');
         Polymer.flush();
       });
 
@@ -3284,7 +3287,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - initial stamping', function() {
-        let unconfigured = fixture('unconfigured');
+        let unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         var stamped1 = unconfigured.$.el1.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3294,7 +3297,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - change from inside', function() {
-        let unconfigured = fixture('unconfigured');
+        let unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         var stamped1 = unconfigured.$.el1.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3311,7 +3314,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - sorted', function() {
-        let unconfigured = fixture('unconfigured');
+        let unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
@@ -3347,7 +3350,7 @@ <h4>x-repeat-chunked</h4>
           836, 564, 721, 540, 372,
           244, 145, 525, 958, 595,
           207, 103, 602, 769, 190];
-        let primitive = fixture('primitive');
+        let primitive = setupFixture('primitive');
         primitive.items = items;
         setTimeout(function() {
           var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3374,7 +3377,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = fixture('unconfigured');
+        unconfigured = setupFixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -3483,7 +3486,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('renderedItemCount', function() {
-        let primitive = fixture('primitive');
+        let primitive = setupFixture('primitive');
         Polymer.flush();
         var repeater1 = primitive.$.repeater1;
         primitive.items = [ 'a', 'b', 'c', 'd', 'e' ];
@@ -3503,7 +3506,7 @@ <h4>x-repeat-chunked</h4>
 
       test('_showHideChildren', function() {
         // Initially all showing
-        let primitive = fixture('primitive');
+        let primitive = setupFixture('primitive');
         Polymer.flush();
         var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
         assert.equal(getComputedStyle(stamped1[0]).display, 'block');
@@ -4046,7 +4049,7 @@ <h4>x-repeat-chunked</h4>
     suite('misc', function() {
 
       test('large splice', function(done) {
-        let primitiveLarge = fixture('primitiveLarge');
+        let primitiveLarge = setupFixture('primitiveLarge');
         Polymer.flush();
         primitiveLarge.splice('items', 0, 10);
         setTimeout(function() {
@@ -4062,7 +4065,7 @@ <h4>x-repeat-chunked</h4>
         if (!window.ShadyDOM) {
           this.skip();
         }
-        let simple = fixture('simple');
+        let simple = setupFixture('simple');
         Polymer.flush();
         var removed;
         // Confirm initial scoping
@@ -4093,7 +4096,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('paths update on observed properties', function() {
-        let simple = fixture('simple');
+        let simple = setupFixture('simple');
         Polymer.flush();
         //debugger;
         var stamped = simple.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -4123,7 +4126,7 @@ <h4>x-repeat-chunked</h4>
     suite('dom-change composed', function() {
 
       test('dom-change event composed, bubbles outside dom-if scope', function() {
-        var el = fixture('simple');
+        var el = setupFixture('simple');
         var domChangeFired = 0;
         el.addEventListener('dom-change', function() {
           domChangeFired++;