From 3aa47a198f2424ba91bb7a8ac07d6b2c53da9698 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 2 Nov 2012 13:10:33 +0000
Subject: Initial synchronization commit. Synchronization is via
 `chrome.storage.sync.*`; data is cached in `localStorage`.

---
 background_scripts/settings.coffee |  44 ++++++++++--
 background_scripts/sync.coffee     | 136 +++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+), 6 deletions(-)
 create mode 100644 background_scripts/sync.coffee

(limited to 'background_scripts')

diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index 0fe1e1bb..64fce308 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -7,17 +7,49 @@ root.Settings = Settings =
   get: (key) ->
     if (key of localStorage) then JSON.parse(localStorage[key]) else @defaults[key]
 
-  set: (key, value) ->
+  # The doNotSync argument suppresses calls to chrome.storage.sync.* while running unit tests
+  set: (key, value, doNotSync) ->
     # don't store the value if it is equal to the default, so we can change the defaults in the future
-    if (value == @defaults[key])
-      @clear(key)
-    else
-      localStorage[key] = JSON.stringify(value)
+    # warning: this test is always false for settings with numeric default values (such as scrollStepSize)
+    if ( value == @defaults[key] )
+      return @clear(key,doNotSync)
+    # don't update the key/value if it's unchanged; thereby suppressing unnecessary calls to chrome.storage
+    valueJSON = JSON.stringify value
+    if localStorage[key] == valueJSON
+      return localStorage[key]
+    # we have a new value: so update chrome.storage and localStorage
+    root.Sync.set key, valueJSON if root?.Sync?.set
+    localStorage[key] = valueJSON
 
-  clear: (key) -> delete localStorage[key]
+  # The doNotSync argument suppresses calls to chrome.storage.sync.* while running unit tests
+  clear: (key, doNotSync) ->
+    if @has key
+      root.Sync.clear key if root?.Sync?.clear
+      delete localStorage[key]
 
   has: (key) -> key of localStorage
 
+  # the postUpdateHooks handler below is called each time an option changes:
+  #    either from options/options.coffee          (when the options page is saved)
+  #        or from background_scripts/sync.coffee  (when an update propagates from chrome.storage)
+  # 
+  # NOTE:
+  # this has been refactored and renamed from options.coffee(postSaveHooks):
+  #   - refactored because it is now also called from sync.coffee
+  #   - renamed because it is no longer associated only with "Save" operations
+  #
+  postUpdateHooks:
+    keyMappings: (value) ->
+      root.Commands.clearKeyMappingsAndSetDefaults()
+      root.Commands.parseCustomKeyMappings value
+      root.refreshCompletionKeysAfterMappingSave()
+  
+  # postUpdateHooks convenience wrapper
+  doPostUpdateHook: (key, value) ->
+    if @postUpdateHooks[key]
+      @postUpdateHooks[key] value 
+
+
   # options/options.(coffee|html) only handle booleans and strings; therefore
   # all defaults must be booleans or strings
   defaults:
diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
new file mode 100644
index 00000000..d3a18b85
--- /dev/null
+++ b/background_scripts/sync.coffee
@@ -0,0 +1,136 @@
+
+#
+# * Sync.set() and Sync.clear() propagate local changes to chrome.storage.
+# * Sync.listener() listens for changes to chrome.storage and propagates those
+#   changes to localStorage and into vimium's internal state.
+# * Sync.pull() polls chrome.storage at startup, similarly propagating changes
+#   to localStorage and into vimium's internal state.
+#
+# Changes are propagated into vimium's state using the same mechanism that is
+# used when options are changed on the options page.
+#
+# The effect is best-effort synchronization of vimium options/settings between
+# chrome/vimium instances, whenever:
+#   - chrome is logged in to the user's Google account, and
+#   - chrome synchronization is enabled.
+#
+# NOTE:
+#   Values handled within this module are ALWAYS already JSON.stringifed, so
+#   they're always non-empty strings.
+#
+
+console.log ">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"
+root = exports ? window
+root.Sync = Sync = 
+
+  # ##################
+  # constants
+
+  debug: true
+  storage: chrome.storage.sync
+  doNotSync: [ "settingsVersion", "previousVersion" ]
+
+  init: ->
+    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
+    @pull()
+    @log "Sync.init()"
+
+  # asynchronous fetch from synced storage, called at startup
+  pull: ->
+    @storage.get null, (items) ->
+      Sync.log "pull callback: #{Sync.callbackStatus()}"
+      if not chrome.runtime.lastError
+        for own key, value of items
+          Sync.storeAndPropagate key, value
+
+  # asynchronous message from synced storage
+  listener: (changes, area) ->
+    @log "listener: #{area}"
+    for own key, change of changes
+      @storeAndPropagate key, change.newValue
+  
+  # only ever called from asynchronous synced-storage callbacks (pull and listener)
+  storeAndPropagate: (key, value) ->
+    # must be JSON.stringifed or undefined
+    @checkHaveStringOrUndefined value
+    # ignore, if not accepting this key
+    if not @syncKey key
+       @log "callback ignoring: #{key}"
+       return
+    # ignore, if unchanged
+    if localStorage[key] == value
+       @log "callback unchanged: #{key}"
+       return
+
+    # ok: accept, store and propagate update
+    defaultValue = root.Settings.defaults[key]
+    defaultValueJSON = JSON.stringify(defaultValue) # could cache this to avoid repeated recalculation
+
+    if value && value != defaultValueJSON
+      # key/value has been changed to non-default value at remote instance
+      @log "callback set: #{key}=#{value}"
+      localStorage[key] = value
+      root.Settings.doPostUpdateHook key, JSON.parse(value)
+    else
+      # key has been reset to default value at remote instance
+      @log "callback clear: #{key}=#{value}"
+      delete localStorage[key]
+      root.Settings.doPostUpdateHook key, defaultValue
+
+  # only called synchronously from within vimium, never on a callback
+  # no need to propagate updates into the rest of vimium (because that will already have been handled externally)
+  set: (key, value) ->
+    # value must be JSON.stringifed
+    @checkHaveString value
+    if value
+      if @syncKey key
+        @storage.set @mkKeyValue(key,value), -> Sync.logCallback "DONE set", key
+        @log "set scheduled: #{key}=#{value}"
+    else
+      # unreachable? (because value is a JSON string)
+      @log "UNREACHABLE in Sync.set(): #{key}"
+      @clear key
+
+  # only called synchronously from within vimium, never on a callback
+  # no need to propagate updates into the rest of  vimium (because that will already have been handled by externally)
+  clear: (key) ->
+    if @syncKey key
+      @storage.remove key, -> Sync.logCallback "DONE clear", key
+      @log "clear scheduled: #{key}"
+
+  # ##################
+  # utilities 
+
+  syncKey: (key) ->
+    key not in @doNotSync
+
+  # there has to be a more elegant way to do this!
+  mkKeyValue: (key, value) ->
+    obj = {}
+    obj[key] = value
+    obj
+
+  # debugging messages
+  # disable these by setting root.Sync.debug to anything falsy
+  log: (msg) ->
+    console.log "sync debug: #{msg}" if @debug
+
+  logCallback: (where, key) ->
+    @log "#{where} callback: #{key} #{@callbackStatus()}"
+
+  callbackStatus: ->
+    if chrome.runtime.lastError then "ERROR: #{chrome.runtime.lastError.message}" else "(OK)"
+
+  checkHaveString: (thing) ->
+    if typeof(thing) != "string" or not thing
+      @log "sync.coffee: Yikes! this should be a non-empty string: #{typeof(thing)} #{thing}"
+
+  checkHaveStringOrUndefined: (thing) ->
+    if ( typeof(thing) != "string" and typeof(thing) != "undefined" ) or ( typeof(thing) == "string" and not thing )
+      @log "sync.coffee: Yikes! this should be a non-empty string or undefined: #{typeof(thing)} #{thing}"
+  
+  # end of Sync object
+  # ##################
+
+Sync.init()
+
-- 
cgit v1.2.3


From 75234c78cf13be08608ccd2b6bcecf2757b199ff Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 18 Apr 2014 23:09:39 +0100
Subject: Code review of Sync().

---
 background_scripts/settings.coffee |  37 ++++------
 background_scripts/sync.coffee     | 139 +++++++++++++++++--------------------
 2 files changed, 78 insertions(+), 98 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index 64fce308..063287db 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -7,47 +7,36 @@ root.Settings = Settings =
   get: (key) ->
     if (key of localStorage) then JSON.parse(localStorage[key]) else @defaults[key]
 
-  # The doNotSync argument suppresses calls to chrome.storage.sync.* while running unit tests
   set: (key, value, doNotSync) ->
     # don't store the value if it is equal to the default, so we can change the defaults in the future
