Response to @philc's comments regarding sync.

1 files changed, 22 insertions, 28 deletions

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 1c6c7fb6..56c74b81 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -1,12 +1,12 @@
 #
 # * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync.
-# * Sync.listener() listens for changes to chrome.storage.sync and propagates those
+# * Sync.handleStorageUpdate() listens for changes to chrome.storage.sync and propagates those
 #   changes to localStorage and into vimium's internal state.
-# * Sync.pull() polls chrome.storage.sync at startup, similarly propagating
+# * Sync.fetchAsync() polls chrome.storage.sync at startup, similarly propagating
 #   changes to localStorage and into vimium's internal state.
 #
 # Changes are propagated into vimium's state using the same mechanism
-# (Settings.doPostUpdateHook) that is used when options are changed on
+# (Settings.performPostUpdateHook) that is used when options are changed on
 # the options page.
 #
 # The effect is best-effort synchronization of vimium options/settings between
@@ -26,40 +26,38 @@ root.Sync = Sync =
   # However, if users have problems, they are unlikely to notice and make sense of console logs on
   # background pages.  So disable it, by default.
   # For genuine errors, we call console.log directly.
-  debug: false
+  debug: true
   storage: chrome.storage.sync
   doNotSync: [ "settingsVersion", "previousVersion" ]
 
-  # This is called in main().
+  # This is called in main.coffee.
   init: ->
-    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
-    @pull()
+    chrome.storage.onChanged.addListener (changes, area) -> Sync.handleStorageUpdate changes, area
+    @fetchAsync()
 
   # Asynchronous fetch from synced storage, called only at startup.
-  pull: ->
+  fetchAsync: ->
     @storage.get null, (items) =>
       # Chrome sets chrome.runtime.lastError if there is an error.
       if chrome.runtime.lastError is undefined
         for own key, value of items
-          @log "pull: #{key} <- #{value}"
+          @log "fetchAsync: #{key} <- #{value}"
           @storeAndPropagate key, value
       else
-        console.log "callback for Sync.pull() indicates error"
+        console.log "callback for Sync.fetchAsync() indicates error"
         console.log chrome.runtime.lastError
 
   # Asynchronous message from synced storage.
-  listener: (changes, area) ->
+  handleStorageUpdate: (changes, area) ->
     for own key, change of changes
-      @log "listener: #{key} <- #{change.newValue}"
+      @log "handleStorageUpdate: #{key} <- #{change.newValue}"
       @storeAndPropagate key, change?.newValue
   
-  # Only ever called from asynchronous synced-storage callbacks (pull and listener).
+  # Only ever called from asynchronous synced-storage callbacks (fetchAsync and handleStorageUpdate).
   storeAndPropagate: (key, value) ->
     return if not key of Settings.defaults
-    return if not @isSyncKey key
+    return if not @shouldSyncKey key
     return if value and key of localStorage and localStorage[key] is value
-
-    # Ok: store and propagate this update.
     defaultValue = Settings.defaults[key]
     defaultValueJSON = JSON.stringify(defaultValue)
 
@@ -67,20 +65,22 @@ root.Sync = Sync =
       # Key/value has been changed to non-default value at remote instance.
       @log "storeAndPropagate update: #{key}=#{value}"
       localStorage[key] = value
-      Settings.doPostUpdateHook key, JSON.parse(value)
+      Settings.performPostUpdateHook key, JSON.parse(value)
     else
       # Key has been reset to default value at remote instance.
       @log "storeAndPropagate clear: #{key}"
       if key of localStorage
         delete localStorage[key]
-      Settings.doPostUpdateHook key, defaultValue
+      Settings.performPostUpdateHook key, defaultValue
 
   # Only called synchronously from within vimium, never on a callback.
   # No need to propagate updates to the rest of vimium, that's already been done.
   set: (key, value) ->
-    if @isSyncKey key
+    if @shouldSyncKey key
       @log "set scheduled: #{key}=#{value}"
-      @storage.set @mkKeyValue(key,value), =>
+      key_value = {}
+      key_value[key] = value
+      @storage.set key_value, =>
         # Chrome sets chrome.runtime.lastError if there is an error.
         if chrome.runtime.lastError
           console.log "callback for Sync.set() indicates error: #{key} <- #{value}"
@@ -88,7 +88,7 @@ root.Sync = Sync =
 
   # Only called synchronously from within vimium, never on a callback.
   clear: (key) ->
-    if @isSyncKey key
+    if @shouldSyncKey key
       @log "clear scheduled: #{key}"
       @storage.remove key, =>
         # Chrome sets chrome.runtime.lastError if there is an error.
@@ -97,15 +97,9 @@ root.Sync = Sync =
           console.log chrome.runtime.lastError
 
   # Should we synchronize this key?
-  isSyncKey: (key) ->
+  shouldSyncKey: (key) ->
     key not in @doNotSync
 
-  # There has to be a more elegant way to do this!
-  mkKeyValue: (key, value) ->
-    obj = {}
-    obj[key] = value
-    obj
-
   log: (msg) ->
     console.log "Sync: #{msg}" if @debug