Wait on a pipe for the test server to start up

This should speed up testserver-based unit tests considerably
and make them less flakey.

R=agl,cpu,phajdan,wtc
BUG=49680
TEST=net_unittests

Review URL: http://codereview.chromium.org/3368012

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

4 files changed, 73 insertions, 20 deletions

diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 4c698b0..536d351 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -27,7 +27,6 @@
 #include "net/base/host_resolver.h"
 #include "net/base/test_completion_callback.h"
 #include "net/socket/tcp_client_socket.h"
-#include "net/socket/tcp_pinger.h"
 #include "net/test/python_utils.h"
 #include "testing/platform_test.h"
 
@@ -256,22 +255,6 @@ bool TestServer::SetPythonPath() {
   return true;
 }
 
-bool TestServer::WaitToStart() {
-  net::AddressList addr;
-  if (!GetAddressList(&addr))
-    return false;
-
-  net::TCPPinger pinger(addr);
-  int rv = pinger.Ping(
-      base::TimeDelta::FromMilliseconds(kServerConnectionTimeoutMs),
-      kServerConnectionAttempts);
-  bool result = (rv == net::OK);
-  if (!result) {
-    LOG(ERROR) << "Failed to connect to server";
-  }
-  return result;
-}
-
 FilePath TestServer::GetRootCertificatePath() {
   return certificates_dir_.AppendASCII("root_ca_cert.crt");
 }
diff --git a/net/test/test_server.h b/net/test/test_server.h
index 8b6dc63..6baaaec 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -12,6 +12,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
+#include "base/file_util.h"
 #include "base/process_util.h"
 #include "net/base/host_port_pair.h"
 
@@ -109,6 +110,15 @@ class TestServer {
 #if defined(OS_WIN)
   // JobObject used to clean up orphaned child processes.
   ScopedHandle job_handle_;
+
+  // The file handle the child writes to when it starts.
+  ScopedHandle child_fd_;
+#endif
+
+#if defined(OS_POSIX)
+  // The file descriptor the child writes to when it starts.
+  int child_fd_;
+  file_util::ScopedFD child_fd_closer_;
 #endif
 
 #if defined(USE_NSS)
diff --git a/net/test/test_server_posix.cc b/net/test/test_server_posix.cc
index 7741d89..262845c 100644
--- a/net/test/test_server_posix.cc
+++ b/net/test/test_server_posix.cc
@@ -32,8 +32,22 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
   if (type_ == TYPE_HTTPS_CLIENT_AUTH)
     command_line.push_back("--ssl-client-auth");
 
-  base::file_handle_mapping_vector no_mappings;
-  if (!base::LaunchApp(command_line, no_mappings, false, &process_handle_)) {
+  int pipefd[2];
+  if (pipe(pipefd) != 0) {
+    PLOG(ERROR) << "Could not create pipe.";
+    return false;
+  }
+
+  // Save the read half. The write half is sent to the child.
+  child_fd_ = pipefd[0];
+  child_fd_closer_.reset(&child_fd_);
+  file_util::ScopedFD write_closer(&pipefd[1]);
+  base::file_handle_mapping_vector map_write_fd;
+  map_write_fd.push_back(std::make_pair(pipefd[1], pipefd[1]));
+
+  command_line.push_back("--startup-pipe=" + base::IntToString(pipefd[1]));
+
+  if (!base::LaunchApp(command_line, map_write_fd, false, &process_handle_)) {
     LOG(ERROR) << "Failed to launch " << command_line[0] << " ...";
     return false;
   }
@@ -41,6 +55,14 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
   return true;
 }
 
+bool TestServer::WaitToStart() {
+  char buf[8];
+  ssize_t n = HANDLE_EINTR(read(child_fd_, buf, sizeof(buf)));
+  // We don't need the FD anymore.
+  child_fd_closer_.reset(NULL);
+  return n > 0;
+}
+
 bool TestServer::CheckCATrusted() {
   return true;
 }
diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc
index 7e8443f..a8b3678 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -34,7 +34,7 @@ bool LaunchTestServerAsJob(const std::wstring& cmdline,
   // The CREATE_BREAKAWAY_FROM_JOB flag is used to prevent this.
   if (!CreateProcess(NULL,
                      const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
-                     FALSE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL,
+                     TRUE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL,
                      &startup_info, &process_info)) {
     LOG(ERROR) << "Could not create process.";
     return false;
@@ -107,6 +107,36 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
   if (type_ == TYPE_HTTPS_CLIENT_AUTH)
     command_line.append(L" --ssl-client-auth");
 
+  HANDLE child_read = NULL;
+  HANDLE child_write = NULL;
+  if (!CreatePipe(&child_read, &child_write, NULL, 0)) {
+    PLOG(ERROR) << "Failed to create pipe";
+    return false;
+  }
+  child_fd_.Set(child_read);
+  ScopedHandle scoped_child_write(child_write);
+
+  // Have the child inherit the write half.
+  if (!SetHandleInformation(child_write, HANDLE_FLAG_INHERIT,
+                            HANDLE_FLAG_INHERIT)) {
+    PLOG(ERROR) << "Failed to enable pipe inheritance";
+    return false;
+  }
+
+  // Pass the handle on the command-line. Although HANDLE is a
+  // pointer, truncating it on 64-bit machines is okay. See
+  // http://msdn.microsoft.com/en-us/library/aa384203.aspx
+  //
+  // "64-bit versions of Windows use 32-bit handles for
+  // interoperability. When sharing a handle between 32-bit and 64-bit
+  // applications, only the lower 32 bits are significant, so it is
+  // safe to truncate the handle (when passing it from 64-bit to
+  // 32-bit) or sign-extend the handle (when passing it from 32-bit to
+  // 64-bit)."
+  command_line.append(
+      L" --startup-pipe=" +
+      ASCIIToWide(base::IntToString(reinterpret_cast<uintptr_t>(child_write))));
+
   if (!LaunchTestServerAsJob(command_line,
                              true,
                              &process_handle_,
@@ -118,6 +148,14 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
   return true;
 }
 
+bool TestServer::WaitToStart() {
+  char buf[8];
+  DWORD bytes_read;
+  BOOL result = ReadFile(child_fd_, buf, sizeof(buf), &bytes_read, NULL);
+  child_fd_.Close();
+  return result && bytes_read > 0;
+}
+
 bool TestServer::CheckCATrusted() {
   HCERTSTORE cert_store = CertOpenSystemStore(NULL, L"ROOT");
   if (!cert_store) {