-    # warning: this test is always false for settings with numeric default values (such as scrollStepSize)
-    if ( value == @defaults[key] )
-      return @clear(key,doNotSync)
-    # don't update the key/value if it's unchanged; thereby suppressing unnecessary calls to chrome.storage
-    valueJSON = JSON.stringify value
-    if localStorage[key] == valueJSON
-      return localStorage[key]
-    # we have a new value: so update chrome.storage and localStorage
-    root.Sync.set key, valueJSON if root?.Sync?.set
-    localStorage[key] = valueJSON
+    if (value == @defaults[key])
+      @clear(key)
+    else
+      jsonified = JSON.stringify value
+      localStorage[key] = jsonified
+      root.Sync.set key, jsonified
 
-  # The doNotSync argument suppresses calls to chrome.storage.sync.* while running unit tests
-  clear: (key, doNotSync) ->
+  clear: (key) ->
     if @has key
-      root.Sync.clear key if root?.Sync?.clear
       delete localStorage[key]
+    root.Sync.clear key
 
   has: (key) -> key of localStorage
 
-  # the postUpdateHooks handler below is called each time an option changes:
-  #    either from options/options.coffee          (when the options page is saved)
-  #        or from background_scripts/sync.coffee  (when an update propagates from chrome.storage)
-  # 
-  # NOTE:
-  # this has been refactored and renamed from options.coffee(postSaveHooks):
-  #   - refactored because it is now also called from sync.coffee
-  #   - renamed because it is no longer associated only with "Save" operations
+  # postUpdateHooks are called each time an option changes:
+  #   either from options/options.coffee          (when the options page is saved)
+  #       or from background_scripts/sync.coffee  (when an update propagates from chrome.storage)
   #
   postUpdateHooks:
     keyMappings: (value) ->
       root.Commands.clearKeyMappingsAndSetDefaults()
       root.Commands.parseCustomKeyMappings value
       root.refreshCompletionKeysAfterMappingSave()
-  
+
   # postUpdateHooks convenience wrapper
   doPostUpdateHook: (key, value) ->
     if @postUpdateHooks[key]
-      @postUpdateHooks[key] value 
+      @postUpdateHooks[key] value
 
 
   # options/options.(coffee|html) only handle booleans and strings; therefore
diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index d3a18b85..7cb77518 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -1,30 +1,25 @@
 
 #
-# * Sync.set() and Sync.clear() propagate local changes to chrome.storage.
-# * Sync.listener() listens for changes to chrome.storage and propagates those
+# * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync.
+# * Sync.listener() 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
 #   changes to localStorage and into vimium's internal state.
-# * Sync.pull() polls chrome.storage at startup, similarly propagating changes
-#   to localStorage and into vimium's internal state.
 #
-# Changes are propagated into vimium's state using the same mechanism that is
-# used when options are changed on the options page.
+# Changes are propagated into vimium's state using the same mechanism
+# (Settings.doPostUpdateHook) that is used when options are changed on
+# the options page.
 #
 # The effect is best-effort synchronization of vimium options/settings between
-# chrome/vimium instances, whenever:
-#   - chrome is logged in to the user's Google account, and
-#   - chrome synchronization is enabled.
+# chrome/vimium instances.
 #
 # NOTE:
 #   Values handled within this module are ALWAYS already JSON.stringifed, so
 #   they're always non-empty strings.
 #
 
-console.log ">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"
 root = exports ? window
-root.Sync = Sync = 
-
-  # ##################
-  # constants
+root.Sync = Sync =
 
   debug: true
   storage: chrome.storage.sync
@@ -33,104 +28,100 @@ root.Sync = Sync =
   init: ->
     chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
     @pull()
-    @log "Sync.init()"
 
-  # asynchronous fetch from synced storage, called at startup
+  # Asynchronous fetch from synced storage, called only at startup.
   pull: ->
     @storage.get null, (items) ->
-      Sync.log "pull callback: #{Sync.callbackStatus()}"
-      if not chrome.runtime.lastError
+      if chrome.runtime.lastError is undefined
         for own key, value of items
-          Sync.storeAndPropagate key, value
+          @storeAndPropagate key, value
+      else
+        @log "chrome sync callback for Sync.pull() indicates error"
+        @log chrome.runtime.lastError
 
-  # asynchronous message from synced storage
+  # Asynchronous message from synced storage.
   listener: (changes, area) ->
-    @log "listener: #{area}"
     for own key, change of changes
       @storeAndPropagate key, change.newValue
   
-  # only ever called from asynchronous synced-storage callbacks (pull and listener)
+  # Only ever called from asynchronous synced-storage callbacks (pull and listener).
   storeAndPropagate: (key, value) ->
-    # must be JSON.stringifed or undefined
-    @checkHaveStringOrUndefined value
-    # ignore, if not accepting this key
-    if not @syncKey key
-       @log "callback ignoring: #{key}"
+    # Value must be JSON.stringifed or undefined.
+    if not @checkHaveStringOrUndefined value
+      return
+    # Ignore, we're not accepting this key.
+    if not @isSyncKey key
+       @log "ignoring: #{key}"
        return
