Issue 5059 - Simplify I/O API and FilterStorage implementation

lib/filterStorage.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 45 matching lines @@
   {
     return formatVersion;
   },
 
   /**
    * File containing the filter list
    * @type {string}
    */
   get sourceFile()
   {
     return "patterns.ini";

[Wladimir Palant, 2017/04/10 14:59:15]

We are removing configurability here, there is no point any more.

   },
 
   /**
    * Will be set to true if no patterns.ini file exists.
    * @type {boolean}
    */
   firstRun: false,
 
   /**
    * Map of properties listed in the filter storage file before the sections
@@ skipping 374 matching lines @@
         parser(null);
         if (this.subscriptions.length == 0)
         {
           // No filter subscriptions in the file, this isn't right.
           throw new Error("No data in the file");
         }
       });
     }).catch(error =>
     {
       Cu.reportError(error);
       return tryBackup(1);

[kzar, 2017/04/20 06:49:25]

So we return a promise here, which might recursively return more promises while
we attempt to restore from our backups. But if tryBackup gives up it will catch
the error and I think the promise here will resolve anyway. Is that what you
intended? It seems kind of counter-intuitive that the promise isn't rejected in
that case.

[Wladimir Palant, 2017/04/20 07:17:06]

Yes, this is the intended behavior - we already reported an error, and we are
attempting a fallback now. There is little point reporting another error because
the fallback failed as well.

     }).then(() =>
     {
       this.initialized = true;
       FilterNotifier.triggerListeners("load");

[Wladimir Palant, 2017/04/10 14:59:15]

There is a change here: load notification is fired and initialized flag is set
only after we tried backups. This restores the logic we had before
https://issues.adblockplus.org/ticket/5052.

     });
   },
 
   /**
    * Constructs the file name for a patterns.ini backup.
    * @param {number} backupIndex
    *    number of the backup file (1 being the most recent)
    * @return {string} backup file name
    */
   getBackupName(backupIndex)
@@ skipping 133 matching lines @@
       {
         if (index > 0)
         {
           return IO.renameFile(this.getBackupName(index),
                                this.getBackupName(index + 1))
                    .catch(ignoreErrors)
                    .then(() => renameBackup(index - 1));
         }
 
         return IO.renameFile(this.sourceFile, this.getBackupName(1))
                  .catch(ignoreErrors);

[Wladimir Palant, 2017/04/10 14:59:15]

The logic is slightly simplified by not removing the last backup explicitly any
more. The assumption is that IO.renameFile() will always overwrite existing
files (which it does for the remaining implementations).

       };
 
       // Rename existing files
       return renameBackup(Prefs.patternsbackups - 1);
     }).catch(error =>
     {
       // Errors during backup creation shouldn't prevent writing filters.
       Cu.reportError(error);
     }).then(() =>
     {
@@ skipping 25 matching lines @@
    */
 
   /**
    * Returns a promise resolving in a list of existing backup files.
    * @return {Promise.<FileInfo[]>}
    */
   getBackupFiles()
   {
     let backups = [];
 
-    function checkBackupFile(index)
+    let checkBackupFile = index =>
     {
       return IO.statFile(this.getBackupName(index)).then(statData =>
       {
         if (!statData.exists)
           return backups;
 
         backups.push({
           index,
           lastModified: statData.lastModified
         });
         return checkBackupFile(index + 1);
       }).catch(error =>
       {
         // Something went wrong, return whatever data we got so far.
         Cu.reportError(error);
         return backups;
       });
-    }
+    };
 
     return checkBackupFile(1);
   }
 };
 
 /**
  * Joins subscription's filters to the subscription without any notifications.
  * @param {Subscription} subscription
  *   filter subscription that should be connected to its filters
  */
@@ skipping 123 matching lines @@
       else if (this.wantObj === false && val)
         this.curObj.push(val.replace(/\\\[/g, "["));
     }
     finally
     {
       Filter.knownFilters = origKnownFilters;
       Subscription.knownSubscriptions = origKnownSubscriptions;
     }
   }
 };