Bug 1370027: Part 2 - Use Subprocess.jsm rather than nsIProcess to create the Browser Toolbox process. r=bgrins
authorKris Maglione <maglione.k@gmail.com>
Tue, 06 Jun 2017 16:15:38 -0700
changeset 410861 0d16a08bbcfa181535789a43856ef176cc9829c8
parent 410860 f4f2dab8b32338f937a3661946809df4674440d6
child 410862 06967108134bd25250440c4e7429484856f811be
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1370027
milestone55.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 1370027: Part 2 - Use Subprocess.jsm rather than nsIProcess to create the Browser Toolbox process. r=bgrins Using nsIProcess has the side-effect of spawning a NSPR process wait loop thread, which makes it impossible for other IPC code to waitpid on its own processes, and check their exit status. There are other instances that need to be changed as well, but this is the one that developers are most likely to run into. MozReview-Commit-ID: L0WyOxlXbkk
devtools/client/debugger/new/test/mochitest/browser_dbg-chrome-create.js
devtools/client/debugger/test/mochitest/browser_dbg_chrome-create.js
devtools/client/framework/ToolboxProcess.jsm
--- a/devtools/client/debugger/new/test/mochitest/browser_dbg-chrome-create.js
+++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-chrome-create.js
@@ -16,22 +16,20 @@ function initChromeDebugger() {
     BrowserToolboxProcess.init(onClose, (event, _process) => {
       info("Browser toolbox process started successfully.");
       resolve(_process);
     });
   });
 }
 
 function onClose() {
-  ok(!gProcess._dbgProcess.isRunning,
-    "The remote debugger process isn't closed as it should be!");
-  is(gProcess._dbgProcess.exitValue, (Services.appinfo.OS == "WINNT" ? 0 : 256),
+  is(gProcess._dbgProcess.exitCode, (Services.appinfo.OS == "WINNT" ? -9 : -15),
     "The remote debugger process didn't die cleanly.");
 
-  info("process exit value: " + gProcess._dbgProcess.exitValue);
+  info("process exit value: " + gProcess._dbgProcess.exitCode);
 
   info("profile path: " + gProcess._dbgProfilePath);
 
   finish();
 }
 
 registerCleanupFunction(function() {
   Services.prefs.clearUserPref("devtools.debugger.remote-enabled");
@@ -42,17 +40,17 @@ add_task(function* () {
   // Windows XP and 8.1 test slaves are terribly slow at this test.
   requestLongerTimeout(5);
   Services.prefs.setBoolPref("devtools.debugger.remote-enabled", true);
 
   gProcess = yield initChromeDebugger();
 
   ok(gProcess._dbgProcess,
     "The remote debugger process wasn't created properly!");
-  ok(gProcess._dbgProcess.isRunning,
+  ok(gProcess._dbgProcess.exitCode == null,
     "The remote debugger process isn't running!");
   is(typeof gProcess._dbgProcess.pid, "number",
     "The remote debugger process doesn't have a pid (?!)");
 
   info("process location: " + gProcess._dbgProcess.location);
   info("process pid: " + gProcess._dbgProcess.pid);
   info("process name: " + gProcess._dbgProcess.processName);
   info("process sig: " + gProcess._dbgProcess.processSignature);
@@ -63,10 +61,10 @@ add_task(function* () {
   is(
     gProcess._dbgProfilePath,
     OS.Path.join(OS.Constants.Path.profileDir, "chrome_debugger_profile"),
      "The remote debugger profile isn't where we expect it!"
    );
 
   info("profile path: " + gProcess._dbgProfilePath);
 
-  gProcess.close();
+  yield gProcess.close();
 });
--- a/devtools/client/debugger/test/mochitest/browser_dbg_chrome-create.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_chrome-create.js
@@ -20,17 +20,17 @@ function test() {
     info("Starting test...");
     performTest();
   });
 }
 
 function performTest() {
   ok(gProcess._dbgProcess,
     "The remote debugger process wasn't created properly!");
-  ok(gProcess._dbgProcess.isRunning,
+  ok(gProcess._dbgProcess.exitCode == null,
     "The remote debugger process isn't running!");
   is(typeof gProcess._dbgProcess.pid, "number",
     "The remote debugger process doesn't have a pid (?!)");
 
   info("process location: " + gProcess._dbgProcess.location);
   info("process pid: " + gProcess._dbgProcess.pid);
   info("process name: " + gProcess._dbgProcess.processName);
   info("process sig: " + gProcess._dbgProcess.processSignature);
@@ -41,19 +41,17 @@ function performTest() {
      "The remote debugger profile isn't where we expect it!");
 
   info("profile path: " + gProcess._dbgProfilePath);
 
   gProcess.close();
 }
 
 function aOnClose() {
-  ok(!gProcess._dbgProcess.isRunning,
-    "The remote debugger process isn't closed as it should be!");
-  is(gProcess._dbgProcess.exitValue, (Services.appinfo.OS == "WINNT" ? 0 : 256),
+  is(gProcess._dbgProcess.exitCode, (Services.appinfo.OS == "WINNT" ? -9 : -15),
     "The remote debugger process didn't die cleanly.");
 
   info("process exit value: " + gProcess._dbgProcess.exitValue);
 
   info("profile path: " + gProcess._dbgProfilePath);
 
   finish();
 }
--- a/devtools/client/framework/ToolboxProcess.jsm
+++ b/devtools/client/framework/ToolboxProcess.jsm
@@ -1,24 +1,25 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
+const { interfaces: Ci, utils: Cu, results: Cr } = Components;
 
 const DBG_XUL = "chrome://devtools/content/framework/toolbox-process-window.xul";
 const CHROME_DEBUGGER_PROFILE_NAME = "chrome_debugger_profile";
 
 const { require, DevToolsLoader } = Cu.import("resource://devtools/shared/Loader.jsm", {});
 const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "Subprocess", "resource://gre/modules/Subprocess.jsm");
 XPCOMUtils.defineLazyGetter(this, "Telemetry", function () {
   return require("devtools/client/shared/telemetry");
 });
 XPCOMUtils.defineLazyGetter(this, "EventEmitter", function () {
   return require("devtools/shared/event-emitter");
 });
 const promise = require("promise");
 const Services = require("Services");
@@ -226,19 +227,18 @@ BrowserToolboxProcess.prototype = {
     }
   },
 
   /**
    * Creates and initializes the profile & process for the remote debugger.
    */
   _create: function () {
     dumpn("Initializing chrome debugging process.");
-    let process = Cc["@mozilla.org/process/util;1"].createInstance(Ci.nsIProcess);
-    this._dbgProcess = process;
-    process.init(Services.dirsvc.get("XREExeF", Ci.nsIFile));
+
+    let command = Services.dirsvc.get("XREExeF", Ci.nsIFile).path;
 
     let xulURI = DBG_XUL;
 
     if (this._options.addonID) {
       xulURI += "?addonID=" + this._options.addonID;
     }
 
     dumpn("Running chrome debugging process.");
@@ -255,48 +255,52 @@ BrowserToolboxProcess.prototype = {
     // was present in order to also clear the child profile's startup cache as
     // well.
     //
     // As an approximation of "isLocalBuild", check for an unofficial build.
     if (!Services.appinfo.isOfficial) {
       args.push("-purgecaches");
     }
 
-    // Disable safe mode for the new process in case this was opened via the
-    // keyboard shortcut.
-    let nsIEnvironment = Cc["@mozilla.org/process/environment;1"]
-                           .getService(Ci.nsIEnvironment);
-    let originalValue = nsIEnvironment.get("MOZ_DISABLE_SAFE_MODE_KEY");
-    nsIEnvironment.set("MOZ_DISABLE_SAFE_MODE_KEY", "1");
+    this._dbgProcessPromise = Subprocess.call({
+      command,
+      arguments: args,
+      environmentAppend: true,
+      environment: {
+        // Disable safe mode for the new process in case this was opened via the
+        // keyboard shortcut.
+        MOZ_DISABLE_SAFE_MODE_KEY: "1",
+      },
+    }).then(proc => {
+      this._dbgProcess = proc;
 
-    process.runwAsync(args, args.length, { observe: () => this.close() });
+      this._telemetry.toolOpened("jsbrowserdebugger");
 
-    // Now that the process has started, it's safe to reset the env variable.
-    nsIEnvironment.set("MOZ_DISABLE_SAFE_MODE_KEY", originalValue);
+      dumpn("Chrome toolbox is now running...");
+      this.emit("run", this);
 
-    this._telemetry.toolOpened("jsbrowserdebugger");
-
-    dumpn("Chrome toolbox is now running...");
-    this.emit("run", this);
+      proc.stdin.close();
+      proc.wait().then(() => this.close());
+      return proc;
+    });
   },
 
   /**
    * Closes the remote debugging server and kills the toolbox process.
    */
-  close: function () {
+  close: async function () {
     if (this.closed) {
       return;
     }
 
     dumpn("Cleaning up the chrome debugging process.");
     Services.obs.removeObserver(this.close, "quit-application");
 
-    if (this._dbgProcess.isRunning) {
-      this._dbgProcess.kill();
-    }
+    this._dbgProcess.stdout.close();
+    await this._dbgProcess.kill();
 
     this._telemetry.toolClosed("jsbrowserdebugger");
     if (this.debuggerServer) {
       this.debuggerServer.off("connectionchange", this.emit);
       this.debuggerServer.destroy();
       this.debuggerServer = null;
     }