Backed out 2 changesets (bug 1370752) for failures in test_ext_storage.js a=backout
authorWes Kocher <wkocher@mozilla.com>
Mon, 10 Jul 2017 14:34:56 -0700
changeset 368067 43fca39fe75c9af03828c315c6cd8108d3ec597f
parent 368066 95043a5e0f9732453d04812b8b6bb16e7e643ee8
child 368068 64ce651bf43d15e711de6e752d80702cd991b336
push id92401
push userkwierso@gmail.com
push dateMon, 10 Jul 2017 21:35:04 +0000
treeherdermozilla-inbound@43fca39fe75c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1370752
milestone56.0a1
backs out42d3c1599af53b047d7ccd6b1c92ab08975284d7
9c4bf59ab966a8ec17181d85cc1fc4be7450cca3
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 2 changesets (bug 1370752) for failures in test_ext_storage.js a=backout Backed out changeset 42d3c1599af5 (bug 1370752) Backed out changeset 9c4bf59ab966 (bug 1370752) MozReview-Commit-ID: 4M6DsJvJ6RI
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionStorage.jsm
toolkit/components/extensions/ext-c-storage.js
toolkit/components/extensions/ext-storage.js
toolkit/components/extensions/test/mochitest/test_ext_storage_content.html
toolkit/components/extensions/test/xpcshell/test_ext_storage.js
toolkit/modules/JSONFile.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -801,34 +801,29 @@ class ChildAPIManager {
   /**
    * Calls a function in the parent process and returns its result
    * asynchronously.
    *
    * @param {string} path The full name of the method, e.g. "tabs.create".
    * @param {Array} args The parameters for the function.
    * @param {function(*)} [callback] The callback to be called when the function
    *     completes.
-   * @param {object} [options] Extra options.
-   * @param {boolean} [options.noClone = false] If true, do not clone
-   *     the arguments into an extension sandbox before calling the API
-   *     method.
    * @returns {Promise|undefined} Must be void if `callback` is set, and a
    *     promise otherwise. The promise is resolved when the function completes.
    */
-  callParentAsyncFunction(path, args, callback, options = {}) {
+  callParentAsyncFunction(path, args, callback) {
     let callId = getUniqueId();
     let deferred = PromiseUtils.defer();
     this.callPromises.set(callId, deferred);
 
     this.messageManager.sendAsyncMessage("API:Call", {
       childId: this.id,
       callId,
       path,
       args,
-      noClone: options.noClone || false,
     });
 
     return this.context.wrapPromise(deferred.promise, callback);
   }
 
   /**
    * Create a proxy for an event in the parent process. The returned event
    * object shares its internal state with other instances. For instance, if
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1096,17 +1096,17 @@ class SchemaAPIManager extends EventEmit
    * @returns {object} A sandbox that is used as the global by `loadScript`.
    */
   _createExtGlobal() {
     let global = Cu.Sandbox(Services.scriptSecurityManager.getSystemPrincipal(), {
       wantXrays: false,
       sandboxName: `Namespace of ext-*.js scripts for ${this.processType}`,
     });
 
-    Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeWorker, ExtensionCommon, MatchPattern, MatchPatternSet, StructuredCloneHolder, extensions: this});
+    Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeWorker, ExtensionCommon, MatchPattern, MatchPatternSet, extensions: this});
 
     Cu.import("resource://gre/modules/AppConstants.jsm", global);
     Cu.import("resource://gre/modules/ExtensionAPI.jsm", global);
 
     XPCOMUtils.defineLazyGetter(global, "console", getConsole);
 
     XPCOMUtils.defineLazyModuleGetter(global, "ExtensionUtils",
                                       "resource://gre/modules/ExtensionUtils.jsm");
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -662,17 +662,17 @@ ParentAPIManager = {
         "API:CallResult",
         Object.assign({
           childId: data.childId,
           callId: data.callId,
         }, result));
     };
 
     try {
-      let args = data.noClone ? data.args : Cu.cloneInto(data.args, context.sandbox);
+      let args = Cu.cloneInto(data.args, context.sandbox);
       let pendingBrowser = context.pendingEventBrowser;
       let fun = await context.apiCan.asyncFindAPIPath(data.path);
       let result = context.withPendingBrowser(pendingBrowser,
                                               () => fun(...args));
       if (data.callId) {
         result = result || Promise.resolve();
 
         result.then(result => {
--- a/toolkit/components/extensions/ExtensionStorage.jsm
+++ b/toolkit/components/extensions/ExtensionStorage.jsm
@@ -8,306 +8,211 @@ this.EXPORTED_SYMBOLS = ["ExtensionStora
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cu = Components.utils;
 const Cr = Components.results;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
-
-XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
-                                  "resource://gre/modules/ExtensionUtils.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
-                                  "resource://gre/modules/JSONFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
-
-const global = this;
-
-function isStructuredCloneHolder(value) {
-  return (value && typeof value === "object" &&
-          Cu.getClassName(value, true) === "StructuredCloneHolder");
-}
+XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
+                                  "resource://gre/modules/AsyncShutdown.jsm");
 
-class SerializeableMap extends Map {
-  toJSON() {
-    let result = {};
-    for (let [key, value] of this) {
-      if (isStructuredCloneHolder(value)) {
-        value = value.deserialize(global);
-        this.set(key, value);
+/**
+ * Helper function used to sanitize the objects that have to be saved in the ExtensionStorage.
+ *
+ * @param {BaseContext} context
+ *   The current extension context.
+ * @param {string} key
+ *   The key of the current JSON property.
+ * @param {any} value
+ *   The value of the current JSON property.
+ *
+ * @returns {any}
+ *   The sanitized value of the property.
+ */
+function jsonReplacer(context, key, value) {
+  switch (typeof(value)) {
+    // Serialize primitive types as-is.
+    case "string":
+    case "number":
+    case "boolean":
+      return value;
+
+    case "object":
+      if (value === null) {
+        return value;
       }
 
-      result[key] = value;
-    }
-    return result;
+      switch (Cu.getClassName(value, true)) {
+        // Serialize arrays and ordinary objects as-is.
+        case "Array":
+        case "Object":
+          return value;
+
+        // Serialize Date objects and regular expressions as their
+        // string representations.
+        case "Date":
+        case "RegExp":
+          return String(value);
+      }
+      break;
   }
 
-  /**
-   * Like toJSON, but attempts to serialize every value separately, and
-   * elides any which fail to serialize. Should only be used if initial
-   * JSON serialization fails.
-   *
-   * @returns {object}
-   */
-  toJSONSafe() {
-    let result = {};
-    for (let [key, value] of this) {
-      try {
-        void JSON.serialize(value);
-
-        result[key] = value;
-      } catch (e) {
-        Cu.reportError(new Error(`Failed to serialize browser.storage key "${key}": ${e}`));
-      }
-    }
-    return result;
+  if (!key) {
+    // If this is the root object, and we can't serialize it, serialize
+    // the value to an empty object.
+    return new context.cloneScope.Object();
   }
-}
 
-/**
- * Serializes an arbitrary value into a StructuredCloneHolder, if
- * appropriate. Existing StructuredCloneHolders are returned unchanged.
- * Non-object values are also returned unchanged. Anything else is
- * serialized, and a new StructuredCloneHolder returned.
- *
- * This allows us to avoid a second structured clone operation after
- * sending a storage value across a message manager, before cloning it
- * into an extension scope.
- *
- * @param {StructuredCloneHolder|*} value
- *        A value to serialize.
- * @returns {*}
- */
-function serialize(value) {
-  if (value && typeof value === "object" && !isStructuredCloneHolder(value)) {
-    return new StructuredCloneHolder(value);
-  }
-  return value;
+  // Everything else, omit entirely.
+  return undefined;
 }
 
 this.ExtensionStorage = {
-  // Map<extension-id, Promise<JSONFile>>
-  jsonFilePromises: new Map(),
-
+  cache: new Map(),
   listeners: new Map(),
 
   /**
-   * Asynchronously reads the storage file for the given extension ID
-   * and returns a Promise for its initialized JSONFile object.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to return a file.
-   * @returns {Promise<JSONFile>}
-   */
-  async _readFile(extensionId) {
-    OS.File.makeDir(this.getExtensionDir(extensionId), {
-      ignoreExisting: true,
-      from: OS.Constants.Path.profileDir,
-    });
-
-    let jsonFile = new JSONFile({path: this.getStorageFile(extensionId)});
-    await jsonFile.load();
-
-    jsonFile.data = new SerializeableMap(Object.entries(jsonFile.data));
-
-    return jsonFile;
-  },
-
-  /**
-   * Returns a Promise for initialized JSONFile instance for the
-   * extension's storage file.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to return a file.
-   * @returns {Promise<JSONFile>}
-   */
-  getFile(extensionId) {
-    let promise = this.jsonFilePromises.get(extensionId);
-    if (!promise) {
-      promise = this._readFile(extensionId);
-      this.jsonFilePromises.set(extensionId, promise);
-    }
-    return promise;
-  },
-
-  /**
    * Sanitizes the given value, and returns a JSON-compatible
    * representation of it, based on the privileges of the given global.
    *
    * @param {value} value
    *        The value to sanitize.
    * @param {Context} context
    *        The extension context in which to sanitize the value
    * @returns {value}
    *        The sanitized value.
    */
   sanitize(value, context) {
-    let json = context.jsonStringify(value === undefined ? null : value);
-    if (json == undefined) {
-      throw new ExtensionUtils.ExtensionError("DataCloneError: The object could not be cloned.");
-    }
+    let json = context.jsonStringify(value, jsonReplacer.bind(null, context));
     return JSON.parse(json);
   },
 
-
-  /**
-   * Returns the path to the storage directory within the profile for
-   * the given extension ID.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to return a directory path.
-   * @returns {string}
-   */
   getExtensionDir(extensionId) {
     return OS.Path.join(this.extensionDir, extensionId);
   },
 
-  /**
-   * Returns the path to the JSON storage file for the given extension
-   * ID.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to return a file path.
-   * @returns {string}
-   */
   getStorageFile(extensionId) {
     return OS.Path.join(this.extensionDir, extensionId, "storage.js");
   },
 
-  /**
-   * Asynchronously sets the values of the given storage items for the
-   * given extension.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to set storage values.
-   * @param {object} items
-   *        The storage items to set. For each property in the object,
-   *        the storage value for that property is set to its value in
-   *        said object. Any values which are StructuredCloneHolder
-   *        instances are deserialized before being stored.
-   * @returns {Promise<void>}
-   */
-  async set(extensionId, items) {
-    let jsonFile = await this.getFile(extensionId);
-
-    let changes = {};
-    for (let prop in items) {
-      let item = items[prop];
-      changes[prop] = {oldValue: serialize(jsonFile.data.get(prop)), newValue: serialize(item)};
-      jsonFile.data.set(prop, item);
+  read(extensionId) {
+    if (this.cache.has(extensionId)) {
+      return this.cache.get(extensionId);
     }
 
-    this.notifyListeners(extensionId, changes);
-
-    jsonFile.saveSoon();
-    return null;
+    let path = this.getStorageFile(extensionId);
+    let decoder = new TextDecoder();
+    let promise = OS.File.read(path);
+    promise = promise.then(array => {
+      return JSON.parse(decoder.decode(array));
+    }).catch((error) => {
+      if (!error.becauseNoSuchFile) {
+        Cu.reportError("Unable to parse JSON data for extension storage.");
+      }
+      return {};
+    });
+    this.cache.set(extensionId, promise);
+    return promise;
   },
 
-  /**
-   * Asynchronously removes the given storage items for the given
-   * extension ID.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to remove storage values.
-   * @param {Array<string>} items
-   *        A list of storage items to remove.
-   * @returns {Promise<void>}
-   */
-  async remove(extensionId, items) {
-    let jsonFile = await this.getFile(extensionId);
+  write(extensionId) {
+    let promise = this.read(extensionId).then(extData => {
+      let encoder = new TextEncoder();
+      let array = encoder.encode(JSON.stringify(extData));
+      let path = this.getStorageFile(extensionId);
+      OS.File.makeDir(this.getExtensionDir(extensionId), {
+        ignoreExisting: true,
+        from: OS.Constants.Path.profileDir,
+      });
+      let promise = OS.File.writeAtomic(path, array);
+      return promise;
+    }).catch(() => {
+      // Make sure this promise is never rejected.
+      Cu.reportError("Unable to write JSON data for extension storage.");
+    });
 
-    let changed = false;
-    let changes = {};
+    AsyncShutdown.profileBeforeChange.addBlocker(
+      "ExtensionStorage: Finish writing extension data",
+      promise);
 
-    for (let prop of [].concat(items)) {
-      if (jsonFile.data.has(prop)) {
-        changes[prop] = {oldValue: serialize(jsonFile.data.get(prop))};
-        jsonFile.data.delete(prop);
-        changed = true;
-      }
-    }
-
-    if (changed) {
-      this.notifyListeners(extensionId, changes);
-      jsonFile.saveSoon();
-    }
-    return null;
+    return promise.then(() => {
+      AsyncShutdown.profileBeforeChange.removeBlocker(promise);
+    });
   },
 
-  /**
-   * Asynchronously clears all storage entries for the given extension
-   * ID.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to clear storage.
-   * @returns {Promise<void>}
-   */
-  async clear(extensionId) {
-    let jsonFile = await this.getFile(extensionId);
+  set(extensionId, items) {
+    return this.read(extensionId).then(extData => {
+      let changes = {};
+      for (let prop in items) {
+        let item = items[prop];
+        changes[prop] = {oldValue: extData[prop], newValue: item};
+        extData[prop] = item;
+      }
+
+      this.notifyListeners(extensionId, changes);
 
-    let changed = false;
-    let changes = {};
+      return this.write(extensionId);
+    });
+  },
 
-    for (let [prop, oldValue] of jsonFile.data.entries()) {
-      changes[prop] = {oldValue: serialize(oldValue)};
-      jsonFile.data.delete(prop);
-      changed = true;
-    }
+  remove(extensionId, items) {
+    return this.read(extensionId).then(extData => {
+      let changes = {};
+      for (let prop of [].concat(items)) {
+        changes[prop] = {oldValue: extData[prop]};
+        delete extData[prop];
+      }
 
-    if (changed) {
       this.notifyListeners(extensionId, changes);
-      jsonFile.saveSoon();
-    }
-    return null;
+
+      return this.write(extensionId);
+    });
   },
 
-  /**
-   * Asynchronously retrieves the values for the given storage items for
-   * the given extension ID.
-   *
-   * @param {string} extensionId
-   *        The ID of the extension for which to get storage values.
-   * @param {Array<string>|object|null} [keys]
-   *        The storage items to get. If an array, the value of each key
-   *        in the array is returned. If null, the values of all items
-   *        are returned. If an object, the value for each key in the
-   *        object is returned, or that key's value if the item is not
-   *        set.
-   * @returns {Promise<object>}
-   *        An object which a property for each requested key,
-   *        containing that key's storage value. Values are
-   *        StructuredCloneHolder objects which can be deserialized to
-   *        the original storage value.
-   */
-  async get(extensionId, keys) {
-    let jsonFile = await this.getFile(extensionId);
-    let {data} = jsonFile;
+  clear(extensionId) {
+    return this.read(extensionId).then(extData => {
+      let changes = {};
+      for (let prop of Object.keys(extData)) {
+        changes[prop] = {oldValue: extData[prop]};
+        delete extData[prop];
+      }
+
+      this.notifyListeners(extensionId, changes);
+
+      return this.write(extensionId);
+    });
+  },
 
-    let result = {};
-    if (keys === null) {
-      Object.assign(result, data.toJSON());
-    } else if (typeof(keys) == "object" && !Array.isArray(keys)) {
-      for (let prop in keys) {
-        if (data.has(prop)) {
-          result[prop] = serialize(data.get(prop));
-        } else {
-          result[prop] = keys[prop];
+  get(extensionId, keys) {
+    return this.read(extensionId).then(extData => {
+      let result = {};
+      if (keys === null) {
+        Object.assign(result, extData);
+      } else if (typeof(keys) == "object" && !Array.isArray(keys)) {
+        for (let prop in keys) {
+          if (prop in extData) {
+            result[prop] = extData[prop];
+          } else {
+            result[prop] = keys[prop];
+          }
+        }
+      } else {
+        for (let prop of [].concat(keys)) {
+          if (prop in extData) {
+            result[prop] = extData[prop];
+          }
         }
       }
-    } else {
-      for (let prop of [].concat(keys)) {
-        if (data.has(prop)) {
-          result[prop] = serialize(data.get(prop));
-        }
-      }
-    }
 
-    return result;
+      return result;
+    });
   },
 
   addOnChangedListener(extensionId, listener) {
     let listeners = this.listeners.get(extensionId) || new Set();
     listeners.add(listener);
     this.listeners.set(extensionId, listeners);
   },
 
@@ -333,20 +238,17 @@ this.ExtensionStorage = {
     Services.obs.addObserver(this, "xpcom-shutdown");
   },
 
   observe(subject, topic, data) {
     if (topic == "xpcom-shutdown") {
       Services.obs.removeObserver(this, "extension-invalidate-storage-cache");
       Services.obs.removeObserver(this, "xpcom-shutdown");
     } else if (topic == "extension-invalidate-storage-cache") {
-      for (let promise of this.jsonFilePromises.values()) {
-        promise.then(jsonFile => { jsonFile.finalize(); });
-      }
-      this.jsonFilePromises.clear();
+      this.cache.clear();
     }
   },
 };
 
 XPCOMUtils.defineLazyGetter(ExtensionStorage, "extensionDir",
   () => OS.Path.join(OS.Constants.Path.profileDir, "browser-extension-data"));
 
 ExtensionStorage.init();
--- a/toolkit/components/extensions/ext-c-storage.js
+++ b/toolkit/components/extensions/ext-c-storage.js
@@ -1,76 +1,21 @@
 "use strict";
 
-/* import-globals-from ext-c-toolkit.js */
-
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage",
                                   "resource://gre/modules/ExtensionStorage.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
                                   "resource://gre/modules/TelemetryStopwatch.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 
-var {
-  ExtensionError,
-} = ExtensionUtils;
-
 const storageGetHistogram = "WEBEXT_STORAGE_LOCAL_GET_MS";
 const storageSetHistogram = "WEBEXT_STORAGE_LOCAL_SET_MS";
 
 this.storage = class extends ExtensionAPI {
   getAPI(context) {
-    /**
-     * Serializes the given storage items for transporting to the parent
-     * process.
-     *
-     * @param {Array<string>|object} items
-     *        The items to serialize. If an object is provided, its
-     *        values are serialized to StructuredCloneHolder objects.
-     *        Otherwise, it is returned as-is.
-     * @returns {Array<string>|object}
-     */
-    function serialize(items) {
-      if (items && typeof items === "object" && !Array.isArray(items)) {
-        let result = {};
-        for (let [key, value] of Object.entries(items)) {
-          try {
-            result[key] = new StructuredCloneHolder(value, context.cloneScope);
-          } catch (e) {
-            throw new ExtensionError(String(e));
-          }
-        }
-        return result;
-      }
-      return items;
-    }
-
-    /**
-     * Deserializes the given storage items from the parent process into
-     * the extension context.
-     *
-     * @param {object} items
-     *        The items to deserialize. Any property of the object which
-     *        is a StructuredCloneHolder instance is deserialized into
-     *        the extension scope. Any other object is cloned into the
-     *        extension scope directly.
-     * @returns {object}
-     */
-    function deserialize(items) {
-      let result = new context.cloneScope.Object();
-      for (let [key, value] of Object.entries(items)) {
-        if (value && typeof value === "object" && Cu.getClassName(value, true) === "StructuredCloneHolder") {
-          value = value.deserialize(context.cloneScope);
-        } else {
-          value = Cu.cloneInto(value, context.cloneScope);
-        }
-        result[key] = value;
-      }
-      return result;
-    }
-
     function sanitize(items) {
       // The schema validator already takes care of arrays (which are only allowed
       // to contain strings). Strings and null are safe values.
       if (typeof items != "object" || items === null || Array.isArray(items)) {
         return items;
       }
       // If we got here, then `items` is an object generated by `ObjectType`'s
       // `normalize` method from Schemas.jsm. The object returned by `normalize`
@@ -80,41 +25,42 @@ this.storage = class extends ExtensionAP
       // it is not allowed to access properties of `items`.
       // So we enumerate all properties and sanitize each value individually.
       let sanitized = {};
       for (let [key, value] of Object.entries(items)) {
         sanitized[key] = ExtensionStorage.sanitize(value, context);
       }
       return sanitized;
     }
-
     return {
       storage: {
         local: {
           get: async function(keys) {
             const stopwatchKey = {};
             TelemetryStopwatch.start(storageGetHistogram, stopwatchKey);
             try {
+              keys = sanitize(keys);
               let result = await context.childManager.callParentAsyncFunction("storage.local.get", [
-                serialize(keys),
-              ], null, {noClone: true}).then(deserialize);
+                keys,
+              ]);
               TelemetryStopwatch.finish(storageGetHistogram, stopwatchKey);
               return result;
             } catch (e) {
               TelemetryStopwatch.cancel(storageGetHistogram, stopwatchKey);
               throw e;
             }
           },
           set: async function(items) {
             const stopwatchKey = {};
             TelemetryStopwatch.start(storageSetHistogram, stopwatchKey);
             try {
+              items = sanitize(items);
               let result = await context.childManager.callParentAsyncFunction("storage.local.set", [
-                serialize(items),
-              ], null, {noClone: true});
+                items,
+              ]);
               TelemetryStopwatch.finish(storageSetHistogram, stopwatchKey);
               return result;
             } catch (e) {
               TelemetryStopwatch.cancel(storageSetHistogram, stopwatchKey);
               throw e;
             }
           },
         },
@@ -128,28 +74,12 @@ this.storage = class extends ExtensionAP
           },
           set: function(items) {
             items = sanitize(items);
             return context.childManager.callParentAsyncFunction("storage.sync.set", [
               items,
             ]);
           },
         },
-
-        onChanged: new EventManager(context, "storage.onChanged", fire => {
-          let onChanged = (data, area) => {
-            let changes = new context.cloneScope.Object();
-            for (let [key, value] of Object.entries(data)) {
-              changes[key] = deserialize(value);
-            }
-            fire.raw(changes, area);
-          };
-
-          let parent = context.childManager.getParentEvent("storage.onChanged");
-          parent.addListener(onChanged);
-          return () => {
-            parent.removeListener(onChanged);
-          };
-        }).api(),
       },
     };
   }
 };
--- a/toolkit/components/extensions/ext-storage.js
+++ b/toolkit/components/extensions/ext-storage.js
@@ -60,17 +60,17 @@ this.storage = class extends ExtensionAP
           clear: function() {
             enforceNoTemporaryAddon(extension.id);
             return extensionStorageSync.clear(extension, context);
           },
         },
 
         onChanged: new EventManager(context, "storage.onChanged", fire => {
           let listenerLocal = changes => {
-            fire.raw(changes, "local");
+            fire.async(changes, "local");
           };
           let listenerSync = changes => {
             fire.async(changes, "sync");
           };
 
           ExtensionStorage.addOnChangedListener(extension.id, listenerLocal);
           extensionStorageSync.addOnChangedListener(extension, listenerSync, context);
           return () => {
--- a/toolkit/components/extensions/test/mochitest/test_ext_storage_content.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_storage_content.html
@@ -144,71 +144,60 @@ async function contentScript(checkGet) {
 
       // Make sure we can store complex JSON data.
       // known previous values
       await storage.set({"test-prop1": "value1", "test-prop2": "value2"});
 
       // Make sure the set() handler landed.
       await globalChanges;
 
-      let date = new Date(0);
-
       clearGlobalChanges();
       await 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,
         },
       });
 
-      await browser.test.assertRejects(
-        storage.set({
-          window,
-        }),
-        /DataCloneError|cyclic object value/);
-
-      await browser.test.assertRejects(
-        storage.set({"test-prop2": function func() {}}),
-        /DataCloneError/);
-
+      await storage.set({"test-prop2": function func() {}});
       const recentChanges = await globalChanges;
 
       browser.test.assertEq("value1", recentChanges["test-prop1"].oldValue, "oldValue correct");
       browser.test.assertEq("object", typeof(recentChanges["test-prop1"].newValue), "newValue is obj");
       clearGlobalChanges();
 
       data = await storage.get({"test-prop1": undefined, "test-prop2": undefined});
       let obj = data["test-prop1"];
 
-      if (areaName === "local") {
-        browser.test.assertEq(String(date), String(obj.date), "date part correct");
-        browser.test.assertEq("/regexp/", obj.regexp.toSource(), "regexp part correct");
-      } else {
-        browser.test.assertEq("1970-01-01T00:00:00.000Z", String(obj.date), "date part correct");
-
-        browser.test.assertEq("object", typeof obj.regexp, "regexp part is an object");
-        browser.test.assertEq(0, Object.keys(obj.regexp).length, "regexp part is an empty object");
-      }
-
       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;
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
@@ -248,71 +248,60 @@ add_task(async function test_backgroundS
 
         // Make sure we can store complex JSON data.
         // known previous values
         await storage.set({"test-prop1": "value1", "test-prop2": "value2"});
 
         // Make sure the set() handler landed.
         await globalChanges;
 
-        let date = new Date(0);
-
         clearGlobalChanges();
         await storage.set({
           "test-prop1": {
             str: "hello",
             bool: true,
             null: null,
             undef: undefined,
             obj: {},
             arr: [1, 2],
-            date,
+            date: new Date(0),
             regexp: /regexp/,
+            func: function func() {},
+            window,
           },
         });
 
-        await browser.test.assertRejects(
-          storage.set({
-            window,
-          }),
-          /DataCloneError|cyclic object value/);
-
-        await browser.test.assertRejects(
-          storage.set({"test-prop2": function func() {}}),
-          /DataCloneError/);
-
+        await storage.set({"test-prop2": function func() {}});
         const recentChanges = await globalChanges;
 
         browser.test.assertEq("value1", recentChanges["test-prop1"].oldValue, "oldValue correct");
         browser.test.assertEq("object", typeof(recentChanges["test-prop1"].newValue), "newValue is obj");
         clearGlobalChanges();
 
         data = await storage.get({"test-prop1": undefined, "test-prop2": undefined});
         let obj = data["test-prop1"];
 
-        if (areaName === "local") {
-          browser.test.assertEq(String(date), String(obj.date), "date part correct");
-          browser.test.assertEq("/regexp/", obj.regexp.toSource(), "regexp part correct");
-        } else {
-          browser.test.assertEq("1970-01-01T00:00:00.000Z", String(obj.date), "date part correct");
-
-          browser.test.assertEq("object", typeof obj.regexp, "regexp part is an object");
-          browser.test.assertEq(0, Object.keys(obj.regexp).length, "regexp part is an empty object");
-        }
-
         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;
--- a/toolkit/modules/JSONFile.jsm
+++ b/toolkit/modules/JSONFile.jsm
@@ -281,30 +281,18 @@ JSONFile.prototype = {
    *
    * If an error occurs, the previous file is not deleted.
    *
    * @return {Promise}
    * @resolves When the operation finished successfully.
    * @rejects JavaScript exception.
    */
   async _save() {
-    let json;
-    try {
-      json = JSON.stringify(this._data);
-    } catch (e) {
-      // If serialization fails, try fallback safe JSON converter.
-      if (typeof this._data.toJSONSafe == "function") {
-        json = JSON.stringify(this._data.toJSONSafe());
-      } else {
-        throw e;
-      }
-    }
-
     // Create or overwrite the file.
-    let bytes = gTextEncoder.encode(json);
+    let bytes = gTextEncoder.encode(JSON.stringify(this._data));
     if (this._beforeSave) {
       await Promise.resolve(this._beforeSave());
     }
     await OS.File.writeAtomic(this.path, bytes,
                               Object.assign(
                                 { tmpPath: this.path + ".tmp" },
                                 this._options));
   },