Integration tests for HTTP pipelining.

net/http/http_pipelined_network_transaction_unittest.cc

Index: net/http/http_pipelined_network_transaction_unittest.cc
diff --git a/net/http/http_pipelined_network_transaction_unittest.cc b/net/http/http_pipelined_network_transaction_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..009699da211578184f0788de8d3c26bab94ec821
--- /dev/null
+++ b/net/http/http_pipelined_network_transaction_unittest.cc
@@ -0,0 +1,463 @@
+// Copyright (c) 2011 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 <string>
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_vector.h"
+#include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
+#include "net/base/address_list.h"
+#include "net/base/host_cache.h"
+#include "net/base/io_buffer.h"
+#include "net/base/mock_host_resolver.h"
+#include "net/base/net_errors.h"
+#include "net/base/net_util.h"
+#include "net/base/request_priority.h"
+#include "net/base/ssl_config_service_defaults.h"
+#include "net/http/http_auth_handler_mock.h"
+#include "net/http/http_network_session.h"
+#include "net/http/http_network_transaction.h"
+#include "net/http/http_request_info.h"
+#include "net/http/http_server_properties_impl.h"
+#include "net/proxy/proxy_service.h"
+#include "net/socket/client_socket_handle.h"
+#include "net/socket/client_socket_pool_histograms.h"
+#include "net/socket/socket_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::StrEq;
+
+namespace net {
+

[willchan no longer on Chromium, 2011/11/08 22:44:54]

If nothing needs to be friended or otherwise externally referenced, can you put
all these tests within the net::(anonymous) namespace?

[James Simonsen, 2011/11/09 05:24:41]

Done.

+class DummySocketParams : public base::RefCounted<DummySocketParams> {

[mmenke, 2011/11/08 18:12:41]

Come to think of it, this isn't needed, either.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+ private:
+  friend class base::RefCounted<DummySocketParams>;
+};
+
+REGISTER_SOCKET_PARAMS_FOR_POOL(MockTransportClientSocketPool,
+                                DummySocketParams);
+
+class HttpPipelinedNetworkTransactionTest : public testing::Test {
+ public:
+  HttpPipelinedNetworkTransactionTest()
+      : histograms_("a"),
+        pool_(1, 1, &histograms_, &factory_) {
+  }
+
+  virtual void SetUp() OVERRIDE {
+    HttpStreamFactory::set_http_pipelining_enabled(true);

[mmenke, 2011/11/08 18:12:41]

This should be set to false again (Or restored to its original value) in
TearDown() to avoid affecting other tests, since all tests run in the same
process.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+  }
+
+  virtual void TearDown() OVERRIDE {
+    MessageLoop::current()->RunAllPending();
+  }
+
+  void Initialize() {
+    proxy_service_.reset(ProxyService::CreateDirect());
+    mock_resolver_.reset(new MockHostResolver);

[mmenke, 2011/11/08 18:12:41]

nit:  You don't need to use a scoped_ptr for either this or
HttpServerPropertiesImpl.  Not a huge simplification, I suppose.

[James Simonsen, 2011/11/09 05:24:41]

I may be misunderstanding you... If I remove the scoped_ptrs and just pass the
raw pointer into session_params and forget about it, then valgrind complains.
I'd prefer to use the scoped_ptr than manually delete.

[mmenke, 2011/11/09 16:10:58]

HttpPipelinedNetworkTransactionTest doesn't need pointers for them at all.  You
can make them non-pointer class members, since they don't need to be created by
funky static constructors.  Doesn't really matter.

+    mock_resolver_->rules()->AddRule("localhost", "127.0.0.1");

[mmenke, 2011/11/08 18:12:41]

nit:  Not needed.  By default, the MockHostResolver maps everything to
127.0.0.1.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+    auth_handler_factory_.reset(new HttpAuthHandlerMock::Factory());
+    http_server_properties_.reset(new HttpServerPropertiesImpl);
+
+    HttpNetworkSession::Params session_params;
+    session_params.client_socket_factory = &factory_;
+    session_params.proxy_service = proxy_service_.get();
+    session_params.host_resolver = mock_resolver_.get();
+    session_params.ssl_config_service = new SSLConfigServiceDefaults;

[mmenke, 2011/11/08 18:12:41]

|ssl_config_| should be a scoped_refptr, and you should have it refer to your
shiny new SSLConfigServiceDefaults.  While HttpNetworkSession will actually
maintain a reference to it, that behavior isn't documented, so I think it's best
to do what the other tests do, and not to rely on it.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+    session_params.http_auth_handler_factory = auth_handler_factory_.get();
+    session_params.http_server_properties = http_server_properties_.get();
+    session_ = new HttpNetworkSession(session_params);
+  }
+
+  void AddExpectedConnection(MockRead* reads, size_t reads_count,
+                             MockWrite* writes, size_t writes_count) {
+    DeterministicSocketData* data = new DeterministicSocketData(
+        reads, reads_count, writes, writes_count);
+    data->set_connect_data(MockConnect(false, 0));
+    if (reads_count || writes_count) {
+      data->StopAfter(reads_count + writes_count);
+    }
+    factory_.AddSocketDataProvider(data);
+    data_vector_.push_back(data);
+  }
+
+  const HttpRequestInfo* GetRequestInfo(const char* filename) {
+    std::string url = StringPrintf("http://localhost/%s", filename);
+    HttpRequestInfo* request_info = new HttpRequestInfo;
+    request_info->url = GURL(url);
+    request_info->method = "GET";
+    request_info_vector_.push_back(request_info);
+    return request_info;
+  }
+
+  void ExpectResponse(const std::string& expected,
+                      HttpNetworkTransaction& transaction) {
+    scoped_refptr<IOBuffer> buffer(new IOBuffer(expected.size()));
+    EXPECT_EQ(static_cast<int>(expected.size()),
+              transaction.Read(buffer.get(), expected.size(), &callback_));
+    std::string actual(buffer->data(), expected.size());
+    EXPECT_THAT(actual, StrEq(expected));
+    EXPECT_EQ(OK, transaction.Read(buffer.get(), expected.size(), &callback_));
+  }
+
+  void CompleteTwoRequests() {
+    HttpNetworkTransaction one_transaction(session_.get());
+    TestOldCompletionCallback one_callback;
+    EXPECT_EQ(ERR_IO_PENDING,
+              one_transaction.Start(GetRequestInfo("one.html"), &one_callback,
+                                    BoundNetLog()));
+    EXPECT_EQ(OK, one_callback.WaitForResult());
+
+    HttpNetworkTransaction two_transaction(session_.get());
+    TestOldCompletionCallback two_callback;
+    EXPECT_EQ(ERR_IO_PENDING,
+              two_transaction.Start(GetRequestInfo("two.html"), &two_callback,
+                                    BoundNetLog()));
+
+    ExpectResponse("one.html", one_transaction);
+    EXPECT_EQ(OK, two_callback.WaitForResult());
+    ExpectResponse("two.html", two_transaction);
+  }
+
+  DeterministicMockClientSocketFactory factory_;
+  ClientSocketPoolHistograms histograms_;
+  MockTransportClientSocketPool pool_;
+  std::vector<scoped_refptr<DeterministicSocketData> > data_vector_;
+
+  scoped_ptr<ProxyService> proxy_service_;
+  scoped_ptr<MockHostResolver> mock_resolver_;
+  scoped_ptr<HttpAuthHandlerMock::Factory> auth_handler_factory_;
+  scoped_ptr<HttpServerPropertiesImpl> http_server_properties_;
+  scoped_refptr<HttpNetworkSession> session_;
+
+  SSLConfig ssl_config_;

[mmenke, 2011/11/08 18:12:41]

Once you use this (As per comment above), you should put this above |session_|,
so it gets destroyed after the |session_|.  With refptrs, not strictly
necessary, but on the off chance that ever changes, just seems safer.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+  ProxyInfo proxy_info_;

[mmenke, 2011/11/08 18:12:41]

This is not used.

[James Simonsen, 2011/11/09 05:24:41]

Done.

+  TestOldCompletionCallback callback_;
+  ScopedVector<HttpRequestInfo> request_info_vector_;
+};
+
+TEST_F(HttpPipelinedNetworkTransactionTest, OneRequest) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /test.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 9\r\n\r\n"),
+    MockRead(false, 3, "test.html"),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  HttpNetworkTransaction transaction(session_.get());
+  EXPECT_EQ(ERR_IO_PENDING,
+            transaction.Start(GetRequestInfo("test.html"), &callback_,
+                              BoundNetLog()));
+  EXPECT_EQ(OK, callback_.WaitForResult());
+  ExpectResponse("test.html", transaction);
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, ReusePipeline) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 4, "GET /two.html HTTP/1.1\r\n"

