Bug 1253740 - Implement storage.sync, r=bsilverberg,kmag

MozReview-Commit-ID: 5v9nYBTgekj

--HG--
extra : rebase_source : 26cf64cf3f4fd07ae2d4dc842b299755a35217b7

modules/libpref/init/all.js

@@ -5444,6 +5444,9 @@ 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", "");
 

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);
+      }
+    }
+  },
+};

toolkit/components/extensions/ext-c-storage.js

@@ -41,6 +41,21 @@ function storageApiFactory(context) {
           ]);
         },
       },
+
+      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,
+          ]);
+        },
+      },
     },
   };
 }

toolkit/components/extensions/ext-storage.js

@@ -4,6 +4,8 @@ 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 {
@@ -15,28 +17,48 @@ function storageApiFactory(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(),
     },

toolkit/components/extensions/moz.build

@@ -11,6 +11,7 @@ EXTRA_JS_MODULES += [
     'ExtensionContent.jsm',
     'ExtensionManagement.jsm',
     'ExtensionStorage.jsm',
+    'ExtensionStorageSync.jsm',
     'ExtensionUtils.jsm',
     'LegacyExtensionsUtils.jsm',
     'MessageChannel.jsm',

toolkit/components/extensions/schemas/storage.json

@@ -179,7 +179,6 @@
     ],
     "properties": {
       "sync": {
-        "unsupported": true,
         "$ref": "StorageArea",
         "description": "Items in the <code>sync</code> storage area are synced by the browser.",
         "properties": {

toolkit/components/extensions/test/xpcshell/test_ext_storage.js

@@ -2,119 +2,276 @@
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
-function backgroundScript() {
-  let storage = browser.storage.local;
-  function check(prop, value) {
+const STORAGE_SYNC_PREF = "webextensions.storage.sync.enabled";
+Cu.import("resource://gre/modules/Preferences.jsm");
+
+/**
+ * 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);
+    browser.test.assertEq(value, data[prop], `null getter worked for ${prop} in ${areaName}`);
     return storage.get(prop);
   }).then(data => {
-      browser.test.assertEq(value, data[prop], "string getter worked for " + prop);
+    browser.test.assertEq(value, data[prop], `string getter worked for ${prop} in ${areaName}`);
     return storage.get([prop]);
   }).then(data => {
-      browser.test.assertEq(value, data[prop], "array getter worked for " + prop);
+    browser.test.assertEq(value, data[prop], `array getter worked for ${prop} in ${areaName}`);
     return storage.get({[prop]: undefined});
   }).then(data => {
-      browser.test.assertEq(value, data[prop], "object getter worked for " + prop);
+    browser.test.assertEq(value, data[prop], `object getter worked for ${prop} in ${areaName}`);
   });
 }
 
-  let globalChanges = {};
-
-  browser.storage.onChanged.addListener((changes, storage) => {
-    browser.test.assertEq("local", storage, "storage is local");
-    Object.assign(globalChanges, changes);
+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");
+        });
+      }
     });
 
-  function checkChanges(changes) {
+    browser.test.sendMessage("ready");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["storage"],
+    },
+    background: `(${background})(${checkGet})`,
+  });
+
+  yield extension.startup();
+  yield extension.awaitMessage("ready");
+
+  extension.sendMessage("set-initial");
+  yield extension.awaitMessage("set-initial-done");
+
+  Services.obs.notifyObservers(null, "extension-invalidate-storage-cache", "");
+
+  extension.sendMessage("check");
+  yield extension.awaitMessage("check-done");
+
+  yield extension.unload();
+});
+
+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.assertEq(obj1[prop].oldValue, obj2[prop].oldValue);
-        browser.test.assertEq(obj1[prop].newValue, obj2[prop].newValue);
+          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})`);
         }
       }
 
-    checkSub(changes, globalChanges);
-    checkSub(globalChanges, changes);
-    globalChanges = {};
+      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.
-  storage.set({"test-prop1": "value1", "test-prop2": "value2"}).then(() => {
-    checkChanges({"test-prop1": {newValue: "value1"}, "test-prop2": {newValue: "value2"}});
-    return check("test-prop1", "value1");
+      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 check("test-prop2", "value2");
+        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");
-    browser.test.assertEq("value2", data["test-prop2"], "prop2 correct");
+        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");
-    browser.test.assertEq("value2", data["test-prop2"], "prop2 correct");
+        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(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}});
+        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");
-    browser.test.assertTrue("test-prop2" in data, "prop2 present");
+        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(() => {
-    checkChanges({"test-prop1": {newValue: "value1"}});
+        return checkChanges(areaName, {"test-prop1": {newValue: "value1"}}, "set (c)");
+      }).then(() => {
         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");
+        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(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}});
+        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");
-    browser.test.assertFalse("test-prop2" in data, "prop2 absent");
+        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(() => {
-    checkChanges({"test-prop1": {oldValue: "value1"}, "test-prop2": {oldValue: "value2"}});
+        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");
-    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");
+        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",
@@ -131,10 +288,10 @@ function backgroundScript() {
         });
       }).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 = {};
+      }).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"];
@@ -146,7 +303,7 @@ function backgroundScript() {
         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("/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");
@@ -157,32 +314,44 @@ function backgroundScript() {
 
         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.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,
+    background: `(${backgroundScript})(${checkGet})`,
     manifest: {
       permissions: ["storage"],
     },
   };
 
-add_task(function* test_backgroundScript() {
+  Preferences.set(STORAGE_SYNC_PREF, true);
+
   let extension = ExtensionTestUtils.loadExtension(extensionData);
-
   yield extension.startup();
+  yield extension.awaitMessage("ready");
 
-  yield extension.awaitMessage("invalidate");
+  extension.sendMessage("test-local");
+  yield extension.awaitMessage("test-finished");
 
-  Services.obs.notifyObservers(null, "extension-invalidate-storage-cache", "");
+  extension.sendMessage("test-sync");
+  yield extension.awaitMessage("test-finished");
 
-  extension.sendMessage("invalidated");
-
-  yield extension.awaitFinish("storage");
+  Preferences.reset(STORAGE_SYNC_PREF);
   yield extension.unload();
 });

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_"), "-");
+});

toolkit/components/extensions/test/xpcshell/xpcshell.ini

@@ -57,6 +57,7 @@ skip-if = release_or_beta
 [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]