From 8ff1aef751a533c17e683207dae1eb165b210f92 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 17 Jun 2015 05:08:03 +0100
Subject: Fix non-default front-end settings.

(@mrmr1993: This is yet another approach to the Settings problem.)

With the new Settings implemetation, settings which have a non-default
value and which are not in synced storage (that is, they have not been
changed since synced storage was introduced) are not currently
accessible to content scripts.  This commit makes such settings
accessible via chrome.storage.local.

Important:
- There's a change to the established settings data model here.
  Previously, settings with default values were not stored; here, they
  are.  This eliminates a considerable amount logic from Settings, but
  means that migrations will be required if default values are changed
  in future.  (Other than type changes, have we ever changed a default
  value?)
- There's also a change (bug fix?) to the behaviour when an affected
  setting is reset to its default value.  Previously, the change would
  *not* have been synced (whereas all other changes are).  Here, such
  changes *are* synced.  The previous behaviour was inconsistent with
  the syncing behaviour of all other options changes.

Note:
- This isn't particularly well tested.  It's being committed mainly just
  for consideration of the approach, initially.
---
 lib/settings.coffee | 68 ++++++++++++++++++++++++++---------------------------
 1 file changed, 33 insertions(+), 35 deletions(-)

(limited to 'lib')

diff --git a/lib/settings.coffee b/lib/settings.coffee
index 842f7618..adbe2bef 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -6,19 +6,16 @@ Settings =
   onLoadedCallbacks: []
 
   init: ->
-    if Utils.isExtensionPage()
-      # On extension pages, we use localStorage (or a copy of it) as the cache.
-      @cache = if Utils.isBackgroundPage() then localStorage else extend {}, localStorage
-      @onLoaded()
+    chrome.storage.local.get null, (localItems) =>
+      localItems = {} if chrome.runtime.lastError
+      @storage.get null, (syncedItems) =>
+        unless chrome.runtime.lastError
+          @handleUpdateFromChromeStorage key, value for own key, value of extend localItems, syncedItems
 
-    @storage.get null, (items) =>
-      unless chrome.runtime.lastError
-        @handleUpdateFromChromeStorage key, value for own key, value of items
+        chrome.storage.onChanged.addListener (changes, area) =>
+          @propagateChangesFromChromeStorage changes if area == "sync"
 
-      chrome.storage.onChanged.addListener (changes, area) =>
-        @propagateChangesFromChromeStorage changes if area == "sync"
-
-      @onLoaded()
+        @onLoaded()
 
   # Called after @cache has been initialized.  On extension pages, this will be called twice, but that does
   # not matter because it's idempotent.
@@ -37,38 +34,25 @@ Settings =
     # false, 0 or "") are truthy here.  Only null is falsy.
     if @shouldSyncKey key
       unless value and key of @cache and @cache[key] == value
-        defaultValue = @defaults[key]
-        defaultValueJSON = JSON.stringify defaultValue
-
-        if value and value != defaultValueJSON
-          # Key/value has been changed to a non-default value.
-          @cache[key] = value
-          @performPostUpdateHook key, JSON.parse value
-        else
-          # The key has been reset to its default value.
-          delete @cache[key] if key of @cache
-          @performPostUpdateHook key, defaultValue
+        value ?= JSON.stringify @defaults[key]
+        @set key, JSON.parse(value), false
 
   get: (key) ->
     console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded
     if key of @cache and @cache[key]? 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.
-    if JSON.stringify(value) == JSON.stringify @defaults[key]
-      @clear key
-    else
-      jsonValue = JSON.stringify value
-      @cache[key] = jsonValue
-      if @shouldSyncKey key
-        setting = {}; setting[key] = jsonValue
+  set: (key, value, shouldSetInSyncedStorage = true) ->
+    @cache[key] = JSON.stringify value
+    if @shouldSyncKey key
+      if shouldSetInSyncedStorage
+        setting = {}; setting[key] = @cache[key]
         @storage.set setting
-      @performPostUpdateHook key, value
+      # Remove settings installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below.
+      chrome.storage.local.remove key if Utils.isBackgroundPage()
+    @performPostUpdateHook key, value
 
   clear: (key) ->
-    delete @cache[key] if @has key
-    @storage.remove key if @shouldSyncKey key
-    @performPostUpdateHook key, @get key
+    @set key, @defaults[key]
 
   has: (key) -> key of @cache
 
@@ -169,5 +153,19 @@ if Utils.isBackgroundPage()
       rawQuery = Settings.get "findModeRawQuery"
       chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else [])
 
+  # Migration (after 1.51, 2015/6/17).
+  # Copy settings with non-default values (and which are not in synced storage) to chrome.storage.local;
+  # thereby making these settings accessible within content scripts.
+  do (migrationKey = "copyNonDefaultsToChromeStorage-20150717") ->
+    unless localStorage[migrationKey]
+      chrome.storage.sync.get null, (items) ->
+        unless chrome.runtime.lastError
+          updates = {}
+          for own key of localStorage
+            if Settings.shouldSyncKey(key) and not items[key]
+              updates[key] = localStorage[key]
+          chrome.storage.local.set updates, ->
+            localStorage[migrationKey] = not chrome.runtime.lastError
+
 root = exports ? window
 root.Settings = Settings
-- 
cgit v1.2.3