Bug 1253740 - Implement storage.sync, r=bsilverberg,kmag
☠☠ backed out by 796ad3765645 ☠ ☠
authorMichiel de Jong <mbdejong@mozilla.com>
Thu, 11 Aug 2016 18:16:37 -0400
changeset 320158 8f100caf82bf582efcebfd7df8d53e07ce90672f
parent 320157 9a3b8780710b720148346572aec2bf2c235602c2
child 320159 eb537ef54d55ce066a59696ceca7da2f6871acb7
push id20751
push userphilringnalda@gmail.com
push dateSun, 30 Oct 2016 18:06:35 +0000
treeherderfx-team@e3279760cd97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsilverberg, kmag
bugs1253740
milestone52.0a1
Bug 1253740 - Implement storage.sync, r=bsilverberg,kmag MozReview-Commit-ID: 5v9nYBTgekj
modules/libpref/init/all.js
toolkit/components/extensions/ExtensionStorageSync.jsm
toolkit/components/extensions/ext-c-storage.js
toolkit/components/extensions/ext-storage.js
toolkit/components/extensions/moz.build
toolkit/components/extensions/schemas/storage.json
toolkit/components/extensions/test/xpcshell/test_ext_storage.js
toolkit/components/extensions/test/xpcshell/test_ext_storage_sync.js
toolkit/components/extensions/test/xpcshell/xpcshell.ini
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5434,16 +5434,19 @@ pref("toolkit.pageThumbs.screenSizeDivis
 pref("toolkit.pageThumbs.minWidth", 0);
 pref("toolkit.pageThumbs.minHeight", 0);
 
 pref("webextensions.tests", false);
 
 // 16MB default non-parseable upload limit for requestBody.raw.bytes
 pref("webextensions.webRequest.requestBodyMaxRawBytes", 16777216);
 
+// This functionality is still experimental
+pref("webextensions.storage.sync.enabled", false);
+
 // Allow customization of the fallback directory for file uploads
 pref("dom.input.fallbackUploadDir", "");
 
 // Turn rewriting of youtube embeds on/off
 pref("plugins.rewrite_youtube_embeds", true);
 
 // Don't hide Flash from navigator.plugins when it is click-to-activate
 pref("plugins.navigator_hide_disabled_flash", false);
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/ExtensionStorageSync.jsm
@@ -0,0 +1,337 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["ExtensionStorageSync"];
+
+const Ci = Components.interfaces;
+const Cc = Components.classes;
+const Cu = Components.utils;
+const Cr = Components.results;
+const global = this;
+
+const STORAGE_SYNC_ENABLED_PREF = "webextensions.storage.sync.enabled";
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+const {
+  runSafeSyncWithoutClone,
+} = Cu.import("resource://gre/modules/ExtensionUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "AppsUtils",
+                                  "resource://gre/modules/AppsUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage",
+                                  "resource://gre/modules/ExtensionStorage.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "loadKinto",
+                                  "resource://services-common/kinto-offline-client.js");
+XPCOMUtils.defineLazyModuleGetter(this, "Task",
+                                  "resource://gre/modules/Task.jsm");
+XPCOMUtils.defineLazyPreferenceGetter(this, "prefPermitsStorageSync",
+                                      STORAGE_SYNC_ENABLED_PREF, false);
+
+/* globals prefPermitsStorageSync */
+
+// Map of Extensions to Promise<Collections>.
+const collectionPromises = new Map();
+// Map of Extensions to Set<Contexts> to track contexts that are still
+// "live" and could still use this collection.
+const extensionContexts = new WeakMap();
+
+// Kinto record IDs have two condtions:
+//
+// - They must contain only ASCII alphanumerics plus - and _. To fix
+// this, we encode all non-letters using _C_, where C is the
+// percent-encoded character, so space becomes _20_
+// and underscore becomes _5F_.
+//
+// - They must start with an ASCII letter. To ensure this, we prefix
+// all keys with "key-".
+function keyToId(key) {
+  function escapeChar(match) {
+    return "_" + match.codePointAt(0).toString(16).toUpperCase() + "_";
+  }
+  return "key-" + key.replace(/[^a-zA-Z0-9]/g, escapeChar);
+}
+
+// Convert a Kinto ID back into a chrome.storage key.
+// Returns null if a key couldn't be parsed.
+function idToKey(id) {
+  function unescapeNumber(match, group1) {
+    return String.fromCodePoint(parseInt(group1, 16));
+  }
+  // An escaped ID should match this regex.
+  // An escaped ID should consist of only letters and numbers, plus
+  // code points escaped as _[0-9a-f]+_.
+  const ESCAPED_ID_FORMAT = /^(?:[a-zA-Z0-9]|_[0-9A-F]+_)*$/;
+
+  if (!id.startsWith("key-")) {
+    return null;
+  }
+  const unprefixed = id.slice(4);
+  // Verify that the ID is the correct format.
+  if (!ESCAPED_ID_FORMAT.test(unprefixed)) {
+    return null;
+  }
+  return unprefixed.replace(/_([0-9A-F]+)_/g, unescapeNumber);
+}
+
+// An "id schema" used to validate Kinto IDs and generate new ones.
+const storageSyncIdSchema = {
+  // We should never generate IDs; chrome.storage only acts as a
+  // key-value store, so we should always have a key.
+  generate() {
+    throw new Error("cannot generate IDs");
+  },
+
+  // See keyToId and idToKey for more details.
+  validate(id) {
+    return idToKey(id) !== null;
+  },
+};
+
+/**
+ * Return a KintoBase object, suitable for using in Firefox.
+ *
+ * This centralizes the logic used to create Kinto instances, which
+ * we will need to do in several places.
+ *
+ * @returns {Kinto}
+ */
+function makeKinto() {
+  const Kinto = loadKinto();
+  return new Kinto({
+    adapter: Kinto.adapters.FirefoxAdapter,
+    adapterOptions: {path: "storage-sync.sqlite"},
+  });
+}
+
+/**
+ * Actually for-real close the collection associated with a
+ * collection.
+ *
+ * @param {Extension} extension
+ *                    The extension whose uses are all over.
+ * @returns {Promise<()>} Promise that resolves when everything is clean.
+ */
+function closeExtensionCollection(extension) {
+  const collectionPromise = collectionPromises.get(extension);
+  if (!collectionPromise) {
+    Cu.reportError(new Error(`Internal error: trying to close extension ${extension.id}` +
+                             "that doesn't have a collection"));
+    return;
+  }
+  collectionPromises.delete(extension);
+  return collectionPromise.then(coll => {
+    return coll.db.close();
+  });
+}
+
+/**
+ * Clean up now that one context is no longer using this extension's collection.
+ *
+ * @param {Extension} extension
+ *                    The extension whose context just ended.
+ * @param {Context} context
+ *                  The context that just ended.
+ * @returns {Promise<()>} Promise that resolves when everything is clean.
+ */
+function cleanUpForContext(extension, context) {
+  const contexts = extensionContexts.get(extension);
+  if (!contexts) {
+    Cu.reportError(new Error(`Internal error: cannot find any contexts for extension ${extension.id}`));
+    // Try to shut down cleanly anyhow?
+    return closeExtensionCollection(extension);
+  }
+  contexts.delete(context);
+  if (contexts.size === 0) {
+    // Nobody else is using this collection. Clean up.
+    extensionContexts.delete(extension);
+    return closeExtensionCollection(extension);
+  }
+}
+
+/**
+ * Generate a promise that produces the Collection for an extension.
+ *
+ * @param {Extension} extension
+ *                    The extension whose collection needs to
+ *                    be opened.
+ * @param {Context} context
+ *                  The context for this extension. The Collection
+ *                  will shut down automatically when all contexts
+ *                  close.
+ * @returns {Promise<Collection>}
+ */
+const openCollection = Task.async(function* (extension, context) {
+  // FIXME: This leaks metadata about what extensions a user has
+  // installed.  We should calculate collection ID using a hash of
+  // user ID, extension ID, and some secret.
+  let collectionId = extension.id;
+  // TODO: implement sync process
+  const db = makeKinto();
+  const coll = db.collection(collectionId, {
+    idSchema: storageSyncIdSchema,
+  });
+  yield coll.db.open();
+  return coll;
+});
+
+this.ExtensionStorageSync = {
+  listeners: new WeakMap(),
+
+  /**
+   * Get the collection for an extension, consulting a cache to
+   * save time.
+   *
+   * @param {Extension} extension
+   *                    The extension for which we are seeking
+   *                    a collection.
+   * @param {Context} context
+   *                  The context of the extension, so that we can
+   *                  clean up the collection when the extension ends.
+   * @returns {Promise<Collection>}
+   */
+  getCollection(extension, context) {
+    if (prefPermitsStorageSync !== true) {
+      return Promise.reject({message: `Please set ${STORAGE_SYNC_ENABLED_PREF} to true in about:config`});
+    }
+    if (!collectionPromises.has(extension)) {
+      const collectionPromise = openCollection(extension, context);
+      collectionPromises.set(extension, collectionPromise);
+      collectionPromise.catch(Cu.reportError);
+    }
+
+    // Register that the extension and context are in use.
+    if (!extensionContexts.has(extension)) {
+      extensionContexts.set(extension, new Set());
+    }
+    const contexts = extensionContexts.get(extension);
+    if (!contexts.has(context)) {
+      // New context. Register it and make sure it cleans itself up
+      // when it closes.
+      contexts.add(context);
+      context.callOnClose({
+        close: () => cleanUpForContext(extension, context),
+      });
+    }
+
+    return collectionPromises.get(extension);
+  },
+
+  set: Task.async(function* (extension, items, context) {
+    const coll = yield this.getCollection(extension, context);
+    const keys = Object.keys(items);
+    const ids = keys.map(keyToId);
+    const changes = yield coll.execute(txn => {
+      let changes = {};
+      for (let [i, key] of keys.entries()) {
+        const id = ids[i];
+        let item = items[key];
+        let {oldRecord} = txn.upsert({
+          id,
+          key,
+          data: item,
+        });
+        changes[key] = {
+          newValue: item,
+        };
+        if (oldRecord && oldRecord.data) {
+          // Extract the "data" field from the old record, which
+          // represents the value part of the key-value store
+          changes[key].oldValue = oldRecord.data;
+        }
+      }
+      return changes;
+    }, {preloadIds: ids});
+    this.notifyListeners(extension, changes);
+  }),
+
+  remove: Task.async(function* (extension, keys, context) {
+    const coll = yield this.getCollection(extension, context);
+    keys = [].concat(keys);
+    const ids = keys.map(keyToId);
+    let changes = {};
+    yield coll.execute(txn => {
+      for (let [i, key] of keys.entries()) {
+        const id = ids[i];
+        const res = txn.deleteAny(id);
+        if (res.deleted) {
+          changes[key] = {
+            oldValue: res.data.data,
+          };
+        }
+      }
+      return changes;
+    }, {preloadIds: ids});
+    if (Object.keys(changes).length > 0) {
+      this.notifyListeners(extension, changes);
+    }
+  }),
+
+  clear: Task.async(function* (extension, context) {
+    // We can't call Collection#clear here, because that just clears
+    // the local database. We have to explicitly delete everything so
+    // that the deletions can be synced as well.
+    const coll = yield this.getCollection(extension, context);
+    const res = yield coll.list();
+    const records = res.data;
+    const keys = records.map(record => record.key);
+    yield this.remove(extension, keys, context);
+  }),
+
+  get: Task.async(function* (extension, spec, context) {
+    const coll = yield this.getCollection(extension, context);
+    let keys, records;
+    if (spec === null) {
+      records = {};
+      const res = yield coll.list();
+      for (let record of res.data) {
+        records[record.key] = record.data;
+      }
+      return records;
+    }
+    if (typeof spec === "string") {
+      keys = [spec];
+      records = {};
+    } else if (Array.isArray(spec)) {
+      keys = spec;
+      records = {};
+    } else {
+      keys = Object.keys(spec);
+      records = Cu.cloneInto(spec, global);
+    }
+
+    for (let key of keys) {
+      const res = yield coll.getAny(keyToId(key));
+      if (res.data && res.data._status != "deleted") {
+        records[res.data.key] = res.data.data;
+      }
+    }
+
+    return records;
+  }),
+
+  addOnChangedListener(extension, listener) {
+    let listeners = this.listeners.get(extension) || new Set();
+    listeners.add(listener);
+    this.listeners.set(extension, listeners);
+  },
+
+  removeOnChangedListener(extension, listener) {
+    let listeners = this.listeners.get(extension);
+    listeners.delete(listener);
+    if (listeners.size == 0) {
+      this.listeners.delete(extension);
+    }
+  },
+
+  notifyListeners(extension, changes) {
+    let listeners = this.listeners.get(extension) || new Set();
+    if (listeners) {
+      for (let listener of listeners) {
+        runSafeSyncWithoutClone(listener, changes);
+      }
+    }
+  },
+};
--- a/toolkit/components/extensions/ext-c-storage.js
+++ b/toolkit/components/extensions/ext-c-storage.js
@@ -36,13 +36,28 @@ function storageApiFactory(context) {
         },
         set: function(items) {
           items = sanitize(items);
           return context.childManager.callParentAsyncFunction("storage.local.set", [
             items,
           ]);
         },
       },
