Backed out changeset 9b91c7e0aa11 (bug 946813) for xpcshell crashes. a=backout
authorRyan VanderMeulen <ryanvm@gmail.com>
Fri, 17 Jan 2014 15:53:55 -0500
changeset 175850 e9358bde4d0f774eb90233f52eb652d29f6a5869
parent 175849 594af05e92a341c4ecf67bfda65af680ad5167f7
child 175851 67e3616f0b17d189745eb09465dcf05397cc5a5b
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)
reviewersbackout
bugs946813
milestone28.0a2
backs out9b91c7e0aa11117a3b6aa4d8dcb6e58f084fa942
Backed out changeset 9b91c7e0aa11 (bug 946813) for xpcshell crashes. a=backout
browser/devtools/framework/ToolboxProcess.jsm
toolkit/devtools/Loader.jsm
toolkit/devtools/tests/mochitest/test_loader_paths.html
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,20 +51,18 @@ 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, especially since we can mark this loader as
-      // invisible to the debugger (unlike the usual loader settings).
+      // DebuggingServer itself.
       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,18 +39,17 @@ 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)
-function BuiltinProvider() {}
-BuiltinProvider.prototype = {
+var BuiltinProvider = {
   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
@@ -69,34 +68,32 @@ BuiltinProvider.prototype = {
         "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,
-      invisibleToDebugger: this.invisibleToDebugger
+      globals: loaderGlobals
     });
 
     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.
-function SrcdirProvider() {}
-SrcdirProvider.prototype = {
+var SrcdirProvider = {
   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);
@@ -133,18 +130,17 @@ SrcdirProvider.prototype = {
         "devtools/output-parser": outputParserURI,
         "devtools/touch-events": touchEventsURI,
         "devtools/client": clientURI,
         "devtools/pretty-fast": prettyFastURI,
 
         "acorn": acornURI,
         "acorn_loose": acornLoosseURI
       },
-      globals: loaderGlobals,
-      invisibleToDebugger: this.invisibleToDebugger
+      globals: loaderGlobals
     });
 
     return this._writeManifest(devtoolsDir).then(null, Cu.reportError);
   },
 
   unload: function(reason) {
     loader.unload(this.loader, reason);
     delete this.loader;
@@ -218,64 +214,47 @@ SrcdirProvider.prototype = {
 /**
  * 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.require = this.require.bind(this);
+  this._chooseProvider();
 };
 
 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]);
     });
   },
 
   /**
@@ -288,55 +267,43 @@ 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(new SrcdirProvider());
+      this.setProvider(SrcdirProvider);
     } else {
-      this.setProvider(new BuiltinProvider());
+      this.setProvider(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/test_loader_paths.html
+++ b/toolkit/devtools/tests/mochitest/test_loader_paths.html
@@ -32,21 +32,19 @@
                       .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", {});
 
-      let builtin = new BuiltinProvider();
-      builtin.load();
-      let srcdir = new SrcdirProvider();
-      srcdir.load();
+      BuiltinProvider.load();
+      SrcdirProvider.load();
 
-      is(builtin.loader.mapping.length,
-         srcdir.loader.mapping.length + 1,
+      is(BuiltinProvider.loader.mapping.length,
+         SrcdirProvider.loader.mapping.length + 1,
          "The built-in loader should have only one more mapping for testing.");
 
       Services.prefs.clearUserPref(SRCDIR_PREF);
     </script>
   </body>
 </html>
deleted file mode 100644
--- a/toolkit/devtools/tests/unit/test_independent_loaders.js
+++ /dev/null
@@ -1,23 +0,0 @@
-/* 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);
-}
deleted file mode 100644
--- a/toolkit/devtools/tests/unit/test_invisible_loader.js
+++ /dev/null
@@ -1,50 +0,0 @@
-/* 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);
-  }
-}
--- a/toolkit/devtools/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/tests/unit/xpcshell.ini
@@ -1,8 +1,6 @@
 [DEFAULT]
 head = head_devtools.js
 tail =
 
-[test_independent_loaders.js]
-[test_invisible_loader.js]
 [test_safeErrorString.js]
 [test_defineLazyPrototypeGetter.js]