From e8c24ff4f590fe67196b152cddccacbefba417e3 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 14:55:57 -0800 Subject: [PATCH 1/9] Use set and clear debouncer upon completion. Fixes #5250. --- lib/utils/debounce.js | 36 +++++++++++++++++++++++++++++++ lib/utils/flush.js | 28 ++---------------------- test/unit/debounce.html | 48 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 26 deletions(-) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index eac7dd0ee8..cabc24f8eb 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -47,6 +47,7 @@ export class Debouncer { if (this.isActive()) { this._asyncModule.cancel(/** @type {number} */(this._timer)); this._timer = null; + debouncerQueue.delete(this); } } /** @@ -112,3 +113,38 @@ export class Debouncer { return debouncer; } } + +let debouncerQueue = new Set(); + +/** + * Adds a `Debouncer` to a list of globally flushable tasks. + * + * @param {!Debouncer} debouncer Debouncer to enqueue + * @return {void} + */ +export const enqueueDebouncer = function(debouncer) { + if (debouncerQueue.has(debouncer)) { + debouncerQueue.delete(debouncer); + } + debouncerQueue.add(debouncer); +}; + +/** + * Flushes any enqueued debouncers + * + * @return {void} + */ +export const flushDebouncers = function() { + const didFlush = Boolean(debouncerQueue.size); + debouncerQueue.forEach(debouncer => { + try { + debouncer.flush(); + } catch(e) { + setTimeout(() => { + throw e; + }); + } + }); + debouncerQueue = new Set(); + return didFlush; +}; \ No newline at end of file diff --git a/lib/utils/flush.js b/lib/utils/flush.js index 065a58a901..a719047010 100644 --- a/lib/utils/flush.js +++ b/lib/utils/flush.js @@ -11,32 +11,8 @@ import './boot.js'; /* eslint-disable no-unused-vars */ import { Debouncer } from '../utils/debounce.js'; // used in type annotations /* eslint-enable no-unused-vars */ - -let debouncerQueue = []; - -/** - * Adds a `Debouncer` to a list of globally flushable tasks. - * - * @param {!Debouncer} debouncer Debouncer to enqueue - * @return {void} - */ -export const 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; -} +import { flushDebouncers } from '../utils/debounce.js'; // used in type annotations +export { enqueueDebouncer } from '../utils/debounce.js'; // used in type annotations /** * Forces several classes of asynchronously queued tasks to flush: diff --git a/test/unit/debounce.html b/test/unit/debounce.html index ece33b94a4..aed0378b5b 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -33,6 +33,7 @@ import { Polymer } from '../../polymer-legacy.js'; import { Debouncer } from '../../lib/utils/debounce.js'; import { microTask, timeOut, animationFrame, idlePeriod } from '../../lib/utils/async.js'; +import { enqueueDebouncer, flush } from '../../lib/utils/flush.js'; Polymer({is: 'x-basic'}); suite('debounce', function() { @@ -209,6 +210,53 @@ }); }); + + suite('enqueueDebouncer & flush', function() { + function testEnqueue(shouldFlush, done) { + // Longer-running debouncer + const timeoutCallback = sinon.spy(() => actualCallbacks.push(timeoutCallback)); + enqueueDebouncer(Debouncer.debounce(null, timeOut, timeoutCallback)); + // Set of short-running debouncers enqueued in the middle of first set + const nestedCallbacks = new Array(150).fill().map((_, i) => sinon.spy(() => + actualCallbacks.push(nestedCallbacks[i]))); + // First set of short-running debouncers + const microtaskCallbacks = new Array(150).fill().map((_, i) => sinon.spy(() => { + actualCallbacks.push(microtaskCallbacks[i]); + if (i === 125) { + nestedCallbacks.forEach(cb => + enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); + } + })); + microtaskCallbacks.forEach(cb => + enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); + // Expect short before long + let expectedCallbacks; + const actualCallbacks = []; + const verify = () => { + actualCallbacks.forEach(cb => assert.isTrue(cb.calledOnce)); + assert.deepEqual(expectedCallbacks, actualCallbacks); + done(); + }; + if (shouldFlush) { + expectedCallbacks = [timeoutCallback, ...microtaskCallbacks, ...nestedCallbacks]; + flush(); + // When flushing, order is order of enqueing + verify(); + } else { + expectedCallbacks = [...microtaskCallbacks, ...nestedCallbacks, timeoutCallback]; + timeOut.run(verify); + } + } + + test('non-flushed', function(done) { + testEnqueue(false, done); + }); + + test('flushed', function(done) { + testEnqueue(true, done); + }); + + }); }); From 567c10b3d40bab4f83d98947750ea6b44c44616c Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 15:57:24 -0800 Subject: [PATCH 2/9] Add comments and avoid Array.fill --- lib/utils/debounce.js | 4 ++++ test/unit/debounce.html | 24 +++++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index cabc24f8eb..12aef40242 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -123,6 +123,8 @@ let debouncerQueue = new Set(); * @return {void} */ export const enqueueDebouncer = function(debouncer) { + // Re-enqueued debouncers are put at the end of the queue; for Set, this + // means removing and re-adding, since forEach traverses insertion order if (debouncerQueue.has(debouncer)) { debouncerQueue.delete(debouncer); } @@ -136,6 +138,8 @@ export const enqueueDebouncer = function(debouncer) { */ export const 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(); diff --git a/test/unit/debounce.html b/test/unit/debounce.html index aed0378b5b..dfa230bce0 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -217,16 +217,22 @@ const timeoutCallback = sinon.spy(() => actualCallbacks.push(timeoutCallback)); enqueueDebouncer(Debouncer.debounce(null, timeOut, timeoutCallback)); // Set of short-running debouncers enqueued in the middle of first set - const nestedCallbacks = new Array(150).fill().map((_, i) => sinon.spy(() => - actualCallbacks.push(nestedCallbacks[i]))); + const nestedCallbacks = []; + for (let i=0; i<150; i++) { + nestedCallbacks.push(sinon.spy(() => + actualCallbacks.push(nestedCallbacks[i]))); + } // First set of short-running debouncers - const microtaskCallbacks = new Array(150).fill().map((_, i) => sinon.spy(() => { - actualCallbacks.push(microtaskCallbacks[i]); - if (i === 125) { - nestedCallbacks.forEach(cb => - enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); - } - })); + const microtaskCallbacks = []; + for (let i=0; i<150; i++) { + microtaskCallbacks.push(sinon.spy(() => { + actualCallbacks.push(microtaskCallbacks[i]); + if (i === 125) { + nestedCallbacks.forEach(cb => + enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); + } + })); + } microtaskCallbacks.forEach(cb => enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); // Expect short before long From b9d495975824479443ee4acdae21596a19e428b3 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 17:11:31 -0800 Subject: [PATCH 3/9] Fix order of flushed debouncers to match 1.x --- lib/utils/debounce.js | 21 ++++++++----- test/unit/debounce.html | 66 ++++++++++++++++++----------------------- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index 12aef40242..83163bb762 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -45,11 +45,19 @@ export class Debouncer { */ cancel() { if (this.isActive()) { - this._asyncModule.cancel(/** @type {number} */(this._timer)); + this._cancelAsync(); this._timer = null; debouncerQueue.delete(this); } } + /** + * Cancels a debouncer's async callback. + * + * @return {void} + */ + _cancelAsync() { + this._asyncModule.cancel(/** @type {number} */(this._timer)); + } /** * Flushes an active debouncer and returns a reference to itself. * @@ -105,7 +113,9 @@ export class Debouncer { */ 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(); } @@ -123,14 +133,11 @@ let debouncerQueue = new Set(); * @return {void} */ export const enqueueDebouncer = function(debouncer) { - // Re-enqueued debouncers are put at the end of the queue; for Set, this - // means removing and re-adding, since forEach traverses insertion order - if (debouncerQueue.has(debouncer)) { - debouncerQueue.delete(debouncer); - } debouncerQueue.add(debouncer); }; +window.debouncerQueue = debouncerQueue; + /** * Flushes any enqueued debouncers * diff --git a/test/unit/debounce.html b/test/unit/debounce.html index dfa230bce0..2ecdb58eb4 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -212,56 +212,48 @@ }); suite('enqueueDebouncer & flush', function() { - function testEnqueue(shouldFlush, done) { - // Longer-running debouncer - const timeoutCallback = sinon.spy(() => actualCallbacks.push(timeoutCallback)); - enqueueDebouncer(Debouncer.debounce(null, timeOut, timeoutCallback)); - // Set of short-running debouncers enqueued in the middle of first set - const nestedCallbacks = []; - for (let i=0; i<150; i++) { - nestedCallbacks.push(sinon.spy(() => - actualCallbacks.push(nestedCallbacks[i]))); - } - // First set of short-running debouncers - const microtaskCallbacks = []; - for (let i=0; i<150; i++) { - microtaskCallbacks.push(sinon.spy(() => { - actualCallbacks.push(microtaskCallbacks[i]); - if (i === 125) { - nestedCallbacks.forEach(cb => - enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); - } - })); - } - microtaskCallbacks.forEach(cb => - enqueueDebouncer(Debouncer.debounce(null, microTask, cb))); - // Expect short before long - let expectedCallbacks; - const actualCallbacks = []; - const verify = () => { - actualCallbacks.forEach(cb => assert.isTrue(cb.calledOnce)); - assert.deepEqual(expectedCallbacks, actualCallbacks); - done(); + + const testEnqueue = (shouldFlush, done) => { + const actualOrder = []; + let i=1; + const enqueue = (type, {db, cb} = {}) => { + // cb = cb || (() => actualOrder.push(cb)); + if (!cb) { + cb = (() => actualOrder.push(cb)); + Object.defineProperty(cb, 'name', {value: `db${i}`}); i++; + } + 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) { - expectedCallbacks = [timeoutCallback, ...microtaskCallbacks, ...nestedCallbacks]; flush(); - // When flushing, order is order of enqueing - verify(); + assert.deepEqual(actualOrder, [db1.cb, db2.cb, db3.cb, db4.cb]); + done(); } else { - expectedCallbacks = [...microtaskCallbacks, ...nestedCallbacks, timeoutCallback]; - timeOut.run(verify); + 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) { + test.only('flushed', function(done) { testEnqueue(true, done); }); + test('re-enqueued ') + }); }); From 1526626b721e304a2e65702219d0ac6f515c4f27 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 17:14:15 -0800 Subject: [PATCH 4/9] Remove test.only --- test/unit/debounce.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/debounce.html b/test/unit/debounce.html index 2ecdb58eb4..bc836e9fe8 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -248,12 +248,10 @@ testEnqueue(false, done); }); - test.only('flushed', function(done) { + test('flushed', function(done) { testEnqueue(true, done); }); - test('re-enqueued ') - }); }); From b750a52dfbc9ef3b2eeae50d324eab840c463290 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 17:59:08 -0800 Subject: [PATCH 5/9] Remove debug code --- lib/utils/debounce.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index 83163bb762..c602239d0c 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -136,8 +136,6 @@ export const enqueueDebouncer = function(debouncer) { debouncerQueue.add(debouncer); }; -window.debouncerQueue = debouncerQueue; - /** * Flushes any enqueued debouncers * From be1afacc0c42dcce8cd79a6c862e0ae8ce0a6cf1 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 18:02:18 -0800 Subject: [PATCH 6/9] Re-add the queue removal in setConfig --- lib/utils/debounce.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index c602239d0c..2c9a42a65f 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -36,6 +36,7 @@ export class Debouncer { this._timer = this._asyncModule.run(() => { this._timer = null; this._callback(); + debouncerQueue.delete(this); }); } /** From cc6ef0e1fcfa545058a957850f979b2ce0ab1efd Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 18:05:09 -0800 Subject: [PATCH 7/9] Remove debug code --- test/unit/debounce.html | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/unit/debounce.html b/test/unit/debounce.html index bc836e9fe8..e27bc8b42f 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -217,11 +217,7 @@ const actualOrder = []; let i=1; const enqueue = (type, {db, cb} = {}) => { - // cb = cb || (() => actualOrder.push(cb)); - if (!cb) { - cb = (() => actualOrder.push(cb)); - Object.defineProperty(cb, 'name', {value: `db${i}`}); i++; - } + cb = cb || (() => actualOrder.push(cb)); db = Debouncer.debounce(db, type, cb); enqueueDebouncer(db); return {db, cb}; From 1e56b0e9c27be22b16c7557a1efb48ae3585523c Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 18:18:35 -0800 Subject: [PATCH 8/9] FIx lint --- test/unit/debounce.html | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/debounce.html b/test/unit/debounce.html index e27bc8b42f..9f528b6ac6 100644 --- a/test/unit/debounce.html +++ b/test/unit/debounce.html @@ -215,7 +215,6 @@ const testEnqueue = (shouldFlush, done) => { const actualOrder = []; - let i=1; const enqueue = (type, {db, cb} = {}) => { cb = cb || (() => actualOrder.push(cb)); db = Debouncer.debounce(db, type, cb); From b63c887f2d396d3f7f6329a301ad30d9c7ff58e2 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Wed, 27 Feb 2019 18:32:28 -0800 Subject: [PATCH 9/9] Add comment about flush order when re-debouncing --- lib/utils/debounce.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/utils/debounce.js b/lib/utils/debounce.js index 2c9a42a65f..6cafd8cb47 100644 --- a/lib/utils/debounce.js +++ b/lib/utils/debounce.js @@ -48,6 +48,10 @@ export class Debouncer { 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) + // Re-debouncing via the `debounce` API retains the 1.x FIFO flush order debouncerQueue.delete(this); } }