Merge pull request #5500 from Polymer/5250-fix-memleak-2.x

[2.x] Use set and clear debouncer upon completion. Fixes #5250

lib/utils/debounce.html

@@ -39,6 +39,7 @@
       this._callback = callback;
       this._timer = this._asyncModule.run(() => {
         this._timer = null;
+        debouncerQueue.delete(this);
         this._callback();
       });
     }
@@ -49,7 +50,22 @@
      */
     cancel() {
       if (this.isActive()) {
-        this._asyncModule.cancel(this._timer);
+        this._cancelAsync();
+        // Canceling a debouncer removes its spot from the flush queue,
+        // so if a debouncer is manually canceled and re-debounced, it
+        // will reset its flush order (this is a very minor difference from 1.x)
+        // Re-debouncing via the `debounce` API retains the 1.x FIFO flush order
+        debouncerQueue.delete(this);
+      }
+    }
+    /**
+     * Cancels a debouncer's async callback.
+     *
+     * @return {void}
+     */
+    _cancelAsync() {
+      if (this.isActive()) {
+        this._asyncModule.cancel(/** @type {number} */(this._timer));
         this._timer = null;
       }
     }
@@ -104,7 +120,9 @@
      */
     static debounce(debouncer, asyncModule, callback) {
       if (debouncer instanceof Debouncer) {
-        debouncer.cancel();
+        // Cancel the async callback, but leave in debouncerQueue if it was
+        // enqueued, to maintain 1.x flush order
+        debouncer._cancelAsync();
       } else {
         debouncer = new Debouncer();
       }
@@ -115,5 +133,35 @@
 
   /** @const */
   Polymer.Debouncer = Debouncer;
+
+  let debouncerQueue = new Set();
+
+  /**
+   * Adds a `Debouncer` to a list of globally flushable tasks.
+   *
+   * @memberof Polymer
+   * @param {!Debouncer} debouncer Debouncer to enqueue
+   * @return {void}
+   */
+  Polymer.enqueueDebouncer = function(debouncer) {
+    debouncerQueue.add(debouncer);
+  };
+
+  Polymer.flushDebouncers = function() {
+    const didFlush = Boolean(debouncerQueue.size);
+    // If new debouncers are added while flushing, Set.forEach will ensure
+    // newly added ones are also flushed
+    debouncerQueue.forEach(debouncer => {
+      try {
+        debouncer.flush();
+      } catch(e) {
+        setTimeout(() => {
+          throw e;
+        });
+      }
+    });
+    return didFlush;
+  };
+
 })();
 </script>

lib/utils/flush.html

@@ -8,37 +8,11 @@
 subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt
 -->
 <link rel="import" href="boot.html">
