Issue 4721 - Use IndexedDB for storage in Edge

edge/ext/background.js

+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2016 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/>.
+ */
+
+(function()
+{
+  /* 
+  * Storage redirection to localforage. In Windows 10 Anniversary Update

[kzar, 2016/12/13 10:24:04]

The comment looks much more useful now thanks. Could you fix the indentation
though?

/*
 *
 ...
 */

not

/*
*
...
 */

(Yes you'll need to wrap the long lines.)

+  * Microsoft Edge has a 1Mb limit on the amount of bytes per transaction in
+  * storage.local. Also unlimitedStorage permision is not honoured. Which is why
+  * we have to use IndexedDB until those issues are fixed in Creators Update.
+  * For reference it is supposed to be fixed in Windows 10 build 14986.  
+   */
+
+  ext.storage = {
+    get: function(keys, callback)
+    {
+      var promisedValues = keys.map(key =>
+      {
+        return localforage.getItem(key);
+      });
+      Promise.all(promisedValues).then(values =>
+      {
+        var items = {};
+        for (let i = 0; i < keys.length; i++)
+        {
+          if (values[i] != null)
+            items[keys[i]] = values[i];
+        }
+        callback(items);
+    }).catch(err =>

[kzar, 2016/12/13 10:24:04]

The indentation here for the catch is wrong. Also why not just do something like
this?

.catch(callback.bind(null, null));

+    {
+      callback(null);
+    });
+    },
+    set: function(key, value, callback)
+    {
+      function internalSet(oldValue)

[kzar, 2016/12/13 10:24:04]

Nit: setThenDispatch or setThenCallback or something like that?

+      {
+        localforage.setItem(key, value).then(() =>
+        {
+          if (callback)
+            callback(null);
+          var changes = {};

[kzar, 2016/12/13 10:24:04]

Please move the `var changes = {};` line down, so that the blank line is above
it. That keeps the event dispatching logic together.

+          var change = changes[key] = {};
+          change.oldValue = oldValue;
+          change.newValue = value;
+
+          ext.storage.onChanged._dispatch(changes);
+        }).catch((err) => 
+        {
+          if (callback)
+            callback(err);
+        });
+      };
+      
+      localforage.getItem(key).then(oldValue => internalSet(oldValue))

[kzar, 2016/12/13 10:24:04]

No need to wrap internalSet with another function. Also then can take a second
argument for the onRejected function. How about this?

localforage.getItem(key).then(internalSet, internalSet.bind(null, null));

+        .catch(err => internalSet(null));
+    },
+    remove: function(key, callback)
+    {
+      localforage.removeItem(key, callback);

[kzar, 2016/12/13 10:24:04]

Why wrap localforage.removeItem at all? How about this?

remove: localforage.removeItem,

+    },
+    onChanged: new ext._EventTarget()
+  };
+})();