+
+      sync: {
+        get: function(keys) {
+          keys = sanitize(keys);
+          return context.childManager.callParentAsyncFunction("storage.sync.get", [
+            keys,
+          ]);
+        },
+        set: function(items) {
+          items = sanitize(items);
+          return context.childManager.callParentAsyncFunction("storage.sync.set", [
+            items,
+          ]);
+        },
+      },
     },
   };
 }
 extensions.registerSchemaAPI("storage", "addon_child", storageApiFactory);
 extensions.registerSchemaAPI("storage", "content_child", storageApiFactory);
--- a/toolkit/components/extensions/ext-storage.js
+++ b/toolkit/components/extensions/ext-storage.js
@@ -1,46 +1,68 @@
 "use strict";
 
 var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage",
                                   "resource://gre/modules/ExtensionStorage.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorageSync",
+                                  "resource://gre/modules/ExtensionStorageSync.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   EventManager,
 } = ExtensionUtils;
 
 function storageApiFactory(context) {
   let {extension} = context;
   return {
     storage: {
       local: {
-        get: function(keys) {
-          return ExtensionStorage.get(extension.id, keys);
+        get: function(spec) {
+          return ExtensionStorage.get(extension.id, spec);
         },
         set: function(items) {
           return ExtensionStorage.set(extension.id, items, context);
         },
-        remove: function(items) {
-          return ExtensionStorage.remove(extension.id, items);
+        remove: function(keys) {
+          return ExtensionStorage.remove(extension.id, keys);
         },
         clear: function() {
           return ExtensionStorage.clear(extension.id);
         },
       },
 
-      onChanged: new EventManager(context, "storage.local.onChanged", fire => {
-        let listener = changes => {
+      sync: {
+        get: function(spec) {
+          return ExtensionStorageSync.get(extension, spec, context);
+        },
+        set: function(items) {
+          return ExtensionStorageSync.set(extension, items, context);
+        },
+        remove: function(keys) {
+          return ExtensionStorageSync.remove(extension, keys, context);
+        },
+        clear: function() {
+          return ExtensionStorageSync.clear(extension, context);
+        },
+      },
+
+      onChanged: new EventManager(context, "storage.onChanged", fire => {
+        let listenerLocal = changes => {
           fire(changes, "local");
         };
+        let listenerSync = changes => {
+          fire(changes, "sync");
+        };
 
-        ExtensionStorage.addOnChangedListener(extension.id, listener);
+        ExtensionStorage.addOnChangedListener(extension.id, listenerLocal);
+        ExtensionStorageSync.addOnChangedListener(extension, listenerSync);
         return () => {
-          ExtensionStorage.removeOnChangedListener(extension.id, listener);
+          ExtensionStorage.removeOnChangedListener(extension.id, listenerLocal);
+          ExtensionStorageSync.removeOnChangedListener(extension, listenerSync);
         };
       }).api(),
     },
   };
 }
 extensions.registerSchemaAPI("storage", "addon_parent", storageApiFactory);
 extensions.registerSchemaAPI("storage", "content_parent", storageApiFactory);
--- a/toolkit/components/extensions/moz.build
+++ b/toolkit/components/extensions/moz.build
@@ -6,16 +6,17 @@
 
 EXTRA_JS_MODULES += [
     'Extension.jsm',
     'ExtensionAPI.jsm',
     'ExtensionChild.jsm',
     'ExtensionContent.jsm',
     'ExtensionManagement.jsm',
     'ExtensionStorage.jsm',
+    'ExtensionStorageSync.jsm',
     'ExtensionUtils.jsm',
     'LegacyExtensionsUtils.jsm',
     'MessageChannel.jsm',
     'NativeMessaging.jsm',
     'Schemas.jsm',
 ]
 
 EXTRA_COMPONENTS += [
--- a/toolkit/components/extensions/schemas/storage.json
+++ b/toolkit/components/extensions/schemas/storage.json
@@ -174,17 +174,16 @@
             "type": "string",
             "description": "The name of the storage area (<code>\"sync\"</code>, <code>\"local\"</code> or <code>\"managed\"</code>) the changes are for."
           }
         ]
       }
     ],
     "properties": {
       "sync": {
-        "unsupported": true,
         "$ref": "StorageArea",
         "description": "Items in the <code>sync</code> storage area are synced by the browser.",
         "properties": {
           "QUOTA_BYTES": {
             "value": 102400,
             "description": "The maximum total amount (in bytes) of data that can be stored in sync storage, as measured by the JSON stringification of every value plus every key's length. Updates that would cause this limit to be exceeded fail immediately and set $(ref:runtime.lastError)."
           },
           "QUOTA_BYTES_PER_ITEM": {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
@@ -1,188 +1,357 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-function backgroundScript() {
-  let storage = browser.storage.local;
-  function check(prop, value) {
-    return storage.get(null).then(data => {
-      browser.test.assertEq(value, data[prop], "null getter worked for " + prop);
-      return storage.get(prop);
-    }).then(data => {
-      browser.test.assertEq(value, data[prop], "string getter worked for " + prop);
-      return storage.get([prop]);
-    }).then(data => {
-      browser.test.assertEq(value, data[prop], "array getter worked for " + prop);
-      return storage.get({[prop]: undefined});
-    }).then(data => {
-      browser.test.assertEq(value, data[prop], "object getter worked for " + prop);
-    });
-  }
-
-  let globalChanges = {};
-
-  browser.storage.onChanged.addListener((changes, storage) => {
-    browser.test.assertEq("local", storage, "storage is local");
-    Object.assign(globalChanges, changes);
-  });
-
-  function checkChanges(changes) {
-    function checkSub(obj1, obj2) {
-      for (let prop in obj1) {
-        browser.test.assertEq(obj1[prop].oldValue, obj2[prop].oldValue);
-        browser.test.assertEq(obj1[prop].newValue, obj2[prop].newValue);
-      }
-    }
-
-    checkSub(changes, globalChanges);
-    checkSub(globalChanges, changes);
-    globalChanges = {};
-  }
-
-  /* eslint-disable dot-notation */
+const STORAGE_SYNC_PREF = "webextensions.storage.sync.enabled";
+Cu.import("resource://gre/modules/Preferences.jsm");
 
-  // Set some data and then test getters.
-  storage.set({"test-prop1": "value1", "test-prop2": "value2"}).then(() => {
-    checkChanges({"test-prop1": {newValue: "value1"}, "test-prop2": {newValue: "value2"}});
-    return check("test-prop1", "value1");
-  }).then(() => {
-    return check("test-prop2", "value2");
-  }).then(() => {
-    return storage.get({"test-prop1": undefined, "test-prop2": undefined, "other": "default"});
-  }).then(data => {
-    browser.test.assertEq("value1", data["test-prop1"], "prop1 correct");
-    browser.test.assertEq("value2", data["test-prop2"], "prop2 correct");
-    browser.test.assertEq("default", data["other"], "other correct");
-    return storage.get(["test-prop1", "test-prop2", "other"]);
-  }).then(data => {
-    browser.test.assertEq("value1", data["test-prop1"], "prop1 correct");
-    browser.test.assertEq("value2", data["test-prop2"], "prop2 correct");
-    browser.test.assertFalse("other" in data, "other correct");
-
-  // Remove data in various ways.
-  }).then(() => {
-    return storage.remove("test-prop1");
-  }).then(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}});
-    return storage.get(["test-prop1", "test-prop2"]);
-  }).then(data => {
-    browser.test.assertFalse("test-prop1" in data, "prop1 absent");
-    browser.test.assertTrue("test-prop2" in data, "prop2 present");
-
-    return storage.set({"test-prop1": "value1"});
-  }).then(() => {
-    checkChanges({"test-prop1": {newValue: "value1"}});
-    return storage.get(["test-prop1", "test-prop2"]);
-  }).then(data => {
-    browser.test.assertEq("value1", data["test-prop1"], "prop1 correct");
-    browser.test.assertEq("value2", data["test-prop2"], "prop2 correct");
-  }).then(() => {
-    return storage.remove(["test-prop1", "test-prop2"]);
-  }).then(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}});
-    return storage.get(["test-prop1", "test-prop2"]);
+/**
+ * Utility function to ensure that all supported APIs for getting are
+ * tested.
+ *
+ * @param {string} areaName
+ *        either "local" or "sync" according to what we want to test
+ * @param {string} prop
+ *        "key" to look up using the storage API
+ * @param {Object} value
+ *        "value" to compare against
+ * @returns {Promise}
+ */
+function checkGet(areaName, prop, value) {
+  let storage = browser.storage[areaName];
+  return storage.get(null).then(data => {
+    browser.test.assertEq(value, data[prop], `null getter worked for ${prop} in ${areaName}`);
+    return storage.get(prop);
   }).then(data => {
-    browser.test.assertFalse("test-prop1" in data, "prop1 absent");
-    browser.test.assertFalse("test-prop2" in data, "prop2 absent");
-
-  // test storage.clear
-  }).then(() => {
-    return storage.set({"test-prop1": "value1", "test-prop2": "value2"});
-  }).then(() => {
-    return storage.clear();
-  }).then(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}});
-    return storage.get(["test-prop1", "test-prop2"]);
+    browser.test.assertEq(value, data[prop], `string getter worked for ${prop} in ${areaName}`);
+    return storage.get([prop]);
   }).then(data => {
-    browser.test.assertFalse("test-prop1" in data, "prop1 absent");
-    browser.test.assertFalse("test-prop2" in data, "prop2 absent");
-
-  // Test cache invalidation.
-  }).then(() => {
-    return storage.set({"test-prop1": "value1", "test-prop2": "value2"});
-  }).then(() => {
-    globalChanges = {};
-    // Schedule sendMessage after onMessage because the other end immediately
-    // sends a message.
-    Promise.resolve().then(() => {
-      browser.test.sendMessage("invalidate");
-    });
-    return new Promise(resolve => browser.test.onMessage.addListener(resolve));
-  }).then(() => {
-    return check("test-prop1", "value1");
-  }).then(() => {
-    return check("test-prop2", "value2");
-
-  // Make sure we can store complex JSON data.
-  }).then(() => {
-    return storage.set({
-      "test-prop1": {
-        str: "hello",
-        bool: true,
-        null: null,
-        undef: undefined,
-        obj: {},
-        arr: [1, 2],
-        date: new Date(0),
-        regexp: /regexp/,
-        func: function func() {},
-        window,
-      },
-    });
-  }).then(() => {
-    return storage.set({"test-prop2": function func() {}});
-  }).then(() => {
-    browser.test.assertEq("value1", globalChanges["test-prop1"].oldValue, "oldValue correct");
-    browser.test.assertEq("object", typeof(globalChanges["test-prop1"].newValue), "newValue is obj");
-    globalChanges = {};
-    return storage.get({"test-prop1": undefined, "test-prop2": undefined});
+    browser.test.assertEq(value, data[prop], `array getter worked for ${prop} in ${areaName}`);
+    return storage.get({[prop]: undefined});
   }).then(data => {
-    let obj = data["test-prop1"];
-
-    browser.test.assertEq("hello", obj.str, "string part correct");
-    browser.test.assertEq(true, obj.bool, "bool part correct");
-    browser.test.assertEq(null, obj.null, "null part correct");
-    browser.test.assertEq(undefined, obj.undef, "undefined part correct");
-    browser.test.assertEq(undefined, obj.func, "function part correct");
-    browser.test.assertEq(undefined, obj.window, "window part correct");
-    browser.test.assertEq("1970-01-01T00:00:00.000Z", obj.date, "date part correct");
-    browser.test.assertEq("/regexp/", obj.regexp, "date part correct");
-    browser.test.assertEq("object", typeof(obj.obj), "object part correct");
-    browser.test.assertTrue(Array.isArray(obj.arr), "array part present");
-    browser.test.assertEq(1, obj.arr[0], "arr[0] part correct");
-    browser.test.assertEq(2, obj.arr[1], "arr[1] part correct");
-    browser.test.assertEq(2, obj.arr.length, "arr.length part correct");
-
-    obj = data["test-prop2"];
-
-    browser.test.assertEq("[object Object]", {}.toString.call(obj), "function serialized as a plain object");
-    browser.test.assertEq(0, Object.keys(obj).length, "function serialized as an empty object");
-  }).then(() => {
-    browser.test.notifyPass("storage");
-  }).catch(e => {
-    browser.test.fail(`Error: ${e} :: ${e.stack}`);
-    browser.test.notifyFail("storage");
+    browser.test.assertEq(value, data[prop], `object getter worked for ${prop} in ${areaName}`);
   });
 }
 
