Implemented prefs module properly, data is actually being persisted now

lib/prefs.js

Index: lib/prefs.js
===================================================================
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -14,23 +14,76 @@
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 //
 // The values are hardcoded for now.
 //
 
-var Prefs = exports.Prefs = {
+let defaults = {
+  __proto__: null,
   enabled: true,
   patternsfile: "patterns.ini",
   patternsbackups: 5,
   patternsbackupinterval: 24,
   data_directory: "",
   savestats: false,
   privateBrowsing: false,
   subscriptions_fallbackerrors: 5,
   subscriptions_fallbackurl: "https://adblockplus.org/getSubscription?version=%VERSION%&url=%SUBSCRIPTION%&downloadURL=%URL%&error=%ERROR%&channelStatus=%CHANNELSTATUS%&responseStatus=%RESPONSESTATUS%",
   subscriptions_autoupdate: true,
   subscriptions_exceptionsurl: "https://easylist-downloads.adblockplus.org/exceptionrules.txt",
   documentation_link: "https://adblockplus.org/redirect?link=%LINK%&lang=%LANG%",
-  addListener: function() {}
 };
+
+let listeners = [];
+
+function defineProperty(key)
+{
+  Prefs.__defineGetter__(key, function()
+  {
+    if (key in localStorage)
+    {
+      try
+      {
+        return JSON.parse(localStorage[key]);
+      }
+      catch(e)
+      {
+        Cu.reportError(e);
+      }
+    }
+    return defaults[key];
+  });
+  Prefs.__defineSetter__(key, function(value)
+  {
+    if (typeof value != typeof defaults[key])
+      throw new Error("Attempt to change preference type");

[Thomas Greiner, 2013/07/25 13:20:36]

I'd also include a check for the existence of the key before that because even
though this check catches it anyway, "Attempt to change preference type" is not
the best response for that case.

e.g.
if (!(key in defaults))
  throw new Error("Attempt to set unknown preference");

Alternatively, you could simply change the error message to something like
"Attempt to set unknown preference or change type of an existing preference" but
personally, I'm not a fan of that.

[Wladimir Palant, 2013/07/26 09:44:02]

The property handler is only added for properties that exist in the defaults -
so we cannot possibly get here if there is no default.

I don't think we can catch an attempt to set a random property without some
major effort.

+
+    let stringified = JSON.stringify(value);
+    if (stringified != JSON.stringify(defaults[key]))
+      localStorage[key] = stringified;
+    else
+      delete localStorage[key];
+    for each (let listener in listeners)
+      listener(key);
+  });
+}
+
+
+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);
+  },
+};
+
+for (let key in defaults)
+  defineProperty(key);