Bug 1392602 - Simplify SDK loader to only match DevTools needs. r=jdescottes draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 28 Aug 2017 14:52:07 +0200
changeset 657253 03c58d851f6a94d8cf112bcc11a744ac867d90d0
parent 657252 bf3b29ba3cc9c16db11de0ea2304856996237afb
child 657254 23e8440f2e30e9d23f53673cfe85eb94179fe45f
push id77496
push userbmo:poirot.alex@gmail.com
push dateFri, 01 Sep 2017 09:32:27 +0000
reviewersjdescottes
bugs1392602
milestone57.0a1
Bug 1392602 - Simplify SDK loader to only match DevTools needs. r=jdescottes MozReview-Commit-ID: 5MUEmCW6AJM
devtools/client/shared/browser-loader.js
devtools/shared/Loader.jsm
devtools/shared/base-loader.js
--- a/devtools/client/shared/browser-loader.js
+++ b/devtools/client/shared/browser-loader.js
@@ -102,17 +102,16 @@ function BrowserLoaderBuilder({ baseURI,
       "resource://devtools/client/shared/vendor/react-dev";
   }
 
   const opts = {
     id: "browser-loader",
     sharedGlobal: true,
     sandboxPrototype: window,
     sandboxName: "DevTools (UI loader)",
-    noSandboxAddonId: true,
     paths: Object.assign({}, dynamicPaths, loaderOptions.paths),
     invisibleToDebugger: loaderOptions.invisibleToDebugger,
     requireHook: (id, require) => {
       // If |id| requires special handling, simply defer to devtools
       // immediately.
       if (devtools.isLoaderPluginId(id)) {
         return devtools.require(id);
       }
--- a/devtools/shared/Loader.jsm
+++ b/devtools/shared/Loader.jsm
@@ -69,17 +69,16 @@ BuiltinProvider.prototype = {
       paths.promise = "resource://gre/modules/Promise-backend.js";
     }
     this.loader = new Loader({
       id: "fx-devtools",
       paths,
       invisibleToDebugger: this.invisibleToDebugger,
       sharedGlobal: true,
       sandboxName: "DevTools (Module loader)",
-      noSandboxAddonId: true,
       requireHook: (id, require) => {
         if (id.startsWith("raw!")) {
           return requireRawId(id, require);
         }
         return require(id);
       },
     });
   },
--- a/devtools/shared/base-loader.js
+++ b/devtools/shared/base-loader.js
@@ -6,32 +6,30 @@
 
 this.EXPORTED_SYMBOLS = ["Loader", "resolveURI", "Module", "Require"]
 
 const { classes: Cc, Constructor: CC, interfaces: Ci, utils: Cu,
         results: Cr, manager: Cm } = Components;
 const systemPrincipal = CC('@mozilla.org/systemprincipal;1', 'nsIPrincipal')();
 const { loadSubScript } = Cc['@mozilla.org/moz/jssubscript-loader;1'].
                      getService(Ci.mozIJSSubScriptLoader);
-const { addObserver, notifyObservers } = Cc['@mozilla.org/observer-service;1'].
+const { notifyObservers } = Cc['@mozilla.org/observer-service;1'].
                         getService(Ci.nsIObserverService);
 const { XPCOMUtils } = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
-const { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {});
-const { normalize, dirname } = Cu.import("resource://gre/modules/osfile/ospath_unix.jsm");
+const { normalize, dirname } = Cu.import("resource://gre/modules/osfile/ospath_unix.jsm", {});
 
 XPCOMUtils.defineLazyServiceGetter(this, "resProto",
                                    "@mozilla.org/network/protocol;1?name=resource",
                                    "nsIResProtocolHandler");
+XPCOMUtils.defineLazyModuleGetter(this, "NetUtil", "resource://gre/modules/NetUtil.jsm");
 
 const { defineLazyGetter } = XPCOMUtils;
 
 // Define some shortcuts.
 const bind = Function.call.bind(Function.bind);
-const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
-const prototypeOf = Object.getPrototypeOf;
 function* getOwnIdentifiers(x) {
   yield* Object.getOwnPropertyNames(x);
   yield* Object.getOwnPropertySymbols(x);
 }
 
 function sourceURI(uri) { return String(uri).split(" -> ").pop(); }
 
 function isntLoaderFrame(frame) { return frame.fileName !== __URI__ }
@@ -107,62 +105,44 @@ function join(base, ...paths) {
 // Function takes set of options and returns a JS sandbox. Function may be
 // passed set of options:
 //  - `name`: A string value which identifies the sandbox in about:memory. Will
 //    throw exception if omitted.
 // - `principal`: String URI or `nsIPrincipal` for the sandbox. Defaults to
 //    system principal.
 // - `prototype`: Ancestor for the sandbox that will be created. Defaults to
 //    `{}`.
-// - `wantXrays`: A Boolean value indicating whether code outside the sandbox
-//    wants X-ray vision with respect to objects inside the sandbox. Defaults
-//    to `true`.
 // - `sandbox`: A sandbox to share JS compartment with. If omitted new
 //    compartment will be created.
-// - `metadata`: A metadata object associated with the sandbox. It should
-//    be JSON-serializable.
 // For more details see:
 // https://developer.mozilla.org/en/Components.utils.Sandbox
 function Sandbox(options) {
   // Normalize options and rename to match `Cu.Sandbox` expectations.
   options = {
     // Do not expose `Components` if you really need them (bad idea!) you
     // still can expose via prototype.
     wantComponents: false,
     sandboxName: options.name,
-    principal: 'principal' in options ? options.principal : systemPrincipal,
-    wantXrays: 'wantXrays' in options ? options.wantXrays : true,
     sandboxPrototype: 'prototype' in options ? options.prototype : {},
     invisibleToDebugger: 'invisibleToDebugger' in options ?
                          options.invisibleToDebugger : false,
-    metadata: 'metadata' in options ? options.metadata : {},
-    waiveIntereposition: !!options.waiveIntereposition
+    waiveInterposition: false
   };
 
-  if (options.metadata && options.metadata.addonID) {
-    options.addonId = options.metadata.addonID;
-  }
-
-  let sandbox = Cu.Sandbox(options.principal, options);
+  let sandbox = Cu.Sandbox(systemPrincipal, options);
 
-  // Each sandbox at creation gets set of own properties that will be shadowing
-  // ones from it's prototype. We override delete such `sandbox` properties
-  // to avoid shadowing.
-  delete sandbox.Iterator;
   delete sandbox.Components;
-  delete sandbox.importFunction;
-  delete sandbox.debug;
 
   return sandbox;
 }
 
 // Populates `exports` of the given CommonJS `module` object, in the context
 // of the given `loader` by evaluating code associated with it.
 function load(loader, module) {
-  let { sandboxes, globals, loadModuleHook } = loader;
+  let { sandboxes, globals } = loader;
   let require = Require(loader, module);
 
   // We expose set of properties defined by `CommonJS` specification via
   // prototype of the sandbox. Also globals are deeper in the prototype
   // chain so that each module has access to them as well.
   let descriptors = {
     require: {
       configurable: true,
@@ -180,17 +160,17 @@ function load(loader, module) {
       configurable: true,
       enumerable: true,
       writable: true,
       value: module.exports
     },
   };
 
   let sandbox;
-  if (loader.useSharedGlobalSandbox || isSystemURI(module.uri)) {
+  if (loader.useSharedGlobalSandbox) {
     // Create a new object in this sandbox, that will be used as
     // the scope object for this particular module
     sandbox = new loader.sharedGlobalSandbox.Object();
     descriptors.lazyRequire = {
       configurable: true,
       value: lazyRequire.bind(sandbox),
     };
     descriptors.lazyRequireModule = {
@@ -212,22 +192,17 @@ function load(loader, module) {
     if ("DOMParser" in globals)
       descriptors.DOMParser = Object.getOwnPropertyDescriptor(globals, "DOMParser");
     Object.defineProperties(sandbox, descriptors);
   }
   else {
     sandbox = Sandbox({
       name: module.uri,
       prototype: Object.create(globals, descriptors),
-      wantXrays: false,
-      invisibleToDebugger: loader.invisibleToDebugger,
-      metadata: {
-        addonID: loader.id,
-        URI: module.uri
-      }
+      invisibleToDebugger: loader.invisibleToDebugger
     });
   }
   sandboxes[module.uri] = sandbox;
 
   let originalExports = module.exports;
   try {
     loadSubScript(module.uri, sandbox, 'UTF-8');
   }
@@ -264,20 +239,16 @@ function load(loader, module) {
       message: { value: message, writable: true, configurable: true },
       fileName: { value: fileName, writable: true, configurable: true },
       lineNumber: { value: lineNumber, writable: true, configurable: true },
       stack: { value: serializeStack(frames), writable: true, configurable: true },
       toString: { value: () => toString, writable: true, configurable: true },
     });
   }
 
-  if (loadModuleHook) {
-    module = loadModuleHook(module, require);
-  }
-
   // Only freeze the exports object if we created it ourselves. Modules
   // which completely replace the exports object and still want it
   // frozen need to freeze it themselves.
   if (module.exports === originalExports)
     Object.freeze(module.exports);
 
   return module;
 }
@@ -308,20 +279,16 @@ function resolve(id, base) {
   // Joining and normalizing removes the './' from relative files.
   // We need to ensure the resolution still has the root
   if (base.startsWith('./'))
     resolved = './' + resolved;
 
   return resolved;
 }
 
-function addTrailingSlash(path) {
-  return path.replace(/\/*$/, "/");
-}
-
 function compileMapping(paths) {
   // Make mapping array that is sorted from longest path to shortest path.
   let mapping = Object.keys(paths)
                       .sort((a, b) => b.length - a.length)
                       .map(path => [path, paths[path]]);
 
   const PATTERN = /([.\\?+*(){}[\]^$])/g;
   const escapeMeta = str => str.replace(PATTERN, '\\$1')
@@ -421,18 +388,17 @@ function lazyRequireModule(obj, moduleId
 
 
 // Creates version of `require` that will be exposed to the given `module`
 // in the context of the given `loader`. Each module gets own limited copy
 // of `require` that is allowed to load only a modules that are associated
 // with it during link time.
 function Require(loader, requirer) {
   let {
-    modules, mapping, mappingCache,
-    manifest, rootURI, isNative, requireHook
+    modules, mapping, mappingCache, requireHook
   } = loader;
 
   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) {
@@ -575,34 +541,30 @@ function unload(loader, reason) {
   // asserted against loader.destructor or require('@loader/unload')
   // Note: We don not destroy loader's module cache or sandboxes map as
   // some modules may do cleanup in subsequent turns of event loop. Destroying
   // cache may cause module identity problems in such cases.
   let subject = { wrappedJSObject: loader.destructor };
   notifyObservers(subject, 'sdk:loader:destroy', reason);
 };
 
-// Function makes new loader that can be used to load CommonJS modules
-// described by a given `options.manifest`. Loader takes following options:
+// Function makes new loader that can be used to load CommonJS modules.
+// Loader takes following options:
 // - `globals`: Optional map of globals, that all module scopes will inherit
 //   from. Map is also exposed under `globals` property of the returned loader
 //   so it can be extended further later. Defaults to `{}`.
 // - `modules` Optional map of built-in module exports mapped by module id.
 //   These modules will incorporated into module cache. Each module will be
 //   frozen.
 // - `resolve` Optional module `id` resolution function. If given it will be
 //   used to resolve module URIs, by calling it with require term, requirer
 //   module object (that has `uri` property) and `baseURI` of the loader.
 //   If `resolve` does not returns `uri` string exception will be thrown by
 //   an associated `require` call.
 function Loader(options) {
-  function normalizeRootURI(uri) {
-    return addTrailingSlash(join(uri));
-  }
-
   let { paths, sharedGlobal, globals } = options;
   if (!globals) {
     globals = {};
   }
 
   // We create an identity object that will be dispatched on an unload
   // event as subject. This way unload listeners will be able to assert
   // which loader is unloaded. Please note that we intentionally don't
@@ -644,32 +606,27 @@ function Loader(options) {
   }
 
   // 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: options.sandboxName || "Addon-SDK",
-    wantXrays: false,
     invisibleToDebugger: options.invisibleToDebugger || false,
-    metadata: {
-      addonID: options.noSandboxAddonId ? undefined : options.id,
-      URI: options.sandboxName || "Addon-SDK"
-    },
     prototype: options.sandboxPrototype || globals,
   });
 
   if (options.sandboxPrototype) {
     // If we were given a sandboxPrototype, we have to define the globals on
     // the sandbox directly. Note that this will not work for callers who
     // depend on being able to add globals after the loader was created.
     for (let name of getOwnIdentifiers(globals))
       Object.defineProperty(sharedGlobalSandbox, name,
-                            getOwnPropertyDescriptor(globals, name));
+                            Object.getOwnPropertyDescriptor(globals, name));
   }
 
   // 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 },
@@ -682,23 +639,19 @@ function Loader(options) {
     // Map of module sandboxes indexed by module URIs.
     sandboxes: { enumerable: false, value: {} },
     // ID of the addon, if provided.
     id: { enumerable: false, value: options.id },
     // Whether the modules loaded should be ignored by the debugger
     invisibleToDebugger: { enumerable: false,
                            value: options.invisibleToDebugger || false },
     requireHook: { enumerable: false, value: options.requireHook },
-    loadModuleHook: { enumerable: false, value: options.loadModuleHook },
   };
 
   return Object.create(null, returnObj);
 };
 
-var SystemRegExp = /^resource:\/\/(gre|devtools|testing-common)\//;
-var isSystemURI = uri => SystemRegExp.test(uri);
-
 var isJSONURI = uri => uri.endsWith('.json');
 var isJSMURI = uri => uri.endsWith('.jsm');
 var isJSURI = uri => uri.endsWith('.js');
 var AbsoluteRegExp = /^(resource|chrome|file|jar):/;
 var isAbsoluteURI = uri => AbsoluteRegExp.test(uri);
 var isRelative = id => id.startsWith(".");