diff options
| -rw-r--r-- | background_scripts/main.coffee | 2 | ||||
| -rw-r--r-- | background_scripts/settings.coffee | 13 | ||||
| -rw-r--r-- | background_scripts/sync.coffee | 50 | ||||
| -rw-r--r-- | pages/options.coffee | 2 | ||||
| -rw-r--r-- | tests/unit_tests/settings_test.coffee | 45 | ||||
| -rw-r--r-- | tests/unit_tests/test_chrome_stubs.coffee | 114 |
6 files changed, 97 insertions, 129 deletions
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 diff --git a/pages/options.coffee b/pages/options.coffee index 0aeb8e2d..34696f68 100644 --- a/pages/options.coffee +++ b/pages/options.coffee @@ -67,7 +67,7 @@ saveOptions = -> bgSettings.set fieldName, fieldValue $(fieldName).value = fieldValue $(fieldName).setAttribute "savedValue", fieldValue - bgSettings.doPostUpdateHook fieldName, fieldValue + bgSettings.performPostUpdateHook fieldName, fieldValue $("saveOptions").disabled = true diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee index dd2815f0..25bb3628 100644 --- a/tests/unit_tests/settings_test.coffee +++ b/tests/unit_tests/settings_test.coffee @@ -13,7 +13,7 @@ context "settings", setup -> stub global, 'localStorage', {} - should "all settings stored in localStorage must be JSONified strings", -> + should "save settings in localStorage as JSONified strings", -> Settings.set 'dummy', "" assert.equal localStorage.dummy, '""' @@ -36,58 +36,39 @@ context "settings", Settings.clear 'scrollStepSize' assert.equal Settings.get('scrollStepSize'), 60 - should "remote changes take effect locally, non-default value", -> + should "propagate non-default value via synced storage listener", -> Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 - Sync.listener { scrollStepSize: { newValue: "40" } } + Sync.handleStorageUpdate { scrollStepSize: { newValue: "40" } } assert.equal Settings.get('scrollStepSize'), 40 - should "remote changes take effect locally, default value", -> + should "propagate default value via synced storage listener", -> Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 - Sync.listener { scrollStepSize: { newValue: "60" } } + Sync.handleStorageUpdate { scrollStepSize: { newValue: "60" } } assert.isFalse Settings.has 'scrollStepSize' - should "remote changes are propagated, non-default value", -> - # Prime Sync. - Settings.set 'scrollStepSize', 20 - assert.equal Settings.get('scrollStepSize'), 20 - # Set a bogus value in localStorage, bypassing Settings and Sync. - localStorage['scrollStepSize'] = JSON.stringify(10) - assert.equal Settings.get('scrollStepSize'), 10 - # Pull Sync's version of scrollStepSize, this should reset it to the correct value (20). - Sync.pull() + 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 "remote changes are propagated, default value", -> - # Prime Sync with a default value. + should "propagate default values from synced storage", -> + Settings.set 'scrollStepSize', 20 chrome.storage.sync.set { scrollStepSize: JSON.stringify(60) } - assert.isFalse Settings.has 'scrollStepSize' - # Set a bogus value in localStorage, bypassing Settings and Sync. - localStorage['scrollStepSize'] = JSON.stringify(10) - assert.equal Settings.get('scrollStepSize'), 10 - # Pull Sync's version of scrollStepSize, this should delete scrollStepSize in localStorage, because it's a default value. - Sync.pull() + Sync.fetchAsync() assert.isFalse Settings.has 'scrollStepSize' - should "remote setting cleared", -> - # Prime localStorage. + should "clear a setting from synced storage", -> Settings.set 'scrollStepSize', 20 - assert.equal Settings.get('scrollStepSize'), 20 - # Prime Sync with a non-default value. - chrome.storage.sync.set { scrollStepSize: JSON.stringify(40) } chrome.storage.sync.remove 'scrollStepSize' assert.isFalse Settings.has 'scrollStepSize' should "trigger a postUpdateHook", -> message = "Hello World" - # Install a bogus update hook for an existing setting. Settings.postUpdateHooks['scrollStepSize'] = (value) -> Sync.message = value chrome.storage.sync.set { scrollStepSize: JSON.stringify(message) } - # Was the update hook triggered? assert.equal message, Sync.message - should "sync a key which is not a setting", -> - # There is nothing to test, here. It's purpose is just to ensure that, should additional settings be - # added in future, then the Sync won't cause unexpected crashes. + 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 index dd8dccf8..e9c48f31 100644 --- a/tests/unit_tests/test_chrome_stubs.coffee +++ b/tests/unit_tests/test_chrome_stubs.coffee @@ -1,65 +1,61 @@ -global.chrome ||= {} -global.runtime ||= {} -global.chrome.storage ||= {} # # 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.storage.onChanged ||= - addListener: (func) -> @func = func - - # Fake a callback from chrome.storage.sync. - call: (key,value) -> - chrome.runtime = { lastError: undefined } - if @func - @func( @mkKeyValue(key,value), 'synced storage stub' ) - - callEmpty: (key) -> - chrome.runtime = { lastError: undefined } - if @func - items = {} - items[key] = {} - @func( items, 'synced storage stub' ) - - mkKeyValue: (key, value) -> - obj = {} - obj[key] = { newValue: value } - obj - -global.chrome.storage.sync ||= - store: {} - - set: (items,callback) -> - chrome.runtime = { lastError: undefined } - for own key, value of items - @store[key] = value - if callback - 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 - if callback - callback items - - remove: (key,callback) -> - chrome.runtime = { lastError: undefined } - if key of @store - delete @store[key] - if callback - callback() - # Now, generate (supposedly asynchronous) notification for listeners. - global.chrome.storage.onChanged.callEmpty(key) - +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) |