Remove 1 second timeouts from HttpServerTests.

net/server/http_server_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/server/http_server.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 41 matching lines @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using net::test::IsOk;
 
 namespace net {
 
 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) {}
 
   int ConnectAndWait(const IPEndPoint& address) {
     AddressList addresses(address);
     NetLogSource source;
     socket_.reset(new TCPClientSocket(addresses, NULL, NULL, source));
 
     base::RunLoop run_loop;
     connect_result_ = socket_->Connect(base::Bind(&TestHttpClient::OnConnect,
                                                   base::Unretained(this),
                                                   run_loop.QuitClosure()));
     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_;
   }
 
   void Send(const std::string& data) {
     write_buffer_ =
         new DrainableIOBuffer(new StringIOBuffer(data), data.length());
     Write();
   }
 
   bool Read(std::string* message, int expected_bytes) {
@@ skipping 131 matching lines @@
     NOTREACHED();
   }
 
   void OnClose(int connection_id) override {
     DCHECK(connection_map_.find(connection_id) != connection_map_.end());
     connection_map_[connection_id] = false;
     if (connection_id == quit_on_close_connection_)
       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) {
     return requests_[request_index].first;
   }
 
   size_t num_requests() const { return requests_.size(); }
 
   int GetConnectionId(size_t request_index) {
     return requests_[request_index].second;
@@ skipping 33 matching lines @@
   }
 
   void OnWebSocketMessage(int connection_id, const std::string& data) override {
   }
 };
 
 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);
   ASSERT_EQ(0u, GetRequest(0).headers.size());
   ASSERT_TRUE(base::StartsWith(GetRequest(0).peer.ToString(), "127.0.0.1",
                                base::CompareCase::SENSITIVE));
 }
 
 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();
 }
 
 TEST_F(HttpServerTest, RequestWithHeaders) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   const char* const kHeaders[][3] = {
       {"Header", ": ", "1"},
       {"HeaderWithNoWhitespace", ":", "1"},
       {"HeaderWithWhitespace", "   :  \t   ", "1 1 1 \t  "},
       {"HeaderWithColon", ": ", "1:1"},
       {"EmptyHeader", ":", ""},
       {"EmptyHeaderWithWhitespace", ":  \t  ", ""},
       {"HeaderWithNonASCII", ":  ", "\xf7"},
   };
   std::string headers;
   for (size_t i = 0; i < arraysize(kHeaders); ++i) {
     headers +=
         std::string(kHeaders[i][0]) + kHeaders[i][1] + kHeaders[i][2] + "\r\n";
   }
 
   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) {
     std::string field = base::ToLowerASCII(std::string(kHeaders[i][0]));
     std::string value = kHeaders[i][2];
     ASSERT_EQ(1u, GetRequest(0).headers.count(field)) << field;
     ASSERT_EQ(value, GetRequest(0).headers[field]) << kHeaders[i][0];
   }
 }
 
 TEST_F(HttpServerTest, RequestWithDuplicateHeaders) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   const char* const kHeaders[][3] = {
       {"FirstHeader", ": ", "1"},
       {"DuplicateHeader", ": ", "2"},
       {"MiddleHeader", ": ", "3"},
       {"DuplicateHeader", ": ", "4"},
       {"LastHeader", ": ", "5"},
   };
   std::string headers;
   for (size_t i = 0; i < arraysize(kHeaders); ++i) {
     headers +=
         std::string(kHeaders[i][0]) + kHeaders[i][1] + kHeaders[i][2] + "\r\n";
   }
 
   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) {
     std::string field = base::ToLowerASCII(std::string(kHeaders[i][0]));
     std::string value = (field == "duplicateheader") ? "2,4" : kHeaders[i][2];
     ASSERT_EQ(1u, GetRequest(0).headers.count(field)) << field;
     ASSERT_EQ(value, GetRequest(0).headers[field]) << kHeaders[i][0];
   }
 }
 
