Bug 1370752: Part 3 - Use structured clone rather than JSON to sanitize storage values. r=aswan

This gives us performance wins in sevaral areas:

- Creating a structured clone blob of storage data directly from the source
  compartment allows us to avoid X-ray and JSON serialization overhead when
  storing new values.

- Storing the intermediate StructuredCloneBlob, rather than JSON values,
  in-memory saves us additional JSON and structured clone overhead when
  passing the values to listeners and API callers, and saves us a fair amount
  of memory to boot.

- Serializing storage values before sending them over a message manager allows
  us to deserialize them directly into an extension scope on the other side,
  saving us a lot of additional structured clone overhead and intermediate
  garbage generation.

- Using JSONFile.jsm for storage lets us consolidate multiple storage file
  write operations, rather than performing a separate JSON serialization for
  each individual storage write.

- Additionally, this paves the way for us to transition to IndexedDB as a
  storage backend, with full support for arbitrary structured-clone-compatible
  data structures.

MozReview-Commit-ID: JiRE7EFMYxn

--HG--
extra : rebase_source : 04a5681c604c5d2acd781b7ce4f66a757465071a
This commit is contained in:
Kris Maglione 2017-06-29 14:11:05 -07:00
parent 45acce829f
commit a835678477
8 changed files with 369 additions and 174 deletions

View File

@ -806,10 +806,14 @@ class ChildAPIManager {
* @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) {
callParentAsyncFunction(path, args, callback, options = {}) {
let callId = getUniqueId();
let deferred = PromiseUtils.defer();
this.callPromises.set(callId, deferred);
@ -819,6 +823,7 @@ class ChildAPIManager {
callId,
path,
args,
noClone: options.noClone || false,
});
return this.context.wrapPromise(deferred.promise, callback);

View File

@ -1101,7 +1101,7 @@ class SchemaAPIManager extends EventEmitter {
sandboxName: `Namespace of ext-*.js scripts for ${this.processType}`,
});
Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeWorker, ExtensionCommon, MatchPattern, MatchPatternSet, extensions: this});
Object.assign(global, {global, Cc, Ci, Cu, Cr, XPCOMUtils, ChromeWorker, ExtensionCommon, MatchPattern, MatchPatternSet, StructuredCloneHolder, extensions: this});
Cu.import("resource://gre/modules/AppConstants.jsm", global);
Cu.import("resource://gre/modules/ExtensionAPI.jsm", global);

View File

