Issue 1488 - Add pre-configurable preference to suppress first run page

lib/prefs.js

Index: lib/prefs.js
===================================================================
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -17,22 +17,33 @@
 
 const keyPrefix = "pref:";
 
-let prefs = Object.create(null);
-let listeners = [];
+let defaults = Object.create(null);
+let overrides = Object.create(null);
+

[kzar, 2015/03/19 15:20:37]

Maybe these defaults should be stored in their own section in the metadata files
instead? It seems like we're muddling up data and code here. (I guess it's kind
of offtopic for this review though.)

+defaults.enabled = true;
+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;
+defaults.suppress_first_run_page = false;
 
 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)
@@ -48,33 +59,28 @@
   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);
       }
     },
@@ -82,70 +88,87 @@
   });
 }
 
-function setPreference(pref, value)
+function init()
 {
-  prefs[pref] = value;
+  let prefs = Object.keys(defaults);
+  prefs.forEach(addPreference);
 
-  for (let listener of listeners)
-    listener(pref);
+  let localLoaded = false;
+  let managedLoaded = false;
+
+  // 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)
+      {
+        key = prefToKey(key);
+        try
+        {
+          value = JSON.parse(value);
+        }
+        catch (e)
+        {
+          return null;
+        }
+      }
+      else if (key != "currentVersion")
+      {
+        return null;
+      }
+
+      return {key: key, value: value};
+    },
+
+    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)

[Sebastian Noack, 2015/03/20 13:26:12]

The key is what's stored in ext.storage (e.g. "pref:enabled"). pref is the name
of the pref (e.g. "enabled") as it is defined above.

+            {
+              let change = changes[key];
+              if ("newValue" in change)
+                overrides[pref] = change.newValue;
+              else
+                delete overrides[pref];
+
+              Prefs.onChanged._dispatch(pref);
+            }
+          }
+        });
+
+        localLoaded = true;
+        if (managedLoaded)

[Sebastian Noack, 2015/03/20 13:26:12]

Done.

+          Prefs.onLoaded._dispatch();
+      });
+    }
+  });
+
+  if (require("info").platform == "chromium" && "managed" in chrome.storage)
+  {
+    chrome.storage.managed.get(null, function(items)
+    {
+      for (let key in items)
+        defaults[key] = items[key];
+
+      managedLoaded = true;
+      if (localLoaded)
+        Prefs.onLoaded._dispatch();
+    });
+  }
+  else
+  {
+    managedLoaded = true;

[Wladimir Palant, 2015/03/19 16:57:04]

This relies on the callbacks to read local prefs to execute asynchronously,
otherwise localLoaded might get set to true before we get here. That assumption
is currently only true on Safari because you made the callback use setTimeout().
Maybe just move reading managed prefs up, before we read local prefs?
Alternatively, this branch could get the same conditional Prefs.onLoaded call
and be consistent.

+  }
 }
 
-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(function(key, value)
-{
-  if (key in prefs)
-  {
-    key = prefToKey(key);
-    try
-    {
-      value = JSON.parse(value);
-    }
-    catch (e)
-    {
-      return null;
-    }
-  }
-  else if (key != "currentVersion")
-  {
-    return null;
-  }
-
-  return {key: key, value: value};
-});
-
-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();