Issue 2040 - Replaced localStorage with chrome.storage.local

lib/prefs.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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/>.
  */
 
 const keyPrefix = "pref:";
 
-let prefs = Object.create(null);
-let listeners = [];
+let defaults = Object.create(null);
+let overrides = Object.create(null);
+
+defaults.enabled = true;
+defaults.currentVersion = "";
+defaults.data_directory = "";
+defaults.patternsbackups = 5;
+defaults.patternsbackupinterval = 24;
+defaults.savestats = false;
+defaults.privateBrowsing = false;
+defaults.subscriptions_fallbackerrors = 5;
+defaults.subscriptions_fallbackurl = "https://adblockplus.org/getSubscription?version=%VERSION%&url=%SUBSCRIPTION%&downloadURL=%URL%&error=%ERROR%&channelStatus=%CHANNELSTATUS%&responseStatus=%RESPONSESTATUS%";
+defaults.subscriptions_autoupdate = true;
+defaults.subscriptions_exceptionsurl = "https://easylist-downloads.adblockplus.org/exceptionrules.txt";
+defaults.subscriptions_antiadblockurl = "https://easylist-downloads.adblockplus.org/antiadblockfilters.txt";
+defaults.documentation_link = "https://adblockplus.org/redirect?link=%LINK%&lang=%LANG%";
+defaults.notificationdata = {};
+defaults.notificationurl = "https://notification.adblockplus.org/notification.json";
+defaults.stats_total = {};
+defaults.show_statsinicon = true;
+defaults.show_statsinpopup = true;
+defaults.shouldShowBlockElementMenu = true;
+defaults.hidePlaceholders = true;
 
 let Prefs = exports.Prefs = {
-  addListener: function(listener)
-  {
-    if (listeners.indexOf(listener) < 0)
-      listeners.push(listener);
-  },
-
-  removeListener: function(listener)
-  {
-    let index = listeners.indexOf(listener);
-    if (index >= 0)
-      listeners.splice(index, 1);
-  }
+  onChanged: new ext._EventTarget(),
+  onLoaded: new ext._EventTarget()
 };
 
 function keyToPref(key)
 {
   if (key.indexOf(keyPrefix) != 0)
     return null;
 
   return key.substr(keyPrefix.length);
 }
 
 function prefToKey(pref)
 {
   return keyPrefix + pref;
 }
 
-function addPreference(pref, defaultValue)
+function addPreference(pref)
 {
-  prefs[pref] = undefined;
-
   Object.defineProperty(Prefs, pref, {
     get: function()
     {
-      let value = prefs[pref];
-
-      if (typeof value != "undefined")
-        return value;
-
-      return defaultValue;
+      return (pref in overrides ? overrides : defaults)[pref];
     },
     set: function(value)
     {
+      let defaultValue = defaults[pref];
+
       if (typeof value != typeof defaultValue)
         throw new Error("Attempt to change preference type");
 
       if (value == defaultValue)
       {
-        prefs[pref] = undefined;
+        delete overrides[pref];
         ext.storage.remove(prefToKey(pref));
       }
       else
       {
-        prefs[pref] = value;
+        overrides[pref] = value;
         ext.storage.set(prefToKey(pref), value);
       }
     },
     enumerable: true
   });
 }
 
-function setPreference(pref, value)
+function init()
 {
-  prefs[pref] = value;
+  let prefs = Object.keys(defaults);
+  prefs.forEach(addPreference);
 
-  for (let listener of listeners)
-    listener(pref);
+  // Migrate preferences for users updating from old versions.
+  // TODO: Remove the migration code after a few releases.
+  ext.storage.migratePrefs({
+    map: function(key, value)
+    {
+      if (key in defaults)
+      {
+        if (key != "currentVersion")
+        {
+          try
+          {
+            value = JSON.parse(value);
+          }
+          catch (e)
+          {
+            return null;
+          }
+        }
+
+        return {key: prefToKey(key), value: value};
+      }
+
+      return null;
+    },
+
+    done: function()
+    {
+      ext.storage.get(prefs.map(prefToKey), function(items)
+      {
+        for (let key in items)
+          overrides[keyToPref(key)] = items[key];
+
+        ext.storage.onChanged.addListener(function(changes)
+        {
+          for (let key in changes)
+          {
+            let pref = keyToPref(key);
+            if (pref && pref in defaults)
+            {
+              let change = changes[key];
+              if ("newValue" in change && change.newValue != defaults[pref])
+                overrides[pref] = change.newValue;
+              else
+                delete overrides[pref];
+
+              Prefs.onChanged._dispatch(pref);
+            }
+          }
+        });
+
+        Prefs.onLoaded._dispatch();
+      });
+    }
+  });
 }

