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");
 
+const BACKUP_NAME = "file:indexedDB-backup";
 
-const BACKUP_NAME = "file:indexedDB-backup";
-let MIN_WRITE_INTERVAL;
-let pendingUpdate;
-let isSaving;
-let lastBackup;
-let lastRun;
-let saveToStorage;
+let backupDelay;
+let pendingBackup = false;
 
-function init(backupInterval, saveFunction)
+function setBackupInterval(backupInterval = 60 * 1000)
 {
-  clearInterval(lastBackup);
-  pendingUpdate = false;
-  isSaving = false;
-  lastRun = false;
-  MIN_WRITE_INTERVAL = backupInterval || 60 * 1000;

[Sebastian Noack, 2018/08/21 20:26:24]

Just out of curiosity? Could you experience any performance difference with vs
without this delay (in terms of measurable CPU usage, and/or irresponsive UI)?

[geo, 2018/08/31 15:49:24]

I've tried to measure CPU usage without too much luck, but when it comes to UI,
it really depends on what the user does.
I don't notice any difference between toggling subscriptions or adding more.
Custom filters are a different story. I've tested with about 433KB. Saving
custom filters is slow at this point, without performing any backup so having a
small delay here is beneficial.
Maybe it would be better to handle different updates separately. I'm not sure.

-  saveToStorage = saveFunction ||

[Sebastian Noack, 2018/08/21 20:26:24]

I'd rather mock browser.storage.local.set() in the tests than passing in this
function.

[geo, 2018/08/31 15:49:24]

Done.

-  function()
-  {
-    return browser.storage.local.set({
-      [BACKUP_NAME]: {
-        content: serialize(),
-        lastModified: Date.now()
-      }
-    });
-  };
+  backupDelay = backupInterval;
 }
 
-init();
+setBackupInterval();
 
 function scheduleBackup()
 {
-  pendingUpdate = true;
-  if (!lastRun)
+  if (!pendingBackup)
   {
-    lastRun = performance.now();
-    pendingUpdate = false;
-    isSaving = true;
+    pendingBackup = true;
 
-    saveToStorage().then(() =>
-    {
-      isSaving = false;
-      lastRun = performance.now();
-      if (pendingUpdate)
-        scheduleBackup();
-    });
-  }
-  else
-  {
-    clearInterval(lastBackup);
-
-    lastBackup = setTimeout(() =>
-    {
-      if (!isSaving && (performance.now() - lastRun) >= MIN_WRITE_INTERVAL)
+    setTimeout(
+      () =>
       {
-        isSaving = true;
-        pendingUpdate = false;
-        lastRun = performance.now();
-        saveToStorage().then(() =>
-        {
-          isSaving = false;
-          lastRun = performance.now();
-          if (pendingUpdate)
-            scheduleBackup();
-        });
-      }
-    }, MIN_WRITE_INTERVAL - (performance.now() - lastRun));
+        saveToStorage();
+        pendingBackup = false;
+      },
+      backupDelay
+    );
   }
 }
 
-function serialize(subscriptions = FilterStorage.subscriptions)
+function saveToStorage()
+{
+  browser.storage.local.set({
+    [BACKUP_NAME]: {
+      content: serialize(),
+      lastModified: Date.now()
+    }
+  });
+}
+
+function serialize()
 {
   let buffer = [];
-  for (let subscription of subscriptions)
+
+  for (let subscription of FilterStorage.subscriptions)
   {
     if (subscription instanceof SpecialSubscription)
     {
-      buffer.push("");

[Sebastian Noack, 2018/08/21 20:26:24]

Do we have to add an empty line here?

[geo, 2018/08/31 15:49:24]

Done.

       subscription.serialize(buffer);
-      buffer.push("", "[Subscription filters]");
+      buffer.push("[Subscription filters]");
       subscription.serializeFilters(buffer);
     }
     else if (subscription instanceof DownloadableSubscription)
     {
       let {homepage, title, url, disabled} = subscription;
 
       buffer.push(

[Sebastian Noack, 2018/08/21 20:26:24]

Can't we just use Subscription.serialize() (but without calling
Subscription.serializeFilters()) here?

[geo, 2018/08/31 15:49:24]

As discussed on IRC, I'm leaving the current implementation as it is, since
Subscription.serialize() add some extra fields that we don't need.

-        "",
         "[Subscription]",
         `homepage=${homepage}`,
         `title=${title}`,
         `url=${url}`,
-        `disabled=${disabled}`,
+        `disabled=${disabled}`
       );
     }
   }
   return buffer;
 }
 
 function getBackupData()
 {
   return browser.storage.local.get(BACKUP_NAME).then(items =>
   {
@@ skipping 12 matching lines @@
 FilterNotifier.on("subscription.disabled", scheduleBackup);
 FilterNotifier.on("filter.added", scheduleBackup);
 FilterNotifier.on("filter.removed", scheduleBackup);
 FilterNotifier.on("filter.moved", scheduleBackup);
 FilterNotifier.on("filter.disabled", scheduleBackup);
 
 exports.IndexedDBBackup =
 {
   getBackupData,
   // Non-public API, just for tests.
-  init,
-  scheduleBackup,

[Sebastian Noack, 2018/08/21 20:26:24]

Can't we just emit one of the above events in the tests rather than calling
sheduleBackup directly?

[geo, 2018/08/31 15:49:24]

Done.

-  serialize
+  setBackupInterval
 };