Merge pull request #5508 from Polymer/5250-ensure-canceled

Ensure the debouncer is not already canceled before canceling.
Polymer · Mar 8, 2019 · ef82ddf · ef82ddf
2 parents 4b54d18 + d48336d
commit ef82ddf
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 39 deletions.
diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js
@@ -47,7 +47,6 @@ export class Debouncer {
   cancel() {
     if (this.isActive()) {
       this._cancelAsync();
-      this._timer = null;
       // 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)
@@ -61,7 +60,10 @@ export class Debouncer {
    * @return {void}
    */
   _cancelAsync() {
-    this._asyncModule.cancel(/** @type {number} */(this._timer));
+    if (this.isActive()) {
+      this._asyncModule.cancel(/** @type {number} */(this._timer));
+      this._timer = null;
+    }
   }
   /**
    * Flushes an active debouncer and returns a reference to itself.

diff --git a/test/unit/debounce.html b/test/unit/debounce.html
@@ -36,6 +36,61 @@
 import { enqueueDebouncer, flush } from '../../lib/utils/flush.js';
 Polymer({is: 'x-basic'});
 
+suite('enqueueDebouncer & flush', function() {
+
+  // NOTE: This is a regression test; the bug it fixed only occured 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 = Debouncer.debounce(null, microTask, cb);
+    enqueueDebouncer(db);
+    db.cancel();
+    assert.equal(db.isActive(), false);
+    assert.equal(cb.callCount, 0);
+    db = Debouncer.debounce(db, microTask, cb);
+    enqueueDebouncer(db);
+    flush();
+    assert.isTrue(cb.calledOnce);
+  });
+
+  const testEnqueue = (shouldFlush, done) => {
+    const actualOrder = [];
+    const enqueue = (type, {db, cb} = {}) => {
+      cb = cb || (() => actualOrder.push(cb));
+      db = Debouncer.debounce(db, type, cb);
+      enqueueDebouncer(db);
+      return {db, cb};
+    };
+    const db1 = enqueue(microTask);
+    const db2 = enqueue(microTask);
+    const db3 = enqueue(timeOut);
+    const db4 = enqueue(microTask);
+    enqueue(microTask, db2);
+    enqueue(microTask, db1);
+    if (shouldFlush) {
+      flush();
+      assert.deepEqual(actualOrder, [db1.cb, db2.cb, db3.cb, db4.cb]);
+      done();
+    } else {
+      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);
+  });
+
+});
+
 suite('debounce', function() {
   var element;
 
@@ -211,43 +266,6 @@
 
   });
 
-  suite('enqueueDebouncer & flush', function() {
-
-    const testEnqueue = (shouldFlush, done) => {
-      const actualOrder = [];
-      const enqueue = (type, {db, cb} = {}) => {
-        cb = cb || (() => actualOrder.push(cb));
-        db = Debouncer.debounce(db, type, cb);
-        enqueueDebouncer(db);
-        return {db, cb};
-      };
-      const db1 = enqueue(microTask);
-      const db2 = enqueue(microTask);
-      const db3 = enqueue(timeOut);
-      const db4 = enqueue(microTask);
-      enqueue(microTask, db2);
-      enqueue(microTask, db1);
-      if (shouldFlush) {
-        flush();
-        assert.deepEqual(actualOrder, [db1.cb, db2.cb, db3.cb, db4.cb]);
-        done();
-      } else {
-        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);
-    });
-
-  });
 });
 </script>
 </body>