[Wladimir Palant, 2015/03/12 23:05:37]

The flow here is rather convoluted, a change to prefs.foo does through several
instances and prefs[pref] is set twice. The point seems to be considering
external changes (is there even a UI for that?). However, if we do that more
effort is required:

* What if an external change changes preference type? We probably want to log an
error and fall back to default.

* What if an external change stores the default value in storage? We probably
want prefs[pref] to be undefined then.

Did you find code paths that rely on Prefs.foo value changing synchronously?

[Sebastian Noack, 2015/03/12 23:35:29]

Currently there is only one case where we externally change prefs, that is the
migration code. However, in the long-term when we don't import the Prefs object
into the options page anymore, calling ext.storage.local.set will be the easiest
and only cross-platform way for users to set hidden preferences. Moreover, when
we implement chrome.storage.managed it there can be external changes we have to
respond to via chrome.storage.on changed as well. So yes we need to handle
external changes.

Do we need to check the type for those? Not for chrome.storage.managed since it
is strictly typed anyway (we have to provide a manifest listing the possible
keys and the type of their values). For the migration code it can be ignored
IMO. The prefs imported there were matter of the same type checks before. So
they should be valid unless somebody messed with localStorage directly, a case
that also wasn't handled before. For the case that some user manually calls
chrhrome.storage.set this might make sense, but when messing with the JS console
there are so many ways to screw up ABP that it isnt probably worth handling this
particular one.

 
-addPreference("enabled", true);
-addPreference("data_directory", "");
-addPreference("patternsbackups", 5);
-addPreference("patternsbackupinterval", 24);
-addPreference("savestats", false);
-addPreference("privateBrowsing", false);
-addPreference("subscriptions_fallbackerrors", 5);
-addPreference("subscriptions_fallbackurl", "https://adblockplus.org/getSubscription?version=%VERSION%&url=%SUBSCRIPTION%&downloadURL=%URL%&error=%ERROR%&channelStatus=%CHANNELSTATUS%&responseStatus=%RESPONSESTATUS%");
-addPreference("subscriptions_autoupdate", true);
-addPreference("subscriptions_exceptionsurl", "https://easylist-downloads.adblockplus.org/exceptionrules.txt");
-addPreference("subscriptions_antiadblockurl", "https://easylist-downloads.adblockplus.org/antiadblockfilters.txt");
-addPreference("documentation_link", "https://adblockplus.org/redirect?link=%LINK%&lang=%LANG%");
-addPreference("notificationdata", {});
-addPreference("notificationurl", "https://notification.adblockplus.org/notification.json");
-addPreference("stats_total", {});
-addPreference("show_statsinicon", true);
-addPreference("show_statsinpopup", true);
-addPreference("shouldShowBlockElementMenu", true);
-addPreference("hidePlaceholders", true);
-
-// Migrate preferences for users updating from old versions.
-// TODO: Remove the migration code after a few releases.
-ext.storage.migratePrefs(prefs);
-
-ext.storage.get(Object.keys(prefs).map(prefToKey), function(items)
-{
-  for (let key in items)
-    setPreference(keyToPref(key), items[key]);
-
-  ext.storage.onChanged.addListener(function(changes)
-  {
-    for (let key in changes)
-    {
-      let pref = keyToPref(key);
-      if (pref && pref in prefs)
-        setPreference(pref, changes[key].newValue);
-    }
-  });
-});
+init();