Modify chrome_frame_test_utils to react gracefully if crash_service is already running before test launch (i.e., no spurious error message, leave it running as it was). Also, if it needs to be started, rather than waiting 500ms and assuming it is launched, poll the pipe and return when server is listening.

BUG=None
TEST="chrome_frame_tests" (optionally with a --gtest_filter=...). Allow it to run to completion (notice crash service starts and then stops). Launch test suite again, kill it after crash_service starts, launch it again. Notice it does not start new crash_service, nor log a needless "Failed to terminate process" warning. Watch debug.log for traces about the launch of crash_service.
Review URL: http://codereview.chromium.org/3437012

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

1 files changed, 86 insertions, 5 deletions

diff --git a/chrome_frame/test/chrome_frame_test_utils.cc b/chrome_frame/test/chrome_frame_test_utils.cc
index c0670fe..3b0c132 100644
--- a/chrome_frame/test/chrome_frame_test_utils.cc
+++ b/chrome_frame/test/chrome_frame_test_utils.cc
@@ -27,6 +27,16 @@
 
 namespace chrome_frame_test {
 
+const wchar_t kCrashServicePipeName[] = L"\\\\.\\pipe\\ChromeCrashServices";
+
+const DWORD kCrashServicePipeDesiredAccess = FILE_READ_DATA |
+                                             FILE_WRITE_DATA |
+                                             FILE_WRITE_ATTRIBUTES;
+
+const DWORD kCrashServicePipeFlagsAndAttributes = SECURITY_IDENTIFICATION |
+                                                  SECURITY_SQOS_PRESENT;
+const int kCrashServiceStartupTimeoutMs = 500;
+
 const wchar_t kIEImageName[] = L"iexplore.exe";
 const wchar_t kIEBrokerImageName[] = L"ieuser.exe";
 const wchar_t kFirefoxImageName[] = L"firefox.exe";
@@ -489,7 +499,63 @@ CloseIeAtEndOfScope::~CloseIeAtEndOfScope() {
   DLOG_IF(ERROR, closed != 0) << "Closed " << closed << " windows forcefully";
 }
 
+// Attempt to connect to a running crash_service instance. Success occurs if we
+// can actually connect to the service's pipe or we receive ERROR_PIPE_BUSY.
+// Waits up to |timeout_ms| for success. |timeout_ms| may be 0, meaning only try
+// once, or negative, meaning wait forever.
+bool DetectRunningCrashService(int timeout_ms) {
+  // Wait for the crash_service.exe to be ready for clients.
+  base::Time start = base::Time::Now();
+  ScopedHandle new_pipe;
+
+  while (true) {
+    new_pipe.Set(::CreateFile(kCrashServicePipeName,
+                              kCrashServicePipeDesiredAccess,
+                              0,  // dwShareMode
+                              NULL,  // lpSecurityAttributes
+                              OPEN_EXISTING,
+                              kCrashServicePipeFlagsAndAttributes,
+                              NULL));  // hTemplateFile
+
+    if (new_pipe.IsValid()) {
+      return true;
+    }
+
+    switch (::GetLastError()) {
+      case ERROR_PIPE_BUSY:
+        // OK, it exists, let's assume that clients will eventually be able to
+        // connect to it.
+        return true;
+      case ERROR_FILE_NOT_FOUND:
+        // Wait a bit longer
+        break;
+      default:
+        DLOG(WARNING) << "Unexpected error while checking crash_service.exe's "
+                      << "pipe: " << win_util::FormatLastWin32Error();
+        // Go ahead and wait in case it clears up.
+        break;
+    }
+
+    if (timeout_ms == 0) {
+      return false;
+    } else if (timeout_ms > 0) {
+      base::TimeDelta duration = base::Time::Now() - start;
+      if (duration.InMilliseconds() > timeout_ms) {
+        return false;
+      }
+    }
+
+    Sleep(10);
+  }
+}
+
 base::ProcessHandle StartCrashService() {
+  if (DetectRunningCrashService(kCrashServiceStartupTimeoutMs)) {
+    DLOG(INFO) << "crash_service.exe is already running. We will use the "
+               << "existing process and leave it running after tests complete.";
+    return NULL;
+  }
+
   FilePath exe_dir;
   if (!PathService::Get(base::DIR_EXE, &exe_dir)) {
     DCHECK(false);
@@ -498,6 +564,8 @@ base::ProcessHandle StartCrashService() {
 
   base::ProcessHandle crash_service = NULL;
 
+  DLOG(INFO) << "Starting crash_service.exe so you know if a test crashes!";
+
   FilePath crash_service_path = exe_dir.AppendASCII("crash_service.exe");
   if (!base::LaunchApp(crash_service_path.value(), false, false,
                        &crash_service)) {
@@ -505,11 +573,24 @@ base::ProcessHandle StartCrashService() {
     return NULL;
   }
 
-  DLOG(INFO) << "Started crash_service.exe so you know if a test crashes!";
-  // This sleep is to ensure that the crash service is done initializing, i.e
-  // the pipe creation, etc.
-  Sleep(500);
-  return crash_service;
+  base::Time start = base::Time::Now();
+
+  if (DetectRunningCrashService(kCrashServiceStartupTimeoutMs)) {
+    DLOG(INFO) << "crash_service.exe is ready for clients in "
+               << (base::Time::Now() - start).InMilliseconds() << "ms.";
+    return crash_service;
+  } else {
+    DLOG(ERROR) << "crash_service.exe failed to accept client connections "
+                << "within " << kCrashServiceStartupTimeoutMs << "ms. "
+                << "Terminating it now.";
+
+    // First check to see if it's even still running just to minimize the
+    // likelihood of spurious error messages from KillProcess.
+    if (WAIT_OBJECT_0 != ::WaitForSingleObject(crash_service, 0)) {
+      base::KillProcess(crash_service, 0, false);
+    }
+    return NULL;
+  }
 }
 
 }  // namespace chrome_frame_test