Issue 6775 - Work around filter data stored in IndexedDB getting lost on Microsoft Edge

lib/indexedDBBackup.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
  * 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";
 
 const {FilterNotifier} = require("../adblockpluscore/lib/filterNotifier");
 const {FilterStorage} = require("../adblockpluscore/lib/filterStorage");
-const {DownloadableSubscription,
-  SpecialSubscription} =
+const {DownloadableSubscription, SpecialSubscription} =
   require("../adblockpluscore/lib/subscriptionClasses");

[Sebastian Noack, 2018/09/04 21:00:01]

Nit: No reason to wrap twice here (also more consistent with existing code):

  const {DownloadableSubscription, SpecialSubscription} = 
    require("../adblockpluscore/lib/subscriptionClasses");

[geo, 2018/09/05 14:04:33]

Done.

 
 const BACKUP_NAME = "file:indexedDB-backup";
 
-let minWriteInterval;
-let lastBackup;
-let lastRun = -minWriteInterval;
+let backupDelay;
+let pendingBackup = false;
 
 function setBackupInterval(backupInterval = 60 * 1000)
 {
-  minWriteInterval = backupInterval;
+  backupDelay = backupInterval;
 }
 
 setBackupInterval();
 
 function scheduleBackup()
 {
-  if (!lastBackup)
+  if (!pendingBackup)
   {
-    lastBackup = setTimeout(

[Sebastian Noack, 2018/09/04 21:00:01]

It seems now the only reason to have the lastBackup variable is to now if there
is a pending backup (we never clear it anymore). So a boolean (e.g. called
pendingPackup) seems more appropriate.

[geo, 2018/09/05 14:04:33]

Done.

+    pendingBackup = true;
+
+    setTimeout(
       () =>
       {
         saveToStorage();
-        lastBackup = null;
-        lastRun = performance.now();
+        pendingBackup = false;
       },
-      lastRun + minWriteInterval - performance.now()

[Sebastian Noack, 2018/09/04 21:00:01]

Just an idea, I'm wondering if a simpler throttling strategy would make more
sense. What if we always defer the backup by a fixed delay, ignoring the time
elapsed since the last backup.

With the current strategy, the first write will trigger an immediate backup, and
when one write occurs, another one is often going to follow, so that we might
end up writing another backup exactly 60 seconds later.

However, if we already defer the backup for the first write by 60 seconds, any
further write that might occur within in the next 60 seconds, wouldn't trigger
another backup. Also the implementation might be somewhat simpler.

What do you think?

[geo, 2018/09/05 14:04:33]

Yup, it does make things simpler. And you are probably right about the updates.
The first one is triggered by the `load` event, and we might as well defer it,
plus I think that is pretty likely that if a user plays with the settings,
he/she is going to change more than one setting at a time. So maybe we'll
capture them in one write instead of two.

I've also updated the tests, to reflect this change.

+      backupDelay
     );
   }
 }
 
 function saveToStorage()
 {
   browser.storage.local.set({
     [BACKUP_NAME]: {
       content: serialize(),
       lastModified: Date.now()
@@ skipping 50 matching lines @@
 FilterNotifier.on("filter.removed", scheduleBackup);
 FilterNotifier.on("filter.moved", scheduleBackup);
 FilterNotifier.on("filter.disabled", scheduleBackup);
 
 exports.IndexedDBBackup =
 {
   getBackupData,
   // Non-public API, just for tests.
   setBackupInterval
 };