Backed out 2 changesets (bug 1622718) as requested by dev for causing Bug 1626727.

Backed out changeset eb632d0b3a7b (bug 1622718)
Backed out changeset 016b86b83932 (bug 1622718)

devtools/shared/Loader.jsm

@@ -44,13 +44,10 @@ var gNextLoaderID = 0;
  *        We use this in order to debug modules loaded in this shared system
  *        compartment. The debugger actor has to be running in a distinct
  *        compartment than the context it is debugging.
- * @param hasJSMLifetime boolean
- *        See base-loader.js for documentation of this option.
  */
 function DevToolsLoader({
   invisibleToDebugger = false,
   freshCompartment = false,
-  hasJSMLifetime = false,
 } = {}) {
   const paths = {
     // ⚠ DISCUSSION ON DEV-DEVELOPER-TOOLS REQUIRED BEFORE MODIFYING ⚠
@@ -82,7 +79,6 @@ function DevToolsLoader({
   }
 
   this.loader = new Loader({
-    hasJSMLifetime,
     paths,
     invisibleToDebugger,
     freshCompartment,
@@ -172,13 +168,6 @@ DevToolsLoader.prototype = {
 
 // Export the standard instance of DevToolsLoader used by the tools.
 var loader = new DevToolsLoader({
-  /**
-   * This instance of the loader is a global singleton and is thus never
-   * destroyed, so we set this option to avoid a little bit of work when
-   * loading scripts. See base-loader.js for more information about this.
-   */
-  hasJSMLifetime: true,
-
   /**
    * Sets whether the compartments loaded by this instance should be invisible
    * to the debugger.  Invisibility is needed for loaders that support debugging

devtools/shared/base-loader.js

@@ -157,17 +157,7 @@ function load(loader, module) {
 
   const originalExports = module.exports;
   try {
-    Services.scriptloader.loadSubScriptWithOptions(module.uri, {
-      target: sandbox,
-      // Calls to loadSubScriptWithOptions run the risk of leaking the target's
-      // global by default because they may strongly cache the JSScript for the
-      // loaded URI. When the JSScript is strongly held, it will also hold onto
-      // the global it is tied to, potentially along with the loader itself.
-      // By using the compilation scope by default, we instead force any cached
-      // script to be one that is tied to the JSM global object, instead of the
-      // sandbox global object (bug 1622718).
-      useCompilationScope: !loader.hasJSMLifetime,
-    });
+    Services.scriptloader.loadSubScript(module.uri, sandbox);
   } catch (error) {
     // loadSubScript sometime throws string errors, which includes no stack.
     // At least provide the current stack by re-throwing a real Error object.
@@ -449,12 +439,6 @@ function Module(id, uri) {
 // Takes `loader`, and unload `reason` string and notifies all observers that
 // they should cleanup after them-self.
 function unload(loader, reason) {
-  // JSM-lifetime loaders leak, so destroying them would indicate a
-  // misunderstanding of this behavior.
-  if (loader.hasJSMLifetime) {
-    throw new Error("A JSM-lifetime loader can't be destroyed");
-  }
-
   // subject is a unique object created per loader instance.
   // This allows any code to cleanup on loader unload regardless of how
   // it was loaded. To handle unload for specific loader subject may be
@@ -482,10 +466,6 @@ function unload(loader, reason) {
 // - `requireHook`: Optional function used to replace native require function
 //   from loader. This function receive the module path as first argument,
 //   and native require method as second argument.
-// - `hasJSMLifetime`: To avoid memory leaks, by default this loader will
-//   load scripts so that they can be fully GCed when the loader is destroyed.
-//   If this loader will always be reachable and will never be destroyed,
-//   you can set this option to true to avoid a small amount of work.
 function Loader(options) {
   let { paths, globals } = options;
   if (!globals) {
@@ -579,11 +559,6 @@ function Loader(options) {
       enumerable: false,
       value: options.invisibleToDebugger || false,
     },
-    hasJSMLifetime: {
-      enumerable: false,
-      writable: false,
-      value: options.hasJSMLifetime || false,
-    },
     requireHook: {
       enumerable: false,
       writable: true,

devtools/shared/tests/xpcshell/test_eventemitter_destroy.js

@@ -5,10 +5,13 @@
 "use strict";
 
 add_task(function() {
-  const { DevToolsLoader } = ChromeUtils.import(
+  const { DevToolsLoader, require } = ChromeUtils.import(
     "resource://devtools/shared/Loader.jsm"
   );
 
+  // Force-load the module once in the global loader to avoid Bug 1622718.
+  require("devtools/shared/event-emitter");
+
   const emitterRef = (function() {
     const loader = new DevToolsLoader();
 

js/xpconnect/idl/mozIJSSubScriptLoader.idl

@@ -40,13 +40,9 @@ interface mozIJSSubScriptLoader : nsISupports
      * @param optionsObject an object with parameters. Valid parameters are:
      *                      - target:  an object to evaluate onto (default: global object of the caller)
      *                      - ignoreCache: if set to true, will bypass the cache for reading the file.
-     *                      - useCompilationScope: If true, the script will be
-     *                        parsed in the compilation cache realm first, and
-     *                        then cloned into the target realm for execution.
-     *                        The script cache strongly holds scripts, so if you
-     *                        load a script into an arbitrary realm without this
-     *                        option enabled, you risk leaking that realm.
-     *                        Defaults to false.
+     *                      - async: if set to true, the script will be loaded
+     *                        asynchronously, and a Promise is returned which
+     *                        resolves to its result when execution is complete.
      *                      - wantReturnValue: If true, the script will return
      *                        the value of the last statement that it evaluated.
      *                        This option disables most optimizations in the

js/xpconnect/loader/mozJSSubScriptLoader.cpp

@@ -47,19 +47,16 @@ class MOZ_STACK_CLASS LoadSubScriptOptions : public OptionsBase {
       : OptionsBase(cx, options),
         target(cx),
         ignoreCache(false),
-        useCompilationScope(false),
         wantReturnValue(false) {}
 
   virtual bool Parse() override {
     return ParseObject("target", &target) &&
            ParseBoolean("ignoreCache", &ignoreCache) &&
-           ParseBoolean("useCompilationScope", &useCompilationScope) &&
            ParseBoolean("wantReturnValue", &wantReturnValue);
   }
 
   RootedObject target;
   bool ignoreCache;
-  bool useCompilationScope;
   bool wantReturnValue;
 };
 
@@ -450,9 +447,6 @@ nsresult mozJSSubScriptLoader::DoLoadSubScriptWithOptions(
       isSystem = true;
     }
   }
-  if (options.useCompilationScope) {
-    useCompilationScope = true;
-  }
   bool ignoreCache =
       options.ignoreCache || !isSystem || scheme.EqualsLiteral("blob");