@ -667,7 +667,7 @@ ParentAPIManager = {
};
try {
let args = Cu.cloneInto(data.args, context.sandbox);
let args = data.noClone ? data.args : Cu.cloneInto(data.args, context.sandbox);
let pendingBrowser = context.pendingEventBrowser;
let fun = await context.apiCan.asyncFindAPIPath(data.path);
let result = context.withPendingBrowser(pendingBrowser,

View File

@ -13,66 +13,123 @@ 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");
XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
"resource://gre/modules/AsyncShutdown.jsm");
const global = this;
function isStructuredCloneHolder(value) {
return (value && typeof value === "object" &&
Cu.getClassName(value, true) === "StructuredCloneHolder");
}
class SerializeableMap extends Map {
toJSON() {
let result = {};
for (let [key, value] of this) {
if (isStructuredCloneHolder(value)) {
value = value.deserialize(global);
this.set(key, value);
}
result[key] = value;
}
return result;
}
/**
* 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;
}
}
/**
* Helper function used to sanitize the objects that have to be saved in the ExtensionStorage.
* 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.
*
* @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.
* 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.
*
* @returns {any}
* The sanitized value of the property.
* @param {StructuredCloneHolder|*} value
* A value to serialize.
* @returns {*}
*/
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;
}
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;
function serialize(value) {
if (value && typeof value === "object" && !isStructuredCloneHolder(value)) {
return new StructuredCloneHolder(value);
}
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();
}
// Everything else, omit entirely.
return undefined;
return value;
}
this.ExtensionStorage = {
cache: new Map(),
// Map<extension-id, Promise<JSONFile>>
jsonFilePromises: 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.
@ -85,129 +142,167 @@ this.ExtensionStorage = {
* The sanitized value.
*/
sanitize(value, context) {
let json = context.jsonStringify(value, jsonReplacer.bind(null, context));
let json = context.jsonStringify(value === undefined ? null : value);
if (json == undefined) {
throw new ExtensionUtils.ExtensionError("DataCloneError: The object could not be cloned.");
}
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");
},
read(extensionId) {
if (this.cache.has(extensionId)) {
return this.cache.get(extensionId);
/**
* 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);
}
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;
this.notifyListeners(extensionId, changes);
jsonFile.saveSoon();
return null;
},
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.");
});
/**
* 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);
AsyncShutdown.profileBeforeChange.addBlocker(
"ExtensionStorage: Finish writing extension data",
promise);
let changed = false;
let changes = {};
return promise.then(() => {
AsyncShutdown.profileBeforeChange.removeBlocker(promise);
});
},
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;
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);
return this.write(extensionId);
});
jsonFile.saveSoon();
}
return null;
},
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];
}
/**
* 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);
let changed = false;
let changes = {};
for (let [prop, oldValue] of jsonFile.data.entries()) {
changes[prop] = {oldValue: serialize(oldValue)};
jsonFile.data.delete(prop);
changed = true;
}
if (changed) {
this.notifyListeners(extensionId, changes);
return this.write(extensionId);
});
jsonFile.saveSoon();
}
return null;
},
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];
}
/**
* 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;
this.notifyListeners(extensionId, changes);
return this.write(extensionId);
});
},
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];
}
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];
}
}
} else {
for (let prop of [].concat(keys)) {
if (data.has(prop)) {
result[prop] = serialize(data.get(prop));
}
}
}
return result;
});
return result;
},
addOnChangedListener(extensionId, listener) {
@ -243,7 +338,10 @@ this.ExtensionStorage = {
Services.obs.removeObserver(this, "extension-invalidate-storage-cache");
Services.obs.removeObserver(this, "xpcom-shutdown");
} else if (topic == "extension-invalidate-storage-cache") {
this.cache.clear();
for (let promise of this.jsonFilePromises.values()) {
promise.then(jsonFile => { jsonFile.finalize(); });
}
this.jsonFilePromises.clear();
}
},
};

View File

@ -1,16 +1,71 @@
"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.
@ -30,6 +85,7 @@ this.storage = class extends ExtensionAPI {
}
return sanitized;
}
return {
storage: {
local: {
@ -37,10 +93,9 @@ this.storage = class extends ExtensionAPI {
const stopwatchKey = {};
TelemetryStopwatch.start(storageGetHistogram, stopwatchKey);
try {
keys = sanitize(keys);
let result = await context.childManager.callParentAsyncFunction("storage.local.get", [
keys,
]);
serialize(keys),
], null, {noClone: true}).then(deserialize);
TelemetryStopwatch.finish(storageGetHistogram, stopwatchKey);
return result;
} catch (e) {
@ -52,10 +107,9 @@ this.storage = class extends ExtensionAPI {
const stopwatchKey = {};
TelemetryStopwatch.start(storageSetHistogram, stopwatchKey);
try {
items = sanitize(items);
let result = await context.childManager.callParentAsyncFunction("storage.local.set", [
items,
]);
serialize(items),
], null, {noClone: true});
TelemetryStopwatch.finish(storageSetHistogram, stopwatchKey);
return result;
} catch (e) {
@ -79,6 +133,22 @@ this.storage = class extends ExtensionAPI {
]);
},
},
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(),
},
};
}

View File

@ -65,7 +65,7 @@ this.storage = class extends ExtensionAPI {
onChanged: new EventManager(context, "storage.onChanged", fire => {
let listenerLocal = changes => {
fire.async(changes, "local");
fire.raw(changes, "local");
};
let listenerSync = changes => {
fire.async(changes, "sync");

View File

@ -149,6 +149,8 @@ async function contentScript(checkGet) {
// Make sure the set() handler landed.
await globalChanges;
let date = new Date(0);
clearGlobalChanges();
await storage.set({
"test-prop1": {
@ -160,12 +162,19 @@ async function contentScript(checkGet) {
arr: [1, 2],
date: new Date(0),
regexp: /regexp/,
func: function func() {},
window,
},
});
await storage.set({"test-prop2": function func() {}});
await browser.test.assertRejects(
storage.set({
window,
}),
/DataCloneError|cyclic object value/);
await browser.test.assertRejects(
storage.set({"test-prop2": function func() {}}),
/DataCloneError/);
const recentChanges = await globalChanges;
browser.test.assertEq("value1", recentChanges["test-prop1"].oldValue, "oldValue correct");
@ -175,24 +184,26 @@ async function contentScript(checkGet) {
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");

View File

@ -253,6 +253,8 @@ add_task(async function test_backgroundScript() {
// Make sure the set() handler landed.
await globalChanges;
let date = new Date(0);
clearGlobalChanges();
await storage.set({
"test-prop1": {
@ -262,14 +264,21 @@ add_task(async function test_backgroundScript() {
undef: undefined,
obj: {},
arr: [1, 2],
date: new Date(0),
date,
regexp: /regexp/,
func: function func() {},
window,
},
});
await storage.set({"test-prop2": function func() {}});
await browser.test.assertRejects(
storage.set({
window,
}),
/DataCloneError|cyclic object value/);
await browser.test.assertRejects(
storage.set({"test-prop2": function func() {}}),
/DataCloneError/);
const recentChanges = await globalChanges;
browser.test.assertEq("value1", recentChanges["test-prop1"].oldValue, "oldValue correct");
@ -279,24 +288,26 @@ add_task(async function test_backgroundScript() {
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");