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
--- 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" ? 0x7f : -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" ? 0x7f : -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;
}