-    # ignore, if unchanged
-    if localStorage[key] == value
-       @log "callback unchanged: #{key}"
+    # Ignore, it's unchanged
+    if localStorage[key] is value
+       @log "unchanged: #{key}"
        return
 
-    # ok: accept, store and propagate update
+    # Ok: accept, store and propagate this update.
     defaultValue = root.Settings.defaults[key]
-    defaultValueJSON = JSON.stringify(defaultValue) # could cache this to avoid repeated recalculation
+    defaultValueJSON = JSON.stringify(defaultValue)
 
     if value && value != defaultValueJSON
-      # key/value has been changed to non-default value at remote instance
-      @log "callback set: #{key}=#{value}"
+      # Key/value has been changed to non-default value at remote instance.
+      @log "update: #{key}=#{value}"
       localStorage[key] = value
       root.Settings.doPostUpdateHook key, JSON.parse(value)
     else
-      # key has been reset to default value at remote instance
-      @log "callback clear: #{key}=#{value}"
+      # Key has been reset to default value at remote instance.
+      @log "clear: #{key}"
       delete localStorage[key]
       root.Settings.doPostUpdateHook key, defaultValue
 
-  # only called synchronously from within vimium, never on a callback
-  # no need to propagate updates into the rest of vimium (because that will already have been handled externally)
+  # Only called synchronously from within vimium, never on a callback.
+  # No need to propagate updates into the rest of vimium.
   set: (key, value) ->
-    # value must be JSON.stringifed
-    @checkHaveString value
-    if value
-      if @syncKey key
-        @storage.set @mkKeyValue(key,value), -> Sync.logCallback "DONE set", key
-        @log "set scheduled: #{key}=#{value}"
-    else
-      # unreachable? (because value is a JSON string)
-      @log "UNREACHABLE in Sync.set(): #{key}"
-      @clear key
-
-  # only called synchronously from within vimium, never on a callback
-  # no need to propagate updates into the rest of  vimium (because that will already have been handled by externally)
+    # value has already been JSON.stringifed
+    if not @checkHaveString value
+      return
+    #
+    if @isSyncKey key
+      @storage.set @mkKeyValue(key,value), ->
+        if chrome.runtime.lastError
+          @log "chrome sync callback for Sync.set() indicates error: " + key
+          @log chrome.runtime.lastError
+      @log "set scheduled: #{key}=#{value}"
+
+  # Only called synchronously from within vimium, never on a callback.
   clear: (key) ->
-    if @syncKey key
-      @storage.remove key, -> Sync.logCallback "DONE clear", key
-      @log "clear scheduled: #{key}"
-
-  # ##################
-  # utilities 
-
-  syncKey: (key) ->
+    if @isSyncKey key
+      @storage.remove key, ->
+        if chrome.runtime.lastError
+          @log "chrome sync callback for Sync.clear() indicates error: " + key
+          @log chrome.runtime.lastError
+
+  # Should we synchronize this key?
+  isSyncKey: (key) ->
     key not in @doNotSync
 
-  # there has to be a more elegant way to do this!
+  # There has to be a more elegant way to do this!
   mkKeyValue: (key, value) ->
     obj = {}
     obj[key] = value
     obj
 
-  # debugging messages
-  # disable these by setting root.Sync.debug to anything falsy
+  # Debugging messages.
+  # Disable debugginf by setting root.Sync.debug to anything falsy.
+  # Enabled for the time being (18/4/14) -- smblott.
   log: (msg) ->
-    console.log "sync debug: #{msg}" if @debug
-
-  logCallback: (where, key) ->
-    @log "#{where} callback: #{key} #{@callbackStatus()}"
-
-  callbackStatus: ->
-    if chrome.runtime.lastError then "ERROR: #{chrome.runtime.lastError.message}" else "(OK)"
+    console.log "Sync: #{msg}" if @debug
 
   checkHaveString: (thing) ->
     if typeof(thing) != "string" or not thing
-      @log "sync.coffee: Yikes! this should be a non-empty string: #{typeof(thing)} #{thing}"
+      @log "Sync: Yikes! this should be a non-empty string: #{typeof(thing)} #{thing}"
+      return false
+    return true
 
   checkHaveStringOrUndefined: (thing) ->
     if ( typeof(thing) != "string" and typeof(thing) != "undefined" ) or ( typeof(thing) == "string" and not thing )
