Revert 80472 - GTTF: Detect browser crashes on shutdown in UI tests.Previously the automation framework could miss a browsercrash during shutdown on POSIX (on Windows there iscrash_service.exe that should catch all crashes).This change makes the automation framework avoid losinginformation 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 WaitForExitCodeWithTimeoutinstead of WaitForSingleProcess. This way we can get the exit statusinformation 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 changethe test failed with an information that the browser has not exited cleanly.BUG=56644Review URL: http://codereview.chromium.org/6689014

TBR=phajdan.jr@chromium.org

[----------] 1 test from MultipartResponseUITest
[ RUN      ] MultipartResponseUITest.SingleVisit
[3538:3538:0405/104633:11326126024137:ERROR:process_util_posix.cc(108)] Received signal 11
	base::debug::StackTrace::StackTrace() [0xcd194a]
	base::(anonymous namespace)::StackDumpSignalHandler() [0xcb0e5a]
	0x2b835e391100
	AutomationProxy::GetBrowserWindowCount() [0x2055e86]
	ProxyLauncher::IsBrowserRunning() [0xc3f1a2]
	ProxyLauncher::QuitBrowser() [0xc454b3]
	ProxyLauncher::CloseBrowserAndServer() [0xc472d6]
	UITestBase::TearDown() [0xc50d54]
	UITest::TearDown() [0xc51260]
	testing::TestInfo::Run() [0xe8de78]
	testing::TestCase::Run() [0xe8df35]
	testing::internal::UnitTestImpl::RunAllTests() [0xe8f6e7]
	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0xe804d5]
	testing::internal::HandleExceptionsInMethodIfSupported<>() [0xe8ba92]
	testing::UnitTest::Run() [0xe8badb]
	base::TestSuite::Run() [0x212c26d]
	main [0xc48e41]
	0x2b835e37d1c4
	0x42fec9
Review URL: http://codereview.chromium.org/6794056

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@80488 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 33 insertions, 8 deletions

diff --git a/base/process_util.h b/base/process_util.h
index bdf9b9e..b3d311e 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -373,11 +373,10 @@ BASE_API TerminationStatus GetTerminationStatus(ProcessHandle handle,
 BASE_API bool WaitForExitCode(ProcessHandle handle, int* exit_code);
 
 // Waits for process to exit. If it did exit within |timeout_milliseconds|,
-// then puts the exit code in |exit_code|, and returns true.
+// then puts the exit code in |exit_code|, closes |handle|, and returns true.
 // In POSIX systems, if the process has been signaled then |exit_code| is set
 // to -1. Returns false on failure (the caller is then responsible for closing
 // |handle|).
-// The caller is always responsible for closing the |handle|.
 BASE_API bool WaitForExitCodeWithTimeout(ProcessHandle handle, int* exit_code,
                                          int64 timeout_milliseconds);
 
@@ -396,6 +395,10 @@ BASE_API bool WaitForProcessesToExit(
 BASE_API bool WaitForSingleProcess(ProcessHandle handle,
                                    int64 wait_milliseconds);
 
+// Returns true when |wait_milliseconds| have elapsed and the process
+// is still running.
+BASE_API bool CrashAwareSleep(ProcessHandle handle, int64 wait_milliseconds);
+
 // Waits a certain amount of time (can be 0) for all the processes with a given
 // executable name to exit, then kills off any of them that are still around.
 // If filter is non-null, then only processes selected by the filter are waited
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 26301d2..6c7a9f6 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -724,15 +724,14 @@ bool WaitForExitCodeWithTimeout(ProcessHandle handle, int* exit_code,
     return false;
   if (!waitpid_success)
     return false;
+  if (!WIFEXITED(status))
+    return false;
   if (WIFSIGNALED(status)) {
     *exit_code = -1;
     return true;
   }
-  if (WIFEXITED(status)) {
-    *exit_code = WEXITSTATUS(status);
-    return true;
-  }
-  return false;
+  *exit_code = WEXITSTATUS(status);
+  return true;
 }
 
 #if defined(OS_MACOSX)
@@ -822,6 +821,19 @@ bool WaitForSingleProcess(ProcessHandle handle, int64 wait_milliseconds) {
   }
 }
 
+bool CrashAwareSleep(ProcessHandle handle, int64 wait_milliseconds) {
+  bool waitpid_success;
+  int status = WaitpidWithTimeout(handle, wait_milliseconds, &waitpid_success);
+  if (status != -1) {
+    DCHECK(waitpid_success);
+    return !(WIFEXITED(status) || WIFSIGNALED(status));
+  } else {
+    // If waitpid returned with an error, then the process doesn't exist
+    // (which most probably means it didn't exist before our call).
+    return waitpid_success;
+  }
+}
+
 int64 TimeValToMicroseconds(const struct timeval& tv) {
   static const int kMicrosecondsPerSecond = 1000000;
   int64 ret = tv.tv_sec;  // Avoid (int * int) integer overflow.
diff --git a/base/process_util_win.cc b/base/process_util_win.cc
index 8ade06e..895ec3b 100644
--- a/base/process_util_win.cc
+++ b/base/process_util_win.cc
@@ -464,7 +464,8 @@ TerminationStatus GetTerminationStatus(ProcessHandle handle, int* exit_code) {
 
 bool WaitForExitCode(ProcessHandle handle, int* exit_code) {
   bool success = WaitForExitCodeWithTimeout(handle, exit_code, INFINITE);
-  CloseProcessHandle(handle);
+  if (!success)
+    CloseProcessHandle(handle);
   return success;
 }
 
@@ -476,6 +477,10 @@ bool WaitForExitCodeWithTimeout(ProcessHandle handle, int* exit_code,
   if (!::GetExitCodeProcess(handle, &temp_code))
     return false;
 
+  // Only close the handle on success, to give the caller a chance to forcefully
+  // terminate the process if he wants to.
+  CloseProcessHandle(handle);
+
   *exit_code = temp_code;
   return true;
 }
@@ -539,6 +544,11 @@ bool WaitForSingleProcess(ProcessHandle handle, int64 wait_milliseconds) {
   return retval;
 }
 
+bool CrashAwareSleep(ProcessHandle handle, int64 wait_milliseconds) {
+  bool retval = WaitForSingleObject(handle, wait_milliseconds) == WAIT_TIMEOUT;
+  return retval;
+}
+
 bool CleanupProcesses(const std::wstring& executable_name,
                       int64 wait_milliseconds,
                       int exit_code,