Bug 1385609 - Fix backoff issue that makes SB lists no longer update r=francois
authorThomas Nguyen <tnguyen@mozilla.com>
Wed, 30 Aug 2017 18:04:10 +0800
changeset 660346 fdbb8e7b65f29b7fadad69be2b70ea3bafac6ef9
parent 660345 8cd07fc93f8bc17c7114949adf813d26aeed55d0
child 660347 36b32026b08deeb2b8d7aaae8eff92bb3c6129b0
push id78390
push userbmo:emilio@crisal.io
push dateWed, 06 Sep 2017 23:04:15 +0000
reviewersfrancois
bugs1385609
milestone57.0a1
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
toolkit/components/url-classifier/nsUrlClassifierLib.js
toolkit/components/url-classifier/nsUrlClassifierListManager.js
toolkit/components/url-classifier/tests/unit/test_backoff.js
--- a/toolkit/components/url-classifier/nsUrlClassifierLib.js
+++ b/toolkit/components/url-classifier/nsUrlClassifierLib.js
@@ -72,27 +72,30 @@ this.HTTP_TEMPORARY_REDIRECT    = 307;
  * @param maxErrors Number of times to request before backing off.
  * @param retryIncrement Time (ms) for each retry before backing off.
  * @param maxRequests Number the number of requests needed to trigger backoff
  * @param requestPeriod Number time (ms) in which maxRequests have to occur to
  *     trigger the backoff behavior (0 to disable maxRequests)
  * @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_ = [];
 
   this.numErrors_ = 0;
   this.errorTimeout_ = 0;
   this.nextRequestTime_ = 0;
 }
@@ -106,17 +109,19 @@ RequestBackoff.prototype.reset = functio
   this.nextRequestTime_ = 0;
 }
 
 /**
  * Check to see if we can make a request.
  */
 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;
   }
 
   return (this.requestTimes_.length < this.MAX_REQUESTS_ ||
           (now - this.requestTimes_[0]) > this.REQUEST_PERIOD_);
 }
 
 RequestBackoff.prototype.noteRequest = function() {
@@ -175,17 +180,18 @@ function RequestBackoffV4(maxRequests, r
   let retryInterval = Math.floor(15 * 60 * 1000 * (rand + 1));   // 15 ~ 30 min.
   let backoffInterval = Math.floor(30 * 60 * 1000 * (rand + 1)); // 30 ~ 60 min.
 
   return new RequestBackoff(2 /* max errors */,
                 retryInterval /* retry interval, 15~30 min */,
                   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.
 var lib = this;
 
 function UrlClassifierLib() {
   this.wrappedJSObject = lib;
 }
--- a/toolkit/components/url-classifier/nsUrlClassifierListManager.js
+++ b/toolkit/components/url-classifier/nsUrlClassifierListManager.js
@@ -231,17 +231,20 @@ PROT_ListManager.prototype.requireTableU
  *  Set timer to check update after delay
  */
 PROT_ListManager.prototype.setUpdateCheckTimer = function(updateUrl,
                                                           delay) {
   this.updateCheckers_[updateUrl] = Cc["@mozilla.org/timer;1"]
                                     .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);
 }
 /**
  * Acts as a nsIUrlClassifierCallback for getTables.
  */
 PROT_ListManager.prototype.kickoffUpdate_ = function(onDiskTableData) {
   this.startingUpdate_ = false;
   var initialUpdateDelay = 3000;
--- a/toolkit/components/url-classifier/tests/unit/test_backoff.js
+++ b/toolkit/components/url-classifier/tests/unit/test_backoff.js
@@ -6,17 +6,17 @@ function setNow(time) {
   jslib.Date.now = function() {
     return 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());
   setNow(2);
   do_check_true(rb.canMakeRequest());
 
   // First error should trigger a 1ms delay
   rb.noteServerResponse(500);