Move the ProcessWatcher methods out of content/common/process_watcher into base/process_util, alo...

Move the ProcessWatcher methods out of content/common/process_watcher into base/process_util, alongside the other process methods.

The only non-trivial move change is to the Windows implementation, where I changed KillProcess to use an exit code of kProcessKilledExitCode instead of content::RESULT_CODE_HUNG. cpu said that the existing code was incorrect, since GetTerminationStatus() should be mapping that result to TERMINATION_STATUS_PROCESS_WAS_KILLED. So I changed the exit code to kProcessKilledExitCode. This might make the UMA stats for killed processes to go up (and crashed to go down), but that will be an accounting change and should be zero-sum.

BUG=98716
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111371

[jam, 2011-11-23 00:59:27]

Will: last one I promise :)
Carlos: FYI per our discussion and your helpful background

[cpu_(ooo_6.6-7.5), 2011-11-23 01:59:49]

I am not sure I can get to this code review in the next 24 hours. If it is
urgent you probably want to add another reviewer.

[jam, 2011-11-23 02:09:35]

On 2011/11/23 01:59:49, cpu wrote:
> I am not sure I can get to this code review in the next 24 hours. If it is
> urgent you probably want to add another reviewer.

I'm sending it to you as an FYI :) The one line change I refer to in the cl
description is what we talked about over chat. No need to review.

[willchan no longer on Chromium, 2011-11-23 17:46:36]

LGTM, although you should probably tell people who look at the stability metrics
so they don't get confused. jar/laforge/et al.

http://codereview.chromium.org/8674003/diff/1/base/process_util_posix.cc
File base/process_util_posix.cc (right):

http://codereview.chromium.org/8674003/diff/1/base/process_util_posix.cc#newc...
base/process_util_posix.cc:1308: #endif
Up to you, but it's my personal preference to terminate lengthy #if/#endif
blocks with a // !defined(OS_MACOSX) or something.

[jam, 2011-11-23 17:48:33]

done, will tell laforge/jar

http://codereview.chromium.org/8674003/diff/1/base/process_util_posix.cc
File base/process_util_posix.cc (right):

http://codereview.chromium.org/8674003/diff/1/base/process_util_posix.cc#newc...
base/process_util_posix.cc:1308: #endif
On 2011/11/23 17:46:37, willchan wrote:
> Up to you, but it's my personal preference to terminate lengthy #if/#endif
> blocks with a // !defined(OS_MACOSX) or something.

Done.

[jar (doing other things), 2011-11-23 18:14:23]

+cc mal

Since we should watch for changes in crash rates on renderer processes as this
rolls out.

[cpu_(ooo_6.6-7.5), 2011-11-24 00:22:07]

http://codereview.chromium.org/8674003/diff/1/base/process_util_unittest.cc
File base/process_util_unittest.cc (right):

http://codereview.chromium.org/8674003/diff/1/base/process_util_unittest.cc#n...
base/process_util_unittest.cc:850: base::WaitForSingleProcess(child_process,
5000);
the problem here is that EnsureProcessTerminated closes the handle so line 850
is a silent fail.

http://codereview.chromium.org/8674003/diff/1/base/process_util_win.cc
File base/process_util_win.cc (right):

http://codereview.chromium.org/8674003/diff/1/base/process_util_win.cc#newcod...
base/process_util_win.cc:139: CloseHandle(process_);
I see a problem, here we close the handle, but we don't do that if the timer
expired.