Properly save modified objects in Prefs

lib/prefs.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 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
@@ skipping 21 matching lines @@
   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%",
   notificationdata: {},
   notificationurl: "https://notification.adblockplus.org/notification.json"
 };
 
 let listeners = [];
 
+let cachedObjects = {};

[Wladimir Palant, 2013/07/24 08:49:40]

Our property getter and setter are closures, we can use a local variable to
cache their value - no need for a global one for all prefs.

+
 function defineProperty(key)
 {
   Prefs.__defineGetter__(key, function()
   {
+    if (key in cachedObjects)
+      return cachedObjects[key];
+
+    var value = null;
     if (key in localStorage)
     {
       try
       {
-        return JSON.parse(localStorage[key]);
+        value = JSON.parse(localStorage[key]);
       }
       catch(e)
       {
         Cu.reportError(e);
       }
     }
-    return defaults[key];
+    value = value || defaults[key];

[Wladimir Palant, 2013/07/24 08:49:40]

What if value is false?

+
+    if (typeof value !== "object")

[Wladimir Palant, 2013/07/24 08:49:40]

I don't really like the special case for objects here - if we cache preferences
then we should cache all of them.

+      return value;
+
+    cachedObjects[key] = JSON.parse(JSON.stringify(value));

[Wladimir Palant, 2013/07/24 08:49:40]

This is wasteful - we only need the parse/stringify dance this if we take a
value from defaults, not if we already parsed it from localStorage.

+    return cachedObjects[key];
   });
   Prefs.__defineSetter__(key, function(value)
   {
     if (typeof value != typeof defaults[key])
       throw new Error("Attempt to change preference type");
 
     let stringified = JSON.stringify(value);
     if (stringified != JSON.stringify(defaults[key]))
       localStorage[key] = stringified;
     else
       delete localStorage[key];
+
+    if (key in cachedObjects)
+      delete cachedObjects[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);