Remove SpawnedTestServer timeouts.

These timeouts are ancient. The test infrastructure has its own
timeouts, which should make these obsolete. Also, this may help with
https://crbug.com/869227, where starting the Python test server
times out.

Bug: 869227
Change-Id: I9ea4f3174bcaa86daf09b965c82921432d8d556a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869717
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707894}

net/test/spawned_test_server/local_test_server_posix.cc

@@ -19,7 +19,6 @@
 #include "base/process/process_iterator.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
-#include "base/test/test_timeouts.h"
 #include "net/test/python_utils.h"
 
 namespace {
@@ -59,13 +58,9 @@ class OrphanedTestServerFilter : public base::ProcessFilter {
 
 // 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_t* buffer,
-              base::TimeDelta* remaining_time) {
+// if the read was successful.
+bool ReadData(int fd, ssize_t bytes_max, uint8_t* buffer) {
   ssize_t bytes_read = 0;
-  base::TimeTicks previous_time = base::TimeTicks::Now();
   while (bytes_read < bytes_max) {
     struct pollfd poll_fds[1];
 
@@ -73,8 +68,8 @@ bool ReadData(int fd,
     poll_fds[0].events = POLLIN | POLLPRI;
     poll_fds[0].revents = 0;
 
-    int rv = HANDLE_EINTR(poll(poll_fds, 1,
-                               remaining_time->InMilliseconds()));
+    // Each test itself has its own timeout, so no need to use one here.
+    int rv = HANDLE_EINTR(poll(poll_fds, 1, -1));
     if (rv == 0) {
       LOG(ERROR) << "poll() timed out; bytes_read=" << bytes_read;
       return false;
@@ -84,12 +79,6 @@ bool ReadData(int fd,
       return false;
     }
 
-    base::TimeTicks current_time = base::TimeTicks::Now();
-    base::TimeDelta elapsed_time_cycle = current_time - previous_time;
-    DCHECK_GE(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)
@@ -163,18 +152,15 @@ bool LocalTestServer::LaunchPython(
 bool LocalTestServer::WaitToStart() {
   base::ScopedFD our_fd(child_fd_.release());
 
-  base::TimeDelta remaining_time = TestTimeouts::action_timeout();
-
   uint32_t server_data_len = 0;
   if (!ReadData(our_fd.get(), sizeof(server_data_len),
-                reinterpret_cast<uint8_t*>(&server_data_len),
-                &remaining_time)) {
+                reinterpret_cast<uint8_t*>(&server_data_len))) {
     LOG(ERROR) << "Could not read server_data_len";
     return false;
   }
   std::string server_data(server_data_len, '\0');
   if (!ReadData(our_fd.get(), server_data_len,
-                reinterpret_cast<uint8_t*>(&server_data[0]), &remaining_time)) {
+                reinterpret_cast<uint8_t*>(&server_data[0]))) {
     LOG(ERROR) << "Could not read server_data (" << server_data_len
                << " bytes)";
     return false;

net/test/spawned_test_server/local_test_server_win.cc

@@ -13,32 +13,14 @@
 #include "base/files/file_path.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
-#include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/test_timeouts.h"
-#include "base/threading/thread.h"
-#include "base/threading/thread_restrictions.h"
 #include "base/win/scoped_handle.h"
 #include "net/test/python_utils.h"
 
 namespace {
 
-// Writes |size| bytes to |handle| and sets |*unblocked| to true.
-// Used as a crude timeout mechanism by ReadData().
-void UnblockPipe(HANDLE handle, DWORD size, bool* unblocked) {
-  std::string unblock_data(size, '\0');
-  // Unblock the ReadFile in LocalTestServer::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 by writing "
-               << size << " bytes";
-  CHECK(WriteFile(handle, unblock_data.data(), size, &bytes_written, nullptr));
-  CHECK_EQ(size, 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.
@@ -46,16 +28,6 @@ bool ReadData(HANDLE read_fd,
               HANDLE write_fd,
               DWORD bytes_max,
               uint8_t* 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.task_runner()->PostDelayedTask(
-      FROM_HERE, base::BindOnce(UnblockPipe, write_fd, bytes_max, &unblocked),
-      TestTimeouts::action_max_timeout());
-
   DWORD bytes_read = 0;
   while (bytes_read < bytes_max) {
     DWORD num_bytes;
@@ -71,15 +43,6 @@ bool ReadData(HANDLE read_fd,
     bytes_read += num_bytes;
   }
 
-  base::ScopedAllowBaseSyncPrimitivesForTesting allow_thread_join;
-  thread.Stop();
-
-  // If the timeout kicked in, abort.
-  if (unblocked) {
-    LOG(ERROR) << "Timeout exceeded for ReadData";
-    return false;
-  }
-
   return true;
 }
 

net/test/spawned_test_server/remote_test_server_spawner_request.cc

@@ -12,10 +12,7 @@
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/waitable_event.h"
-#include "base/test/test_timeouts.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "base/time/time.h"
-#include "base/timer/timer.h"
 #include "build/build_config.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/io_buffer.h"
@@ -50,7 +47,6 @@ class RemoteTestServerSpawnerRequest::Core : public URLRequest::Delegate {
 
   void ReadResponse();
   void OnCommandCompleted(int net_error);
-  void OnTimeout();
 
   // Request results.
   int result_code_ = 0;
@@ -64,8 +60,6 @@ class RemoteTestServerSpawnerRequest::Core : public URLRequest::Delegate {
 
   scoped_refptr<IOBuffer> read_buffer_;
 
-  std::unique_ptr<base::OneShotTimer> timeout_timer_;
-
   THREAD_CHECKER(thread_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(Core);
@@ -102,10 +96,6 @@ void RemoteTestServerSpawnerRequest::Core::SendRequest(
                                           /*override=*/true);
   }
 
-  timeout_timer_ = std::make_unique<base::OneShotTimer>();
-  timeout_timer_->Start(FROM_HERE, TestTimeouts::action_max_timeout(),
-                        base::Bind(&Core::OnTimeout, base::Unretained(this)));
-
   request_->Start();
 }
 
@@ -124,13 +114,6 @@ bool RemoteTestServerSpawnerRequest::Core::WaitForCompletion(
   return result_code_ == OK;
 }
 
-void RemoteTestServerSpawnerRequest::Core::OnTimeout() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  int result = request_->CancelWithError(ERR_TIMED_OUT);
-  OnCommandCompleted(result);
-}
-
 void RemoteTestServerSpawnerRequest::Core::OnCommandCompleted(int net_error) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK_NE(ERR_IO_PENDING, net_error);
@@ -152,7 +135,6 @@ void RemoteTestServerSpawnerRequest::Core::OnCommandCompleted(int net_error) {
 
   request_.reset();
   context_.reset();
-  timeout_timer_.reset();
 
   event_.Signal();
 }