Bug 946813 - Part 3: Use independent loaders, mark them invisible. r=past

browser/devtools/framework/ToolboxProcess.jsm

@@ -56,8 +56,10 @@ BrowserToolboxProcess.prototype = {
       // 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.");

toolkit/devtools/Loader.jsm

@@ -43,7 +43,8 @@ let loaderGlobals = {
 };
 
 // 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: {
@@ -72,7 +73,8 @@ var BuiltinProvider = {
         // Allow access to xpcshell test items from the loader.
         "xpcshell-test": "resource://test"
       },
-      globals: loaderGlobals
+      globals: loaderGlobals,
+      invisibleToDebugger: this.invisibleToDebugger
     });
 
     return promise.resolve(undefined);
@@ -87,7 +89,8 @@ var BuiltinProvider = {
 // 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;
@@ -134,7 +137,8 @@ var SrcdirProvider = {
         "acorn": acornURI,
         "acorn_loose": acornLoosseURI
       },
-      globals: loaderGlobals
+      globals: loaderGlobals,
+      invisibleToDebugger: this.invisibleToDebugger
     });
 
     return this._writeManifest(devtoolsDir).then(null, Cu.reportError);
@@ -218,12 +222,29 @@ var SrcdirProvider = {
  * then a new one can also be created.
  */
 this.DevToolsLoader = function DevToolsLoader() {
-  this._chooseProvider();
+  this.require = this.require.bind(this);
 };
 
 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
@@ -234,7 +255,7 @@ DevToolsLoader.prototype = {
    */
   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;
   },
 
   /**
@@ -248,7 +269,7 @@ DevToolsLoader.prototype = {
    */
   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 => {
@@ -271,6 +292,7 @@ DevToolsLoader.prototype = {
       this._provider.unload("newprovider");
     }
     this._provider = provider;
+    this._provider.invisibleToDebugger = this.invisibleToDebugger;
     this._provider.load();
     this.require = loader.Require(this._provider.loader, { id: "devtools" });
 
@@ -284,9 +306,9 @@ DevToolsLoader.prototype = {
    */
   _chooseProvider: function() {
     if (Services.prefs.prefHasUserValue("devtools.loader.srcdir")) {
-      this.setProvider(SrcdirProvider);
+      this.setProvider(new SrcdirProvider());
     } else {
-      this.setProvider(BuiltinProvider);
+      this.setProvider(new BuiltinProvider());
     }
   },
 
@@ -302,6 +324,17 @@ DevToolsLoader.prototype = {
     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.

toolkit/devtools/tests/mochitest/test_loader_paths.html

@@ -37,11 +37,13 @@
       const { BuiltinProvider, SrcdirProvider } =
         Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
 
-      BuiltinProvider.load();
-      SrcdirProvider.load();
+      let builtin = new BuiltinProvider();
+      builtin.load();
+      let srcdir = new SrcdirProvider();
+      srcdir.load();
 
-      is(BuiltinProvider.loader.mapping.length,
-         SrcdirProvider.loader.mapping.length + 1,
+      is(builtin.loader.mapping.length,
+         srcdir.loader.mapping.length + 1,
          "The built-in loader should have only one more mapping for testing.");
 
       Services.prefs.clearUserPref(SRCDIR_PREF);

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 color1 = loader1.require("devtools/css-color");
+  let color2 = loader2.require("devtools/css-color");
+
+  do_check_true(color1 !== color2);
+
+  do_check_true(loader1._provider !== loader2._provider);
+  do_check_true(loader1._provider.loader !== loader2._provider.loader);
+}

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 COLOR_URI = "resource://gre/modules/devtools/css-color.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/css-color");
+
+  let dbg = new Debugger();
+  let sandbox = loader._provider.loader.sandboxes[COLOR_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/css-color");
+
+  let dbg = new Debugger();
+  let sandbox = loader._provider.loader.sandboxes[COLOR_URI];
+
+  try {
+    dbg.addDebuggee(sandbox);
+    do_throw("debugger added invisible value");
+  } catch(e) {
+    do_check_true(true);
+  }
+}

toolkit/devtools/tests/unit/xpcshell.ini

@@ -2,5 +2,7 @@
 head = head_devtools.js
 tail =
 
+[test_independent_loaders.js]
+[test_invisible_loader.js]
 [test_safeErrorString.js]
 [test_defineLazyPrototypeGetter.js]