-let extensionData = {
-  background: backgroundScript,
-  manifest: {
-    permissions: ["storage"],
-  },
-};
+add_task(function* test_local_cache_invalidation() {
+  function background(checkGet) {
+    browser.test.onMessage.addListener(msg => {
+      if (msg === "set-initial") {
+        browser.storage.local.set({"test-prop1": "value1", "test-prop2": "value2"}).then(() => {
+          browser.test.sendMessage("set-initial-done");
+        });
+      } else if (msg === "check") {
+        checkGet("local", "test-prop1", "value1").then(() => {
+          return checkGet("local", "test-prop2", "value2");
+        }).then(() => {
+          browser.test.sendMessage("check-done");
+        });
+      }
+    });
 
-add_task(function* test_backgroundScript() {
-  let extension = ExtensionTestUtils.loadExtension(extensionData);
+    browser.test.sendMessage("ready");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["storage"],
+    },
+    background: `(${background})(${checkGet})`,
+  });
 
   yield extension.startup();
+  yield extension.awaitMessage("ready");
 
-  yield extension.awaitMessage("invalidate");
+  extension.sendMessage("set-initial");
+  yield extension.awaitMessage("set-initial-done");
 
   Services.obs.notifyObservers(null, "extension-invalidate-storage-cache", "");
 
-  extension.sendMessage("invalidated");
+  extension.sendMessage("check");
+  yield extension.awaitMessage("check-done");
+
+  yield extension.unload();
+});
 