[mmenke, 2011/11/08 18:12:41]

I think it's a little odd that we always send the second "pipelined" request
after reading the results from earlier requests.  Not sure if there's an easy
fix for this.

[James Simonsen, 2011/11/09 05:24:41]

Yeah, it makes the code uglier. But you're right, we really need to test the
pipeline aspect of it, so I changed the tests that use CompleteTwoRequests().

+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "one.html"),
+    MockRead(false, 5, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 6, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 7, "two.html"),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  CompleteTwoRequests();
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, ReusesOnSpaceAvailable) {
+  int old_max_sockets = ClientSocketPoolManager::max_sockets_per_group();
+  ClientSocketPoolManager::set_max_sockets_per_group(1);
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 4, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 7, "GET /three.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 12, "GET /four.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "one.html"),
+    MockRead(false, 5, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 6, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 8, "two.html"),
+    MockRead(false, 9, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 10, "Content-Length: 10\r\n\r\n"),
+    MockRead(false, 11, "three.html"),
+    MockRead(false, 13, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 14, "Content-Length: 9\r\n\r\n"),
+    MockRead(false, 15, "four.html"),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  scoped_ptr<HttpNetworkTransaction> one_transaction(
+      new HttpNetworkTransaction(session_.get()));
+  TestOldCompletionCallback one_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            one_transaction->Start(GetRequestInfo("one.html"), &one_callback,
+                                   BoundNetLog()));
+  EXPECT_EQ(OK, one_callback.WaitForResult());
+
+  HttpNetworkTransaction two_transaction(session_.get());
+  TestOldCompletionCallback two_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            two_transaction.Start(GetRequestInfo("two.html"), &two_callback,
+                                  BoundNetLog()));
+
+  HttpNetworkTransaction three_transaction(session_.get());
+  TestOldCompletionCallback three_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            three_transaction.Start(GetRequestInfo("three.html"),
+                                    &three_callback, BoundNetLog()));
+
+  HttpNetworkTransaction four_transaction(session_.get());
+  TestOldCompletionCallback four_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            four_transaction.Start(GetRequestInfo("four.html"), &four_callback,
+                                   BoundNetLog()));
+
+  ExpectResponse("one.html", *one_transaction.get());
+  EXPECT_EQ(OK, two_callback.WaitForResult());
+  ExpectResponse("two.html", two_transaction);
+  EXPECT_EQ(OK, three_callback.WaitForResult());
+  ExpectResponse("three.html", three_transaction);
+
+  one_transaction.reset();
+  EXPECT_EQ(OK, four_callback.WaitForResult());
+  ExpectResponse("four.html", four_transaction);
+
+  ClientSocketPoolManager::set_max_sockets_per_group(old_max_sockets);
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, UnknownSizeEvictsToNewPipeline) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 4, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n\r\n"),
+    MockRead(false, 2, "one.html"),
+    MockRead(false, OK, 3),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  MockWrite writes2[] = {
+    MockWrite(false, 0, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads2[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "two.html"),
+  };
+  AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2));
+
+  CompleteTwoRequests();
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, ConnectionCloseEvictToNewPipeline) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 4, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "one.html"),
+    MockRead(false, ERR_SOCKET_NOT_CONNECTED, 5),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  MockWrite writes2[] = {
+    MockWrite(false, 0, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads2[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "two.html"),
+  };
+  AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2));
+
+  CompleteTwoRequests();
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, ErrorEvictsToNewPipeline) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 3, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n\r\n"),
+    MockRead(false, ERR_FAILED, 2),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  MockWrite writes2[] = {
+    MockWrite(false, 0, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads2[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "two.html"),
+  };
+  AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2));
+
+  HttpNetworkTransaction one_transaction(session_.get());
+  TestOldCompletionCallback one_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            one_transaction.Start(GetRequestInfo("one.html"), &one_callback,
+                                  BoundNetLog()));
+  EXPECT_EQ(OK, one_callback.WaitForResult());
+
+  HttpNetworkTransaction two_transaction(session_.get());
+  TestOldCompletionCallback two_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            two_transaction.Start(GetRequestInfo("two.html"), &two_callback,
+                                  BoundNetLog()));
+
+  scoped_refptr<IOBuffer> buffer(new IOBuffer(1));
+  EXPECT_EQ(ERR_FAILED, one_transaction.Read(buffer.get(), 1, &callback_));
+  EXPECT_EQ(OK, two_callback.WaitForResult());
+  ExpectResponse("two.html", two_transaction);
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, SendErrorEvictsToNewPipeline) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(true, ERR_FAILED, 0),
+  };
+  AddExpectedConnection(NULL, 0, writes, arraysize(writes));
+
+  MockWrite writes2[] = {
+    MockWrite(false, 0, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads2[] = {
+    MockRead(false, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 3, "two.html"),
+  };
+  AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2));
+
+  HttpNetworkTransaction one_transaction(session_.get());
+  TestOldCompletionCallback one_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            one_transaction.Start(GetRequestInfo("one.html"), &one_callback,
+                                  BoundNetLog()));
+
+  HttpNetworkTransaction two_transaction(session_.get());
+  TestOldCompletionCallback two_callback;
+  EXPECT_EQ(ERR_IO_PENDING,
+            two_transaction.Start(GetRequestInfo("two.html"), &two_callback,
+                                  BoundNetLog()));
+
+  data_vector_[0]->RunFor(1);
+  EXPECT_EQ(ERR_FAILED, one_callback.WaitForResult());
+
+  EXPECT_EQ(OK, two_callback.WaitForResult());
+  ExpectResponse("two.html", two_transaction);
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, BasicHttpAuthentication) {
+  Initialize();
+
+  MockWrite writes[] = {
+    MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(false, 5, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n"
+              "Authorization: auth_token\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(false, 1, "HTTP/1.1 401 Authentication Required\r\n"),
+    MockRead(false, 2, "WWW-Authenticate: Basic realm=\"Secure Area\"\r\n"),
+    MockRead(false, 3, "Content-Length: 20\r\n\r\n"),
+    MockRead(false, 4, "needs authentication"),
+    MockRead(false, 6, "HTTP/1.1 200 OK\r\n"),
+    MockRead(false, 7, "Content-Length: 8\r\n\r\n"),
+    MockRead(false, 8, "one.html"),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  HttpAuthHandlerMock* mock_auth = new HttpAuthHandlerMock;
+  std::string challenge_text = "Basic";
+  HttpAuth::ChallengeTokenizer challenge(challenge_text.begin(),
+                                         challenge_text.end());
+  GURL origin("localhost");
+  EXPECT_TRUE(mock_auth->InitFromChallenge(&challenge,
+                                           HttpAuth::AUTH_SERVER,
+                                           origin,
+                                           BoundNetLog()));
+  auth_handler_factory_->AddMockHandler(mock_auth, HttpAuth::AUTH_SERVER);
+
+  HttpNetworkTransaction transaction(session_.get());
+  EXPECT_EQ(
+      ERR_IO_PENDING,
+      transaction.Start(GetRequestInfo("one.html"), &callback_, BoundNetLog()));
+  EXPECT_EQ(OK, callback_.WaitForResult());
+
+  AuthCredentials credentials(ASCIIToUTF16("user"), ASCIIToUTF16("pass"));
+  EXPECT_EQ(OK, transaction.RestartWithAuth(credentials, &callback_));
+
+  ExpectResponse("one.html", transaction);
+}
+
+}  // namespace net