Revert 66879 - 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

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

TBR=akalin@chromium.org
Review URL: http://codereview.chromium.org/5177008

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

5 files changed, 66 insertions, 134 deletions

diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 1fd20c9..0e8c461 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -17,14 +17,11 @@
 #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"
@@ -281,11 +278,6 @@ 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")));
 
@@ -381,28 +373,4 @@ 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 5c11038..f819365 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -146,10 +146,6 @@ 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 74dbcd4..9c0210b 100644
--- a/net/test/test_server_posix.cc
+++ b/net/test/test_server_posix.cc
@@ -53,42 +53,6 @@ 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 {
@@ -134,22 +98,45 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  file_util::ScopedFD child_fd_closer(child_fd_closer_.release());
-
+  uint16 port;
+  uint8* buffer = reinterpret_cast<uint8*>(&port);
+  ssize_t bytes_read = 0;
+  ssize_t bytes_max = sizeof(port);
   base::TimeDelta remaining_time = base::TimeDelta::FromMilliseconds(
       TestTimeouts::action_max_timeout_ms());
-  uint32 server_data_len = 0;
-  if (!ReadData(child_fd_, sizeof(server_data_len),
-                reinterpret_cast<uint8*>(&server_data_len),
-                &remaining_time))
-    return false;
-  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;
+  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;
+    }
 
-  return ParseServerData(server_data);
+    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)
+    return false;
+  host_port_pair_.set_port(port);
+  return true;
 }
 
 bool TestServer::CheckCATrusted() {
diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc
index 0fa7003..64437cd 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -86,26 +86,6 @@ void UnblockPipe(HANDLE handle, bool* unblocked) {
   *unblocked = true;
 }
 
-// Given a file handle, reads into |buffer| until |bytes_max| bytes
-// has been read or an error has been encountered (which includes
-// |unblocked| being set to true; see UnblockPipe() above).  Returns
-// true if the read was successful.
-bool ReadData(bool* unblocked, HANDLE fd, DWORD bytes_max, uint8* buffer) {
-  DWORD bytes_read = 0;
-  while (bytes_read < bytes_max) {
-    DWORD num_bytes;
-    if (!ReadFile(fd, buffer + bytes_read, bytes_max - bytes_read,
-                  &num_bytes, NULL))
-      return false;
-    if (num_bytes <= 0)
-      return false;
-    if (*unblocked)
-      return false;
-    bytes_read += num_bytes;
-  }
-  return true;
-}
-
 }  // namespace
 
 namespace net {
@@ -166,9 +146,6 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  ScopedHandle read_fd(child_read_fd_.Take());
-  ScopedHandle write_fd(child_write_fd_.Take());
-
   base::Thread thread("test_server_watcher");
   if (!thread.Start())
     return false;
@@ -176,19 +153,37 @@ bool TestServer::WaitToStart() {
   // Prepare a timeout in case the server fails to start.
   bool unblocked = false;
   thread.message_loop()->PostDelayedTask(FROM_HERE,
-      NewRunnableFunction(UnblockPipe, write_fd.Get(), &unblocked),
+      NewRunnableFunction(UnblockPipe, child_write_fd_.Get(), &unblocked),
       TestTimeouts::action_max_timeout_ms());
 
-  uint32 server_data_len = 0;
-  if (!ReadData(&unblocked, read_fd.Get(), sizeof(server_data_len),
-                reinterpret_cast<uint8*>(&server_data_len)))
+  // 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;
+  }
+  thread.Stop();
+  child_read_fd_.Close();
+  child_write_fd_.Close();
+
+  // If we hit the timeout, fail.
+  if (unblocked)
     return false;
-  std::string server_data(server_data_len, '\0');
-  if (!ReadData(&unblocked, read_fd.Get(), server_data_len,
-                reinterpret_cast<uint8*>(&server_data[0])))
+
+  // If not enough bytes were read, fail.
+  if (bytes_read < bytes_max)
     return false;
 
-  return ParseServerData(server_data);
+  host_port_pair_.set_port(port);
+  return true;
 }
 
 bool TestServer::CheckCATrusted() {
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index b69eb2f..2551139 100755
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -27,13 +27,6 @@ import time
 import urlparse
 import warnings
 
-# If we use simplejson always, we get some warnings when we run under
-# 2.6.
-if sys.version_info < (2, 6):
-  import simplejson as json
-else:
-  import json
-
 # Ignore deprecation warnings, they make our output more cluttered.
 warnings.filterwarnings("ignore", category=DeprecationWarning)
 
@@ -1328,22 +1321,15 @@ def main(options, args):
   # Notify the parent that we've started. (BaseServer subclasses
   # bind their sockets on construction.)
   if options.startup_pipe is not None:
-    server_data = {
-      'port': listen_port
-    }
-    server_data_json = json.dumps(server_data)
-    debug('sending server_data: %s' % server_data_json)
-    server_data_len = len(server_data_json)
     if sys.platform == 'win32':
       fd = msvcrt.open_osfhandle(options.startup_pipe, 0)
     else:
       fd = options.startup_pipe
     startup_pipe = os.fdopen(fd, "w")
-    # First write the data length as an unsigned 4-byte value.  This
-    # is _not_ using network byte ordering since the other end of the
-    # pipe is on the same machine.
-    startup_pipe.write(struct.pack('=L', server_data_len))
-    startup_pipe.write(server_data_json)
+    # Write the listening port as a 2 byte value. This is _not_ using
+    # network byte ordering since the other end of the pipe is on the same
+    # machine.
+    startup_pipe.write(struct.pack('@H', listen_port))
     startup_pipe.close()
 
   try: