Issue 6741 - Use ES2015 classes in lib/downloader.js

lib/downloader.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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
@@ skipping 14 matching lines @@
 
 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;
 
 class Downloader
 {
   /**
    * Creates a new downloader instance.
-   * @param {Function} dataSource

[Manish Jethani, 2018/08/22 06:13:34]

Another JSDoc convention that I think that we should follow is to use the
lower-case type name for primitive types [1]. In this case, let's make it
`function` instead of `Function` (throughout the file).

[1]:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Data_...

[Jon Sonesen, 2018/08/22 18:18:51]

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(dataSource, initialDelay, checkInterval)
   {
     /**
      * Maximal time interval that the checks can be left out until the soft
@@ skipping 16 matching lines @@
     this.maxExpirationInterval = 14 * MILLIS_IN_DAY;
 
     /**
      * Maximal number of redirects before the download is considered as failed.
      * @type {number}
      */
     this.maxRedirects = 5;
 
     /**
      * Called whenever expiration intervals for an object need to be adapted.
-     * @type {Function}
+     * @type {function?}
      */
     this.onExpirationChange = null;
 
     /**
      * Callback to be triggered whenever a download starts.
-     * @type {Function}
+     * @type {function?}
      */
     this.onDownloadStarted = null;
 
     /**
      * Callback to be triggered whenever a download finishes successfully. The
      * callback can return an error code to indicate that the data is wrong.
-     * @type {Function}
+     * @type {function?}
      */
     this.onDownloadSuccess = null;
 
     /**
      * Callback to be triggered whenever a download fails.
-     * @type {Function}
+     * @type {function?}
      */
     this.onDownloadError = null;
 
     /**
      * Function that will yield downloadable objects on each check.
-     * @type {Function}
+     * @type {function}
      */
     this.dataSource = dataSource;
 
     /**
      * Timer triggering the downloads.
      * @type {nsITimer}
      */
     this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
     this._timer.initWithCallback(() =>
     {
@@ skipping 59 matching lines @@
    * Stops the periodic checks.
    */
   cancel()
   {
     this._timer.cancel();
   }
 
   /**
    * Checks whether an address is currently being downloaded.
    * @param {string} url
-   * @return {boolean}
+   * @returns {boolean}
    */
   isDownloading(url)
   {
     return this._downloading.has(url);
   }
 
   /**
    * Starts downloading for an object.
    * @param {Downloadable} downloadable
    */
   download(downloadable)
   {
     // Make sure to detach download from the current execution context
     Utils.runAsync(this._download.bind(this, downloadable, 0));
   }
 
   /**
    * Generates the real download URL for an object by appending various
    * parameters.
    * @param {Downloadable} downloadable
-   * @return {string}
+   * @returns {string}
    */
   getDownloadUrl(downloadable)
   {
     const {addonName, addonVersion, application, applicationVersion,
            platform, platformVersion} = require("info");
     let url = downloadable.redirectURL || downloadable.url;
     if (url.includes("?"))
       url += "&";
     else
       url += "?";
@@ skipping 127 matching lines @@
 
     this._downloading.add(downloadable.url);
     if (this.onDownloadStarted)
       this.onDownloadStarted(downloadable);
   }
 
   /**
    * Produces a soft and a hard expiration interval for a given supplied
    * expiration interval.
    * @param {number} interval
-   * @return {Array} soft and hard expiration interval
+   * @returns {Array.<number>} soft and hard expiration interval
    */
   processExpirationInterval(interval)
   {
     interval = Math.min(Math.max(interval, 0), this.maxExpirationInterval);
     let soft = Math.round(interval * (Math.random() * 0.4 + 0.8));
     let hard = interval * 2;
     let now = Date.now();
     return [now + soft, now + hard];
   }
 }
+
 exports.Downloader = Downloader;
 
 class Downloadable
 {
   /**
-   * An object that can be downloaded by the downloadable

[Manish Jethani, 2018/08/22 06:13:35]

I think this should be "Creates an object that can be downloaded by the
downloader." (Note: downloader, not downloadable (that's a mistake), and a
period at the end.)

[Jon Sonesen, 2018/08/22 18:18:51]

Done.

+   * Creates an object that can be downloaded by the downloader.
    * @param {string} url  URL that has to be requested for the object
    */
   constructor(url)
   {
+    /**
+     * URL that the download was redirected to if any.
+     * @type {string?}
+     */
+    this.redirectURL = null;
+
+    /**
+     * Time of last download error or 0 if the last download was successful.
+     * @type {number}
+     */
+    this.lastError = 0;
+
+    /**
+     * Time of last check whether the object needs downloading.
+     * @type {number}
+     */
+    this.lastCheck = 0;
+
+    /**
+     * Object version corresponding to the last successful download.
+     * @type {number}
+     */
+    this.lastVersion = 0;
+
+    /**
+     * Soft expiration interval; will increase if no checks are performed for a
+     * while.
+     * @type {number}
+     */
+    this.softExpiration = 0;
+
+    /**
+     * Hard expiration interval; this is fixed.
+     * @type {number}
+     */
+    this.hardExpiration = 0;
+
+    /**
+     * Number indicating how often the object was downloaded.
+     * @type {number}
+     */
+    this.downloadCount = 0;
+
     /**
      * URL that has to be requested for the object.
      * @type {string}
      */
-    this.url = null;

[Manish Jethani, 2018/08/22 06:13:35]

Let's also just assign `this.url` directly at the end of the constructor and
move the JSDoc comment there.

[Jon Sonesen, 2018/08/22 18:18:51]

Done.

-
-    /**
-     * URL that the download was redirected to if any.
-     * @type {string}
-     */
-    this.redirectURL = null;
-
-    /**
-     * Time of last download error or 0 if the last download was successful.
-     * @type {number}
-     */
-    this.lastError = 0;
-
-    /**
-     * Time of last check whether the object needs downloading.
-     * @type {number}
-     */
-    this.lastCheck = 0;
-
-    /**
-     * Object version corresponding to the last successful download.
-     * @type {number}
-     */
-    this.lastVersion = 0;
-
-    /**
-     * Soft expiration interval; will increase if no checks are performed for a
-     * while.
-     * @type {number}
-     */
-    this.softExpiration = 0;
-
-    /**
-     * Hard expiration interval; this is fixed.
-     * @type {number}
-     */
-    this.hardExpiration = 0;
-
-    /**
-     * Number indicating how often the object was downloaded.
-     * @type {number}
-     */
-    this.downloadCount = 0;
     this.url = url;
   }
 }
+
 exports.Downloadable = Downloadable;