Skip to content

Commit

Permalink
Merge pull request #5499 from Polymer/5250-fix-memleak
Browse files Browse the repository at this point in the history
[3.x] Use set and clear debouncer upon completion. Fixes #5250.
  • Loading branch information
kevinpschaaf authored Feb 28, 2019
2 parents 187f10a + b63c887 commit e4ee5f8
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 28 deletions.
54 changes: 52 additions & 2 deletions lib/utils/debounce.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class Debouncer {
this._timer = this._asyncModule.run(() => {
this._timer = null;
this._callback();
debouncerQueue.delete(this);
});
}
/**
Expand All @@ -45,10 +46,23 @@ export class Debouncer {
*/
cancel() {
if (this.isActive()) {
this._asyncModule.cancel(/** @type {number} */(this._timer));
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);
}
}
/**
* 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.
*
Expand Down Expand Up @@ -104,11 +118,47 @@ 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();
}
debouncer.setConfig(asyncModule, callback);
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) {
debouncerQueue.add(debouncer);
};

/**
* Flushes any enqueued debouncers
*
* @return {void}
*/
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();
} catch(e) {
setTimeout(() => {
throw e;
});
}
});
debouncerQueue = new Set();
return didFlush;
};
28 changes: 2 additions & 26 deletions lib/utils/flush.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
39 changes: 39 additions & 0 deletions test/unit/debounce.html
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -209,6 +210,44 @@
});

});

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>
Expand Down

0 comments on commit e4ee5f8

Please sign in to comment.