@@ skipping 10 matching lines @@
       "EmptyHeader:",
       "EmptyHeaderWithWhitespace:  \t  ",
       "HeaderWithNonASCII:  \xf7",
   };
   std::string headers;
   for (size_t i = 0; i < arraysize(kHeaders); ++i) {
     headers += std::string(kHeaders[i]) + "\r\n";
   }
 
   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"));
   ASSERT_FALSE(GetRequest(0).HasHeaderValue("header", "bc"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithnowhitespace", "e"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithwhitespace", "f"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("duplicateheader", "g"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "h"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "i"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithcomma", "j"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("duplicateheader", "k"));
   ASSERT_FALSE(GetRequest(0).HasHeaderValue("emptyheader", "x"));
   ASSERT_FALSE(GetRequest(0).HasHeaderValue("emptyheaderwithwhitespace", "x"));
   ASSERT_TRUE(GetRequest(0).HasHeaderValue("headerwithnonascii", "\xf7"));
 }
 
 TEST_F(HttpServerTest, RequestWithBody) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   std::string body = "a" + std::string(1 << 10, 'b') + "c";
   client.Send(base::StringPrintf(
       "GET /test HTTP/1.1\r\n"
       "SomeHeader: 1\r\n"
       "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]);
   ASSERT_EQ('c', *body.rbegin());
 }
 
 TEST_F(WebSocketTest, RequestWebSocket) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send(
       "GET /test HTTP/1.1\r\n"
       "Upgrade: WebSocket\r\n"
       "Connection: SomethingElse, Upgrade\r\n"
       "Sec-WebSocket-Version: 8\r\n"
       "Sec-WebSocket-Key: key\r\n"
       "\r\n");
-  ASSERT_TRUE(RunUntilRequestsReceived(1));
+  RunUntilRequestsReceived(1);
 }
 
 TEST_F(WebSocketTest, RequestWebSocketTrailingJunk) {
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   client.Send(
       "GET /test HTTP/1.1\r\n"
       "Upgrade: WebSocket\r\n"
       "Connection: SomethingElse, Upgrade\r\n"
       "Sec-WebSocket-Version: 8\r\n"
       "Sec-WebSocket-Key: key\r\n"
       "\r\nHello? Anyone");
-  ASSERT_TRUE(RunUntilConnectionIdClosed(1));
+  RunUntilConnectionIdClosed(1);
   client.ExpectUsedThenDisconnectedWithNoData();
 }
 
 TEST_F(HttpServerTest, RequestWithTooLargeBody) {
   class TestURLFetcherDelegate : public URLFetcherDelegate {
    public:
     TestURLFetcherDelegate(const base::Closure& quit_loop_func)
         : quit_loop_func_(quit_loop_func) {}
     ~TestURLFetcherDelegate() override {}
 
@@ skipping 13 matching lines @@
       new TestURLRequestContextGetter(base::ThreadTaskRunnerHandle::Get()));
   std::unique_ptr<URLFetcher> fetcher = URLFetcher::Create(
       GURL(base::StringPrintf("http://127.0.0.1:%d/test",
                               server_address_.port())),
       URLFetcher::GET, &delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
   fetcher->SetRequestContext(request_context_getter.get());
   fetcher->AddExtraRequestHeader(
       base::StringPrintf("content-length:%d", 1 << 30));
   fetcher->Start();
 
-  ASSERT_TRUE(RunLoopWithTimeout(&run_loop));
+  run_loop.Run();
   ASSERT_EQ(0u, requests_.size());
 }
 
 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;
   ASSERT_TRUE(client.ReadResponse(&response));
   ASSERT_TRUE(base::StartsWith(response, "HTTP/1.1 200 OK",
                                base::CompareCase::SENSITIVE));
   ASSERT_TRUE(
       base::EndsWith(response, "Response!", base::CompareCase::SENSITIVE));
 }
 
 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");
 
   const std::string expected_response("Raw Data More DataThird Piece of Data");
   std::string response;
   ASSERT_TRUE(client.Read(&response, expected_response.length()));
   ASSERT_EQ(expected_response, response);
 }
 
 TEST_F(HttpServerTest, WrongProtocolRequest) {
   const char* const kBadProtocolRequests[] = {
       "GET /test HTTP/1.0\r\n\r\n",
       "GET /test foo\r\n\r\n",
       "GET /test \r\n\r\n",
   };
 
   for (size_t i = 0; i < arraysize(kBadProtocolRequests); ++i) {
     TestHttpClient client;
     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();
   }
 }
 
 class MockStreamSocket : public StreamSocket {
  public:
   MockStreamSocket()
       : connected_(true),
@@ skipping 114 matching lines @@
   // The idea behind this test is that requests with or without bodies should
   // not break parsing of the next request.
   TestHttpClient client;
   ASSERT_THAT(client.ConnectAndWait(server_address_), IsOk());
   std::string body = "body";
   client.Send(base::StringPrintf(
       "GET /test HTTP/1.1\r\n"
       "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);
   server_->Send200(client_connection_id, "Content for /test", "text/plain");
   std::string response1;
   ASSERT_TRUE(client.ReadResponse(&response1));
   ASSERT_TRUE(base::StartsWith(response1, "HTTP/1.1 200 OK",
                                base::CompareCase::SENSITIVE));
   ASSERT_TRUE(base::EndsWith(response1, "Content for /test",
                              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));
   server_->Send404(client_connection_id);
   std::string response2;
   ASSERT_TRUE(client.ReadResponse(&response2));
   ASSERT_TRUE(base::StartsWith(response2, "HTTP/1.1 404 Not Found",
                                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));
   server_->Send200(client_connection_id, "Content for /test3", "text/plain");
   std::string response3;
   ASSERT_TRUE(client.ReadResponse(&response3));
   ASSERT_TRUE(base::StartsWith(response3, "HTTP/1.1 200 OK",
                                base::CompareCase::SENSITIVE));
   ASSERT_TRUE(base::EndsWith(response3, "Content for /test3",
                              base::CompareCase::SENSITIVE));
 }
 
 class CloseOnConnectHttpServerTest : public HttpServerTest {
  public:
   void OnConnect(int connection_id) override {
     HttpServerTest::OnConnect(connection_id);
     connection_ids_.push_back(connection_id);
     server_->Close(connection_id);
   }
 
  protected:
   std::vector<int> connection_ids_;
 };
 
 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
 
 }  // namespace net