From fed97654aef63740b5031e1587613ca3817f26a2 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 7 Mar 2019 12:51:18 -0800 Subject: [PATCH 1/2] Ensure the debouncer is not already canceled before canceling. --- lib/utils/debounce.js | 6 ++- test/unit/debounce.html | 90 ++++++++++++++++++++++++----------------- 2 files changed, 57 insertions(+), 39 deletions(-) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index 622e22d35e..6fb246052b 100644 --- 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 index 9f528b6ac6..f4d9cd3d78 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -36,6 +36,59 @@ 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); + }); + + 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 +264,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); - }); - - }); }); From d48336d61493056569cbbbb7f03609ec7d341779 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Thu, 7 Mar 2019 14:56:56 -0800 Subject: [PATCH 2/2] Assert the callback was called. --- test/unit/debounce.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/debounce.html b/test/unit/debounce.html index f4d9cd3d78..5d20439147 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -40,7 +40,7 @@ // 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. + // first in this file test('re-enqueue canceled debouncer', function() { const cb = sinon.spy(); let db; @@ -51,6 +51,8 @@ assert.equal(cb.callCount, 0); db = Debouncer.debounce(db, microTask, cb); enqueueDebouncer(db); + flush(); + assert.isTrue(cb.calledOnce); }); const testEnqueue = (shouldFlush, done) => {