Merge branch 'smblott-github-sync-chrome-instances'

Conflicts:
	tests/unit_tests/utils_test.coffee

8 files changed, 240 insertions, 11 deletions

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 207cbccb..b2b4669c 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -599,3 +599,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 synchronized storage.
+Sync.init()
diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index 0fe1e1bb..73a7a04b 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -12,12 +12,30 @@ root.Settings = Settings =
     if (value == @defaults[key])
       @clear(key)
     else
-      localStorage[key] = JSON.stringify(value)
+      jsonValue = JSON.stringify value
+      localStorage[key] = jsonValue
+      Sync.set key, jsonValue
 
-  clear: (key) -> delete localStorage[key]
+  clear: (key) ->
+    if @has key
+      delete localStorage[key]
+    Sync.clear key
 
   has: (key) -> key of localStorage
 
+  # 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()
+      root.Commands.parseCustomKeyMappings value
+      root.refreshCompletionKeysAfterMappingSave()
+
+  # postUpdateHooks convenience wrapper
+  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
   defaults:
diff --git a/background_scripts/sync.coffee b/background_scripts/sync.coffee
new file mode 100644
index 00000000..56c74b81
--- /dev/null
+++ b/background_scripts/sync.coffee
@@ -0,0 +1,105 @@
+#
+# * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync.
+# * Sync.handleStorageUpdate() listens for changes to chrome.storage.sync and propagates those
+#   changes to localStorage and into vimium's internal state.
+# * 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.performPostUpdateHook) 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.
+#
+# NOTE:
+#   Values handled within this module are ALWAYS already JSON.stringifed, so
+#   they're always non-empty strings.
+#
+
+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: true
+  storage: chrome.storage.sync
+  doNotSync: [ "settingsVersion", "previousVersion" ]
+
+  # This is called in main.coffee.
+  init: ->
+    chrome.storage.onChanged.addListener (changes, area) -> Sync.handleStorageUpdate changes, area
+    @fetchAsync()
+
+  # Asynchronous fetch from synced storage, called only at startup.
+  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 "fetchAsync: #{key} <- #{value}"
+          @storeAndPropagate key, value
+      else
+        console.log "callback for Sync.fetchAsync() indicates error"
+        console.log chrome.runtime.lastError
+
+  # Asynchronous message from synced storage.
+  handleStorageUpdate: (changes, area) ->
+    for own key, change of changes
+      @log "handleStorageUpdate: #{key} <- #{change.newValue}"
+      @storeAndPropagate key, change?.newValue
+  
+  # Only ever called from asynchronous synced-storage callbacks (fetchAsync and handleStorageUpdate).
+  storeAndPropagate: (key, value) ->
+    return if not key of Settings.defaults
+    return if not @shouldSyncKey key
+    return if value and key of localStorage and localStorage[key] is value
+    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 "storeAndPropagate update: #{key}=#{value}"
+      localStorage[key] = 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.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 @shouldSyncKey key
+      @log "set scheduled: #{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}"
+          console.log chrome.runtime.lastError
+
+  # Only called synchronously from within vimium, never on a callback.
+  clear: (key) ->
+    if @shouldSyncKey key
+      @log "clear scheduled: #{key}"
+      @storage.remove key, =>
+        # Chrome sets chrome.runtime.lastError if there is an error.
+        if chrome.runtime.lastError
+          console.log "for Sync.clear() indicates error: #{key}"
+          console.log chrome.runtime.lastError
+
+  # Should we synchronize this key?
+  shouldSyncKey: (key) ->
+    key not in @doNotSync
+
+  log: (msg) ->
+    console.log "Sync: #{msg}" if @debug
+
diff --git a/manifest.json b/manifest.json
index 8de7f009..5692fe75 100644
--- a/manifest.json
+++ b/manifest.json
@@ -11,6 +11,7 @@
       "lib/utils.js",
       "background_scripts/commands.js",
       "lib/clipboard.js",
+      "background_scripts/sync.js",
       "background_scripts/settings.js",
       "background_scripts/completion.js",
       "background_scripts/marks.js",
@@ -23,6 +24,7 @@
     "bookmarks",
     "history",
     "clipboardRead",
+    "storage",
     "<all_urls>"
   ],
   "content_scripts": [
diff --git a/pages/options.coffee b/pages/options.coffee
index 117ce4a6..34696f68 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -8,12 +8,6 @@ editableFields = [ "scrollStepSize", "excludedUrls", "linkHintCharacters", "link
 
 canBeEmptyFields = ["excludedUrls", "keyMappings", "userDefinedLinkHintCss"]
 
-postSaveHooks = keyMappings: (value) ->
-  commands = chrome.extension.getBackgroundPage().Commands
-  commands.clearKeyMappingsAndSetDefaults()
-  commands.parseCustomKeyMappings value
-  chrome.extension.getBackgroundPage().refreshCompletionKeysAfterMappingSave()
-
 document.addEventListener "DOMContentLoaded", ->
   populateOptions()
 
@@ -73,7 +67,7 @@ saveOptions = ->
       bgSettings.set fieldName, fieldValue
     $(fieldName).value = fieldValue
     $(fieldName).setAttribute "savedValue", fieldValue
-    postSaveHooks[fieldName] fieldValue if postSaveHooks[fieldName]
+    bgSettings.performPostUpdateHook fieldName, fieldValue
 
   $("saveOptions").disabled = true
 
diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee
index b2c5484b..25bb3628 100644
--- a/tests/unit_tests/settings_test.coffee
+++ b/tests/unit_tests/settings_test.coffee
@@ -1,15 +1,22 @@
 require "./test_helper.js"
+require "./test_chrome_stubs.js"
 
 extend(global, require "../../lib/utils.js")
 Utils.getCurrentVersion = -> '1.44'
 global.localStorage = {}
-{Settings} = require "../../background_scripts/settings.js"
+extend(global,require "../../background_scripts/sync.js")
+extend(global,require "../../background_scripts/settings.js")
+Sync.init()
 
 context "settings",
 
   setup ->
     stub global, 'localStorage', {}
 
+  should "save settings in localStorage as JSONified strings", ->
+    Settings.set 'dummy', ""
+    assert.equal localStorage.dummy, '""'
+
   should "obtain defaults if no key is stored", ->
     assert.isFalse Settings.has 'scrollStepSize'
     assert.equal Settings.get('scrollStepSize'), 60
@@ -28,3 +35,40 @@ context "settings",
     Settings.set 'scrollStepSize', 20
     Settings.clear 'scrollStepSize'
     assert.equal Settings.get('scrollStepSize'), 60
+
+  should "propagate non-default value via synced storage listener", ->
+    Settings.set 'scrollStepSize', 20
+    assert.equal Settings.get('scrollStepSize'), 20
+    Sync.handleStorageUpdate { scrollStepSize: { newValue: "40" } }
+    assert.equal Settings.get('scrollStepSize'), 40
+
+  should "propagate default value via synced storage listener", ->
+    Settings.set 'scrollStepSize', 20
+    assert.equal Settings.get('scrollStepSize'), 20
+    Sync.handleStorageUpdate { scrollStepSize: { newValue: "60" } }
+    assert.isFalse Settings.has 'scrollStepSize'
+
+  should "propagate non-default values from synced storage", ->
+    chrome.storage.sync.set { scrollStepSize: JSON.stringify(20) }
+    Sync.fetchAsync()
+    assert.equal Settings.get('scrollStepSize'), 20
+
+  should "propagate default values from synced storage", ->
+    Settings.set 'scrollStepSize', 20
+    chrome.storage.sync.set { scrollStepSize: JSON.stringify(60) }
+    Sync.fetchAsync()
+    assert.isFalse Settings.has 'scrollStepSize'
+
+  should "clear a setting from synced storage", ->
+    Settings.set 'scrollStepSize', 20
+    chrome.storage.sync.remove 'scrollStepSize'
+    assert.isFalse Settings.has 'scrollStepSize'
+
+  should "trigger a postUpdateHook", ->
+    message = "Hello World"
+    Settings.postUpdateHooks['scrollStepSize'] = (value) -> Sync.message = value
+    chrome.storage.sync.set { scrollStepSize: JSON.stringify(message) }
+    assert.equal message, Sync.message
+
+  should "sync a key which is not a known setting (without crashing)", ->
+    chrome.storage.sync.set { notASetting: JSON.stringify("notAUsefullValue") }
diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee
new file mode 100644
index 00000000..e9c48f31
--- /dev/null
+++ b/tests/unit_tests/test_chrome_stubs.coffee
@@ -0,0 +1,61 @@
+
+#
+# This is a stub for chrome.strorage.sync for testing.
+# It does what chrome.storage.sync should do (roughly), but does so synchronously.
+#
+
+global.chrome =
+  runtime: {}
+
+  storage:
+
+    # chrome.storage.onChanged
+    onChanged:
+      addListener: (func) -> @func = func
+
+      # Fake a callback from chrome.storage.sync.
+      call: (key, value) ->
+        chrome.runtime = { lastError: undefined }
+        key_value = {}
+        key_value[key] = { newValue: value }
+        @func(key_value,'synced storage stub') if @func
+
+      callEmpty: (key) ->
+        chrome.runtime = { lastError: undefined }
+        if @func
+          items = {}
+          items[key] = {}
+          @func(items,'synced storage stub')
+
+    # chrome.storage.sync
+    sync:
+      store: {}
+
+      set: (items, callback) ->
+        chrome.runtime = { lastError: undefined }
+        for own key, value of items
+          @store[key] = value
+        callback() if callback
+        # Now, generate (supposedly asynchronous) notifications for listeners.
+        for own key, value of items
+          global.chrome.storage.onChanged.call(key,value)
+
+      get: (keys, callback) ->
+        chrome.runtime = { lastError: undefined }
+        if keys == null
+          keys = []
+          for own key, value of @store
+            keys.push key
+        items = {}
+        for key in keys
+          items[key] = @store[key]
+        # Now, generate (supposedly asynchronous) callback
+        callback items if callback
+
+      remove: (key, callback) ->
+        chrome. runtime = { lastError: undefined }
+        if key of @store
+          delete @store[key]
+        callback() if callback
+        # Now, generate (supposedly asynchronous) notification for listeners.
+        global.chrome.storage.onChanged.callEmpty(key)
diff --git a/tests/unit_tests/utils_test.coffee b/tests/unit_tests/utils_test.coffee
index 91a06135..c4139dbb 100644
--- a/tests/unit_tests/utils_test.coffee
+++ b/tests/unit_tests/utils_test.coffee
@@ -1,8 +1,10 @@
 require "./test_helper.js"
+require "./test_chrome_stubs.js"
 extend(global, require "../../lib/utils.js")
 Utils.getCurrentVersion = -> '1.43'
-global.localStorage = {}
+extend(global, require "../../background_scripts/sync.js")
 extend(global, require "../../background_scripts/settings.js")
+Sync.init()
 
 context "isUrl",
   should "accept valid URLs", ->