Remove 1 second timeouts from HttpServerTests.

net/server/http_server_unittest.cc

Index: net/server/http_server_unittest.cc
diff --git a/net/server/http_server_unittest.cc b/net/server/http_server_unittest.cc
index 6e3198fe0155582b7e1aaa59500962716411121c..5245485bcc3cbe840e7151190bb99f03f7c0246d 100644
--- a/net/server/http_server_unittest.cc
+++ b/net/server/http_server_unittest.cc
@@ -59,26 +59,6 @@ namespace {
 
 const int kMaxExpectedResponseLength = 2048;
 
-void SetTimedOutAndQuitLoop(const base::WeakPtr<bool> timed_out,
-                            const base::Closure& quit_loop_func) {
-  if (timed_out) {
-    *timed_out = true;
-    quit_loop_func.Run();
-  }
-}
-
-bool RunLoopWithTimeout(base::RunLoop* run_loop) {
-  bool timed_out = false;
-  base::WeakPtrFactory<bool> timed_out_weak_factory(&timed_out);
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&SetTimedOutAndQuitLoop, timed_out_weak_factory.GetWeakPtr(),
-                 run_loop->QuitClosure()),
-      base::TimeDelta::FromSeconds(1));
-  run_loop->Run();
-  return !timed_out;
-}
-
 class TestHttpClient {
  public:
   TestHttpClient() : connect_result_(OK) {}
@@ -95,8 +75,7 @@ class TestHttpClient {
     if (connect_result_ != OK && connect_result_ != ERR_IO_PENDING)
       return connect_result_;
 
-    if (!RunLoopWithTimeout(&run_loop))
-      return ERR_TIMED_OUT;
+    run_loop.Run();
     return connect_result_;
   }
 
@@ -248,31 +227,29 @@ class HttpServerTest : public testing::Test,
       run_loop_quit_func_.Run();
   }
 
