[ChromeOS] Add option to set controlling terminal when launching process.

[ChromeOS] Add option to set controlling terminal when launching process.

This is needed to expose crosh process to hterm (if pseudo terminal used for
communication between terminalPrivateApi and crosh process is not controlling, job
control in the crosh process won't work, and it won't be able to e.g. process signals).

TEST=None
BUG=chromium-os:23273
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118585

[tbarzic, 2012-01-20 18:06:35]

Hi Evan,
Could you, please, take a look at this :)
Thanks,
Toni

[Evan Martin, 2012-01-20 19:03:03]

LGTM with some suggestions

I'm not familiar with how controlling terminals work, so I'll take it on faith
that the code is doing what you want.

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix.cc
File base/process_util_posix.cc (right):

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix...
base/process_util_posix.cc:685: if (options.ctrl_terminal_fd >=0) {
space before 0

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix...
base/process_util_posix.cc:689: RAW_LOG(WARNING, "Unable to set controlling
terminal");
I wonder if it'd be more useful to specify what failed here.  The log will only
be read by someone technical, so perhaps referencing setsid() or
ioctl(TIOCSCTTY) in the message would be helpful.  Your call.

[tbarzic, 2012-01-20 20:42:05]

Thanks for suggestions :)

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix.cc
File base/process_util_posix.cc (right):

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix...
base/process_util_posix.cc:685: if (options.ctrl_terminal_fd >=0) {
On 2012/01/20 19:03:03, Evan Martin wrote:
> space before 0

Done.

https://chromiumcodereview.appspot.com/9176013/diff/1/base/process_util_posix...
base/process_util_posix.cc:689: RAW_LOG(WARNING, "Unable to set controlling
terminal");
On 2012/01/20 19:03:03, Evan Martin wrote:
> I wonder if it'd be more useful to specify what failed here.  The log will
only
> be read by someone technical, so perhaps referencing setsid() or
> ioctl(TIOCSCTTY) in the message would be helpful.  Your call.

Good suggestion...

[commit-bot: I haz the power, 2012-01-20 21:48:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/9176013/6002

[commit-bot: I haz the power, 2012-01-21 01:01:42]

Try job failure for 9176013-6002 (retry) on win_rel for step "base_unittests".
It's a second try, previously, step "base_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...