Merge pull request #3079 from MetaMask/i3076-UseStorageLocalInstead-danjm-1

Additions to the UseStorageLocalInstead branch

app/scripts/background.js

@@ -57,7 +57,13 @@ async function loadStateFromPersistence () {
   // fetch from extension store and merge in data
 
   if (localStore.isSupported) {
-    const localData = await localStore.get()
+    let localData
+    try {
+      localData = await localStore.get()
+    } catch (err) {
+      log.error('error fetching state from local store:', err)
+    }
+
    // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError)
     versionedData = Object.keys(localData).length > 0 ? localData : versionedData
   }
@@ -113,7 +119,11 @@ function setupController (initState) {
 
   function syncDataWithExtension(state) {
     if (localStore.isSupported) {
-      localStore.set(state) // TODO: handle possible exceptions (https://developer.chrome.com/apps/runtime#property-lastError)
+      try {
+        localStore.set(state)
+      } catch (err) {
+        log.error('error setting state in local store:', err)
+      }
     }
     return state
   }

app/scripts/lib/extension-store.js

@@ -15,12 +15,12 @@ const handleDisabledSyncAndResolve = (resolve, toResolve) => {
 
 module.exports = class ExtensionStore {
   constructor() {
-    this.isSupported = !!(extension.storage.sync)
+    this.isSupported = !!(extension.storage && 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) => {
+      extension.storage && extension.storage.sync.get(KEYS_TO_SYNC, (data) => {
         handleDisabledSyncAndResolve(resolve, data)
       })
     })
@@ -31,7 +31,7 @@ module.exports = class ExtensionStore {
       return result
     }, {})
     return new Promise((resolve) => {
-      extension.storage.sync.set(dataToSync, () => {
+      extension.storage && extension.storage.sync.set(dataToSync, () => {
         handleDisabledSyncAndResolve(resolve)
       })
     })

test/unit/extension-store-test.js

@@ -1,5 +1,18 @@
 const assert = require('assert')
+const sinon = require('sinon')
 
+const KEYS_TO_SYNC = ['KeyringController', 'PreferencesController']
+const mockSyncGetResult = 123
+const syncGetStub = sinon.stub().callsFake((str, cb) => cb(mockSyncGetResult))
+const syncSetStub = sinon.stub().callsFake((str, cb) => cb())
+
+window.storage = {
+  sync: {
+    get: syncGetStub,
+    set: syncSetStub,
+  },
+}
+window.runtime = {}
 const ExtensionStore = require('../../app/scripts/lib/extension-store')
 
 describe('Extension Store', function () {
@@ -11,19 +24,44 @@ describe('Extension Store', function () {
 
   describe('#fetch', function () {
     it('should return a promise', function () {
-
+      const extensionStoreFetchResult = extensionStore.fetch()
+      assert.ok(Promise.resolve(extensionStoreFetchResult) === extensionStoreFetchResult)
     })
-    it('after promise resolution, should have loaded the proper data from the extension', function () {
+    it('after promise resolution, should have loaded the proper data from the extension', function (done) {
-
+      extensionStore.fetch()
+        .then((result) => {
+          assert.deepEqual(syncGetStub.getCall(0).args[0], KEYS_TO_SYNC.slice(0))
+          assert.equal(result, mockSyncGetResult)
+          done()
+        })
     })
   })
 
   describe('#sync', function () {
     it('should return a promise', function () {
-
+      const extensionStoreSyncResult = extensionStore.sync()
+      assert.ok(Promise.resolve(extensionStoreSyncResult) === extensionStoreSyncResult)
     })
-    it('after promise resolution, should have synced the proper data from the extension', function () {
+    it('after promise resolution, should have synced the proper data from the extension', function (done) {
-
+      const mockState = {
+        data: {
+          KeyringController: 5,
+          PreferencesController: 6,
+          someOtherData: 7
+        },
+        someOtherProp: {
+          evenMoreData: 8,
+        },
+      }
+      const expectedDataToSync = {
+        KeyringController: 5,
+        PreferencesController: 6,
+      }
+      extensionStore.sync(mockState)
+        .then(() => {
+          assert.deepEqual(syncSetStub.getCall(0).args[0], expectedDataToSync)
+          done()
+        })
     })
   })
 })