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

13 files changed, 150 insertions, 103 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,
diff --git a/chrome/browser/process_singleton_linux_uitest.cc b/chrome/browser/process_singleton_linux_uitest.cc
index 2129ed0..6606eb0 100644
--- a/chrome/browser/process_singleton_linux_uitest.cc
+++ b/chrome/browser/process_singleton_linux_uitest.cc
@@ -171,10 +171,7 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessFailure) {
             NotifyOtherProcess(url, TestTimeouts::action_timeout_ms()));
 
   // Wait for a while to make sure the browser process is actually killed.
-  int exit_code = 0;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(-1, exit_code);  // Expect unclean shutdown.
+  EXPECT_FALSE(CrashAwareSleep(TestTimeouts::action_timeout_ms()));
 }
 
 // Test that we don't kill ourselves by accident if a lockfile with the same pid
@@ -228,10 +225,7 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessDifferingHost) {
   // Kill the browser process, so that it does not respond on the socket.
   kill(pid, SIGKILL);
   // Wait for a while to make sure the browser process is actually killed.
-  int exit_code = 0;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(-1, exit_code);  // Expect unclean shutdown.
+  EXPECT_FALSE(CrashAwareSleep(TestTimeouts::action_timeout_ms()));
 
   EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
@@ -253,10 +247,7 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessOrCreate_DifferingHost) {
   // Kill the browser process, so that it does not respond on the socket.
   kill(pid, SIGKILL);
   // Wait for a while to make sure the browser process is actually killed.
-  int exit_code = 0;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(-1, exit_code);  // Expect unclean shutdown.
+  EXPECT_FALSE(CrashAwareSleep(TestTimeouts::action_timeout_ms()));
 
   EXPECT_EQ(0, unlink(lock_path_.value().c_str()));
   EXPECT_EQ(0, symlink("FAKEFOOHOST-1234", lock_path_.value().c_str()));
diff --git a/chrome/browser/unload_uitest.cc b/chrome/browser/unload_uitest.cc
index 002a27d..5ae47bd 100644
--- a/chrome/browser/unload_uitest.cc
+++ b/chrome/browser/unload_uitest.cc
@@ -103,13 +103,25 @@ class UnloadTest : public UITest {
     UITest::SetUp();
   }
 
+  void WaitForBrowserClosed() {
+    const int kCheckDelayMs = 100;
+    for (int max_wait_time = TestTimeouts::action_max_timeout_ms();
+         max_wait_time > 0; max_wait_time -= kCheckDelayMs) {
+      CrashAwareSleep(kCheckDelayMs);
+      if (!IsBrowserRunning())
+        break;
+    }
+
+    EXPECT_FALSE(IsBrowserRunning());
+  }
+
   void CheckTitle(const std::wstring& expected_title) {
     const int kCheckDelayMs = 100;
     for (int max_wait_time = TestTimeouts::action_max_timeout_ms();
          max_wait_time > 0; max_wait_time -= kCheckDelayMs) {
+      CrashAwareSleep(kCheckDelayMs);
       if (expected_title == GetActiveTabTitle())
         break;
-      base::PlatformThread::Sleep(kCheckDelayMs);
     }
 
     EXPECT_EQ(expected_title, GetActiveTabTitle());
@@ -289,11 +301,7 @@ TEST_F(UnloadTest, BrowserCloseBeforeUnloadOK) {
 
   CloseBrowserAsync(browser.get());
   ClickModalDialogButton(ui::MessageBoxFlags::DIALOGBUTTON_OK);
-
-  int exit_code = -1;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(0, exit_code);  // Expect a clean shutown.
+  WaitForBrowserClosed();
 }
 
 // Tests closing the browser with a beforeunload handler and clicking
@@ -305,19 +313,14 @@ TEST_F(UnloadTest, BrowserCloseBeforeUnloadCancel) {
 
   CloseBrowserAsync(browser.get());
   ClickModalDialogButton(ui::MessageBoxFlags::DIALOGBUTTON_CANCEL);
-
   // There's no real graceful way to wait for something _not_ to happen, so
   // we just wait a short period.
-  base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms());
+  CrashAwareSleep(500);
   ASSERT_TRUE(IsBrowserRunning());
 
   CloseBrowserAsync(browser.get());
   ClickModalDialogButton(ui::MessageBoxFlags::DIALOGBUTTON_OK);
-
-  int exit_code = -1;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(0, exit_code);  // Expect a clean shutdown.
+  WaitForBrowserClosed();
 }
 
 #if defined(OS_LINUX)
@@ -339,11 +342,7 @@ TEST_F(UnloadTest, MAYBE_BrowserCloseWithInnerFocusedFrame) {
 
   CloseBrowserAsync(browser.get());
   ClickModalDialogButton(ui::MessageBoxFlags::DIALOGBUTTON_OK);
-
-  int exit_code = -1;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::action_max_timeout_ms(), &exit_code));
-  EXPECT_EQ(0, exit_code);  // Expect a clean shutdown.
+  WaitForBrowserClosed();
 }
 
 // Tests closing the browser with a beforeunload handler that takes
