commit dd42019824ed Author: Thomas Nguyen Date: Wed Aug 30 18:04:10 2017 +0800 Bug 1385609 - Fix backoff issue that makes SB lists no longer update r=francois The issue occurs when nsITimer is fired earlier than the backoff time. In that case, the update doesn't proceed and we never make another attempt because the backoff update timer was oneshot. We fix the issue in two ways: - Add a tolerance of 1 second in case the timer fires too early. - Set another oneshot timer whenever we are prevented from updating due to backoff. MozReview-Commit-ID: E2ogNRsHJVK --HG-- extra : rebase_source : c81fa77934f6c39e1c5d07b19785a01546e02542 --- toolkit/components/url-classifier/nsUrlClassifierLib.js | 12 +++++++++--- .../components/url-classifier/nsUrlClassifierListManager.js | 5 ++++- toolkit/components/url-classifier/tests/unit/test_backoff.js | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git toolkit/components/url-classifier/nsUrlClassifierLib.js toolkit/components/url-classifier/nsUrlClassifierLib.js index e2457d85474c..38d1977e4641 100644 --- toolkit/components/url-classifier/nsUrlClassifierLib.js +++ toolkit/components/url-classifier/nsUrlClassifierLib.js @@ -77,17 +77,20 @@ this.HTTP_TEMPORARY_REDIRECT = 307; * @param timeoutIncrement Number time (ms) the starting timeout period * we double this time for consecutive errors * @param maxTimeout Number time (ms) maximum timeout period + * @param tolerance Checking next request tolerance. */ this.RequestBackoff = function RequestBackoff(maxErrors, retryIncrement, maxRequests, requestPeriod, - timeoutIncrement, maxTimeout) { + timeoutIncrement, maxTimeout, + tolerance) { this.MAX_ERRORS_ = maxErrors; this.RETRY_INCREMENT_ = retryIncrement; this.MAX_REQUESTS_ = maxRequests; this.REQUEST_PERIOD_ = requestPeriod; this.TIMEOUT_INCREMENT_ = timeoutIncrement; this.MAX_TIMEOUT_ = maxTimeout; + this.TOLERANCE_ = tolerance; // Queue of ints keeping the time of all requests this.requestTimes_ = []; @@ -111,7 +114,9 @@ RequestBackoff.prototype.reset = function() { */ RequestBackoff.prototype.canMakeRequest = function() { var now = Date.now(); - if (now < this.nextRequestTime_) { + // Note that nsITimer delay is approximate: the timer can be fired before the + // requested time has elapsed. So, give it a tolerance + if (now + this.TOLERANCE_ < this.nextRequestTime_) { return false; } @@ -180,7 +185,8 @@ function RequestBackoffV4(maxRequests, requestPeriod) { maxRequests /* num requests */, requestPeriod /* request time, 60 min */, backoffInterval /* backoff interval, 60 min */, - 24 * 60 * 60 * 1000 /* max backoff, 24hr */); + 24 * 60 * 60 * 1000 /* max backoff, 24hr */, + 1000 /* tolerance of 1 sec */); } // Expose this whole component. diff --git toolkit/components/url-classifier/nsUrlClassifierListManager.js toolkit/components/url-classifier/nsUrlClassifierListManager.js index 1e976b8597b7..5f10933163b6 100644 --- toolkit/components/url-classifier/nsUrlClassifierListManager.js +++ toolkit/components/url-classifier/nsUrlClassifierListManager.js @@ -236,7 +236,10 @@ PROT_ListManager.prototype.setUpdateCheckTimer = function(updateUrl, .createInstance(Ci.nsITimer); this.updateCheckers_[updateUrl].initWithCallback(() => { this.updateCheckers_[updateUrl] = null; - this.checkForUpdates(updateUrl); + if (updateUrl && !this.checkForUpdates(updateUrl)) { + // Make another attempt later. + this.setUpdateCheckTimer(updateUrl, this.updateInterval); + } }, delay, Ci.nsITimer.TYPE_ONE_SHOT); } /** diff --git toolkit/components/url-classifier/tests/unit/test_backoff.js toolkit/components/url-classifier/tests/unit/test_backoff.js index 365568c479e9..205507e83dd6 100644 --- toolkit/components/url-classifier/tests/unit/test_backoff.js +++ toolkit/components/url-classifier/tests/unit/test_backoff.js @@ -11,7 +11,7 @@ function setNow(time) { function run_test() { // 3 errors, 1ms retry period, max 3 requests per ten milliseconds, // 5ms backoff interval, 19ms max delay - var rb = new jslib.RequestBackoff(3, 1, 3, 10, 5, 19); + var rb = new jslib.RequestBackoff(3, 1, 3, 10, 5, 19, 0); setNow(1); rb.noteServerResponse(200); do_check_true(rb.canMakeRequest());