Issue 4023 - Move storage of subscription lists to localStorage

lib/io.js

Index: lib/io.js
===================================================================
--- a/lib/io.js
+++ b/lib/io.js
@@ -15,6 +15,8 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+"use strict";
+
 const keyPrefix = "file:";
 
 function fileToKey(file)
@@ -22,33 +24,36 @@
   return keyPrefix + (file instanceof FakeFile ? file.path : file.spec);
 }
 
-function loadFile(file, successCallback, errorCallback)
+function loadFile(file)
 {
-  let key = fileToKey(file);
-
-  ext.storage.get([key], function(items)
+  return new Promise((resolve, reject) =>
   {
-    let entry = items[key];
-
-    if (entry)
-      successCallback(entry);
-    else
-      errorCallback(new Error("File doesn't exist"));
+    let key = fileToKey(file);
+    ext.storage.get([key], items =>
+    {
+      let entry = items[key];
+      if (!entry)
+      {
+        // Check in localStorage
+        try
+        {
+          entry = JSON.parse(window.localStorage.getItem(key));
+        }
+        catch(err)
+        {
+          entry = null;          

[kzar, 2016/08/22 15:38:32]

Nit: `entry = null` seems redundant since we already know entry is falsey here.

[Oleksandr, 2016/08/31 21:54:35]

Done.

+        }
+      }
+      if (entry)
+        resolve(entry);
+      else
+      {

[kzar, 2016/08/22 15:38:32]

Nit: Please remove the braces for the else clause.

[Oleksandr, 2016/08/31 21:54:35]

Done.

+        reject(new Error("File doesn't exist"));
+      }
+    });
   });
 }
 
-function saveFile(file, data, callback)
-{
-  ext.storage.set(
-    fileToKey(file),
-    {
-      content: data,
-      lastModified: Date.now()
-    },
-    callback
-  );
-}
-
 exports.IO =
 {
   resolveFilePath: function(path)
@@ -60,47 +65,44 @@
   {
     function onLoaded(entry)
     {
-      for (let line of entry.content)
-        listener.process(line);
-
-      listener.process(null);
+      if ("content" in entry)
+      {
+        if ("compressed" in entry)
+          entry.content = JSON.parse(LZString.decompressFromUTF16(entry.content));
+        for (let line of entry.content)
+          listener.process(line);
+      }
       callback(null);
     }
 
-    loadFile(file, onLoaded, callback);
+    loadFile(file).then(onLoaded, callback);
   },
 
   writeToFile: function(file, data, callback)
   {
-    saveFile(file, data, callback);
-  },
+    let entry = {};
+    var key = fileToKey(file);

[kzar, 2016/08/22 15:38:32]

You seem to use let and var inconsistently throughout.

[Oleksandr, 2016/08/31 21:54:35]

Done.

 
-  copyFile: function(fromFile, toFile, callback)

[kzar, 2016/08/22 15:38:32]

copyFile and renameFile have been removed?

[Oleksandr, 2016/08/31 21:54:35]

Yes.

[kzar, 2016/09/01 12:57:37]

But aren't they needed by adblockpluscore/lib/filerStorage.js?

[Oleksandr, 2016/09/02 04:12:26]

I don't think they are. Looks like we are not doing backups anymore, for which
those were used. At least I wasn't able to invoke that code. I have actually
removed it because of Sebastian's comment on this code review:
https://codereview.adblockplus.org/29339112/diff/29339113/lib/io.js#newcode35

[kzar, 2016/09/02 08:57:49]

Ah Ok, fair enough.

-  {
-    function onLoaded(entry)
+    // Edge cannot write files larger than 1Mb (500k string in UTF-16).
+    // However it does not always raise an exception when trying to do so.
+    if (typeof browser == "undefined")

[kzar, 2016/08/22 15:38:32]

Since this code is for the adblockplusedge repository I wonder why we bother do
the `typeof browser == "undefined"` check at all. We already know this is Edge
right?

[Sebastian Noack, 2016/08/22 16:49:19]

As discussed on IRC, this should go into a bookmark in adblockpluschrome rather
than into a separate repo. Moreover, at some point we will merge those changes
upstream, using this code on Edge, Chrome, and Safari.

[Oleksandr, 2016/08/31 21:54:36]

Acknowledged.

     {
-      saveFile(toFile, entry.content, callback);
+      entry[key] = {lastModified: Date.now(), 
+                    content: data};
+      ext.storage.set(entry, callback);
     }
-
-    loadFile(fromFile, onLoaded, callback);
-  },
-
-  renameFile: function(fromFile, newName, callback)
-  {
-    function onLoaded()
+    else
     {
-      ext.storage.remove(fileToKey(fromFile), function()
-      {
-        ext.storage.set(keyPrefix + newName, entry, callback);
-      });
+      // Compress and fallback to localStorage
+      var dataString = JSON.stringify(data);

[kzar, 2016/08/22 15:38:32]

Perhaps do the JSON serialization and compression in one step? That would be
consistent with how you do the reverse process and I guess would avoid keeping a
reference to both. (Not sure that keeping a reference to both would matter, but
since they're both large strings maybe it would.)

[Oleksandr, 2016/08/31 21:54:35]

Done.

+      var processedData = LZString.compressToUTF16(dataString);
+      chrome.storage.local.remove(key);

[kzar, 2016/08/22 15:38:32]

Did you mean to use chrome.storage instead of ext.storage here?

[Oleksandr, 2016/08/31 21:54:36]

Done.

+      entry[key] = {lastModified: Date.now(), 
+                    content: processedData,
+                    compressed: true};
+      window.localStorage.setItem(key, JSON.stringify(entry[key]));
+      callback();
     }
-
-    loadFile(fromFile, onLoaded, callback);
-  },
-
-  removeFile: function(file, callback)
-  {
-    ext.storage.remove(fileToKey(file), callback);
   },
 
   statFile: function(file, callback)
@@ -113,6 +115,6 @@
       });
     }
 
-    loadFile(file, onLoaded, callback);
+    loadFile(file).then(onLoaded, callback);
   }
 };