-      @log "sync.coffee: Yikes! this should be a non-empty string or undefined: #{typeof(thing)} #{thing}"
+      @log "Sync: Yikes! this should be a non-empty string or undefined: #{typeof(thing)} #{thing}"
+      return false
+    return true
   
-  # end of Sync object
-  # ##################
-
 Sync.init()
 
-- 
cgit v1.2.3


From 654738cb4d566c5e0b2d1e229dd9055403af35f6 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 00:13:36 +0100
Subject: Code review of Sync().

---
 background_scripts/settings.coffee | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index 063287db..ad89888e 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -7,14 +7,14 @@ root.Settings = Settings =
   get: (key) ->
     if (key of localStorage) then JSON.parse(localStorage[key]) else @defaults[key]
 
-  set: (key, value, doNotSync) ->
+  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 (value == @defaults[key])
       @clear(key)
     else
-      jsonified = JSON.stringify value
-      localStorage[key] = jsonified
-      root.Sync.set key, jsonified
+      jsonValue = JSON.stringify value
+      localStorage[key] = jsonValue
+      root.Sync.set key, jsonValue
 
   clear: (key) ->
     if @has key
-- 
cgit v1.2.3


From 033d353b265fcd4e9963ef232c17d82d7bea43f1 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 07:41:31 +0100
Subject: Settings tests pass, Sync still has an error.

---
 background_scripts/settings.coffee | 4 ++--
 background_scripts/sync.coffee     | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index ad89888e..f75c1db3 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -14,12 +14,12 @@ root.Settings = Settings =
     else
       jsonValue = JSON.stringify value
       localStorage[key] = jsonValue
-      root.Sync.set key, jsonValue
+      Sync.set key, jsonValue
 
   clear: (key) ->
     if @has key
       delete localStorage[key]
-    root.Sync.clear key
+    Sync.clear key
 
   has: (key) -> key of localStorage
 
diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 7cb77518..40e0c5c3 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -59,19 +59,19 @@ root.Sync = Sync =
        return
 
     # Ok: accept, store and propagate this update.
-    defaultValue = root.Settings.defaults[key]
+    defaultValue = Settings.defaults[key]
     defaultValueJSON = JSON.stringify(defaultValue)
 
     if value && value != defaultValueJSON
       # Key/value has been changed to non-default value at remote instance.
       @log "update: #{key}=#{value}"
       localStorage[key] = value
-      root.Settings.doPostUpdateHook key, JSON.parse(value)
+      Settings.doPostUpdateHook key, JSON.parse(value)
     else
       # Key has been reset to default value at remote instance.
       @log "clear: #{key}"
       delete localStorage[key]
-      root.Settings.doPostUpdateHook key, defaultValue
+      Settings.doPostUpdateHook key, defaultValue
 
   # Only called synchronously from within vimium, never on a callback.
   # No need to propagate updates into the rest of vimium.
-- 
cgit v1.2.3


From 91a32f9ebe577aace222f615e17e924018b232e5 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 07:54:09 +0100
Subject: Functional sync.

---
 background_scripts/sync.coffee | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 40e0c5c3..e498172e 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -31,9 +31,10 @@ root.Sync = Sync =
 
   # Asynchronous fetch from synced storage, called only at startup.
   pull: ->
-    @storage.get null, (items) ->
+    @storage.get null, (items) =>
       if chrome.runtime.lastError is undefined
         for own key, value of items
+          @log "pull: #{key} <- #{value}"
           @storeAndPropagate key, value
       else
         @log "chrome sync callback for Sync.pull() indicates error"
@@ -42,6 +43,7 @@ root.Sync = Sync =
   # Asynchronous message from synced storage.
   listener: (changes, area) ->
     for own key, change of changes
+      @log "listener: #{key} <- #{change.newValue}"
       @storeAndPropagate key, change.newValue
   
   # Only ever called from asynchronous synced-storage callbacks (pull and listener).
-- 
cgit v1.2.3


From 8e0bb0a5c749cfae9586dd797d96a0f6233f828c Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 08:01:29 +0100
Subject: Fix indentation and disable sync.log() messages.

---
 background_scripts/sync.coffee | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index e498172e..afde3e70 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -21,7 +21,7 @@
 root = exports ? window
 root.Sync = Sync =
 
-  debug: true
+  debug: false
   storage: chrome.storage.sync
   doNotSync: [ "settingsVersion", "previousVersion" ]
 
@@ -124,6 +124,6 @@ root.Sync = Sync =
       @log "Sync: Yikes! this should be a non-empty string or undefined: #{typeof(thing)} #{thing}"
       return false
     return true
-  
+
 Sync.init()
 
