From d439960abaa2eee3741837db4cae1b96b4b9168c Mon Sep 17 00:00:00 2001
From: Daniel Freedman <compismyrx@gmail.com>
Date: Wed, 9 Aug 2017 15:49:47 -0700
Subject: [PATCH] Revert "Adds `restamp` mode to dom-repeat."

---
 lib/elements/dom-repeat.html | 197 ++++++++++++-----------------------
 test/runner.html             |   1 -
 test/unit/dom-repeat.html    |  63 +++++------
 3 files changed, 95 insertions(+), 166 deletions(-)

diff --git a/lib/elements/dom-repeat.html b/lib/elements/dom-repeat.html
index 0cbc10f219..17a1401588 100644
--- a/lib/elements/dom-repeat.html
+++ b/lib/elements/dom-repeat.html
@@ -265,32 +265,6 @@
         _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
         }
 
       }
@@ -305,6 +279,7 @@
       super();
       this.__instances = [];
       this.__limit = Infinity;
+      this.__pool = [];
       this.__renderDebouncer = null;
       this.__itemsIdxToInstIdx = {};
       this.__chunkCount = null;
@@ -320,9 +295,8 @@
     disconnectedCallback() {
       super.disconnectedCallback();
       this.__isDetached = true;
-      let instances = this.__instances;
-      for (let i=0; i<instances.length; i++) {
-        this.__detachInstance(instances[i]);
+      for (let i=0; i<this.__instances.length; i++) {
+        this.__detachInstance(i);
       }
     }
 
@@ -331,9 +305,9 @@
       // only perform attachment if the element was previously detached.
       if (this.__isDetached) {
         this.__isDetached = false;
-        let instances = this.__instances;
-        for (let i=0; i<instances.length; i++) {
-          this.__attachInstance(instances[i]);
+        let parent = this.parentNode;
+        for (let i=0; i<this.__instances.length; i++) {
+          this.__attachInstance(i, parent);
         }
       }
     }
@@ -523,35 +497,13 @@
         // No template found yet
         return;
       }
-      let 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, array) =>
-          this.__filterFn(items[i], idx, array));
-      }
-      // 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);
-      }
+      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;
       // Set rendered item count
       this._setRenderedItemCount(this.__instances.length);
       // Notify users
@@ -563,90 +515,65 @@
       this.__tryRenderChunk();
     }
 
-    __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;
+    __applyFullRefresh() {
+      let items = this.items || [];
+      let isntIdxToItemsIdx = new Array(items.length);
+      for (let i=0; i<items.length; i++) {
+        isntIdxToItemsIdx[i] = i;
       }
-      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];
+      // 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]));
+      }
+      // 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];
         let item = items[itemIdx];
-        if (inst) {
+        itemsIdxToInstIdx[itemIdx] = instIdx;
+        if (inst && instIdx < this.__limit) {
           inst._setPendingProperty(this.as, item);
-          inst._setPendingProperty(this.indexAs, i);
+          inst._setPendingProperty(this.indexAs, instIdx);
           inst._setPendingProperty(this.itemsIndexAs, itemIdx);
           inst._flushProperties();
         } else {
-          this.__insertInstance(item, i, itemIdx);
+          this.__insertInstance(item, instIdx, itemIdx);
         }
-        items2inst[itemIdx] = i;
+      }
+      // Remove any extra instances from previous state
+      for (let i=this.__instances.length-1; i>=instIdx; i--) {
+        this.__detachAndRemoveInstance(i);
       }
     }
 
-    __detachInstance(inst) {
+    __detachInstance(idx) {
+      let inst = this.__instances[idx];
       for (let i=0; i<inst.children.length; i++) {
         let el = inst.children[i];
         inst.root.appendChild(el);
       }
+      return inst;
     }
 
-    __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();
-    }
-
-    __attachInstance(inst) {
-      this.parentNode.insertBefore(inst.root, this);
+    __attachInstance(idx, parent) {
+      let inst = this.__instances[idx];
+      parent.insertBefore(inst.root, this);
     }
 
-    __detachAndRemoveInstanceAt(idx) {
-      this.__detachInstance(this.__instances[idx]);
+    __detachAndRemoveInstance(idx) {
+      let inst = this.__detachInstance(idx);
+      if (inst) {
+        this.__pool.push(inst);
+      }
       this.__instances.splice(idx, 1);
     }
 
@@ -659,11 +586,21 @@
     }
 
     __insertInstance(item, instIdx, itemIdx) {
-      let inst = this.__stampInstance(item, instIdx, itemIdx);
-      let beforeRow = this.__instances[instIdx];
+      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 beforeNode = beforeRow ? beforeRow.children[0] : this;
       this.parentNode.insertBefore(inst.root, beforeNode);
-      this.__instances.splice(instIdx, 0, inst);
+      this.__instances[instIdx] = inst;
       return inst;
     }
 
diff --git a/test/runner.html b/test/runner.html
index 2ae6abc917..ad9d00e356 100644
--- a/test/runner.html
+++ b/test/runner.html
@@ -60,7 +60,6 @@
       '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 fa2ed83f04..5fa29eebe5 100644
--- a/test/unit/dom-repeat.html
+++ b/test/unit/dom-repeat.html
@@ -147,14 +147,6 @@ <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);
@@ -178,7 +170,7 @@ <h4>x-repeat-chunked</h4>
       let configured;
 
       setup(function() {
-        configured = setupFixture('configured');
+        configured = fixture('configured');
         Polymer.flush();
       });
 
