Bug 1309351: Part 2 - Use a shared global sandbox and simple module resolution for built-in modules. r?ochameau
This takes another 21% off the Jetpack test suite run time for me.
MozReview-Commit-ID: 1U2lq4PN21w
--- a/addon-sdk/source/lib/sdk/addon/bootstrap.js
+++ b/addon-sdk/source/lib/sdk/addon/bootstrap.js
@@ -152,21 +152,31 @@ Bootstrap.prototype = {
return new Promise(resolve => {
const { loader } = this;
if (loader) {
this.loader = null;
unload(loader, reason);
setTimeout(() => {
for (let uri of Object.keys(loader.sandboxes)) {
- Cu.nukeSandbox(loader.sandboxes[uri]);
+ try {
+ Cu.nukeSandbox(loader.sandboxes[uri]);
+ } catch (e) {
+ // This will throw for shared sandboxes.
+ }
delete loader.sandboxes[uri];
delete loader.modules[uri];
}
+ try {
+ Cu.nukeSandbox(loader.sharedGlobalSandbox);
+ } catch (e) {
+ Cu.reportError(e);
+ }
+
resolve();
}, 1000);
}
else {
resolve();
}
});
}
--- a/addon-sdk/source/lib/sdk/system/globals.js
+++ b/addon-sdk/source/lib/sdk/system/globals.js
@@ -30,10 +30,17 @@ Object.defineProperty(exports, 'define',
// variables remain accessible.
configurable: true,
get: function() {
let sandbox = this;
return function define(factory) {
factory = Array.slice(arguments).pop();
factory.call(sandbox, sandbox.require, sandbox.exports, sandbox.module);
}
- }
+ },
+ set: function(value) {
+ Object.defineProperty(this, 'define', {
+ configurable: true,
+ enumerable: true,
+ value,
+ });
+ },
});
--- a/addon-sdk/source/lib/toolkit/loader.js
+++ b/addon-sdk/source/lib/toolkit/loader.js
@@ -469,24 +469,25 @@ const load = iced(function load(loader,
get Components() {
// Expose `Components` property to throw error on usage with
// additional information
throw new ReferenceError(COMPONENT_ERROR);
}
});
let sandbox;
- if (loader.sharedGlobalSandbox &&
+ if ((loader.useSharedGlobalSandbox || isSystemURI(module.uri)) &&
loader.sharedGlobalBlocklist.indexOf(module.id) == -1) {
// Create a new object in this sandbox, that will be used as
// the scope object for this particular module
sandbox = new loader.sharedGlobalSandbox.Object();
// Inject all expected globals in the scope object
getOwnIdentifiers(globals).forEach(function(name) {
descriptors[name] = getOwnPropertyDescriptor(globals, name)
+ descriptors[name].configurable = true;
});
Object.defineProperties(sandbox, descriptors);
}
else {
sandbox = Sandbox({
name: module.uri,
prototype: Object.create(globals, descriptors),
wantXrays: false,
@@ -730,16 +731,21 @@ Loader.resolveURI = resolveURI;
// with it during link time.
const Require = iced(function Require(loader, requirer) {
let {
modules, mapping, resolve: loaderResolve, load,
manifest, rootURI, isNative, requireMap,
requireHook
} = loader;
+ if (isSystemURI(requirer.uri)) {
+ isNative = false;
+ loaderResolve = Loader.resolve;
+ }
+
function require(id) {
if (!id) // Throw if `id` is not passed.
throw Error('You must provide a module name when calling require() from '
+ requirer.id, requirer.uri);
if (requireHook) {
return requireHook(id, _require);
}
@@ -861,16 +867,19 @@ const Require = iced(function Require(lo
// If not found in the map, not a node module, and wasn't able to be
// looked up, it's something
// found in the paths most likely, like `sdk/tabs`, which should
// be resolved relatively if needed using traditional resolve
if (!requirement) {
requirement = isRelative(id) ? Loader.resolve(id, requirer.id) : id;
}
}
+ else if (modules[id]) {
+ uri = requirement = id;
+ }
else if (requirer) {
// Resolve `id` to its requirer if it's relative.
requirement = loaderResolve(id, requirer.id);
}
else {
requirement = id;
}
@@ -1034,45 +1043,43 @@ function Loader(options) {
get: function() {
return builtinModuleExports[id];
}
});
modules[uri] = freeze(module);
}
- let sharedGlobalSandbox;
- if (sharedGlobal) {
- // Create the unique sandbox we will be using for all modules,
- // so that we prevent creating a new comportment per module.
- // The side effect is that all modules will share the same
- // global objects.
- sharedGlobalSandbox = Sandbox({
- name: "Addon-SDK",
- wantXrays: false,
- wantGlobalProperties: [],
- invisibleToDebugger: options.invisibleToDebugger || false,
- metadata: {
- addonID: options.id,
- URI: "Addon-SDK"
- },
- prototype: options.sandboxPrototype || {}
- });
- }
+ // Create the unique sandbox we will be using for all modules,
+ // so that we prevent creating a new comportment per module.
+ // The side effect is that all modules will share the same
+ // global objects.
+ let sharedGlobalSandbox = Sandbox({
+ name: "Addon-SDK",
+ wantXrays: false,
+ wantGlobalProperties: [],
+ invisibleToDebugger: options.invisibleToDebugger || false,
+ metadata: {
+ addonID: options.id,
+ URI: "Addon-SDK"
+ },
+ prototype: options.sandboxPrototype || {}
+ });
// Loader object is just a representation of a environment
// state. We freeze it and mark make it's properties non-enumerable
// as they are pure implementation detail that no one should rely upon.
let returnObj = {
destructor: { enumerable: false, value: destructor },
globals: { enumerable: false, value: globals },
mapping: { enumerable: false, value: mapping },
// Map of module objects indexed by module URIs.
modules: { enumerable: false, value: modules },
metadata: { enumerable: false, value: metadata },
+ useSharedGlobalSandbox: { enumerable: false, value: !!sharedGlobal },
sharedGlobalSandbox: { enumerable: false, value: sharedGlobalSandbox },
sharedGlobalBlocklist: { enumerable: false, value: sharedGlobalBlocklist },
sharedGlobalBlacklist: { enumerable: false, value: sharedGlobalBlocklist },
// Map of module sandboxes indexed by module URIs.
sandboxes: { enumerable: false, value: {} },
resolve: { enumerable: false, value: resolve },
// ID of the addon, if provided.
id: { enumerable: false, value: options.id },
@@ -1102,16 +1109,18 @@ function Loader(options) {
returnObj.requireMap = { enumerable: false, value: requireMap };
returnObj.rootURI = { enumerable: false, value: addTrailingSlash(rootURI) };
}
return freeze(Object.create(null, returnObj));
};
Loader.Loader = Loader;
+var isSystemURI = uri => /^resource:\/\/(gre|devtools|testing-common)\//.test(uri);
+
var isJSONURI = uri => uri.endsWith('.json');
var isJSMURI = uri => uri.endsWith('.jsm');
var isJSURI = uri => uri.endsWith('.js');
var isAbsoluteURI = uri => uri.startsWith("resource://") ||
uri.startsWith("chrome://") ||
uri.startsWith("file://");
var isRelative = id => id.startsWith(".");
--- a/addon-sdk/source/test/test-content-events.js
+++ b/addon-sdk/source/test/test-content-events.js
@@ -61,22 +61,17 @@ exports["test dead object errors"] = fun
let cleanup = () => system.off("console-api-log-event", onMessage);
let fail = (reason) => {
cleanup();
assert.fail(reason);
}
loader.unload();
- // in order to get a dead object error on this module, we need to nuke
- // the relative sandbox; unload the loader is not enough
- let url = Object.keys(loader.sandboxes).
- find(url => url.endsWith("/sdk/content/events.js"));
-
- nuke(loader.sandboxes[url]);
+ nuke(loader.sharedGlobalSandbox);
system.on("console-api-log-event", onMessage, true);
openBrowserWindow().
then(closeWindow).
then(() => assert.pass("checking dead object errors")).
then(cleanup).
then(done, fail);