-- 
cgit v1.2.3


From a2de5078f8d34c2b65b451556aff07a6a41399a5 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 09:44:50 +0100
Subject: Add test cases for "asynchronous" sync.

---
 background_scripts/sync.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index afde3e70..3a829722 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -83,7 +83,7 @@ root.Sync = Sync =
       return
     #
     if @isSyncKey key
-      @storage.set @mkKeyValue(key,value), ->
+      @storage.set @mkKeyValue(key,value), =>
         if chrome.runtime.lastError
           @log "chrome sync callback for Sync.set() indicates error: " + key
           @log chrome.runtime.lastError
-- 
cgit v1.2.3


From 5ea93cd18611d859867bdd80c44c886d29a7b958 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 10:02:14 +0100
Subject: Add remote sync test for setting cleared.

---
 background_scripts/sync.coffee | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 3a829722..3b34c5a6 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -56,7 +56,7 @@ root.Sync = Sync =
        @log "ignoring: #{key}"
        return
     # Ignore, it's unchanged
-    if localStorage[key] is value
+    if key of localStorage and localStorage[key] is value
        @log "unchanged: #{key}"
        return
 
@@ -64,7 +64,7 @@ root.Sync = Sync =
     defaultValue = Settings.defaults[key]
     defaultValueJSON = JSON.stringify(defaultValue)
 
-    if value && value != defaultValueJSON
+    if value and value != defaultValueJSON
       # Key/value has been changed to non-default value at remote instance.
       @log "update: #{key}=#{value}"
       localStorage[key] = value
@@ -72,7 +72,8 @@ root.Sync = Sync =
     else
       # Key has been reset to default value at remote instance.
       @log "clear: #{key}"
-      delete localStorage[key]
+      if key of localStorage
+        delete localStorage[key]
       Settings.doPostUpdateHook key, defaultValue
 
   # Only called synchronously from within vimium, never on a callback.
-- 
cgit v1.2.3


From 222d53fe43e34d8d2fd7c2fd859de53cf5fee1f3 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 11:00:32 +0100
Subject: Delay initializing sync until other components have started.

---
 background_scripts/main.coffee | 3 +++
 background_scripts/sync.coffee | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index f564f477..dc853803 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -596,3 +596,6 @@ chrome.windows.getAll { populate: true }, (windows) ->
       createScrollPositionHandler = ->
         (response) -> updateScrollPosition(tab, response.scrollX, response.scrollY) if response?
       chrome.tabs.sendMessage(tab.id, { name: "getScrollPosition" }, createScrollPositionHandler())
+
+# Start pulling changes from synchrized storage.
+Sync.init()
diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 3b34c5a6..b1ccb696 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -25,9 +25,11 @@ root.Sync = Sync =
   storage: chrome.storage.sync
   doNotSync: [ "settingsVersion", "previousVersion" ]
 
+  register: ->
+    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
+
   init: ->
     chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
-    @pull()
 
   # Asynchronous fetch from synced storage, called only at startup.
   pull: ->
@@ -126,5 +128,5 @@ root.Sync = Sync =
       return false
     return true
 
-Sync.init()
+Sync.register()
 
-- 
cgit v1.2.3


From 0ff5b7a8ead8ad9051f0a9ba4a9667f07c9c38ac Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 11:19:59 +0100
Subject: Comments for philc.

---
 background_scripts/sync.coffee | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index b1ccb696..dfa260b5 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -1,4 +1,3 @@
-
 #
 # * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync.
 # * Sync.listener() listens for changes to chrome.storage.sync and propagates those
@@ -21,6 +20,12 @@
 root = exports ? window
 root.Sync = Sync =
 
+  # 19/4/14:
+  # Leave logging statements in, but disable debugging.
+  # We may need to come back to this, so removing logging now would be premature.
+  # 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
   storage: chrome.storage.sync
   doNotSync: [ "settingsVersion", "previousVersion" ]
@@ -34,13 +39,14 @@ root.Sync = Sync =
   # Asynchronous fetch from synced storage, called only at startup.
   pull: ->
     @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}"
           @storeAndPropagate key, value
       else
-        @log "chrome sync callback for Sync.pull() indicates error"
-        @log chrome.runtime.lastError
+        console.log "chrome sync callback for Sync.pull() indicates error"
+        console.log chrome.runtime.lastError
 
   # Asynchronous message from synced storage.
   listener: (changes, area) ->
@@ -87,18 +93,20 @@ root.Sync = Sync =
     #
     if @isSyncKey key
       @storage.set @mkKeyValue(key,value), =>
