Issue 4878 - Start using ESLint for adblockpluscore

lib/downloader.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 /**
  * @fileOverview Downloads a set of URLs in regular time intervals.
  */
 
 const {Utils} = require("utils");

[Wladimir Palant, 2017/03/02 14:06:43]

ESLint doesn't seem to require this change. But if you are doing it, you should
also change the declarations below - these are actual constants.

[kzar, 2017/03/08 12:33:27]

Done.

 
-let MILLIS_IN_SECOND = exports.MILLIS_IN_SECOND = 1000;
-let MILLIS_IN_MINUTE = exports.MILLIS_IN_MINUTE = 60 * MILLIS_IN_SECOND;
-let MILLIS_IN_HOUR = exports.MILLIS_IN_HOUR = 60 * MILLIS_IN_MINUTE;
-let MILLIS_IN_DAY = exports.MILLIS_IN_DAY = 24 * MILLIS_IN_HOUR;
+const MILLIS_IN_SECOND = exports.MILLIS_IN_SECOND = 1000;
+const MILLIS_IN_MINUTE = exports.MILLIS_IN_MINUTE = 60 * MILLIS_IN_SECOND;
+const MILLIS_IN_HOUR = exports.MILLIS_IN_HOUR = 60 * MILLIS_IN_MINUTE;
+const MILLIS_IN_DAY = exports.MILLIS_IN_DAY = 24 * MILLIS_IN_HOUR;
 
 let Downloader =
 /**
  * Creates a new downloader instance.
- * @param {Function} dataSource  Function that will yield downloadable objects
- *                               on each check
- * @param {number} initialDelay  Number of milliseconds to wait before the
- *                                 first check
- * @param {number} checkInterval  Interval between the checks

[Wladimir Palant, 2017/03/02 14:06:42]

Nit: Indentation is messy here. I suggest avoiding such issues by putting
comments on the next line:

 * @param {Function} dataSource
 *   Function that will yield downloadable objects on each check
 * @param {number} initialDelay
 *   Number of milliseconds to wait before the first check
 * @param {number} checkInterval
 *   Interval between the checks

[kzar, 2017/03/08 12:33:28]

Done.

+ * @param {Function} dataSource
+ *   Function that will yield downloadable objects on each check
+ * @param {number} initialDelay
+ *   Number of milliseconds to wait before the first check
+ * @param {number} checkInterval
+ *   Interval between the checks
  * @constructor
  */
 exports.Downloader = function(dataSource, initialDelay, checkInterval)
 {
   this.dataSource = dataSource;
   this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
   this._timer.initWithCallback(() =>
   {
     this._timer.delay = checkInterval;
     this._doCheck();
@@ skipping 74 matching lines @@
    * Checks whether anything needs downloading.
    */
   _doCheck()
   {
     let now = Date.now();
     for (let downloadable of this.dataSource())
     {
       if (downloadable.lastCheck &&
           now - downloadable.lastCheck > this.maxAbsenceInterval)
       {
-        // No checks for a long time interval - user must have been
-        // offline, e.g.  during a weekend. Increase soft expiration

[Wladimir Palant, 2017/03/02 14:06:44]

Nit: "offline" should go on the previous line.

[kzar, 2017/03/08 12:33:26]

Done.

-        // to prevent load peaks on the server.
+        // No checks for a long time interval - user must have been offline,
+        // e.g.  during a weekend. Increase soft expiration to prevent load
+        // peaks on the server.
         downloadable.softExpiration += now - downloadable.lastCheck;
       }
       downloadable.lastCheck = now;
 
       // Sanity check: do expiration times make sense? Make sure people changing
       // system clock don't get stuck with outdated subscriptions.
       if (downloadable.hardExpiration - now > this.maxExpirationInterval)
         downloadable.hardExpiration = now + this.maxExpirationInterval;
       if (downloadable.softExpiration - now > this.maxExpirationInterval)
         downloadable.softExpiration = now + this.maxExpirationInterval;
 
       // Notify the caller about changes to expiration parameters
       if (this.onExpirationChange)
         this.onExpirationChange(downloadable);
 
       // Does that object need downloading?
       if (downloadable.softExpiration > now &&
           downloadable.hardExpiration > now)
+      {
         continue;

[Wladimir Palant, 2017/03/02 14:06:43]

Nit: For multi-line conditions I suggest always adding brackets around the body,
makes it more obvious where the if body actually starts. Same below.

[kzar, 2017/03/08 12:33:29]

Done.

+      }
 
       // Do not retry downloads too often
       if (downloadable.lastError &&
           now - downloadable.lastError < this.minRetryInterval)
+      {
         continue;
+      }
 
       this._download(downloadable, 0);
     }
   },
 
   /**
    * Stops the periodic checks.
    */
   cancel()
   {
@@ skipping 74 matching lines @@
                      "Download address: " + downloadUrl + "\n" +
                      "Channel status: " + channelStatus + "\n" +
                      "Server response: " + responseStatus);
 
       if (this.onDownloadError)
       {
         // Allow one extra redirect if the error handler gives us a redirect URL
         let redirectCallback = null;
         if (redirects <= this.maxRedirects)
         {
-          redirectCallback = (url) =>

[Wladimir Palant, 2017/03/02 14:06:44]

Nit: Parentheses around parameter are unnecessary here.

[kzar, 2017/03/08 12:33:26]

Done. (Also added the "as-needed" arrow-parens rule, since that's the convention
here.)

+          redirectCallback = url =>
           {
             downloadable.redirectURL = url;
             this._download(downloadable, redirects + 1);
           };
         }
 
         this.onDownloadError(downloadable, downloadUrl, error, channelStatus,
                              responseStatus, redirectCallback);
       }
     }.bind(this);
@@ skipping 142 matching lines @@
    * @type {number}
    */
   hardExpiration: 0,
 
   /**
    * Number indicating how often the object was downloaded.
    * @type {number}
    */
   downloadCount: 0
 };