Made testserver communicate to parent process with JSON

This is so that if the testserver needs to communicate anything more than
the port in the future (e.g., xmpp port for the test sync server), it
can do so in a flexible manner.

BUG=53934
TEST=manually

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=66879

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67018

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67386

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

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

4 files changed, 141 insertions, 67 deletions

diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 0e8c461..1fd20c9 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -17,11 +17,14 @@
 #include "base/base64.h"
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
+#include "base/json/json_reader.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/scoped_ptr.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
+#include "base/values.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/cert_test_util.h"
 #include "net/base/host_port_pair.h"
@@ -278,6 +281,11 @@ bool TestServer::SetPythonPath() {
   }
   third_party_dir = third_party_dir.Append(FILE_PATH_LITERAL("third_party"));
 
+  // For simplejson. (simplejson, unlike all the other python modules
+  // we include, doesn't have an extra 'simplejson' directory, so we
+  // need to include its parent directory, i.e. third_party_dir).
+  AppendToPythonPath(third_party_dir);
+
   AppendToPythonPath(third_party_dir.Append(FILE_PATH_LITERAL("tlslite")));
   AppendToPythonPath(third_party_dir.Append(FILE_PATH_LITERAL("pyftpdlib")));
 
@@ -373,4 +381,28 @@ bool TestServer::AddCommandLineArguments(CommandLine* command_line) const {
   return true;
 }
 
+bool TestServer::ParseServerData(const std::string& server_data) {
+  VLOG(1) << "Server data: " << server_data;
+  base::JSONReader json_reader;
+  scoped_ptr<Value> value(json_reader.JsonToValue(server_data, true, false));
+  if (!value.get() ||
+      !value->IsType(Value::TYPE_DICTIONARY)) {
+    LOG(ERROR) << "Could not parse server data: "
+               << json_reader.GetErrorMessage();
+    return false;
+  }
+  DictionaryValue* dict = static_cast<DictionaryValue*>(value.get());
+  int port = 0;
+  if (!dict->GetInteger("port", &port)) {
+    LOG(ERROR) << "Could not find port value";
+    return false;
+  }
+  if ((port <= 0) || (port >= kuint16max)) {
+    LOG(ERROR) << "Invalid port value: " << port;
+    return false;
+  }
+  host_port_pair_.set_port(port);
+  return true;
+}
+
 }  // namespace net