+        # Chrome sets chrome.runtime.lastError if there is an error.
         if chrome.runtime.lastError
-          @log "chrome sync callback for Sync.set() indicates error: " + key
-          @log chrome.runtime.lastError
+          console.log "chrome sync callback for Sync.set() indicates error: " + key
+          console.log chrome.runtime.lastError
       @log "set scheduled: #{key}=#{value}"
 
   # Only called synchronously from within vimium, never on a callback.
   clear: (key) ->
     if @isSyncKey key
-      @storage.remove key, ->
+      @storage.remove key, =>
+        # Chrome sets chrome.runtime.lastError if there is an error.
         if chrome.runtime.lastError
-          @log "chrome sync callback for Sync.clear() indicates error: " + key
-          @log chrome.runtime.lastError
+          console.log "chrome sync callback for Sync.clear() indicates error: " + key
+          console.log chrome.runtime.lastError
 
   # Should we synchronize this key?
   isSyncKey: (key) ->
@@ -129,4 +137,3 @@ root.Sync = Sync =
     return true
 
 Sync.register()
-
-- 
cgit v1.2.3


From 5398abdb0af3b0d7938c8b34bfda762c5995d427 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 14:27:34 +0100
Subject: Fix sync.init.

---
 background_scripts/sync.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index dfa260b5..4413a748 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -34,7 +34,7 @@ root.Sync = Sync =
     chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
 
   init: ->
-    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
+    @pull()
 
   # Asynchronous fetch from synced storage, called only at startup.
   pull: ->
-- 
cgit v1.2.3


From 2f343bacd7567cb51901b7f8b54f224ea2f011b2 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 14:49:45 +0100
Subject: Code review, clean up.

---
 background_scripts/sync.coffee | 52 +++++++++++-------------------------------
 1 file changed, 13 insertions(+), 39 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 4413a748..454aa6fd 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -45,67 +45,56 @@ root.Sync = Sync =
           @log "pull: #{key} <- #{value}"
           @storeAndPropagate key, value
       else
-        console.log "chrome sync callback for Sync.pull() indicates error"
+        console.log "callback for Sync.pull() indicates error"
         console.log chrome.runtime.lastError
 
   # Asynchronous message from synced storage.
   listener: (changes, area) ->
     for own key, change of changes
       @log "listener: #{key} <- #{change.newValue}"
-      @storeAndPropagate key, change.newValue
+      @storeAndPropagate key, change?.newValue
   
   # Only ever called from asynchronous synced-storage callbacks (pull and listener).
   storeAndPropagate: (key, value) ->
-    # Value must be JSON.stringifed or undefined.
-    if not @checkHaveStringOrUndefined value
-      return
-    # Ignore, we're not accepting this key.
-    if not @isSyncKey key
-       @log "ignoring: #{key}"
-       return
-    # Ignore, it's unchanged
-    if key of localStorage and localStorage[key] is value
-       @log "unchanged: #{key}"
-       return
+    return if not key of Settings.defaults
+    return if not @isSyncKey key
+    return if value and key of localStorage and localStorage[key] is value
 
-    # Ok: accept, store and propagate this update.
+    # Ok: store and propagate this update.
     defaultValue = Settings.defaults[key]
     defaultValueJSON = JSON.stringify(defaultValue)
 
     if value and value != defaultValueJSON
       # Key/value has been changed to non-default value at remote instance.
-      @log "update: #{key}=#{value}"
+      @log "storeAndPropagate update: #{key}=#{value}"
       localStorage[key] = value
       Settings.doPostUpdateHook key, JSON.parse(value)
     else
       # Key has been reset to default value at remote instance.
-      @log "clear: #{key}"
+      @log "storeAndPropagate clear: #{key}"
       if key of localStorage
         delete localStorage[key]
       Settings.doPostUpdateHook key, defaultValue
 
   # Only called synchronously from within vimium, never on a callback.
-  # No need to propagate updates into the rest of vimium.
+  # No need to propagate updates to the rest of vimium, that's already been done.
   set: (key, value) ->
-    # value has already been JSON.stringifed
-    if not @checkHaveString value
-      return
-    #
     if @isSyncKey key
+      @log "set scheduled: #{key}=#{value}"
       @storage.set @mkKeyValue(key,value), =>
         # Chrome sets chrome.runtime.lastError if there is an error.
         if chrome.runtime.lastError
-          console.log "chrome sync callback for Sync.set() indicates error: " + key
+          console.log "callback for Sync.set() indicates error: #{key} <- #{value}"
           console.log chrome.runtime.lastError
-      @log "set scheduled: #{key}=#{value}"
 
   # Only called synchronously from within vimium, never on a callback.
   clear: (key) ->
     if @isSyncKey key