-  bool RunUntilRequestsReceived(size_t count) {
+  void RunUntilRequestsReceived(size_t count) {
     quit_after_request_count_ = count;
     if (requests_.size() == count)
-      return true;
+      return;
 
     base::RunLoop run_loop;
     run_loop_quit_func_ = run_loop.QuitClosure();
-    bool success = RunLoopWithTimeout(&run_loop);
+    run_loop.Run();
     run_loop_quit_func_.Reset();
-    return success;
   }
 
-  bool RunUntilConnectionIdClosed(int connection_id) {
+  void RunUntilConnectionIdClosed(int connection_id) {
     quit_on_close_connection_ = connection_id;
     auto iter = connection_map_.find(connection_id);
     if (iter != connection_map_.end() && !iter->second) {
       // Already disconnected.
-      return true;
+      return;
     }
 
     base::RunLoop run_loop;
     run_loop_quit_func_ = run_loop.QuitClosure();
-    bool success = RunLoopWithTimeout(&run_loop);
+    run_loop.Run();
     run_loop_quit_func_.Reset();
-    return success;
   }
 
   HttpServerRequestInfo GetRequest(size_t request_index) {
@@ -326,7 +303,7 @@ TEST_F(HttpServerTest, Request) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET /test HTTP/1.1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ("GET", GetRequest(0).method);
   ASSERT_EQ("/test", GetRequest(0).path);
   ASSERT_EQ("", GetRequest(0).data);
@@ -339,7 +316,7 @@ TEST_F(HttpServerTest, RequestBrokenTermination) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET /test HTTP/1.1\r\n\r)");
-  ASSERT_TRUE(RunUntilConnectionIdClosed(1));
+  RunUntilConnectionIdClosed(1);
   EXPECT_EQ(0u, num_requests());
   client.ExpectUsedThenDisconnectedWithNoData();
 }
@@ -363,7 +340,7 @@ TEST_F(HttpServerTest, RequestWithHeaders) {
   }
 
   client.Send("GET /test HTTP/1.1\r\n" + headers + "\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ("", GetRequest(0).data);
 
   for (size_t i = 0; i < arraysize(kHeaders); ++i) {
@@ -391,7 +368,7 @@ TEST_F(HttpServerTest, RequestWithDuplicateHeaders) {
   }
 
   client.Send("GET /test HTTP/1.1\r\n" + headers + "\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ("", GetRequest(0).data);
 
   for (size_t i = 0; i < arraysize(kHeaders); ++i) {
@@ -422,7 +399,7 @@ TEST_F(HttpServerTest, HasHeaderValueTest) {
   }
 
   client.Send("GET /test HTTP/1.1\r\n" + headers + "\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ("", GetRequest(0).data);
 
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("header", "abcd"));
@@ -449,7 +426,7 @@ TEST_F(HttpServerTest, RequestWithBody) {
       "Content-Length: %" PRIuS "\r\n\r\n%s",
       body.length(),
       body.c_str()));
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ(2u, GetRequest(0).headers.size());
   ASSERT_EQ(body.length(), GetRequest(0).data.length());
   ASSERT_EQ('a', body[0]);
@@ -466,7 +443,7 @@ TEST_F(WebSocketTest, RequestWebSocket) {
       "Sec-WebSocket-Version: 8\r\n"
       "Sec-WebSocket-Key: key\r\n"
       "\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
 }
 
 TEST_F(WebSocketTest, RequestWebSocketTrailingJunk) {
@@ -479,7 +456,7 @@ TEST_F(WebSocketTest, RequestWebSocketTrailingJunk) {
       "Sec-WebSocket-Version: 8\r\n"
       "Sec-WebSocket-Key: key\r\n"
       "\r\nHello? Anyone");
-  ASSERT_TRUE(RunUntilConnectionIdClosed(1));
+  RunUntilConnectionIdClosed(1);
   client.ExpectUsedThenDisconnectedWithNoData();
 }
 
@@ -513,7 +490,7 @@ TEST_F(HttpServerTest, RequestWithTooLargeBody) {
       base::StringPrintf("content-length:%d", 1 << 30));
   fetcher->Start();
 
-  ASSERT_TRUE(RunLoopWithTimeout(&run_loop));
+  run_loop.Run();
   ASSERT_EQ(0u, requests_.size());
 }
 
@@ -521,7 +498,7 @@ TEST_F(HttpServerTest, Send200) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET /test HTTP/1.1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   server_->Send200(GetConnectionId(0), "Response!", "text/plain");
 
   std::string response;
@@ -536,7 +513,7 @@ TEST_F(HttpServerTest, SendRaw) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET /test HTTP/1.1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   server_->SendRaw(GetConnectionId(0), "Raw Data ");
   server_->SendRaw(GetConnectionId(0), "More Data");
   server_->SendRaw(GetConnectionId(0), "Third Piece of Data");
@@ -559,13 +536,12 @@ TEST_F(HttpServerTest, WrongProtocolRequest) {
     ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
 
     client.Send(kBadProtocolRequests[i]);
-    ASSERT_FALSE(RunUntilRequestsReceived(1));
+    client.ExpectUsedThenDisconnectedWithNoData();
 
     // Assert that the delegate was updated properly.
     ASSERT_EQ(1u, connection_map().size());
     ASSERT_FALSE(connection_map().begin()->second);
-
-    client.ExpectUsedThenDisconnectedWithNoData();
+    EXPECT_EQ(0ul, requests_.size());
 
     // Reset the state of the connection map.
     connection_map().clear();
@@ -700,7 +676,7 @@ TEST_F(HttpServerTest, MultipleRequestsOnSameConnection) {
       "Content-Length: %" PRIuS "\r\n\r\n%s",
       body.length(),
       body.c_str()));
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
   ASSERT_EQ(body, GetRequest(0).data);
 
   int client_connection_id = GetConnectionId(0);
@@ -713,7 +689,7 @@ TEST_F(HttpServerTest, MultipleRequestsOnSameConnection) {
                              base::CompareCase::SENSITIVE));
 
   client.Send("GET /test2 HTTP/1.1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(2));
+  RunUntilRequestsReceived(2);
   ASSERT_EQ("/test2", GetRequest(1).path);
 
   ASSERT_EQ(client_connection_id, GetConnectionId(1));
@@ -724,7 +700,7 @@ TEST_F(HttpServerTest, MultipleRequestsOnSameConnection) {
                                base::CompareCase::SENSITIVE));
 
   client.Send("GET /test3 HTTP/1.1\r\n\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(3));
+  RunUntilRequestsReceived(3);
   ASSERT_EQ("/test3", GetRequest(2).path);
 
   ASSERT_EQ(client_connection_id, GetConnectionId(2));
@@ -753,9 +729,17 @@ TEST_F(CloseOnConnectHttpServerTest, ServerImmediatelyClosesConnection) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send("GET / HTTP/1.1\r\n\r\n");

[mmenke, 2017/06/13 17:29:26]

The fact that this always succeeds seems a bit weird...

-  ASSERT_FALSE(RunUntilRequestsReceived(1));
-  ASSERT_EQ(1ul, connection_ids_.size());
-  ASSERT_EQ(0ul, requests_.size());
+
+  // The server should close the socket without responding.
+  client.ExpectUsedThenDisconnectedWithNoData();
+
+  // Run any tasks the TestServer posted.
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1ul, connection_ids_.size());
+  // OnHttpRequest() should never have been called, since the connection was
+  // closed without reading from it.
+  EXPECT_EQ(0ul, requests_.size());
 }
 
 }  // namespace