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

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 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.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";
@@ skipping 55 matching lines @@
 }
 
 function init()
 {
   let prefs = Object.keys(defaults);
   prefs.forEach(addPreference);
 
   let localLoaded = false;
   let managedLoaded = false;
 
+  let checkLoaded = function()
+  {
+    if (!localLoaded || !managedLoaded)
+      return;
+
+    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();
+  };
+
   // 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
+        if (key != "currentVersion")
         {
-          value = JSON.parse(value);
+          try
+          {
+            value = JSON.parse(value);
+          }
+          catch (e)
+          {
+            return null;
+          }
         }
-        catch (e)
-        {
-          return null;
-        }
-      }
-      else if (key != "currentVersion")
-      {
-        return null;
+
+        return {key: prefToKey(key), value: value};
       }
 
-      return {key: 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)

[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();
+        checkLoaded();
       });
     }
   });
 
   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();
+      checkLoaded();
     });
   }
   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.

+    checkLoaded();
   }
 }
 
 init();