Bug 946813 - Part 3: Use independent loaders, mark them invisible. r=past, a=lsblakk
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 17 Jan 2014 16:47:06 -0600
changeset 175870 009513120418c90adb9a1a423a08f083bc994988
parent 175869 e7e9d99a29650fdddf208d93edb37c5191f8b00a
child 175871 1679216e579fb6dbf3896bfd9d5b6a80973d9caf
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspast, lsblakk
bugs946813
milestone28.0a2
Bug 946813 - Part 3: Use independent loaders, mark them invisible. r=past, a=lsblakk
browser/devtools/framework/ToolboxProcess.jsm
toolkit/devtools/Loader.jsm
toolkit/devtools/tests/mochitest/chrome.ini
toolkit/devtools/tests/mochitest/test_independent_loaders.html
toolkit/devtools/tests/mochitest/test_invisible_loader.html
toolkit/devtools/tests/mochitest/test_loader_paths.html
--- 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
@@ -39,17 +39,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
@@ -68,32 +69,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);
@@ -130,17 +133,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;
@@ -214,47 +218,64 @@ 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.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
    *    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]);
     });
   },
 
   /**
@@ -267,43 +288,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();
--- a/toolkit/devtools/tests/mochitest/chrome.ini
+++ b/toolkit/devtools/tests/mochitest/chrome.ini
@@ -1,1 +1,3 @@
+[test_independent_loaders.html]
+[test_invisible_loader.html]
 [test_loader_paths.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/tests/mochitest/test_independent_loaders.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+
+<html>
+
+  <head>
+    <meta charset="utf8">
+    <title></title>
+
+    <script type="application/javascript"
+            src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css"
+          href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  </head>
+
+  <body>
+
+    <script type="application/javascript;version=1.8">
+      const { utils: Cu } = Components;
+
+      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.
+       */
+      let loader1 = new DevToolsLoader();
+      let loader2 = new DevToolsLoader();
+
+      let color1 = loader1.require("devtools/css-color");
+      let color2 = loader2.require("devtools/css-color");
+
+      ok(color1 !== color2, "Loaded objects should be unique");
+
+      ok(loader1._provider !== loader2._provider,
+        "Providers should be unique");
+      ok(loader1._provider.loader !== loader2._provider.loader,
+        "Loaders should be unique");
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/tests/mochitest/test_invisible_loader.html
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+
+<html>
+
+  <head>
+    <meta charset="utf8">
+    <title></title>
+
+    <script type="application/javascript"
+            src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+    <link rel="stylesheet" type="text/css"
+          href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  </head>
+
+  <body>
+
+    <script type="application/javascript;version=1.8">
+      const { utils: Cu } = Components;
+
+      /**
+       * Ensure that sandboxes created via the Dev Tools loader respect the
+       * invisibleToDebugger flag.
+       */
+      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";
+
+      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);
+          ok(true, "debugger added visible value");
+        } catch(e) {
+          ok(false, "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);
+          ok(false, "debugger added invisible value");
+        } catch(e) {
+          ok(true, "debugger could not add invisible value");
+        }
+      }
+    </script>
+  </body>
+</html>
--- a/toolkit/devtools/tests/mochitest/test_loader_paths.html
+++ b/toolkit/devtools/tests/mochitest/test_loader_paths.html
@@ -32,19 +32,21 @@
                       .createInstance(Ci.nsISupportsString);
       srcDirStr.data = srcDir;
       Services.prefs.setComplexValue(SRCDIR_PREF, Ci.nsISupportsString,
                                      srcDirStr);
 
       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);
     </script>
   </body>
 </html>