GTTF: Detect browser crashes on shutdown in UI tests.

GTTF: Detect browser crashes on shutdown in UI tests. 

Previously the automation framework could miss a browser 
crash during shutdown on POSIX (on Windows there is 
crash_service.exe that should catch all crashes). 

This change makes the automation framework avoid losing 
information about the browser process' exit status 
(CrashAwareSleep), and fixes a bug in base::WaitForExitCodeWithTimeout 
(which on POSIX never reported the process has been signaled). 

Finally, it makes the automation framework use WaitForExitCodeWithTimeout 
instead of WaitForSingleProcess. This way we can get the exit status 
information in an accurate and cross-platform way. 

To avoid trying to close the same process handle twice (it's only an issue on Windows) I've changed WaitForExitCodeWithTimeout not to close the passed handle. It's only used in few places and I think this CL fixes all of them. 

I've tested this change locally on Mac with a UI test that SIGKILLs the browser. 
Before this change the test passed (it shouldn't), and after this change 
the test failed with an information that the browser has not exited cleanly. 

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

[Paweł Hajdan Jr., 2011-04-01 16:01:07]

Please review:

agl: process_util
jcivelli: everything else

[agl, 2011-04-01 16:58:55]

LGTM

[Paweł Hajdan Jr., 2011-04-05 07:02:51]

+sky, could you take a look?

[sky, 2011-04-05 16:17:58]

LGTM

[Paweł Hajdan Jr., 2011-04-05 19:45:29]

Could you take another look? There was an issue with deleted AutomationProxy,
see http://codereview.chromium.org/6794056/ .

[sky, 2011-04-05 20:08:49]

SLGTM

[Huyen, 2011-04-07 00:07:44]

I know this has already been submitted but I'm noticing a compilation error when
making all.

http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
File chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc (right):

http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:60:
TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code));
Pawel, I think you forgot to include test_timeouts.h. I tried to build all
target and I'm getting an error:

chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc: In member
function ‘virtual void FastShutdown_SlowTermination_Test::TestBody()’:
chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:59: error:
‘TestTimeouts’ was not declared in this scope

[Paweł Hajdan Jr., 2011-04-07 06:25:19]

http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
File chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc (right):

http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:60:
TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code));
On 2011/04/07 00:07:45, Huyen wrote:
> Pawel, I think you forgot to include test_timeouts.h. I tried to build all
> target and I'm getting an error:
> 
> chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc: In member
> function ‘virtual void FastShutdown_SlowTermination_Test::TestBody()’:
> chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:59: error:
> ‘TestTimeouts’ was not declared in this scope

Thank you for commenting here. I took a look at the side-by-side diff, and
TestTimeouts was used here before. Also, the change landed successfully on the
waterfall, which suggests it didn't introduce a compile failure.

Do you have any local changes that could make this fail to compile? I agree
though that test_timeouts.h should be explicitly #included here. Feel free to
TBR such a change to me.

[hnguyen, 2011-04-07 22:35:07]

On Wed, Apr 6, 2011 at 11:25 PM, <phajdan.jr@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
> File chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc
> (right):
>
>
>
http://codereview.chromium.org/6689014/diff/15001/chrome/test/interactive_ui/...
> chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:60:
> TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code));
> On 2011/04/07 00:07:45, Huyen wrote:
>
>> Pawel, I think you forgot to include test_timeouts.h. I tried to build
>>
> all
>
>> target and I'm getting an error:
>>
>
>  chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc: In
>>
> member
>
>> function ‘virtual void FastShutdown_SlowTermination_Test::TestBody()’:
>> chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc:59:
>>
> error:
>
>> ‘TestTimeouts’ was not declared in this scope
>>
>
> Thank you for commenting here. I took a look at the side-by-side diff,
> and TestTimeouts was used here before. Also, the change landed
> successfully on the waterfall, which suggests it didn't introduce a
> compile failure.
>
> Do you have any local changes that could make this fail to compile? I
> agree though that test_timeouts.h should be explicitly #included here.
> Feel free to TBR such a change to me.


This is breaking on the try server and locally for me on patch
http://codereview.chromium.org/6685099/.
I've added fast_shutdown_interactive_uitest.cc to the above patch.


>
> http://codereview.chromium.org/6689014/
>