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

lib/ioIndexedDB.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 139 matching lines @@
   };
 }
 
 function getObjectStore(dbInstance, storeName)
 {
   return dbInstance
     .transaction([storeName], IDBTransaction.READ_WRITE)
     .objectStore(storeName);
 }
 
-function reestablishConnection(dbInstance, retries)
+function reestablishConnection(dbInstance, retries = 10)
 {
   dbInstance.close();
-  retries = retries || 1;
-
-  return new Promise((resolve) =>
-  {
-    setTimeout(() =>
-    {
-      db = openDB(dbConfig);
-      return resolve(db);
-    }, 0);

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

Is this timeout necessary?

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

This part here is mostly from experimentation. If I try to call
`indexedDB.openDB` before calling `close` I'm getting errors. I couldn't find an
event for when the close operation completes, hence the timeout. However it
seems that it works on the second try without the timeout.
Done.

[Sebastian Noack, 2018/08/31 18:07:04]

Ok, let's stick with the delay then.

-  })
-  .catch(err =>
-  {
-    if (retries == 10)
-      return Promise.reject(err);

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

Nit: You can just throw the error in here. This will have the same effect as
returning a rejected promise.

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

Done.

-
-    return reestablishConnection(dbInstance, ++retries);
+  db = openDB(dbConfig);
+
+  return db.catch(err =>
+  {
+    if (!retries)
+      throw err;
+
+    return reestablishConnection(dbInstance, --retries);
   });
 }
 
 function getFile(fileName, dbInstance, storeName)
 {
   return getFromIndexedDB(fileToKey(fileName), dbInstance, storeName)
     .then(indexedDBResult =>
     {
       if (!indexedDBResult)
       {
         const {IndexedDBBackup} = require("./indexedDBBackup");
 
         return IndexedDBBackup.getBackupData()

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

I wonder whether we also have to trigger the subscriptions to be downloaded
again, since the backup doesn't included any filter data for downloadable
subscriptions?

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

No need with the information we are currently saving. 

           .then(backupData =>
             saveFile(
               {
                 fileName: fileToKey(fileName),
                 content: backupData.content,
                 lastModified: backupData.lastModified
               },
               dbInstance,
-              storeName)
-            .then(() => backupData));
+              storeName).then(() => backupData)
+          );
       }
       return indexedDBResult;
     });
 }
 
 function getFromIndexedDB(fileName, dbInstance, storeName)
 {
   return new Promise((resolve, reject) =>
   {
     let store = getObjectStore(dbInstance, storeName);
     let req = store.get(fileName);
 
     req.onsuccess = event => resolve(event.currentTarget.result);
     req.onerror = event => reject(event.target.error);
   })
   .catch(error =>
   {
     if (error.name == "UnknownError")
-      return reestablishConnection(dbInstance)
-        .then(() => Promise.resolve());

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

Nit: This seems redundant.

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

I want this to resolve with `undefined` so that `getFile` will check in
`local.storage` next. I can remove it, but I have to change the `getFile` logic.

[Sebastian Noack, 2018/08/31 18:07:04]

I see, then this should be:

  .then(() => undefined)

The behavior will essentially be the same, but it's less code, and more obvious
that you specifically want to return undefined here.

[geo, 2018/09/04 16:12:07]

Done.

-    return Promise.reject(error);

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

Nit: See above.

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

Done.

+      return reestablishConnection(dbInstance).then(() => undefined);
   });
 }
 
 function saveFile(data, dbInstance, storeName)
 {
   return new Promise((resolve, reject) =>
   {
     let store = getObjectStore(dbInstance, storeName);
     let req = store.put(data);
 
     req.onsuccess = resolve;
     req.onerror = event => reject(event.target.error);
   })
   .catch(error =>
   {
     if (error.name == "UnknownError")
     {
-      return reestablishConnection(dbInstance)
-      .then(newDbInstance =>
-        saveFile(data, newDbInstance, storeName));
+      return reestablishConnection(dbInstance).then(newDbInstance =>
+        saveFile(data, newDbInstance, storeName)
+      );
     }
-    return Promise.reject(error);
   });
 }
 
 function deleteFile(fileName, dbInstance, storeName)
 {
   return new Promise((resolve, reject) =>
   {
     let store = getObjectStore(dbInstance, storeName);
     let req = store.delete(fileToKey(fileName));
 
     req.onsuccess = resolve;
     req.onerror = event => reject(event.target.error);
   })
   .catch(error =>
   {
     if (error.name == "UnknownError")
-    {
-      return reestablishConnection(dbInstance)
-        .then(() => Promise.resolve());

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

Nit: This seems redundant.

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

Done.

-    }
-
-    return Promise.reject(error);

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

Nit: See above.

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

Done.

+      return reestablishConnection(dbInstance);
   });
 }
 
 exports.IO =
 {
   /**
    * Writes text lines to a file.
    * @param {string} fileName
    *    Name of the file to be written
    * @param {Iterable.<string>} data
@@ skipping 83 matching lines @@
                 fileName: fileToKey(newName),
                 content: fileData.content,
                 lastModified: fileData.lastModified
               },
               dbInstance,
               dbConfig.storeName))
           .then(() => deleteFile(fromFile, dbInstance, dbConfig.storeName))));
   }
 };