1 files changed, 53 insertions, 37 deletions

diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc
index a8b3678..c6e10d5 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -8,17 +8,21 @@
 #include <wincrypt.h>
 
 #include "base/base_paths.h"
+#include "base/command_line.h"
 #include "base/file_util.h"
+#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "base/test/test_timeouts.h"
+#include "base/thread.h"
 #include "base/utf_string_conversions.h"
 
 #pragma comment(lib, "crypt32.lib")
 
 namespace {
 
-bool LaunchTestServerAsJob(const std::wstring& cmdline,
+bool LaunchTestServerAsJob(const CommandLine& cmdline,
                            bool start_hidden,
                            base::ProcessHandle* process_handle,
                            ScopedHandle* job_handle) {
@@ -32,10 +36,10 @@ bool LaunchTestServerAsJob(const std::wstring& cmdline,
   // If this code is run under a debugger, the test server process is
   // automatically associated with a job object created by the debugger.
   // The CREATE_BREAKAWAY_FROM_JOB flag is used to prevent this.
-  if (!CreateProcess(NULL,
-                     const_cast<wchar_t*>(cmdline.c_str()), NULL, NULL,
-                     TRUE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL,
-                     &startup_info, &process_info)) {
+  if (!CreateProcess(
+      NULL, const_cast<wchar_t*>(cmdline.command_line_string().c_str()),
+      NULL, NULL, TRUE, CREATE_BREAKAWAY_FROM_JOB, NULL, NULL,
+      &startup_info, &process_info)) {
     LOG(ERROR) << "Could not create process.";
     return false;
   }
@@ -71,9 +75,21 @@ bool LaunchTestServerAsJob(const std::wstring& cmdline,
   return true;
 }
 
+void UnblockPipe(HANDLE handle, bool* unblocked) {
+  static const char kUnblock[] = "UNBLOCK";
+  // Unblock the ReadFile in TestServer::WaitToStart by writing to the pipe.
+  // Make sure the call succeeded, otherwise we are very likely to hang.
+  DWORD bytes_written = 0;
+  CHECK(WriteFile(handle, kUnblock, arraysize(kUnblock), &bytes_written,
+                  NULL));
+  CHECK_EQ(arraysize(kUnblock), bytes_written);
+  *unblocked = true;
+}
+
 }  // namespace
 
 namespace net {
+
 bool TestServer::LaunchPython(const FilePath& testserver_path) {
   FilePath python_exe;
   if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_exe))
@@ -83,29 +99,10 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
       .Append(FILE_PATH_LITERAL("python_24"))
       .Append(FILE_PATH_LITERAL("python.exe"));
 
-  std::wstring command_line =
-      L"\"" + python_exe.value() + L"\" " +
-      L"\"" + testserver_path.value() +
-      L"\" --port=" + ASCIIToWide(base::IntToString(host_port_pair_.port())) +
-      L" --data-dir=\"" + document_root_.value() + L"\"";
-
-  if (type_ == TYPE_FTP)
-    command_line.append(L" -f");
-
-  FilePath certificate_path(GetCertificatePath());
-  if (!certificate_path.value().empty()) {
-    if (!file_util::PathExists(certificate_path)) {
-      LOG(ERROR) << "Certificate path " << certificate_path.value()
-                 << " doesn't exist. Can't launch https server.";
-      return false;
-    }
-    command_line.append(L" --https=\"");
-    command_line.append(certificate_path.value());
-    command_line.append(L"\"");
-  }
-
-  if (type_ == TYPE_HTTPS_CLIENT_AUTH)
-    command_line.append(L" --ssl-client-auth");
+  CommandLine python_command(python_exe);
+  python_command.AppendArgPath(testserver_path);
+  if (!AddCommandLineArguments(&python_command))
+    return false;
 
   HANDLE child_read = NULL;
   HANDLE child_write = NULL;
@@ -113,8 +110,8 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
     PLOG(ERROR) << "Failed to create pipe";
     return false;
   }
-  child_fd_.Set(child_read);
-  ScopedHandle scoped_child_write(child_write);
+  child_read_fd_.Set(child_read);
+  child_write_fd_.Set(child_write);
 
   // Have the child inherit the write half.
   if (!SetHandleInformation(child_write, HANDLE_FLAG_INHERIT,
@@ -133,15 +130,15 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
   // 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))));
+  python_command.AppendSwitchASCII(
+      "startup-pipe",
+      base::IntToString(reinterpret_cast<uintptr_t>(child_write)));
 
-  if (!LaunchTestServerAsJob(command_line,
+  if (!LaunchTestServerAsJob(python_command,
                              true,
                              &process_handle_,
                              &job_handle_)) {
-    LOG(ERROR) << "Failed to launch " << command_line;
+    LOG(ERROR) << "Failed to launch " << python_command.command_line_string();
     return false;
   }
 
@@ -149,10 +146,29 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
+  base::Thread thread("test_server_watcher");
+  if (!thread.Start())
+    return false;
+
+  // Prepare a timeout in case the server fails to start.
+  bool unblocked = false;
+  thread.message_loop()->PostDelayedTask(FROM_HERE,
+      NewRunnableFunction(UnblockPipe, child_write_fd_.Get(), &unblocked),
+      TestTimeouts::action_max_timeout_ms());
+
   char buf[8];
   DWORD bytes_read;
-  BOOL result = ReadFile(child_fd_, buf, sizeof(buf), &bytes_read, NULL);
-  child_fd_.Close();
+  BOOL result = ReadFile(child_read_fd_.Get(), buf, sizeof(buf), &bytes_read,
+                         NULL);
+
+  thread.Stop();
+  child_read_fd_.Close();
+  child_write_fd_.Close();
+
+  // If we hit the timeout, fail.
+  if (unblocked)
+    return false;
+
   return result && bytes_read > 0;
 }