+      @log "clear scheduled: #{key}"
       @storage.remove key, =>
         # Chrome sets chrome.runtime.lastError if there is an error.
         if chrome.runtime.lastError
-          console.log "chrome sync callback for Sync.clear() indicates error: " + key
+          console.log "for Sync.clear() indicates error: #{key}"
           console.log chrome.runtime.lastError
 
   # Should we synchronize this key?
@@ -118,22 +107,7 @@ root.Sync = Sync =
     obj[key] = value
     obj
 
-  # Debugging messages.
-  # Disable debugginf by setting root.Sync.debug to anything falsy.
-  # Enabled for the time being (18/4/14) -- smblott.
   log: (msg) ->
     console.log "Sync: #{msg}" if @debug
 
-  checkHaveString: (thing) ->
-    if typeof(thing) != "string" or not thing
-      @log "Sync: Yikes! this should be a non-empty string: #{typeof(thing)} #{thing}"
-      return false
-    return true
-
-  checkHaveStringOrUndefined: (thing) ->
-    if ( typeof(thing) != "string" and typeof(thing) != "undefined" ) or ( typeof(thing) == "string" and not thing )
-      @log "Sync: Yikes! this should be a non-empty string or undefined: #{typeof(thing)} #{thing}"
-      return false
-    return true
-
 Sync.register()
-- 
cgit v1.2.3


From 4083ee6423c10c34c1d0a3fa48e8f4d79c8615df Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 19 Apr 2014 14:52:38 +0100
Subject: Clean up initialization.

---
 background_scripts/sync.coffee | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
index 454aa6fd..1c6c7fb6 100644
--- a/background_scripts/sync.coffee
+++ b/background_scripts/sync.coffee
@@ -30,10 +30,9 @@ root.Sync = Sync =
   storage: chrome.storage.sync
   doNotSync: [ "settingsVersion", "previousVersion" ]
 
-  register: ->
-    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
-
+  # This is called in main().
   init: ->
+    chrome.storage.onChanged.addListener (changes, area) -> Sync.listener changes, area
     @pull()
 
   # Asynchronous fetch from synced storage, called only at startup.
@@ -110,4 +109,3 @@ root.Sync = Sync =
   log: (msg) ->
     console.log "Sync: #{msg}" if @debug
 
-Sync.register()
-- 
cgit v1.2.3


From db65721aa67b2de75b1e279f01e721676e83b448 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sun, 27 Apr 2014 11:52:30 +0100
Subject: Response to @philc's comments regarding sync.

---
 background_scripts/main.coffee     |  2 +-
 background_scripts/settings.coffee | 13 ++++------
 background_scripts/sync.coffee     | 50 +++++++++++++++++---------------------
 3 files changed, 28 insertions(+), 37 deletions(-)

(limited to 'background_scripts')

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index dc853803..5147aec2 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -597,5 +597,5 @@ chrome.windows.getAll { populate: true }, (windows) ->
         (response) -> updateScrollPosition(tab, response.scrollX, response.scrollY) if response?
       chrome.tabs.sendMessage(tab.id, { name: "getScrollPosition" }, createScrollPositionHandler())
 
-# Start pulling changes from synchrized storage.
+# Start pulling changes from synchronized storage.
 Sync.init()
diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index f75c1db3..73a7a04b 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -23,10 +23,9 @@ root.Settings = Settings =
 
   has: (key) -> key of localStorage
 
-  # postUpdateHooks are called each time an option changes:
-  #   either from options/options.coffee          (when the options page is saved)
-  #       or from background_scripts/sync.coffee  (when an update propagates from chrome.storage)
-  #
+  # for settings which require action when their value changes, add hooks here
+  # called from options/options.coffee (when the options page is saved), and from background_scripts/sync.coffee (when
+  # an update propagates from chrome.storage.sync).
   postUpdateHooks:
     keyMappings: (value) ->
       root.Commands.clearKeyMappingsAndSetDefaults()
@@ -34,10 +33,8 @@ root.Settings = Settings =
       root.refreshCompletionKeysAfterMappingSave()
 
   # postUpdateHooks convenience wrapper
-  doPostUpdateHook: (key, value) ->
-    if @postUpdateHooks[key]
-      @postUpdateHooks[key] value
-
+  performPostUpdateHook: (key, value) ->
+    @postUpdateHooks[key] value if @postUpdateHooks[key]
 
   # options/options.(coffee|html) only handle booleans and strings; therefore
   # all defaults must be booleans or strings
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
 
-- 
cgit v1.2.3