diff --git a/net/test/test_server.h b/net/test/test_server.h
index f819365..5c11038 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -146,6 +146,10 @@ class TestServer {
   // Waits for the server to start. Returns true on success.
   bool WaitToStart() WARN_UNUSED_RESULT;
 
+  // Parses the server data read from the test server.  Returns true
+  // on success.
+  bool ParseServerData(const std::string& server_data) WARN_UNUSED_RESULT;
+
   // Returns path to the root certificate.
   FilePath GetRootCertificatePath();
 
diff --git a/net/test/test_server_posix.cc b/net/test/test_server_posix.cc
index 9c0210b..74dbcd4 100644
--- a/net/test/test_server_posix.cc
+++ b/net/test/test_server_posix.cc
@@ -53,6 +53,42 @@ class OrphanedTestServerFilter : public base::ProcessFilter {
   DISALLOW_COPY_AND_ASSIGN(OrphanedTestServerFilter);
 };
 
+// Given a file descriptor, reads into |buffer| until |bytes_max|
+// bytes has been read or an error has been encountered.  Returns true
+// if the read was successful.  |remaining_time| is used as a timeout.
+bool ReadData(int fd, ssize_t bytes_max, uint8* buffer,
+              base::TimeDelta* remaining_time) {
+  ssize_t bytes_read = 0;
+  base::Time previous_time = base::Time::Now();
+  while (bytes_read < bytes_max) {
+    struct pollfd poll_fds[1];
+
+    poll_fds[0].fd = fd;
+    poll_fds[0].events = POLLIN | POLLPRI;
+    poll_fds[0].revents = 0;
+
+    int rv = HANDLE_EINTR(poll(poll_fds, 1,
+                               remaining_time->InMilliseconds()));
+    if (rv != 1) {
+      LOG(ERROR) << "Failed to poll for the child file descriptor.";
+      return false;
+    }
+
+    base::Time current_time = base::Time::Now();
+    base::TimeDelta elapsed_time_cycle = current_time - previous_time;
+    DCHECK(elapsed_time_cycle.InMilliseconds() >= 0);
+    *remaining_time -= elapsed_time_cycle;
+    previous_time = current_time;
+
+    ssize_t num_bytes = HANDLE_EINTR(read(fd, buffer + bytes_read,
+                                          bytes_max - bytes_read));
+    if (num_bytes <= 0)
+      return false;
+    bytes_read += num_bytes;
+  }
+  return true;
+}
+
 }  // namespace
 
 namespace net {
@@ -98,45 +134,22 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  uint16 port;
-  uint8* buffer = reinterpret_cast<uint8*>(&port);
-  ssize_t bytes_read = 0;
-  ssize_t bytes_max = sizeof(port);
+  file_util::ScopedFD child_fd_closer(child_fd_closer_.release());
+
   base::TimeDelta remaining_time = base::TimeDelta::FromMilliseconds(
       TestTimeouts::action_max_timeout_ms());
-  base::Time previous_time = base::Time::Now();
-  while (bytes_read < bytes_max) {
-    struct pollfd poll_fds[1];
-
-    poll_fds[0].fd = child_fd_;
-    poll_fds[0].events = POLLIN | POLLPRI;
-    poll_fds[0].revents = 0;
-
-    int rv = HANDLE_EINTR(poll(poll_fds, 1, remaining_time.InMilliseconds()));
-    if (rv != 1) {
-      LOG(ERROR) << "Failed to poll for the child file descriptor.";
-      return false;
-    }
-
-    base::Time current_time = base::Time::Now();
-    base::TimeDelta elapsed_time_cycle = current_time - previous_time;
-    DCHECK(elapsed_time_cycle.InMilliseconds() >= 0);
-    remaining_time -= elapsed_time_cycle;
-    previous_time = current_time;
-
-    ssize_t num_bytes = HANDLE_EINTR(read(child_fd_, buffer + bytes_read,
-                                          bytes_max - bytes_read));
-    if (num_bytes <= 0)
-      break;
-    bytes_read += num_bytes;
-  }
-
-  // We don't need the FD anymore.
-  child_fd_closer_.reset(NULL);
-  if (bytes_read < bytes_max)
+  uint32 server_data_len = 0;
+  if (!ReadData(child_fd_, sizeof(server_data_len),
+                reinterpret_cast<uint8*>(&server_data_len),
+                &remaining_time))
     return false;
-  host_port_pair_.set_port(port);
-  return true;
+  std::string server_data(server_data_len, '\0');
+  if (!ReadData(child_fd_, server_data_len,
+                reinterpret_cast<uint8*>(&server_data[0]),
+                &remaining_time))
+    return false;
+
+  return ParseServerData(server_data);
 }
 
 bool TestServer::CheckCATrusted() {
diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc
index a363755..47a3c61 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -80,12 +80,53 @@ void UnblockPipe(HANDLE handle, bool* unblocked) {
   // 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;
+  LOG(WARNING) << "Timeout reached; unblocking pipe";
   CHECK(WriteFile(handle, kUnblock, arraysize(kUnblock), &bytes_written,
                   NULL));
   CHECK_EQ(arraysize(kUnblock), bytes_written);
   *unblocked = true;
 }
 
+// Given a file handle, reads into |buffer| until |bytes_max| bytes
+// has been read or an error has been encountered.  Returns
+// true if the read was successful.
+bool ReadData(HANDLE read_fd, HANDLE write_fd,
+              DWORD bytes_max, uint8* buffer) {
+  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, write_fd, &unblocked),
+      TestTimeouts::action_max_timeout_ms());
+
+  DWORD bytes_read = 0;
+  while (bytes_read < bytes_max) {
+    DWORD num_bytes;
+    if (!ReadFile(read_fd, buffer + bytes_read, bytes_max - bytes_read,
+                  &num_bytes, NULL)) {
+      PLOG(ERROR) << "ReadFile failed";
+      return false;
+    }
+    if (num_bytes <= 0) {
+      LOG(ERROR) << "ReadFile returned invalid byte count: " << num_bytes;
+      return false;
+    }
+    bytes_read += num_bytes;
+  }
+
+  thread.Stop();
+  // If the timeout kicked in, abort.
+  if (unblocked) {
+    LOG(ERROR) << "Timeout exceeded for ReadData";
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 namespace net {
@@ -146,43 +187,27 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  base::Thread thread("test_server_watcher");
-  if (!thread.Start())
-    return false;
+  ScopedHandle read_fd(child_read_fd_.Take());
+  ScopedHandle write_fd(child_write_fd_.Take());
 
-  // 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());
-
-  // Try to read two bytes from the pipe indicating the ephemeral port number.
-  uint16 port;
-  uint8* buffer = reinterpret_cast<uint8*>(&port);
-  DWORD bytes_read = 0;
-  DWORD bytes_max = sizeof(port);
-  while (bytes_read < bytes_max) {
-    DWORD num_bytes;
-    if (!ReadFile(child_read_fd_, buffer + bytes_read, bytes_max - bytes_read,
-                  &num_bytes, NULL))
-      break;
-    if (num_bytes <= 0)
-      break;
-    bytes_read += num_bytes;
+  uint32 server_data_len = 0;
+  if (!ReadData(read_fd.Get(), write_fd.Get(), sizeof(server_data_len),
+                reinterpret_cast<uint8*>(&server_data_len))) {
+    LOG(ERROR) << "Could not read server_data_len";
+    return false;
   }
-  thread.Stop();
-  child_read_fd_.Close();
-  child_write_fd_.Close();
-
-  // If we hit the timeout, fail.
-  if (unblocked)
+  std::string server_data(server_data_len, '\0');
+  if (!ReadData(read_fd.Get(), write_fd.Get(), server_data_len,
+                reinterpret_cast<uint8*>(&server_data[0]))) {
+    LOG(ERROR) << "Could not read server_data of size " << server_data_len;
     return false;
+  }
 
-  // If not enough bytes were read, fail.
-  if (bytes_read < bytes_max)
+  if (!ParseServerData(server_data)) {
+    LOG(ERROR) << "Could not parse " << server_data;
     return false;
+  }
 
-  host_port_pair_.set_port(port);
   return true;
 }