Bug 946813 - Part 3: Use independent loaders and mark them invisible. r=past
☠☠ backed out by 9d410546acc3 ☠ ☠
authorJ. Ryan Stinnett <jryans@gmail.com>
Tue, 07 Jan 2014 12:25:13 -0600
changeset 163076 e4a784a2c735d1326ecc6338075a838889e1e3ed
parent 163075 6520037420c15ce13707f03e0eae38bf2bc70bd4
child 163077 937f571f465568e0eeec4cd5ce9d0a1e41dd8c33
push id38388
push usercbook@mozilla.com
push dateMon, 13 Jan 2014 12:27:31 +0000
treeherdermozilla-inbound@a68d50181ebb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast
bugs946813
milestone29.0a1
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
Bug 946813 - Part 3: Use independent loaders and mark them invisible. r=past
browser/devtools/framework/ToolboxProcess.jsm
toolkit/devtools/Loader.jsm
toolkit/devtools/tests/unit/test_independent_loaders.js
toolkit/devtools/tests/unit/test_invisible_loader.js
toolkit/devtools/tests/unit/xpcshell.ini
--- a/browser/devtools/framework/ToolboxProcess.jsm
+++ b/browser/devtools/framework/ToolboxProcess.jsm
@@ -51,18 +51,20 @@ BrowserToolboxProcess.prototype = {
    */
   _initServer: function() {
     dumpn("Initializing the chrome toolbox server.");
 
     if (!this.loader) {
       // Create a separate loader instance, so that we can be sure to receive a
       // separate instance of the DebuggingServer from the rest of the devtools.
       // This allows us to safely use the tools against even the actors and
-      // DebuggingServer itself.
+      // DebuggingServer itself, especially since we can mark this loader as
+      // invisible to the debugger (unlike the usual loader settings).
       this.loader = new DevToolsLoader();
+      this.loader.invisibleToDebugger = true;
       this.loader.main("devtools/server/main");
       this.debuggerServer = this.loader.DebuggerServer;
       dumpn("Created a separate loader instance for the DebuggerServer.");
     }
 
     if (!this.debuggerServer.initialized) {
       this.debuggerServer.init();
       this.debuggerServer.addBrowserActors();
--- a/toolkit/devtools/Loader.jsm
+++ b/toolkit/devtools/Loader.jsm
@@ -38,17 +38,18 @@ let loaderGlobals = {
   loader: {
     lazyGetter: XPCOMUtils.defineLazyGetter.bind(XPCOMUtils),
     lazyImporter: XPCOMUtils.defineLazyModuleGetter.bind(XPCOMUtils),
     lazyServiceGetter: XPCOMUtils.defineLazyServiceGetter.bind(XPCOMUtils)
   }
 };
 
 // Used when the tools should be loaded from the Firefox package itself (the default)
-var BuiltinProvider = {
+function BuiltinProvider() {}
+BuiltinProvider.prototype = {
   load: function() {
     this.loader = new loader.Loader({
       modules: {
         "toolkit/loader": loader,
         "source-map": SourceMap,
       },
       paths: {
         // When you add a line to this mapping, don't forget to make a
@@ -67,32 +68,34 @@ var BuiltinProvider = {
         "devtools/pretty-fast": "resource://gre/modules/devtools/pretty-fast.js",
 
         "acorn": "resource://gre/modules/devtools/acorn.js",
         "acorn_loose": "resource://gre/modules/devtools/acorn_loose.js",
 
         // Allow access to xpcshell test items from the loader.
         "xpcshell-test": "resource://test"
       },
-      globals: loaderGlobals
+      globals: loaderGlobals,
+      invisibleToDebugger: this.invisibleToDebugger
     });
 
     return promise.resolve(undefined);
   },
 
   unload: function(reason) {
     loader.unload(this.loader, reason);
     delete this.loader;
   },
 };
 
 // Used when the tools should be loaded from a mozilla-central checkout.  In addition
 // to different paths, it needs to write chrome.manifest files to override chrome urls
 // from the builtin tools.
-var SrcdirProvider = {
+function SrcdirProvider() {}
+SrcdirProvider.prototype = {
   fileURI: function(path) {
     let file = new FileUtils.File(path);
     return Services.io.newFileURI(file).spec;
   },
 
   load: function() {
     let srcdir = Services.prefs.getComplexValue("devtools.loader.srcdir",
                                                 Ci.nsISupportsString);
@@ -129,17 +132,18 @@ var SrcdirProvider = {
         "devtools/output-parser": outputParserURI,
         "devtools/touch-events": touchEventsURI,
         "devtools/client": clientURI,
         "devtools/pretty-fast": prettyFastURI,
 
         "acorn": acornURI,
         "acorn_loose": acornLoosseURI
       },
-      globals: loaderGlobals
+      globals: loaderGlobals,
+      invisibleToDebugger: this.invisibleToDebugger
     });
 
     return this._writeManifest(devtoolsDir).then(null, Cu.reportError);
   },
 
   unload: function(reason) {
     loader.unload(this.loader, reason);
     delete this.loader;
@@ -212,48 +216,63 @@ var SrcdirProvider = {
 
 /**
  * The main devtools API.
  * In addition to a few loader-related details, this object will also include all
  * exports from the main module.  The standard instance of this loader is
  * exported as |devtools| below, but if a fresh copy of the loader is needed,
  * then a new one can also be created.
  */
-this.DevToolsLoader = function DevToolsLoader() {
-  this._chooseProvider();
-};
+this.DevToolsLoader = function DevToolsLoader() {};
 
 DevToolsLoader.prototype = {
+  get provider() {
+    if (!this._provider) {
+      this._chooseProvider();
+    }
+    return this._provider;
+  },
+
   _provider: null,
 
   /**
+   * A dummy version of require, in case a provider hasn't been chosen yet when
+   * this is first called.  This will then be replaced by the real version.
+   * @see setProvider
+   */
+  require: function() {
+    this._chooseProvider();
+    return this.require.apply(this, arguments);
+  },
+
+  /**
    * Add a URI to the loader.
    * @param string id
    *    The module id that can be used within the loader to refer to this module.
    * @param string uri
    *    The URI to load as a module.
    * @returns The module's exports.
    */
   loadURI: function(id, uri) {
     let module = loader.Module(id, uri);
-    return loader.load(this._provider.loader, module).exports;
+    return loader.load(this.provider.loader, module).exports;
   },
 
   /**
    * Let the loader know the ID of the main module to load.
    *
    * The loader doesn't need a main module, but it's nice to have.  This
    * will be called by the browser devtools to load the devtools/main module.
    *
    * When only using the server, there's no main module, and this method
    * can be ignored.
    */
   main: function(id) {
     this._mainid = id;
-    this._main = loader.main(this._provider.loader, id);
+    this._main = loader.main(this.provider.loader, id);
 
     // Mirror the main module's exports on this object.
     Object.getOwnPropertyNames(this._main).forEach(key => {
       XPCOMUtils.defineLazyGetter(this, key, () => this._main[key]);
     });
   },
 
   /**
@@ -266,43 +285,55 @@ DevToolsLoader.prototype = {
 
     if (this._provider) {
       var events = this.require("sdk/system/events");
       events.emit("devtools-unloaded", {});
       delete this.require;
       this._provider.unload("newprovider");
     }
     this._provider = provider;
+    this._provider.invisibleToDebugger = this.invisibleToDebugger;
     this._provider.load();
     this.require = loader.Require(this._provider.loader, { id: "devtools" });
 
     if (this._mainid) {
       this.main(this._mainid);
     }
   },
 
   /**
    * Choose a default tools provider based on the preferences.
    */
   _chooseProvider: function() {
     if (Services.prefs.prefHasUserValue("devtools.loader.srcdir")) {
-      this.setProvider(SrcdirProvider);
+      this.setProvider(new SrcdirProvider());
     } else {
-      this.setProvider(BuiltinProvider);
+      this.setProvider(new BuiltinProvider());
     }
   },
 
   /**
    * Reload the current provider.
    */
   reload: function() {
     var events = this.require("sdk/system/events");
     events.emit("startupcache-invalidate", {});
     events.emit("devtools-unloaded", {});
 
     this._provider.unload("reload");
     delete this._provider;
     this._chooseProvider();
   },
+
+  /**
+   * Sets whether the compartments loaded by this instance should be invisible
+   * to the debugger.  Invisibility is needed for loaders that support debugging
+   * of chrome code.  This is true of remote target environments, like Fennec or
+   * B2G.  It is not the default case for desktop Firefox because we offer the
+   * Browser Toolbox for chrome debugging there, which uses its own, separate
+   * loader instance.
+   * @see browser/devtools/framework/ToolboxProcess.jsm
+   */
+  invisibleToDebugger: Services.appinfo.name !== "Firefox"
 };
 
 // Export the standard instance of DevToolsLoader used by the tools.
 this.devtools = new DevToolsLoader();
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/tests/unit/test_independent_loaders.js
@@ -0,0 +1,23 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const { DevToolsLoader } =
+  Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+
+/**
+ * Ensure that each instance of the Dev Tools loader contains its own loader
+ * instance, and also returns unique objects.  This ensures there is no sharing
+ * in place between loaders.
+ */
+function run_test() {
+  let loader1 = new DevToolsLoader();
+  let loader2 = new DevToolsLoader();
+
+  let server1 = loader1.require("devtools/server/main");
+  let server2 = loader2.require("devtools/server/main");
+
+  do_check_true(server1 !== server2);
+
+  do_check_true(loader1._provider !== loader2._provider);
+  do_check_true(loader1._provider.loader !== loader2._provider.loader);
+}
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/tests/unit/test_invisible_loader.js
@@ -0,0 +1,50 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const { DevToolsLoader } =
+  Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+Cu.import("resource://gre/modules/jsdebugger.jsm");
+addDebuggerToGlobal(this);
+
+const MAIN_URI = "resource://gre/modules/devtools/server/main.js";
+
+/**
+ * Ensure that sandboxes created via the Dev Tools loader respect the
+ * invisibleToDebugger flag.
+ */
+function run_test() {
+  visible_loader();
+  invisible_loader();
+}
+
+function visible_loader() {
+  let loader = new DevToolsLoader();
+  loader.invisibleToDebugger = false;
+  loader.require("devtools/server/main");
+
+  let dbg = new Debugger();
+  let sandbox = loader._provider.loader.sandboxes[MAIN_URI];
+
+  try {
+    dbg.addDebuggee(sandbox);
+    do_check_true(true);
+  } catch(e) {
+    do_throw("debugger could not add visible value");
+  }
+}
+
+function invisible_loader() {
+  let loader = new DevToolsLoader();
+  loader.invisibleToDebugger = true;
+  loader.require("devtools/server/main");
+
+  let dbg = new Debugger();
+  let sandbox = loader._provider.loader.sandboxes[MAIN_URI];
+
+  try {
+    dbg.addDebuggee(sandbox);
+    do_throw("debugger added invisible value");
+  } catch(e) {
+    do_check_true(true);
+  }
+}
--- a/toolkit/devtools/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/tests/unit/xpcshell.ini
@@ -1,6 +1,8 @@
 [DEFAULT]
 head = head_devtools.js
 tail =
 
+[test_independent_loaders.js]
+[test_invisible_loader.js]
 [test_safeErrorString.js]
 [test_defineLazyPrototypeGetter.js]