diff --git a/chrome/common/service_process_util_unittest.cc b/chrome/common/service_process_util_unittest.cc
index 58aea81..4155bda 100644
--- a/chrome/common/service_process_util_unittest.cc
+++ b/chrome/common/service_process_util_unittest.cc
@@ -180,8 +180,7 @@ TEST_F(ServiceProcessStateTest, ForceShutdown) {
   ASSERT_TRUE(ForceServiceProcessShutdown(version, pid));
   int exit_code = 0;
   ASSERT_TRUE(base::WaitForExitCodeWithTimeout(handle,
-      &exit_code, TestTimeouts::action_max_timeout_ms()));
-  base::CloseProcessHandle(handle);
+      &exit_code, TestTimeouts::action_timeout_ms() * 2));
   ASSERT_EQ(exit_code, 0);
 }
 
diff --git a/chrome/test/automation/proxy_launcher.cc b/chrome/test/automation/proxy_launcher.cc
index 1928902..90a62b2 100644
--- a/chrome/test/automation/proxy_launcher.cc
+++ b/chrome/test/automation/proxy_launcher.cc
@@ -206,14 +206,13 @@ void ProxyLauncher::QuitBrowser() {
     return;
   }
 
-  base::TimeTicks quit_start = base::TimeTicks::Now();
-
   // There's nothing to do here if the browser is not running.
   // WARNING: There is a race condition here where the browser may shut down
   // after this check but before some later automation call. Your test should
   // use WaitForBrowserProcessToQuit() if it intentionally
   // causes the browser to shut down.
   if (IsBrowserRunning()) {
+    base::TimeTicks quit_start = base::TimeTicks::Now();
     EXPECT_TRUE(automation()->SetFilteredInet(false));
 
     if (WINDOW_CLOSE == shutdown_type_) {
@@ -254,24 +253,21 @@ void ProxyLauncher::QuitBrowser() {
     } else {
       NOTREACHED() << "Invalid shutdown type " << shutdown_type_;
     }
-  }
 
-  // Now, drop the automation IPC channel so that the automation provider in
-  // the browser notices and drops its reference to the browser process.
-  automation()->Disconnect();
-
-  // Wait for the browser process to quit. It should quit once all tabs have
-  // been closed.
-  int exit_code = -1;
-  if (WaitForBrowserProcessToQuit(
-          TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code)) {
-    EXPECT_EQ(0, exit_code);  // Expect a clean shutdown.
-  } else {
-    // We need to force the browser to quit because it didn't quit fast
-    // enough. Take no chance and kill every chrome processes.
-    CleanupAppProcesses();
+    // Now, drop the automation IPC channel so that the automation provider in
+    // the browser notices and drops its reference to the browser process.
+    automation()->Disconnect();
+
+    // Wait for the browser process to quit. It should quit once all tabs have
+    // been closed.
+    if (!WaitForBrowserProcessToQuit(
+        TestTimeouts::wait_for_terminate_timeout_ms())) {
+      // We need to force the browser to quit because it didn't quit fast
+      // enough. Take no chance and kill every chrome processes.
+      CleanupAppProcesses();
+    }
+    browser_quit_time_ = base::TimeTicks::Now() - quit_start;
   }
-  browser_quit_time_ = base::TimeTicks::Now() - quit_start;
 
   // Don't forget to close the handle
   base::CloseProcessHandle(process_);
@@ -280,9 +276,8 @@ void ProxyLauncher::QuitBrowser() {
 }
 
 void ProxyLauncher::TerminateBrowser() {
-  base::TimeTicks quit_start = base::TimeTicks::Now();
-
   if (IsBrowserRunning()) {
+    base::TimeTicks quit_start = base::TimeTicks::Now();
     EXPECT_TRUE(automation()->SetFilteredInet(false));
 #if defined(OS_WIN)
     scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
@@ -297,18 +292,15 @@ void ProxyLauncher::TerminateBrowser() {
 #if defined(OS_POSIX)
     EXPECT_EQ(kill(process_, SIGTERM), 0);
 #endif  // OS_POSIX
-  }
 
-  int exit_code = 0;
-  if (WaitForBrowserProcessToQuit(
-          TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code)) {
-    EXPECT_EQ(0, exit_code);  // Expect a clean shutdown.
-  } else {
-    // We need to force the browser to quit because it didn't quit fast
-    // enough. Take no chance and kill every chrome processes.
-    CleanupAppProcesses();
+    if (!WaitForBrowserProcessToQuit(
+        TestTimeouts::wait_for_terminate_timeout_ms())) {
+      // We need to force the browser to quit because it didn't quit fast
+      // enough. Take no chance and kill every chrome processes.
+      CleanupAppProcesses();
+    }
+    browser_quit_time_ = base::TimeTicks::Now() - quit_start;
   }
-  browser_quit_time_ = base::TimeTicks::Now() - quit_start;
 
   // Don't forget to close the handle
   base::CloseProcessHandle(process_);
@@ -335,18 +327,19 @@ void ProxyLauncher::CleanupAppProcesses() {
   TerminateAllChromeProcesses(process_id_);
 }
 
-bool ProxyLauncher::WaitForBrowserProcessToQuit(int timeout, int* exit_code) {
+bool ProxyLauncher::WaitForBrowserProcessToQuit(int timeout) {
 #ifdef WAIT_FOR_DEBUGGER_ON_OPEN
   timeout = 500000;
 #endif
-  return base::WaitForExitCodeWithTimeout(process_, exit_code, timeout);
+  return base::WaitForSingleProcess(process_, timeout);
 }
 
 bool ProxyLauncher::IsBrowserRunning() {
-  // Send a simple message to the browser. If it comes back, the browser
-  // must be alive.
-  int window_count;
-  return automation_proxy_->GetBrowserWindowCount(&window_count);
+  return CrashAwareSleep(0);
+}
+
+bool ProxyLauncher::CrashAwareSleep(int timeout_ms) {
+  return base::CrashAwareSleep(process_, timeout_ms);
 }
 
 void ProxyLauncher::PrepareTestCommandline(CommandLine* command_line,
diff --git a/chrome/test/automation/proxy_launcher.h b/chrome/test/automation/proxy_launcher.h
index 505c65d..642c539 100644
--- a/chrome/test/automation/proxy_launcher.h
+++ b/chrome/test/automation/proxy_launcher.h
@@ -122,10 +122,13 @@ class ProxyLauncher {
   // window or if the browser process died by itself.
   bool IsBrowserRunning();
 
+  // Returns true when timeout_ms milliseconds have elapsed.
+  // Returns false if the browser process died while waiting.
+  bool CrashAwareSleep(int timeout_ms);
+
   // Wait for the browser process to shut down on its own (i.e. as a result of
-  // some action that your test has taken). If it has exited within |timeout|,
-  // puts the exit code in |exit_code| and returns true.
-  bool WaitForBrowserProcessToQuit(int timeout, int* exit_code);
+  // some action that your test has taken).
+  bool WaitForBrowserProcessToQuit(int timeout);
 
   AutomationProxy* automation() const;
 
diff --git a/chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc b/chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc
index a783f3c..15a0ab8 100644
--- a/chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc
+++ b/chrome/test/interactive_ui/fast_shutdown_interactive_uitest.cc
@@ -54,9 +54,6 @@ TEST_F(FastShutdown, MAYBE_SlowTermination) {
   ASSERT_TRUE(automation()->WaitForAppModalDialog());
   ASSERT_TRUE(automation()->ClickAppModalDialogButton(
                   ui::MessageBoxFlags::DIALOGBUTTON_OK));
-
-  int exit_code = -1;
-  ASSERT_TRUE(launcher_->WaitForBrowserProcessToQuit(
-                  TestTimeouts::wait_for_terminate_timeout_ms(), &exit_code));
-  EXPECT_EQ(0, exit_code);  // Expect a clean shutdown.
+  ASSERT_TRUE(WaitForBrowserProcessToQuit(
+      TestTimeouts::wait_for_terminate_timeout_ms()));
 }
diff --git a/chrome/test/out_of_proc_test_runner.cc b/chrome/test/out_of_proc_test_runner.cc
index 6e4bc3b..db5e54c 100644
--- a/chrome/test/out_of_proc_test_runner.cc
+++ b/chrome/test/out_of_proc_test_runner.cc
@@ -401,8 +401,6 @@ int RunTest(const std::string& test_name, int default_timeout_ms) {
   }
 #endif
 
-  base::CloseProcessHandle(process_handle);
-
   return exit_code;
 }
 
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 95378d7..2e47340 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -297,6 +297,10 @@ void UITestBase::NavigateToURLBlockUntilNavigationsComplete(
                 url, number_of_navigations)) << url.spec();
 }
 
+bool UITestBase::WaitForBrowserProcessToQuit(int timeout) {
+  return launcher_->WaitForBrowserProcessToQuit(timeout);
+}
+
 bool UITestBase::WaitForBookmarkBarVisibilityChange(BrowserProxy* browser,
                                                     bool wait_for_open) {
   const int kCycles = 10;
@@ -309,7 +313,11 @@ bool UITestBase::WaitForBookmarkBarVisibilityChange(BrowserProxy* browser,
       return true;  // Bookmark bar visibility change complete.
 
     // Give it a chance to catch up.
-    base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms() / kCycles);
+    bool browser_survived = CrashAwareSleep(
+        TestTimeouts::action_timeout_ms() / kCycles);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return false;
   }
 
   ADD_FAILURE() << "Timeout reached in WaitForBookmarkBarVisibilityChange";
@@ -357,6 +365,10 @@ bool UITestBase::IsBrowserRunning() {
   return launcher_->IsBrowserRunning();
 }
 
+bool UITestBase::CrashAwareSleep(int timeout_ms) {
+  return launcher_->CrashAwareSleep(timeout_ms);
+}
+
 int UITestBase::GetTabCount() {
   return GetTabCount(0);
 }
@@ -379,10 +391,12 @@ void UITestBase::WaitUntilTabCount(int tab_count) {
   const int kIntervalMs = TestTimeouts::action_timeout_ms() / kMaxIntervals;
 
   for (int i = 0; i < kMaxIntervals; ++i) {
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return;
     if (GetTabCount() == tab_count)
       return;
-
-    base::PlatformThread::Sleep(kIntervalMs);
   }
 
   ADD_FAILURE() << "Timeout reached in WaitUntilTabCount";
@@ -713,6 +727,11 @@ bool UITest::WaitUntilJavaScriptCondition(TabProxy* tab,
 
   // Wait until the test signals it has completed.
   for (int i = 0; i < kMaxIntervals; ++i) {
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return false;
+
     bool done_value = false;
     bool success = tab->ExecuteAndExtractBool(frame_xpath, jscript,
                                               &done_value);
@@ -721,8 +740,6 @@ bool UITest::WaitUntilJavaScriptCondition(TabProxy* tab,
       return false;
     if (done_value)
       return true;
-
-    base::PlatformThread::Sleep(kIntervalMs);
   }
 
   ADD_FAILURE() << "Timeout reached in WaitUntilJavaScriptCondition";
@@ -739,11 +756,14 @@ bool UITest::WaitUntilCookieValue(TabProxy* tab,
 
   std::string cookie_value;
   for (int i = 0; i < kMaxIntervals; ++i) {
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return false;
+
     EXPECT_TRUE(tab->GetCookieByName(url, cookie_name, &cookie_value));
     if (cookie_value == expected_value)
       return true;
-
-    base::PlatformThread::Sleep(kIntervalMs);
   }
 
   ADD_FAILURE() << "Timeout reached in WaitUntilCookieValue";
@@ -758,12 +778,15 @@ std::string UITest::WaitUntilCookieNonEmpty(TabProxy* tab,
   const int kMaxIntervals = timeout_ms / kIntervalMs;
 
   for (int i = 0; i < kMaxIntervals; ++i) {
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return std::string();
+
     std::string cookie_value;
     EXPECT_TRUE(tab->GetCookieByName(url, cookie_name, &cookie_value));
     if (!cookie_value.empty())
       return cookie_value;
-
-    base::PlatformThread::Sleep(kIntervalMs);
   }
 
   ADD_FAILURE() << "Timeout reached in WaitUntilCookieNonEmpty";
@@ -789,7 +812,11 @@ bool UITest::WaitForFindWindowVisibilityChange(BrowserProxy* browser,
       return true;  // Find window visibility change complete.
 
     // Give it a chance to catch up.
-    base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms() / kCycles);
+    bool browser_survived = CrashAwareSleep(
+        TestTimeouts::action_timeout_ms() / kCycles);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return false;
   }
 
   ADD_FAILURE() << "Timeout reached in WaitForFindWindowVisibilityChange";
@@ -813,6 +840,19 @@ bool UITest::WaitForDownloadShelfVisibilityChange(BrowserProxy* browser,
   int incorrect_state_count = 0;
   base::Time start = base::Time::Now();
   for (int i = 0; i < kCycles; i++) {
+    // Give it a chance to catch up.
+    bool browser_survived = CrashAwareSleep(
+        TestTimeouts::action_timeout_ms() / kCycles);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived) {
+      LOG(INFO) << "Elapsed time: " << (base::Time::Now() - start).InSecondsF()
+                << " seconds"
+                << " call failed " << fail_count << " times"
+                << " state was incorrect " << incorrect_state_count << " times";
+      ADD_FAILURE() << "Browser failed in " << __FUNCTION__;
+      return false;
+    }
+
     bool visible = !wait_for_open;
     if (!browser->IsShelfVisible(&visible)) {
       fail_count++;
@@ -826,9 +866,6 @@ bool UITest::WaitForDownloadShelfVisibilityChange(BrowserProxy* browser,
       return true;  // Got the download shelf.
     }
     incorrect_state_count++;
-
-    // Give it a chance to catch up.
-    base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms() / kCycles);
   }
 
   LOG(INFO) << "Elapsed time: " << (base::Time::Now() - start).InSecondsF()
diff --git a/chrome/test/ui/ui_test.h b/chrome/test/ui/ui_test.h
index 5ec4039..f446c6a 100644
--- a/chrome/test/ui/ui_test.h
+++ b/chrome/test/ui/ui_test.h
@@ -139,6 +139,10 @@ class UITestBase {
   // window or if the browser process died by itself.
   bool IsBrowserRunning();
 
+  // Returns true when timeout_ms milliseconds have elapsed.
+  // Returns false if the browser process died while waiting.
+  bool CrashAwareSleep(int timeout_ms);
+
   // Returns the number of tabs in the first window.  If no windows exist,
   // causes a test failure and returns 0.
   int GetTabCount();
@@ -150,6 +154,10 @@ class UITestBase {
   // assert that the tab count is valid at the end of the wait.
   void WaitUntilTabCount(int tab_count);
 
+  // Wait for the browser process to shut down on its own (i.e. as a result of
+  // some action that your test has taken).
+  bool WaitForBrowserProcessToQuit(int timeout);
+
   // Waits until the Bookmark bar has stopped animating and become fully visible
   // (if |wait_for_open| is true) or fully hidden (if |wait_for_open| is false).
   // This function can time out (in which case it returns false).
diff --git a/chrome_frame/test/perf/chrome_frame_perftest.cc b/chrome_frame/test/perf/chrome_frame_perftest.cc
index dac9605..4320bff 100644
--- a/chrome_frame/test/perf/chrome_frame_perftest.cc
+++ b/chrome_frame/test/perf/chrome_frame_perftest.cc
@@ -1358,8 +1358,7 @@ void PrintResultList(const std::string& measurement,
 
 bool RunSingleTestOutOfProc(const std::string& test_name) {
   FilePath path;
-  if (!PathService::Get(base::DIR_EXE, &path))
-    return false;
+  PathService::Get(base::DIR_EXE, &path);
   path = path.Append(L"chrome_frame_tests.exe");
 
   CommandLine cmd_line(path);
@@ -1385,8 +1384,6 @@ bool RunSingleTestOutOfProc(const std::string& test_name) {
       base::KillProcess(process_handle, -1, true);
   }
 
-  base::CloseProcessHandle(process_handle);
-
   return exit_code == 0;
 }