Make Settings explicitly use a cache

The Settings object used by the background page now uses 1 of 3 caches,
depending on the context it is available in:
* localStorage - in the background page
* a copy of localStorage - in non-background extension pages
  (options.html, popup.html, etc.)
* an empty object - in all other pages (where localStorage doesn't point
  to the extension's localStorage object).

For any extension page which is *not* the background page, a copy of
localStorage is used instead of true localStorage:
* Once localStorage is updated by one background page, the others can
  only see the updated copy.
  - Pages with an updated cache can't tell which changes are new, and so
    don't know which postUpdateHooks to run.
* By copying localStorage's contents into a new object, extension pages
  can still access settings synchronously.
  - This is especially important to options.html and popup.html; they
    will not work without it.

2 files changed, 21 insertions, 8 deletions

diff --git a/lib/settings.coffee b/lib/settings.coffee
index 607264a2..27d3efed 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -54,10 +54,20 @@ Sync =
 # Used by all parts of Vimium to manipulate localStorage.
 #
 
+# Select the object to use as the cache for settings.
+if Utils.isExtensionPage()
+  if Utils.isBackgroundPage()
+    settingsCache = localStorage
+  else
+    settingsCache = extend {}, localStorage # Make a copy of the cached settings from localStorage
+else
+  settingsCache = {}
+
 root.Settings = Settings =
+  cache: settingsCache
   init: -> Sync.init()
   get: (key) ->
-    if (key of localStorage) then JSON.parse(localStorage[key]) else @defaults[key]
+    if (key of @cache) then JSON.parse(@cache[key]) else @defaults[key]
 
   set: (key, value) ->
     # Don't store the value if it is equal to the default, so we can change the defaults in the future
@@ -65,15 +75,15 @@ root.Settings = Settings =
       @clear(key)
     else
       jsonValue = JSON.stringify value
-      localStorage[key] = jsonValue
+      @cache[key] = jsonValue
       Sync.set key, jsonValue
 
   clear: (key) ->
     if @has key
-      delete localStorage[key]
+      delete @cache[key]
     Sync.clear key
 
-  has: (key) -> key of localStorage
+  has: (key) -> key of @cache
 
   # For settings which require action when their value changes, add hooks to this object, to be called from
   # options/options.coffee (when the options page is saved), and by Settings.storeAndPropagate (when an
@@ -87,18 +97,18 @@ root.Settings = Settings =
   # Only ever called from asynchronous synced-storage callbacks (fetchAsync and handleStorageUpdate).
   storeAndPropagate: (key, value) ->
     return unless key of @defaults
-    return if value and key of localStorage and localStorage[key] is value
+    return if value and key of @cache and @cache[key] is value
     defaultValue = @defaults[key]
     defaultValueJSON = JSON.stringify(defaultValue)
 
     if value and value != defaultValueJSON
       # Key/value has been changed to non-default value at remote instance.
-      localStorage[key] = value
+      @cache[key] = value
       @performPostUpdateHook key, JSON.parse(value)
     else
       # Key has been reset to default value at remote instance.
-      if key of localStorage
-        delete localStorage[key]
+      if key of @cache
+        delete @cache[key]
       @performPostUpdateHook key, defaultValue
 
   # options.coffee and options.html only handle booleans and strings; therefore all defaults must be booleans
diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee
index 946a1688..ded7b5f8 100644
--- a/tests/unit_tests/settings_test.coffee
+++ b/tests/unit_tests/settings_test.coffee
@@ -3,6 +3,8 @@ extend global, require "./test_chrome_stubs.js"
 
 extend(global, require "../../lib/utils.js")
 Utils.getCurrentVersion = -> '1.44'
+Utils.isBackgroundPage = -> true
+Utils.isExtensionPage = -> true
 global.localStorage = {}
 extend(global,require "../../lib/settings.js")
 
@@ -10,6 +12,7 @@ context "settings",
 
   setup ->
     stub global, 'localStorage', {}
+    Settings.cache = global.localStorage # Point the settings cache to the new localStorage object.
     Settings.postUpdateHooks = {} # Avoid running update hooks which include calls to outside of settings.
     Settings.init()