Revert 64070 - Revert 64065 - testserver.py listens on ephemeral ports by default.

If --port is specified on the command line, testserver.py will listen on that port, otherwise it will listen on an ephemeral port. If --startup_pipe is specified, the port number is written to the pipe as a 2 byte unsigned int in host order.

TestServer by default spawns testserver.py to listen on an ephemeral port and reads the port value from the pipe. If necessary, the port can still be fixed using TestServer::ForcePort, but that will hopefully get deprecated quickly.

BUG=56814
TEST=try bots pass

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

TBR=cbentzel@chromium.org
Review URL: http://codereview.chromium.org/4165004

TBR=cbentzel@chromium.org
Review URL: http://codereview.chromium.org/4146008

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

4 files changed, 58 insertions, 60 deletions

diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 0b1cd085..99cb4a6 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -38,46 +38,6 @@ const int kServerConnectionAttempts = 10;
 // Connection timeout in milliseconds for tests.
 const int kServerConnectionTimeoutMs = 1000;
 
-const char kTestServerShardFlag[] = "test-server-shard";
-
-int GetPortBase(net::TestServer::Type type) {
-  switch (type) {
-    case net::TestServer::TYPE_FTP:
-      return 3117;
-    case net::TestServer::TYPE_HTTP:
-      return 1337;
-    case net::TestServer::TYPE_HTTPS:
-      return 9443;
-    case net::TestServer::TYPE_HTTPS_CLIENT_AUTH:
-      return 9543;
-    case net::TestServer::TYPE_HTTPS_EXPIRED_CERTIFICATE:
-      // TODO(phajdan.jr): Some tests rely on this hardcoded value.
-      // Some uses of this are actually in .html/.js files.
-      return 9666;
-    case net::TestServer::TYPE_HTTPS_MISMATCHED_HOSTNAME:
-      return 9643;
-    default:
-      NOTREACHED();
-  }
-  return -1;
-}
-
-int GetPort(net::TestServer::Type type) {
-  int port = GetPortBase(type);
-  if (CommandLine::ForCurrentProcess()->HasSwitch(kTestServerShardFlag)) {
-    std::string shard_str(CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-                              kTestServerShardFlag));
-    int shard = -1;
-    if (base::StringToInt(shard_str, &shard)) {
-      port += shard;
-    } else {
-      LOG(FATAL) << "Got invalid " << kTestServerShardFlag << " flag value. "
-                 << "An integer is expected.";
-    }
-  }
-  return port;
-}
-
 std::string GetHostname(net::TestServer::Type type) {
   if (type == net::TestServer::TYPE_HTTPS_MISMATCHED_HOSTNAME) {
     // Return a different hostname string that resolves to the same hostname.
@@ -96,9 +56,10 @@ void SetMacTestCertificate(X509Certificate* cert);
 #endif
 
 TestServer::TestServer(Type type, const FilePath& document_root)
-    : host_port_pair_(GetHostname(type), GetPort(type)),
+    : host_port_pair_(GetHostname(type), 0),
       process_handle_(base::kNullProcessHandle),
-      type_(type) {
+      type_(type),
+      started_(false) {
   FilePath src_dir;
   PathService::Get(base::DIR_SOURCE_ROOT, &src_dir);
 
@@ -148,6 +109,7 @@ bool TestServer::Start() {
     return false;
   }
 
+  started_ = true;
   return true;
 }
 
@@ -155,6 +117,8 @@ bool TestServer::Stop() {
   if (!process_handle_)
     return true;
 
+  started_ = false;
+
   // First check if the process has already terminated.
   bool ret = base::WaitForSingleProcess(process_handle_, 0);
   if (!ret)
@@ -170,6 +134,11 @@ bool TestServer::Stop() {
   return ret;
 }
 
+const HostPortPair& TestServer::host_port_pair() const {
+  DCHECK(started_);
+  return host_port_pair_;
+}
+
 std::string TestServer::GetScheme() const {
   switch (type_) {
     case TYPE_FTP:
diff --git a/net/test/test_server.h b/net/test/test_server.h
index 4e68fd9..2ee801e 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -53,7 +53,7 @@ class TestServer {
   bool Stop();
 
   const FilePath& document_root() const { return document_root_; }
-  const HostPortPair& host_port_pair() const { return host_port_pair_; }
+  const HostPortPair& host_port_pair() const;
   std::string GetScheme() const;
   bool GetAddressList(AddressList* address_list) const WARN_UNUSED_RESULT;
 
@@ -121,6 +121,9 @@ class TestServer {
 
   Type type_;
 
+  // Has the server been started?
+  bool started_;
+
   DISALLOW_COPY_AND_ASSIGN(TestServer);
 };
 
diff --git a/net/test/test_server_posix.cc b/net/test/test_server_posix.cc
index 1456ac8..17b2631 100644
--- a/net/test/test_server_posix.cc
+++ b/net/test/test_server_posix.cc
@@ -110,24 +110,37 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  struct pollfd poll_fds[1];
-
-  poll_fds[0].fd = child_fd_;
-  poll_fds[0].events = POLLIN | POLLPRI;
-  poll_fds[0].revents = 0;
+  uint16 port;
+  uint8* buffer = reinterpret_cast<uint8*>(&port);
+  ssize_t bytes_read = 0;
+  ssize_t bytes_max = sizeof(port);
+  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,
+                               TestTimeouts::action_max_timeout_ms()));
+    if (rv != 1) {
+      LOG(ERROR) << "Failed to poll for the child file descriptor.";
+      return false;
+    }
 
-  int rv = HANDLE_EINTR(poll(poll_fds, 1,
-                             TestTimeouts::action_max_timeout_ms()));
-  if (rv != 1) {
-    LOG(ERROR) << "Failed to poll for the child file descriptor.";
-    return false;
+    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;
   }
 
-  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;
+  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 a8b3678..cfc7178 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -149,11 +149,24 @@ bool TestServer::LaunchPython(const FilePath& testserver_path) {
 }
 
 bool TestServer::WaitToStart() {
-  char buf[8];
-  DWORD bytes_read;
-  BOOL result = ReadFile(child_fd_, buf, sizeof(buf), &bytes_read, NULL);
+  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_fd_, buffer + bytes_read, bytes_max - bytes_read,
+                  &num_bytes, NULL))
+      break;
+    if (num_bytes <= 0)
+      break;
+    bytes_read += num_bytes;
+  }
   child_fd_.Close();
-  return result && bytes_read > 0;
+  if (bytes_read < bytes_max)
+    return false;
+  host_port_pair_.set_port(port);
+  return true;
 }
 
 bool TestServer::CheckCATrusted() {