-  yield extension.awaitFinish("storage");
+add_task(function* test_config_flag_needed() {
+  function background() {
+    let promises = [];
+    let apiTests = [
+      {method: "get", args: ["foo"]},
+      {method: "set", args: [{foo: "bar"}]},
+      {method: "remove", args: ["foo"]},
+      {method: "clear", args: []},
+    ];
+    apiTests.forEach(testDef => {
+      const test = browser.storage.sync[testDef.method](...testDef.args).then(() => {
+        browser.test.fail("didn't fail with extension.storage.sync.enabled = false");
+        return Promise.reject();
+      }).catch(error => {
+        browser.test.assertEq("Please set webextensions.storage.sync.enabled to " +
+                              "true in about:config", error.message,
+                              `storage.sync.${testDef.method} is behind a flag`);
+        return Promise.resolve();
+      });
+      promises.push(test);
+    });
+
+    Promise.all(promises).then(() => browser.test.notifyPass("flag needed"));
+  }
+
+  ok(!Preferences.get(STORAGE_SYNC_PREF));
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["storage"],
+    },
+    background: `(${background})(${checkGet})`,
+  });
+
+  yield extension.startup();
+  yield extension.awaitFinish("flag needed");
   yield extension.unload();
 });
+
+add_task(function* test_reloading_extensions_works() {
+  // Just some random extension ID that we can re-use
+  const extensionId = "my-extension-id@1";
+
+  function loadExtension() {
+    function background() {
+      browser.storage.sync.set({"a": "b"}).then(() => {
+        browser.test.notifyPass("set-works");
+      });
+    }
+
+    return ExtensionTestUtils.loadExtension({
+      manifest: {
+        permissions: ["storage"],
+      },
+      background: `(${background})()`,
+    }, extensionId);
+  }
+
+  Preferences.set(STORAGE_SYNC_PREF, true);
+
+  let extension1 = loadExtension();
+
+  yield extension1.startup();
+  yield extension1.awaitFinish("set-works");
+  yield extension1.unload();
+
+  let extension2 = loadExtension();
+
+  yield extension2.startup();
+  yield extension2.awaitFinish("set-works");
+  yield extension2.unload();
+
+  Preferences.reset(STORAGE_SYNC_PREF);
+});
+
+do_register_cleanup(() => {
+  Preferences.reset(STORAGE_SYNC_PREF);
+});
+
+add_task(function* test_backgroundScript() {
+  function backgroundScript(checkGet) {
+    let globalChanges, gResolve;
+    function clearGlobalChanges() {
+      globalChanges = new Promise(resolve => gResolve = resolve);
+    }
+    clearGlobalChanges();
+    let expectedAreaName;
+
+    browser.storage.onChanged.addListener((changes, areaName) => {
+      browser.test.assertEq(expectedAreaName, areaName,
+        "Expected area name received by listener");
+      gResolve(changes);
+    });
+
+    function checkChanges(areaName, changes, message) {
+      function checkSub(obj1, obj2) {
+        for (let prop in obj1) {
+          browser.test.assertTrue(obj1[prop] !== undefined,
+                                  `checkChanges ${areaName} ${prop} is missing (${message})`);
+          browser.test.assertTrue(obj2[prop] !== undefined,
+                                  `checkChanges ${areaName} ${prop} is missing (${message})`);
+          browser.test.assertEq(obj1[prop].oldValue, obj2[prop].oldValue,
+                                `checkChanges ${areaName} ${prop} old (${message})`);
+          browser.test.assertEq(obj1[prop].newValue, obj2[prop].newValue,
+                                `checkChanges ${areaName} ${prop} new (${message})`);
+        }
+      }
+
+      return globalChanges.then(recentChanges => {
+        checkSub(changes, recentChanges);
+        checkSub(recentChanges, changes);
+        clearGlobalChanges();
+      });
+    }
+
+    /* eslint-disable dot-notation */
+    function runTests(areaName) {
+      expectedAreaName = areaName;
+      let storage = browser.storage[areaName];
+      // Set some data and then test getters.
+      return storage.set({"test-prop1": "value1", "test-prop2": "value2"}).then(() => {
+        return checkChanges(areaName,
+          {"test-prop1": {newValue: "value1"}, "test-prop2": {newValue: "value2"}},
+          "set (a)");
+      }).then(() => {
+        return checkGet(areaName, "test-prop1", "value1");
+      }).then(() => {
+        return checkGet(areaName, "test-prop2", "value2");
+      }).then(() => {
+        return storage.get({"test-prop1": undefined, "test-prop2": undefined, "other": "default"});
+      }).then(data => {
+        browser.test.assertEq("value1", data["test-prop1"], "prop1 correct (a)");
+        browser.test.assertEq("value2", data["test-prop2"], "prop2 correct (a)");
+        browser.test.assertEq("default", data["other"], "other correct");
+        return storage.get(["test-prop1", "test-prop2", "other"]);
+      }).then(data => {
+        browser.test.assertEq("value1", data["test-prop1"], "prop1 correct (b)");
+        browser.test.assertEq("value2", data["test-prop2"], "prop2 correct (b)");
+        browser.test.assertFalse("other" in data, "other correct");
+
+        // Remove data in various ways.
+      }).then(() => {
+        return storage.remove("test-prop1");
+      }).then(() => {
+        return checkChanges(areaName, {"test-prop1": {oldValue: "value1"}}, "remove string");
+      }).then(() => {
+        return storage.get(["test-prop1", "test-prop2"]);
+      }).then(data => {
+        browser.test.assertFalse("test-prop1" in data, "prop1 absent (remove string)");
+        browser.test.assertTrue("test-prop2" in data, "prop2 present (remove string)");
+
+        return storage.set({"test-prop1": "value1"});
+      }).then(() => {
+        return checkChanges(areaName, {"test-prop1": {newValue: "value1"}}, "set (c)");
+      }).then(() => {
+        return storage.get(["test-prop1", "test-prop2"]);
+      }).then(data => {
+        browser.test.assertEq(data["test-prop1"], "value1", "prop1 correct (c)");
+        browser.test.assertEq(data["test-prop2"], "value2", "prop2 correct (c)");
+      }).then(() => {
+        return storage.remove(["test-prop1", "test-prop2"]);
+      }).then(() => {
+        return checkChanges(areaName,
+          {"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}},
+          "remove array");
+      }).then(() => {
+        return storage.get(["test-prop1", "test-prop2"]);
+      }).then(data => {
+        browser.test.assertFalse("test-prop1" in data, "prop1 absent (remove array)");
+        browser.test.assertFalse("test-prop2" in data, "prop2 absent (remove array)");
+
+        // test storage.clear
+      }).then(() => {
+        return storage.set({"test-prop1": "value1", "test-prop2": "value2"});
+      }).then(() => {
+        // Make sure that set() handler happened before we clear the
+        // promise again.
+        return globalChanges;
+      }).then(() => {
+        clearGlobalChanges();
+        return storage.clear();
+      }).then(() => {
+        return checkChanges(areaName,
+          {"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}},
+          "clear");
+      }).then(() => {
+        return storage.get(["test-prop1", "test-prop2"]);
+      }).then(data => {
+        browser.test.assertFalse("test-prop1" in data, "prop1 absent (clear)");
+        browser.test.assertFalse("test-prop2" in data, "prop2 absent (clear)");
+
+        // Make sure we can store complex JSON data.
+      }).then(() => {
+        // known previous values
+        return storage.set({"test-prop1": "value1", "test-prop2": "value2"});
+      }).then(() => {
+        // Make sure the set() handler landed.
+        return globalChanges;
+      }).then(() => {
+        clearGlobalChanges();
+        return storage.set({
+          "test-prop1": {
+            str: "hello",
+            bool: true,
+            null: null,
+            undef: undefined,
+            obj: {},
+            arr: [1, 2],
+            date: new Date(0),
+            regexp: /regexp/,
+            func: function func() {},
+            window,
+          },
+        });
+      }).then(() => {
+        return storage.set({"test-prop2": function func() {}});
+      }).then(() => globalChanges).then(recentChanges => {
+        browser.test.assertEq("value1", recentChanges["test-prop1"].oldValue, "oldValue correct");
+        browser.test.assertEq("object", typeof(recentChanges["test-prop1"].newValue), "newValue is obj");
+        clearGlobalChanges();
+        return storage.get({"test-prop1": undefined, "test-prop2": undefined});
+      }).then(data => {
+        let obj = data["test-prop1"];
+
+        browser.test.assertEq("hello", obj.str, "string part correct");
+        browser.test.assertEq(true, obj.bool, "bool part correct");
+        browser.test.assertEq(null, obj.null, "null part correct");
+        browser.test.assertEq(undefined, obj.undef, "undefined part correct");
+        browser.test.assertEq(undefined, obj.func, "function part correct");
+        browser.test.assertEq(undefined, obj.window, "window part correct");
+        browser.test.assertEq("1970-01-01T00:00:00.000Z", obj.date, "date part correct");
+        browser.test.assertEq("/regexp/", obj.regexp, "regexp part correct");
+        browser.test.assertEq("object", typeof(obj.obj), "object part correct");
+        browser.test.assertTrue(Array.isArray(obj.arr), "array part present");
+        browser.test.assertEq(1, obj.arr[0], "arr[0] part correct");
+        browser.test.assertEq(2, obj.arr[1], "arr[1] part correct");
+        browser.test.assertEq(2, obj.arr.length, "arr.length part correct");
+
+        obj = data["test-prop2"];
+
+        browser.test.assertEq("[object Object]", {}.toString.call(obj), "function serialized as a plain object");
+        browser.test.assertEq(0, Object.keys(obj).length, "function serialized as an empty object");
+      }).catch(e => {
+        browser.test.fail(`Error: ${e} :: ${e.stack}`);
+        browser.test.notifyFail("storage");
+      });
+    }
+
+    browser.test.onMessage.addListener(msg => {
+      let promise;
+      if (msg === "test-local") {
+        promise = runTests("local");
+      } else if (msg === "test-sync") {
+        promise = runTests("sync");
+      }
+      promise.then(() => browser.test.sendMessage("test-finished"));
+    });
+
+    browser.test.sendMessage("ready");
+  }
+
+  let extensionData = {
+    background: `(${backgroundScript})(${checkGet})`,
+    manifest: {
+      permissions: ["storage"],
+    },
+  };
+
+  Preferences.set(STORAGE_SYNC_PREF, true);
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  yield extension.startup();
+  yield extension.awaitMessage("ready");
+
+  extension.sendMessage("test-local");
+  yield extension.awaitMessage("test-finished");
+
+  extension.sendMessage("test-sync");
+  yield extension.awaitMessage("test-finished");
+
+  Preferences.reset(STORAGE_SYNC_PREF);
+  yield extension.unload();
+});
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_sync.js
@@ -0,0 +1,31 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const {keyToId, idToKey} = Cu.import("resource://gre/modules/ExtensionStorageSync.jsm");
+
+add_task(function* test_key_to_id() {
+  equal(keyToId("foo"), "key-foo");
+  equal(keyToId("my-new-key"), "key-my_2D_new_2D_key");
+  equal(keyToId(""), "key-");
+  equal(keyToId("™"), "key-_2122_");
+  equal(keyToId("\b"), "key-_8_");
+  equal(keyToId("abc\ndef"), "key-abc_A_def");
+  equal(keyToId("Kinto's fancy_string"), "key-Kinto_27_s_20_fancy_5F_string");
+
+  const KEYS = ["foo", "my-new-key", "", "Kinto's fancy_string", "™", "\b"];
+  for (let key of KEYS) {
+    equal(idToKey(keyToId(key)), key);
+  }
+
+  equal(idToKey("hi"), null);
+  equal(idToKey("-key-hi"), null);
+  equal(idToKey("key--abcd"), null);
+  equal(idToKey("key-%"), null);
+  equal(idToKey("key-_HI"), null);
+  equal(idToKey("key-_HI_"), null);
+  equal(idToKey("key-"), "");
+  equal(idToKey("key-1"), "1");
+  equal(idToKey("key-_2D_"), "-");
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell.ini
@@ -51,16 +51,17 @@ skip-if = release_or_beta
 [test_ext_runtime_sendMessage_no_receiver.js]
 [test_ext_runtime_sendMessage_self.js]
 [test_ext_schemas.js]
 [test_ext_schemas_api_injection.js]
 [test_ext_schemas_async.js]
 [test_ext_schemas_allowed_contexts.js]
 [test_ext_simple.js]
 [test_ext_storage.js]
+[test_ext_storage_sync.js]
 [test_ext_topSites.js]
 skip-if = os == "android"
 [test_getAPILevelForWindow.js]
 [test_ext_legacy_extension_context.js]
 [test_ext_legacy_extension_embedding.js]
 [test_locale_converter.js]
 [test_locale_data.js]
 [test_native_messaging.js]