Revert 65465 - testserver.py listens on ephemeral ports by default.

Some SSL UI tests timed out again on 10.5/10.6 bots when landing this, despite the changes made. Different tests this time however.

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 spawns testserver.py to listen on an ephemeral port and reads the port value from the pipe. A fixed port can not be specified.

BUG=56814
TEST=try bots pass

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

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

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

4 files changed, 77 insertions, 76 deletions

diff --git a/net/test/test_server.cc b/net/test/test_server.cc
index 1e6ff64..4b426eb 100644
--- a/net/test/test_server.cc
+++ b/net/test/test_server.cc
@@ -40,6 +40,59 @@ const int kServerConnectionAttempts = 10;
 // Connection timeout in milliseconds for tests.
 const int kServerConnectionTimeoutMs = 1000;
 
+const char kTestServerShardFlag[] = "test-server-shard";
+
+int GetHTTPSPortBase(const TestServer::HTTPSOptions& options) {
+  if (options.request_client_certificate)
+    return 9543;
+
+  switch (options.server_certificate) {
+    case TestServer::HTTPSOptions::CERT_OK:
+      return 9443;
+    case TestServer::HTTPSOptions::CERT_MISMATCHED_NAME:
+      return 9643;
+    case TestServer::HTTPSOptions::CERT_EXPIRED:
+      // TODO(phajdan.jr): Some tests rely on this hardcoded value.
+      // Some uses of this are actually in .html/.js files.
+      return 9666;
+    default:
+      NOTREACHED();
+  }
+  return -1;
+}
+
+int GetPortBase(TestServer::Type type,
+                const TestServer::HTTPSOptions& options) {
+  switch (type) {
+    case TestServer::TYPE_FTP:
+      return 3117;
+    case TestServer::TYPE_HTTP:
+      return 1337;
+    case TestServer::TYPE_HTTPS:
+      return GetHTTPSPortBase(options);
+    default:
+      NOTREACHED();
+  }
+  return -1;
+}
+
+int GetPort(TestServer::Type type,
+            const TestServer::HTTPSOptions& options) {
+  int port = GetPortBase(type, options);
+  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(TestServer::Type type,
                         const TestServer::HTTPSOptions& options) {
   if (type == TestServer::TYPE_HTTPS &&
@@ -85,16 +138,13 @@ FilePath TestServer::HTTPSOptions::GetCertificateFile() const {
 }
 
 TestServer::TestServer(Type type, const FilePath& document_root)
-    : type_(type),
-      started_(false) {
+    : type_(type) {
   Init(document_root);
 }
 
 TestServer::TestServer(const HTTPSOptions& https_options,
                        const FilePath& document_root)
-    : https_options_(https_options),
-      type_(TYPE_HTTPS),
-      started_(false) {
+    : https_options_(https_options), type_(TYPE_HTTPS) {
   Init(document_root);
 }
 
@@ -106,11 +156,8 @@ TestServer::~TestServer() {
 }
 
 void TestServer::Init(const FilePath& document_root) {
-  // At this point, the port that the testserver will listen on is unknown.
-  // The testserver will listen on an ephemeral port, and write the port
-  // number out over a pipe that this TestServer object will read from. Once
-  // that is complete, the host_port_pair_ will contain the actual port.
-  host_port_pair_ = HostPortPair(GetHostname(type_, https_options_), 0);
+  host_port_pair_ = HostPortPair(GetHostname(type_, https_options_),
+                                 GetPort(type_, https_options_));
   process_handle_ = base::kNullProcessHandle;
 
   FilePath src_dir;
@@ -155,7 +202,6 @@ bool TestServer::Start() {
     return false;
   }
 
-  started_ = true;
   return true;
 }
 
@@ -163,8 +209,6 @@ 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)
@@ -180,11 +224,6 @@ 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 e0cadab..8affc12 100644
--- a/net/test/test_server.h
+++ b/net/test/test_server.h
@@ -113,7 +113,7 @@ class TestServer {
   bool Stop();
 
   const FilePath& document_root() const { return document_root_; }
-  const HostPortPair& host_port_pair() const;
+  const HostPortPair& host_port_pair() const { return host_port_pair_; }
   std::string GetScheme() const;
   bool GetAddressList(AddressList* address_list) const WARN_UNUSED_RESULT;
 
@@ -189,9 +189,6 @@ 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 9c0210b..707eb93 100644
--- a/net/test/test_server_posix.cc
+++ b/net/test/test_server_posix.cc
@@ -98,45 +98,24 @@ 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);
-  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;
-    }
+  struct pollfd poll_fds[1];
+
+  poll_fds[0].fd = child_fd_;
+  poll_fds[0].events = POLLIN | POLLPRI;
+  poll_fds[0].revents = 0;
 
-    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;
+  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;
   }
 
+  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);
-  if (bytes_read < bytes_max)
-    return false;
-  host_port_pair_.set_port(port);
-  return true;
+  return n > 0;
 }
 
 bool TestServer::CheckCATrusted() {
diff --git a/net/test/test_server_win.cc b/net/test/test_server_win.cc
index 64437cd..c6e10d5 100644
--- a/net/test/test_server_win.cc
+++ b/net/test/test_server_win.cc
@@ -156,20 +156,11 @@ bool TestServer::WaitToStart() {
       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;
-  }
+  char buf[8];
+  DWORD bytes_read;
+  BOOL result = ReadFile(child_read_fd_.Get(), buf, sizeof(buf), &bytes_read,
+                         NULL);
+
   thread.Stop();
   child_read_fd_.Close();
   child_write_fd_.Close();
@@ -178,12 +169,7 @@ bool TestServer::WaitToStart() {
   if (unblocked)
     return false;
 
-  // If not enough bytes were read, fail.
-  if (bytes_read < bytes_max)
-    return false;
-
-  host_port_pair_.set_port(port);
-  return true;
+  return result && bytes_read > 0;
 }
 
 bool TestServer::CheckCATrusted() {