From a76324f6d397c3e746ba501cfd858c4869cb0af7 Mon Sep 17 00:00:00 2001 From: Ellie Day Date: Sat, 23 Dec 2017 08:23:34 -0600 Subject: [PATCH 1/3] Add ExtensionStore and add basic store instance syncing to main controller --- app/scripts/background.js | 12 ++++++++++++ app/scripts/lib/extension-store.js | 20 ++++++++++++++++++++ test/unit/extension-store-test.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 app/scripts/lib/extension-store.js create mode 100644 test/unit/extension-store-test.js diff --git a/app/scripts/background.js b/app/scripts/background.js index da022c490641..45da2f6d0a09 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -4,6 +4,7 @@ const pump = require('pump') const log = require('loglevel') const extension = require('extensionizer') const LocalStorageStore = require('obs-store/lib/localStorage') +const ExtensionStore = require('./lib/extension-store') const storeTransform = require('obs-store/lib/transform') const asStream = require('obs-store/lib/asStream') const ExtensionPlatform = require('./platforms/extension') @@ -28,6 +29,7 @@ let popupIsOpen = false // state persistence const diskStore = new LocalStorageStore({ storageKey: STORAGE_KEY }) +const extensionStore = new ExtensionStore() // initialization flow initialize().catch(log.error) @@ -45,8 +47,12 @@ async function initialize () { async function loadStateFromPersistence () { // migrations const migrator = new Migrator({ migrations }) + // fetch from extension store + const extensionData = await extensionStore.fetch() // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) // read from disk let versionedData = diskStore.getState() || migrator.generateInitialState(firstTimeState) + // merge extension and versioned data + versionedData = { ...versionedData, ...extensionData } // migrate data versionedData = await migrator.migrateData(versionedData) // write to disk @@ -76,6 +82,7 @@ function setupController (initState) { pump( asStream(controller.store), storeTransform(versionifyData), + storeTransform(syncDataWithExtension), asStream(diskStore) ) @@ -85,6 +92,11 @@ function setupController (initState) { return versionedData } + function syncDataWithExtension(state) { + extensionStore.sync(state) // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) + return state + } + // // connect to other contexts // diff --git a/app/scripts/lib/extension-store.js b/app/scripts/lib/extension-store.js new file mode 100644 index 000000000000..a8b730a65296 --- /dev/null +++ b/app/scripts/lib/extension-store.js @@ -0,0 +1,20 @@ +const extension = require('extensionizer') + +const KEYS_TO_SYNC = ['KeyringController', 'PreferencesController'] + +module.exports = class ExtensionStore { + async fetch() { + return new Promise((resolve) => { + extension.storage.sync.get(KEYS_TO_SYNC, data => resolve(data)) + }) + } + async sync(state) { + const dataToSync = KEYS_TO_SYNC.reduce((result, key) => { + result[key] = state.data[key] + return result + }, {}) + return new Promise((resolve) => { + extension.storage.sync.set(dataToSync, () => resolve()) + }) + } +} diff --git a/test/unit/extension-store-test.js b/test/unit/extension-store-test.js new file mode 100644 index 000000000000..e3b5713fbd16 --- /dev/null +++ b/test/unit/extension-store-test.js @@ -0,0 +1,29 @@ +const assert = require('assert') + +const ExtensionStore = require('../../app/scripts/lib/extension-store') + +describe('Extension Store', function () { + let extensionStore + + beforeEach(function () { + extensionStore = new ExtensionStore() + }) + + describe('#fetch', function () { + it('should return a promise', function () { + + }) + it('after promise resolution, should have loaded the proper data from the extension', function () { + + }) + }) + + describe('#sync', function () { + it('should return a promise', function () { + + }) + it('after promise resolution, should have synced the proper data from the extension', function () { + + }) + }) +}) From 7184db7632ef79d4bde0e643fdc1a4ee910c77fb Mon Sep 17 00:00:00 2001 From: Ellie Day Date: Tue, 2 Jan 2018 21:31:17 -0800 Subject: [PATCH 2/3] handle situation where storage.sync is disabled in certain versions of firefox --- app/scripts/background.js | 5 ++--- app/scripts/lib/extension-store.js | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 45da2f6d0a09..732f47590b6b 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -47,11 +47,10 @@ async function initialize () { async function loadStateFromPersistence () { // migrations const migrator = new Migrator({ migrations }) - // fetch from extension store - const extensionData = await extensionStore.fetch() // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) // read from disk let versionedData = diskStore.getState() || migrator.generateInitialState(firstTimeState) - // merge extension and versioned data + // fetch from extension store and merge in data + const extensionData = await extensionStore.fetch() // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) versionedData = { ...versionedData, ...extensionData } // migrate data versionedData = await migrator.migrateData(versionedData) diff --git a/app/scripts/lib/extension-store.js b/app/scripts/lib/extension-store.js index a8b730a65296..dd0f82f369e8 100644 --- a/app/scripts/lib/extension-store.js +++ b/app/scripts/lib/extension-store.js @@ -1,11 +1,24 @@ const extension = require('extensionizer') const KEYS_TO_SYNC = ['KeyringController', 'PreferencesController'] +const FIREFOX_SYNC_DISABLED_MESSAGE = 'Please set webextensions.storage.sync.enabled to true in about:config' + +const handleDisabledSyncAndResolve = (resolve, toResolve) => { + // Firefox 52 has sync available on extension.storage, but it is disabled by default + const lastError = extension.runtime.lastError + if (lastError && lastError.message.includes(FIREFOX_SYNC_DISABLED_MESSAGE)) { + resolve({}) + } else { + resolve(toResolve) + } +} module.exports = class ExtensionStore { async fetch() { return new Promise((resolve) => { - extension.storage.sync.get(KEYS_TO_SYNC, data => resolve(data)) + extension.storage.sync.get(KEYS_TO_SYNC, (data) => { + handleDisabledSyncAndResolve(resolve, data) + }) }) } async sync(state) { @@ -14,7 +27,9 @@ module.exports = class ExtensionStore { return result }, {}) return new Promise((resolve) => { - extension.storage.sync.set(dataToSync, () => resolve()) + extension.storage.sync.set(dataToSync, () => { + handleDisabledSyncAndResolve(resolve) + }) }) } } From 3c6a5b16ad37c83f548028d5b6fa3d0f75293ca5 Mon Sep 17 00:00:00 2001 From: Ellie Day Date: Tue, 2 Jan 2018 21:53:11 -0800 Subject: [PATCH 3/3] conditionally use extension store if supported or enabled --- app/scripts/background.js | 12 +++++++++--- app/scripts/lib/extension-store.js | 4 ++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 732f47590b6b..d9a2b0a6e2b6 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -50,8 +50,12 @@ async function loadStateFromPersistence () { // read from disk let versionedData = diskStore.getState() || migrator.generateInitialState(firstTimeState) // fetch from extension store and merge in data - const extensionData = await extensionStore.fetch() // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) - versionedData = { ...versionedData, ...extensionData } + + if (extensionStore.isSupported && extensionStore.isEnabled) { + const extensionData = await extensionStore.fetch() // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) + versionedData = { ...versionedData, ...extensionData } + } + // migrate data versionedData = await migrator.migrateData(versionedData) // write to disk @@ -92,7 +96,9 @@ function setupController (initState) { } function syncDataWithExtension(state) { - extensionStore.sync(state) // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) + if (extensionStore.isSupported && extensionStore.isEnabled) { + extensionStore.sync(state) // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError) + } return state } diff --git a/app/scripts/lib/extension-store.js b/app/scripts/lib/extension-store.js index dd0f82f369e8..67ee71f16bae 100644 --- a/app/scripts/lib/extension-store.js +++ b/app/scripts/lib/extension-store.js @@ -14,6 +14,10 @@ const handleDisabledSyncAndResolve = (resolve, toResolve) => { } module.exports = class ExtensionStore { + constructor() { + this.isSupported = !!(extension.storage.sync) + this.isEnabled = true // TODO: get value from user settings + } async fetch() { return new Promise((resolve) => { extension.storage.sync.get(KEYS_TO_SYNC, (data) => {