@@ -310,7 +302,6 @@ <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);
@@ -337,7 +328,6 @@ <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);
@@ -654,7 +644,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.prop = 'outer';
         unconfigured.$.el1.item = {prop: 'outerItem'};
@@ -787,7 +777,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -875,7 +865,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         unconfigured.$.el1.domUpdateHandlerCount = 0;
@@ -1128,7 +1118,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.filter = function(o) {
           return o.prop.indexOf('2') < 0;
@@ -1338,7 +1328,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -1547,7 +1537,7 @@ <h4>x-repeat-chunked</h4>
       var unconfigured
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
           return b.prop == a.prop ? 0 : b.prop < a.prop ? -1 : 1;
@@ -1746,7 +1736,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured-mutable');
+        unconfigured = fixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -2046,7 +2036,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured-mutable');
+        unconfigured = fixture('unconfigured-mutable');
         unconfigured.items = window.getData();
         unconfigured.$.el1.$.repeater.filter = function(o) {
           return o.prop.indexOf('2') < 0;
@@ -2263,7 +2253,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured-mutable');
+        unconfigured = fixture('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;
@@ -2479,7 +2469,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured-mutable');
+        unconfigured = fixture('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;
@@ -2685,7 +2675,7 @@ <h4>x-repeat-chunked</h4>
       let primitive;
 
       setup(function() {
-        primitive = setupFixture('primitive');
+        primitive = fixture('primitive');
         Polymer.flush();
       });
 
@@ -3231,7 +3221,7 @@ <h4>x-repeat-chunked</h4>
       let primitive;
 
       setup(function() {
-        primitive = setupFixture('primitive');
+        primitive = fixture('primitive');
         Polymer.flush();
       });
 
@@ -3288,7 +3278,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - initial stamping', function() {
-        let unconfigured = setupFixture('unconfigured');
+        let unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         var stamped1 = unconfigured.$.el1.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3298,7 +3288,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - change from inside', function() {
-        let unconfigured = setupFixture('unconfigured');
+        let unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         var stamped1 = unconfigured.$.el1.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3315,7 +3305,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('structured values - sorted', function() {
-        let unconfigured = setupFixture('unconfigured');
+        let unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
         unconfigured.$.el1.$.repeater.sort = function(a, b) {
@@ -3351,7 +3341,7 @@ <h4>x-repeat-chunked</h4>
           836, 564, 721, 540, 372,
           244, 145, 525, 958, 595,
           207, 103, 602, 769, 190];
-        let primitive = setupFixture('primitive');
+        let primitive = fixture('primitive');
         primitive.items = items;
         setTimeout(function() {
           var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -3361,7 +3351,10 @@ <h4>x-repeat-chunked</h4>
           var prev = items.slice();
           items.sort();
           var splices = Polymer.ArraySplice.calculateSplices(items, prev);
-          primitive.notifySplices('items', splices);
+          var change = {
+            indexSplices: splices
+          };
+          primitive.set('items.splices', change);
           setTimeout(function() {
             var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
             for (var i=0; i<items.length; i++) {
@@ -3378,7 +3371,7 @@ <h4>x-repeat-chunked</h4>
       let unconfigured;
 
       setup(function() {
-        unconfigured = setupFixture('unconfigured');
+        unconfigured = fixture('unconfigured');
         unconfigured.items = window.getData();
         Polymer.flush();
       });
@@ -3487,7 +3480,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('renderedItemCount', function() {
-        let primitive = setupFixture('primitive');
+        let primitive = fixture('primitive');
         Polymer.flush();
         var repeater1 = primitive.$.repeater1;
         primitive.items = [ 'a', 'b', 'c', 'd', 'e' ];
@@ -3507,7 +3500,7 @@ <h4>x-repeat-chunked</h4>
 
       test('_showHideChildren', function() {
         // Initially all showing
-        let primitive = setupFixture('primitive');
+        let primitive = fixture('primitive');
         Polymer.flush();
         var stamped1 = primitive.$.container1.querySelectorAll('*:not(template):not(dom-repeat)');
         assert.equal(getComputedStyle(stamped1[0]).display, 'block');
@@ -4050,7 +4043,7 @@ <h4>x-repeat-chunked</h4>
     suite('misc', function() {
 
       test('large splice', function(done) {
-        let primitiveLarge = setupFixture('primitiveLarge');
+        let primitiveLarge = fixture('primitiveLarge');
         Polymer.flush();
         primitiveLarge.splice('items', 0, 10);
         setTimeout(function() {
@@ -4066,7 +4059,7 @@ <h4>x-repeat-chunked</h4>
         if (!window.ShadyDOM) {
           this.skip();
         }
-        let simple = setupFixture('simple');
+        let simple = fixture('simple');
         Polymer.flush();
         var removed;
         // Confirm initial scoping
@@ -4097,7 +4090,7 @@ <h4>x-repeat-chunked</h4>
       });
 
       test('paths update on observed properties', function() {
-        let simple = setupFixture('simple');
+        let simple = fixture('simple');
         Polymer.flush();
         //debugger;
         var stamped = simple.root.querySelectorAll('*:not(template):not(dom-repeat)');
@@ -4127,7 +4120,7 @@ <h4>x-repeat-chunked</h4>
     suite('dom-change composed', function() {
 
       test('dom-change event composed, bubbles outside dom-if scope', function() {
-        var el = setupFixture('simple');
+        var el = fixture('simple');
         var domChangeFired = 0;
         el.addEventListener('dom-change', function() {
           domChangeFired++;