Make a proper TCPSocketParams

BUG=none
TEST=existing unit tests

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

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

8 files changed, 93 insertions, 76 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 5e770d3..1c979bf 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -33,6 +33,7 @@
 #include "net/socket/socks5_client_socket.h"
 #include "net/socket/socks_client_socket.h"
 #include "net/socket/ssl_client_socket.h"
+#include "net/socket/tcp_client_socket_pool.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
 #include "net/spdy/spdy_stream.h"
@@ -655,6 +656,14 @@ int HttpNetworkTransaction::DoInitConnection() {
     port = session_->fixed_http_port();
   }
 
+  // Check first if we have a spdy session for this group.  If so, then go
+  // straight to using that.
+  HostPortPair host_port_pair(host, port);
+  if (session_->spdy_session_pool()->HasSession(host_port_pair)) {
+    using_spdy_ = true;
+    return OK;
+  }
+
   // For a connection via HTTP proxy not using CONNECT, the connection
   // is to the proxy server only. For all other cases
   // (direct, HTTP proxy CONNECT, SOCKS), the connection is upto the
@@ -664,30 +673,14 @@ int HttpNetworkTransaction::DoInitConnection() {
 
   DCHECK(!connection_group.empty());
 
-  HostResolver::RequestInfo resolve_info(host, port);
-  resolve_info.set_priority(request_->priority);
-
-  // The referrer is used by the DNS prefetch system to correlate resolutions
-  // with the page that triggered them. It doesn't impact the actual addresses
-  // that we resolve to.
-  resolve_info.set_referrer(request_->referrer);
-
   // If the user is refreshing the page, bypass the host cache.
-  if (request_->load_flags & LOAD_BYPASS_CACHE ||
-      request_->load_flags & LOAD_DISABLE_CACHE) {
-    resolve_info.set_allow_cached_response(false);
-  }
-
-  HostPortPair host_port_pair(host, port);
+  bool disable_resolver_cache = request_->load_flags & LOAD_BYPASS_CACHE ||
+    request_->load_flags & LOAD_DISABLE_CACHE;
 
-  // Check first if we have a spdy session for this group.  If so, then go
-  // straight to using that.
-  if (session_->spdy_session_pool()->HasSession(host_port_pair)) {
-    using_spdy_ = true;
-    return OK;
-  }
+  TCPSocketParams tcp_params(host, port, request_->priority, request_->referrer,
+                             disable_resolver_cache);
 
-  int rv = connection_->Init(connection_group, resolve_info, request_->priority,
+  int rv = connection_->Init(connection_group, tcp_params, request_->priority,
                              &io_callback_, session_->tcp_socket_pool(),
                              load_log_);
   return rv;
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index dcc2c09..edfba1a 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -29,7 +29,8 @@ const int kDefaultMaxSockets = 4;
 const int kDefaultMaxSocketsPerGroup = 2;
 const net::RequestPriority kDefaultPriority = MEDIUM;
 
-typedef ClientSocketPoolBase<const void*> TestClientSocketPoolBase;
+typedef const void* TestSocketParams;
+typedef ClientSocketPoolBase<TestSocketParams> TestClientSocketPoolBase;
 
 class MockClientSocket : public ClientSocket {
  public:
@@ -335,7 +336,7 @@ class TestClientSocketPool : public ClientSocketPool {
 
 }  // namespace
 
-REGISTER_SOCKET_PARAMS_FOR_POOL(TestClientSocketPool, const void*);
+REGISTER_SOCKET_PARAMS_FOR_POOL(TestClientSocketPool, TestSocketParams);
 
 namespace {
 
@@ -408,7 +409,7 @@ class ClientSocketPoolBaseTest : public ClientSocketPoolTest {
 
   int StartRequest(const std::string& group_name,
                    net::RequestPriority priority) {
-    return StartRequestUsingPool<TestClientSocketPool, const void*>(
+    return StartRequestUsingPool<TestClientSocketPool, TestSocketParams>(
         pool_.get(), group_name, priority, NULL);
   }
 
@@ -443,7 +444,7 @@ int InitHandle(ClientSocketHandle* handle,
                CompletionCallback* callback,
                TestClientSocketPool* pool,
                LoadLog* load_log) {
-  return handle->Init<const void*, TestClientSocketPool>(
+  return handle->Init<TestSocketParams, TestClientSocketPool>(
       group_name, NULL, priority, callback, pool, load_log);
 }
 
diff --git a/net/socket/tcp_client_socket_pool.cc b/net/socket/tcp_client_socket_pool.cc
index 3ea6700..c7a524e 100644
--- a/net/socket/tcp_client_socket_pool.cc
+++ b/net/socket/tcp_client_socket_pool.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -33,14 +33,14 @@ static const int kTCPConnectJobTimeoutInSeconds = 240; // 4 minutes.
 
 TCPConnectJob::TCPConnectJob(
     const std::string& group_name,
-    const HostResolver::RequestInfo& resolve_info,
+    const TCPSocketParams& params,
     base::TimeDelta timeout_duration,
     ClientSocketFactory* client_socket_factory,
     HostResolver* host_resolver,
     Delegate* delegate,
     LoadLog* load_log)
     : ConnectJob(group_name, timeout_duration, delegate, load_log),
-      resolve_info_(resolve_info),
+      params_(params),
       client_socket_factory_(client_socket_factory),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           callback_(this,
@@ -112,7 +112,8 @@ int TCPConnectJob::DoLoop(int result) {
 
 int TCPConnectJob::DoResolveHost() {
   next_state_ = kStateResolveHostComplete;
-  return resolver_.Resolve(resolve_info_, &addresses_, &callback_, load_log());
+  return resolver_.Resolve(params_.destination(), &addresses_, &callback_,
+                           load_log());
 }
 
 int TCPConnectJob::DoResolveHostComplete(int result) {
@@ -182,24 +183,24 @@ TCPClientSocketPool::~TCPClientSocketPool() {}
 
 int TCPClientSocketPool::RequestSocket(
     const std::string& group_name,
-    const void* resolve_info,
+    const void* params,
     RequestPriority priority,
     ClientSocketHandle* handle,
     CompletionCallback* callback,
     LoadLog* load_log) {
-  const HostResolver::RequestInfo* casted_resolve_info =
-      static_cast<const HostResolver::RequestInfo*>(resolve_info);
+  const TCPSocketParams* casted_params =
+      static_cast<const TCPSocketParams*>(params);
 
   if (LoadLog::IsUnbounded(load_log)) {
     LoadLog::AddString(
         load_log,
         StringPrintf("Requested TCP socket to: %s [port %d]",
-                     casted_resolve_info->hostname().c_str(),
-                     casted_resolve_info->port()));
+                     casted_params->destination().hostname().c_str(),
+                     casted_params->destination().port()));
   }
 
-  return base_.RequestSocket(
-      group_name, *casted_resolve_info, priority, handle, callback, load_log);
+  return base_.RequestSocket(group_name, *casted_params, priority, handle,
+                             callback, load_log);
 }
 
 void TCPClientSocketPool::CancelRequest(
diff --git a/net/socket/tcp_client_socket_pool.h b/net/socket/tcp_client_socket_pool.h
index fb1a93b..7896ea9 100644
--- a/net/socket/tcp_client_socket_pool.h
+++ b/net/socket/tcp_client_socket_pool.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -20,12 +20,32 @@ namespace net {
 
 class ClientSocketFactory;
 
+class TCPSocketParams {
+ public:
+  TCPSocketParams(const std::string& host, int port, RequestPriority priority,
+                  const GURL& referrer, bool disable_resolver_cache)
+      :  destination_(host, port) {
+    // The referrer is used by the DNS prefetch system to correlate resolutions
+    // with the page that triggered them. It doesn't impact the actual addresses
+    // that we resolve to.
+    destination_.set_referrer(referrer);
+    destination_.set_priority(priority);
+    if (disable_resolver_cache)
+      destination_.set_allow_cached_response(false);
+  }
+
+  HostResolver::RequestInfo destination() const { return destination_; }
+
+ private:
+  HostResolver::RequestInfo destination_;
+};
+
 // TCPConnectJob handles the host resolution necessary for socket creation
 // and the tcp connect.
 class TCPConnectJob : public ConnectJob {
  public:
   TCPConnectJob(const std::string& group_name,
-                const HostResolver::RequestInfo& resolve_info,
+                const TCPSocketParams& params,
                 base::TimeDelta timeout_duration,
                 ClientSocketFactory* client_socket_factory,
                 HostResolver* host_resolver,
@@ -60,7 +80,7 @@ class TCPConnectJob : public ConnectJob {
   int DoTCPConnect();
   int DoTCPConnectComplete(int result);
 
-  const HostResolver::RequestInfo resolve_info_;
+  const TCPSocketParams params_;
   ClientSocketFactory* const client_socket_factory_;
   CompletionCallbackImpl<TCPConnectJob> callback_;
   SingleRequestHostResolver resolver_;
@@ -115,7 +135,7 @@ class TCPClientSocketPool : public ClientSocketPool {
   virtual ~TCPClientSocketPool();
 
  private:
-  typedef ClientSocketPoolBase<HostResolver::RequestInfo> PoolBase;
+  typedef ClientSocketPoolBase<TCPSocketParams> PoolBase;
 
   class TCPConnectJobFactory
       : public PoolBase::ConnectJobFactory {
@@ -147,7 +167,7 @@ class TCPClientSocketPool : public ClientSocketPool {
   DISALLOW_COPY_AND_ASSIGN(TCPClientSocketPool);
 };
 
-REGISTER_SOCKET_PARAMS_FOR_POOL(TCPClientSocketPool, HostResolver::RequestInfo)
+REGISTER_SOCKET_PARAMS_FOR_POOL(TCPClientSocketPool, TCPSocketParams)
 
 }  // namespace net
 
diff --git a/net/socket/tcp_client_socket_pool_unittest.cc b/net/socket/tcp_client_socket_pool_unittest.cc
index 2678848..c9d80f1 100644
--- a/net/socket/tcp_client_socket_pool_unittest.cc
+++ b/net/socket/tcp_client_socket_pool_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -206,7 +206,7 @@ class MockClientSocketFactory : public ClientSocketFactory {
 class TCPClientSocketPoolTest : public ClientSocketPoolTest {
  protected:
   TCPClientSocketPoolTest()
-      : ignored_request_info_("ignored", 80),
+      : ignored_socket_params_("ignored", 80, MEDIUM, GURL(), false),
         host_resolver_(new MockHostResolver),
         pool_(new TCPClientSocketPool(kMaxSockets,
                                       kMaxSocketsPerGroup,
@@ -217,10 +217,10 @@ class TCPClientSocketPoolTest : public ClientSocketPoolTest {
 
   int StartRequest(const std::string& group_name, RequestPriority priority) {
     return StartRequestUsingPool(
-        pool_.get(), group_name, priority, ignored_request_info_);
+        pool_.get(), group_name, priority, ignored_socket_params_);
   }
 
-  HostResolver::RequestInfo ignored_request_info_;
+  TCPSocketParams ignored_socket_params_;
   scoped_refptr<MockHostResolver> host_resolver_;
   MockClientSocketFactory client_socket_factory_;
   MockNetworkChangeNotifier notifier_;
@@ -230,8 +230,8 @@ class TCPClientSocketPoolTest : public ClientSocketPoolTest {
 TEST_F(TCPClientSocketPoolTest, Basic) {
   TestCompletionCallback callback;
   ClientSocketHandle handle;
-  HostResolver::RequestInfo info("www.google.com", 80);
-  int rv = handle.Init("a", info, LOW, &callback, pool_.get(), NULL);
+  TCPSocketParams dest("www.google.com", 80, LOW, GURL(), false);
+  int rv = handle.Init("a", dest, LOW, &callback, pool_.get(), NULL);
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_FALSE(handle.is_initialized());
   EXPECT_FALSE(handle.socket());
@@ -246,10 +246,11 @@ TEST_F(TCPClientSocketPoolTest, Basic) {
 TEST_F(TCPClientSocketPoolTest, InitHostResolutionFailure) {
   host_resolver_->rules()->AddSimulatedFailure("unresolvable.host.name");
   TestSocketRequest req(&request_order_, &completion_count_);
-  HostResolver::RequestInfo info("unresolvable.host.name", 80);
+  TCPSocketParams dest("unresolvable.host.name", 80, kDefaultPriority, GURL(),
+                       false);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle()->Init(
-                "a", info, kDefaultPriority, &req, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req, pool_.get(), NULL));
   EXPECT_EQ(ERR_NAME_NOT_RESOLVED, req.WaitForResult());
 }
 
@@ -257,17 +258,17 @@ TEST_F(TCPClientSocketPoolTest, InitConnectionFailure) {
   client_socket_factory_.set_client_socket_type(
       MockClientSocketFactory::MOCK_FAILING_CLIENT_SOCKET);
   TestSocketRequest req(&request_order_, &completion_count_);
-  HostResolver::RequestInfo info("a", 80);
+  TCPSocketParams dest("a", 80, kDefaultPriority, GURL(), false);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle()->Init(
-                "a", info, kDefaultPriority, &req, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req, pool_.get(), NULL));
   EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult());
 
   // Make the host resolutions complete synchronously this time.
   host_resolver_->set_synchronous_mode(true);
   EXPECT_EQ(ERR_CONNECTION_FAILED,
             req.handle()->Init(
-                "a", info, kDefaultPriority, &req, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req, pool_.get(), NULL));
 }
 
 TEST_F(TCPClientSocketPoolTest, PendingRequests) {
@@ -371,10 +372,10 @@ TEST_F(TCPClientSocketPoolTest, PendingRequests_NoKeepAlive) {
 // ClientSocketPool which will crash if the group was not cleared properly.
 TEST_F(TCPClientSocketPoolTest, CancelRequestClearGroup) {
   TestSocketRequest req(&request_order_, &completion_count_);
-  HostResolver::RequestInfo info("www.google.com", 80);
+  TCPSocketParams dest("www.google.com", 80, kDefaultPriority, GURL(), false);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle()->Init(
-                "a", info, kDefaultPriority, &req, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req, pool_.get(), NULL));
   req.handle()->Reset();
 
   // There is a race condition here.  If the worker pool doesn't post the task
@@ -390,13 +391,13 @@ TEST_F(TCPClientSocketPoolTest, TwoRequestsCancelOne) {
   TestSocketRequest req(&request_order_, &completion_count_);
   TestSocketRequest req2(&request_order_, &completion_count_);
 
-  HostResolver::RequestInfo info("www.google.com", 80);
+  TCPSocketParams dest("www.google.com", 80, kDefaultPriority, GURL(), false);
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle()->Init(
-                "a", info, kDefaultPriority, &req, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req, pool_.get(), NULL));
   EXPECT_EQ(ERR_IO_PENDING,
             req2.handle()->Init(
-                "a", info, kDefaultPriority, &req2, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &req2, pool_.get(), NULL));
 
   req.handle()->Reset();
 
@@ -411,17 +412,17 @@ TEST_F(TCPClientSocketPoolTest, ConnectCancelConnect) {
   TestCompletionCallback callback;
   TestSocketRequest req(&request_order_, &completion_count_);
 
-  HostResolver::RequestInfo info("www.google.com", 80);
+  TCPSocketParams dest("www.google.com", 80, kDefaultPriority, GURL(), false);
   EXPECT_EQ(ERR_IO_PENDING,
             handle.Init(
-                "a", info, kDefaultPriority, &callback, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &callback, pool_.get(), NULL));
 
   handle.Reset();
 
   TestCompletionCallback callback2;
   EXPECT_EQ(ERR_IO_PENDING,
             handle.Init(
-                "a", info, kDefaultPriority, &callback2, pool_.get(), NULL));
+                "a", dest, kDefaultPriority, &callback2, pool_.get(), NULL));
 
   host_resolver_->set_synchronous_mode(true);
   // At this point, handle has two ConnectingSockets out for it.  Due to the
@@ -519,9 +520,8 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
         MessageLoop::current()->RunAllPending();
       }
       within_callback_ = true;
-      int rv = handle_->Init(
-          "a", HostResolver::RequestInfo("www.google.com", 80), LOWEST,
-          this, pool_.get(), NULL);
+      TCPSocketParams dest("www.google.com", 80, LOWEST, GURL(), false);
+      int rv = handle_->Init("a", dest, LOWEST, this, pool_.get(), NULL);
       EXPECT_EQ(OK, rv);
     }
   }
@@ -540,9 +540,8 @@ class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
 TEST_F(TCPClientSocketPoolTest, RequestTwice) {
   ClientSocketHandle handle;
   RequestSocketCallback callback(&handle, pool_.get());
-  int rv = handle.Init(
-      "a", HostResolver::RequestInfo("www.google.com", 80), LOWEST,
-      &callback, pool_.get(), NULL);
+  TCPSocketParams dest("www.google.com", 80, LOWEST, GURL(), false);
+  int rv = handle.Init("a", dest, LOWEST, &callback, pool_.get(), NULL);
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
   // The callback is going to request "www.google.com". We want it to complete
@@ -604,8 +603,8 @@ TEST_F(TCPClientSocketPoolTest, FailingActiveRequestWithPendingRequests) {
 TEST_F(TCPClientSocketPoolTest, ResetIdleSocketsOnIPAddressChange) {
   TestCompletionCallback callback;
   ClientSocketHandle handle;
-  HostResolver::RequestInfo info("www.google.com", 80);
-  int rv = handle.Init("a", info, LOW, &callback, pool_.get(), NULL);
+  TCPSocketParams dest("www.google.com", 80, LOW, GURL(), false);
+  int rv = handle.Init("a", dest, LOW, &callback, pool_.get(), NULL);
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_FALSE(handle.is_initialized());
   EXPECT_FALSE(handle.socket());
diff --git a/net/spdy/spdy_network_transaction.cc b/net/spdy/spdy_network_transaction.cc
index 9ae31c2b8..3d04c1d 100644
--- a/net/spdy/spdy_network_transaction.cc
+++ b/net/spdy/spdy_network_transaction.cc
@@ -17,6 +17,7 @@
 #include "net/http/http_network_session.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_info.h"
+#include "net/socket/tcp_client_socket_pool.h"
 #include "net/spdy/spdy_stream.h"
 
 using base::Time;
@@ -231,14 +232,15 @@ int SpdyNetworkTransaction::DoInitConnection() {
   std::string connection_group = "spdy.";
   connection_group.append(host);
 
-  HostResolver::RequestInfo resolve_info(host, port);
+  TCPSocketParams tcp_params(host, port, request_->priority, request_->referrer,
+                             false);
   HostPortPair host_port_pair(host, port);
 
   spdy_ = session_->spdy_session_pool()->Get(host_port_pair, session_);
   DCHECK(spdy_);
 
   return spdy_->Connect(
-      connection_group, resolve_info, request_->priority, load_log_);
+      connection_group, tcp_params, request_->priority, load_log_);
 }
 
 int SpdyNetworkTransaction::DoInitConnectionComplete(int result) {
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 172b5d7..e0eb9c5 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -257,7 +257,7 @@ void SpdySession::InitializeWithSSLSocket(ClientSocketHandle* connection) {
 }
 
 net::Error SpdySession::Connect(const std::string& group_name,
-                                const HostResolver::RequestInfo& host,
+                                const TCPSocketParams& destination,
                                 RequestPriority priority,
                                 LoadLog* load_log) {
   DCHECK(priority >= SPDY_PRIORITY_HIGHEST && priority <= SPDY_PRIORITY_LOWEST);
@@ -271,8 +271,9 @@ net::Error SpdySession::Connect(const std::string& group_name,
   static StatsCounter spdy_sessions("spdy.sessions");
   spdy_sessions.Increment();
 
-  int rv = connection_->Init(group_name, host, priority, &connect_callback_,
-                             session_->tcp_socket_pool(), load_log);
+  int rv = connection_->Init(group_name, destination, priority,
+                             &connect_callback_, session_->tcp_socket_pool(),
+                             load_log);
   DCHECK(rv <= 0);
 
   // If the connect is pending, we still return ok.  The APIs enqueue
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 01ac1a7..2f2465b 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -20,7 +20,7 @@
 #include "net/base/upload_data_stream.h"
 #include "net/socket/client_socket.h"
 #include "net/socket/client_socket_handle.h"
-#include "testing/platform_test.h"
+#include "net/socket/tcp_client_socket_pool.h"
 #include "net/spdy/spdy_framer.h"
 #include "net/spdy/spdy_io_buffer.h"
 #include "net/spdy/spdy_protocol.h"
@@ -45,7 +45,7 @@ class SpdySession : public base::RefCounted<SpdySession>,
   // Note that this call does not wait for the connect to complete. Callers can
   // immediately start using the SpdySession while it connects.
   net::Error Connect(const std::string& group_name,
-                     const HostResolver::RequestInfo& host,
+                     const TCPSocketParams& destination,
                      RequestPriority priority,
                      LoadLog* load_log);