+<link rel="import" href="debounce.html">
 <script>
 (function() {
   'use strict';
 
-  let debouncerQueue = [];
-
-  /**
-   * Adds a `Polymer.Debouncer` to a list of globally flushable tasks.
-   *
-   * @memberof Polymer
-   * @param {!Polymer.Debouncer} debouncer Debouncer to enqueue
-   * @return {void}
-   */
-  Polymer.enqueueDebouncer = function(debouncer) {
-    debouncerQueue.push(debouncer);
-  };
-
-  function flushDebouncers() {
-    const didFlush = Boolean(debouncerQueue.length);
-    while (debouncerQueue.length) {
-      try {
-        debouncerQueue.shift().flush();
-      } catch(e) {
-        setTimeout(() => {
-          throw e;
-        });
-      }
-    }
-    return didFlush;
-  }
-
   /**
    * Forces several classes of asynchronously queued tasks to flush:
    * - Debouncers added via `enqueueDebouncer`
@@ -54,7 +28,7 @@
       if (window.ShadyCSS && window.ShadyCSS.ScopingShim) {
         window.ShadyCSS.ScopingShim.flush();
       }
-      debouncers = flushDebouncers();
+      debouncers = Polymer.flushDebouncers();
     } while (shadyDOM || debouncers);
   };
 

test/unit/debounce.html

@@ -34,6 +34,86 @@
     Polymer({is: 'x-basic'});
   });
 
+  suite('enqueueDebouncer & flush', function() {
+
+    // NOTE: This is a regression test; the bug it fixed only occurred if the
+    // debouncer was flushed before any microtasks run, hence it should be
+    // first in this file
+    test('re-enqueue canceled debouncer', function() {
+      const cb = sinon.spy();
+      let db;
+      db = Polymer.Debouncer.debounce(null, Polymer.Async.microTask, cb);
+      Polymer.enqueueDebouncer(db);
+      db.cancel();
+      assert.equal(db.isActive(), false);
+      assert.equal(cb.callCount, 0);
+      db = Polymer.Debouncer.debounce(db, Polymer.Async.microTask, cb);
+      Polymer.enqueueDebouncer(db);
+      Polymer.flush();
+      assert.isTrue(cb.calledOnce);
+    });
+
+    test('flushDebouncers from enqueued debouncer', function(done) {
+      const cb = sinon.spy(() => Polymer.flush());
+      let db = Polymer.Debouncer.debounce(null, Polymer.Async.microTask, cb);
+      Polymer.enqueueDebouncer(db);
+      setTimeout(() => {
+        assert.isTrue(cb.calledOnce);
+        done();
+      });
+    });
+
+    const testEnqueue = (shouldFlush, done) => {
+      const actualOrder = [];
+      const enqueue = (type, {db, cb} = {}) => {
+        cb = cb || (() => actualOrder.push(cb));
+        db = Polymer.Debouncer.debounce(db, type, cb);
+        Polymer.enqueueDebouncer(db);
+        return {db, cb};
+      };
+      const db1 = enqueue(Polymer.Async.microTask);
+      const db2 = enqueue(Polymer.Async.microTask);
+      const db3 = enqueue(Polymer.Async.timeOut);
+      const db4 = enqueue(Polymer.Async.microTask);
+      enqueue(Polymer.Async.microTask, db2);
+      enqueue(Polymer.Async.microTask, db1);
+      if (shouldFlush) {
+        Polymer.flush();
+        assert.deepEqual(actualOrder, [db1.cb, db2.cb, db3.cb, db4.cb]);
+        done();
+      } else {
+        Polymer.Async.timeOut.run(() => {
+          assert.deepEqual(actualOrder, [db4.cb, db2.cb, db1.cb, db3.cb]);
+          done();
+        });
+      }
+    };
+
+    test('non-flushed', function(done) {
+      testEnqueue(false, done);
+    });
+
+    test('flushed', function(done) {
+      testEnqueue(true, done);
+    });
+
+    test('reentrant flush', function() {
+      const cb2 = sinon.spy();
+      let db2;
+      const cb1 = sinon.spy(() => {
+        Polymer.flush();
+        db2 = Polymer.Debouncer.debounce(null, Polymer.Async.microTask, cb2);
+        Polymer.enqueueDebouncer(db2);
+      });
+      const db1 = Polymer.Debouncer.debounce(null, Polymer.Async.microTask, cb1);
+      Polymer.enqueueDebouncer(db1);
+      Polymer.flush();
+      assert.isTrue(cb1.calledOnce);
+      assert.isTrue(cb2.calledOnce);
+    });
+
+  });
+
   suite('debounce', function() {
     var element;
 
@@ -208,6 +288,7 @@
       });
 
     });
+
   });
 </script>
 </body>

types/lib/utils/debounce.d.ts

@@ -68,6 +68,11 @@ declare namespace Polymer {
      */
     cancel(): void;
 
+    /**
+     * Cancels a debouncer's async callback.
+     */
+    _cancelAsync(): void;
+
     /**
      * Flushes an active debouncer and returns a reference to itself.
      */
@@ -80,4 +85,10 @@ declare namespace Polymer {
      */
     isActive(): boolean;
   }
+
+
+  /**
+   * Adds a `Debouncer` to a list of globally flushable tasks.
+   */
+  function enqueueDebouncer(debouncer: Debouncer): void;
 }

types/lib/utils/flush.d.ts

@@ -12,16 +12,11 @@
 // tslint:disable:variable-name Describing an API that's defined elsewhere.
 
 /// <reference path="boot.d.ts" />
+/// <reference path="debounce.d.ts" />
 
 declare namespace Polymer {
 
 
-  /**
-   * Adds a `Polymer.Debouncer` to a list of globally flushable tasks.
-   */
-  function enqueueDebouncer(debouncer: Polymer.Debouncer): void;
-
-
   /**
    * Forces several classes of asynchronously queued tasks to flush:
    * - Debouncers added via `enqueueDebouncer`