Add a new OnHostResolutionCallback to TransportSocketParams

Correctly implement OnHostResolution in HttpStreamFactoryImpl::Job

BUG=106235
TEST=HttpNetworkTransactionSpdy\*Test.UseIPConnectionPoolingAfterResolution


Review URL: https://chromiumcodereview.appspot.com/10238010

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

10 files changed, 110 insertions, 31 deletions

diff --git a/net/socket/client_socket_pool_manager.cc b/net/socket/client_socket_pool_manager.cc
index 70f1edb..d5591b7 100644
--- a/net/socket/client_socket_pool_manager.cc
+++ b/net/socket/client_socket_pool_manager.cc
@@ -80,6 +80,7 @@ int InitSocketPoolHelper(const GURL& request_url,
                          const BoundNetLog& net_log,
                          int num_preconnect_streams,
                          ClientSocketHandle* socket_handle,
+                         const OnHostResolutionCallback& resolution_callback,
                          const CompletionCallback& callback) {
   scoped_refptr<TransportSocketParams> tcp_params;
   scoped_refptr<HttpProxySocketParams> http_proxy_params;
@@ -150,7 +151,8 @@ int InitSocketPoolHelper(const GURL& request_url,
     tcp_params = new TransportSocketParams(origin_host_port,
                                            request_priority,
                                            disable_resolver_cache,
-                                           ignore_limits);
+                                           ignore_limits,
+                                           resolution_callback);
   } else {
     ProxyServer proxy_server = proxy_info.proxy_server();
     proxy_host_port.reset(new HostPortPair(proxy_server.host_port_pair()));
@@ -158,7 +160,8 @@ int InitSocketPoolHelper(const GURL& request_url,
         new TransportSocketParams(*proxy_host_port,
                                   request_priority,
                                   disable_resolver_cache,
-                                  ignore_limits));
+                                  ignore_limits,
+                                  resolution_callback));
 
     if (proxy_info.is_http() || proxy_info.is_https()) {
       std::string user_agent;
@@ -368,13 +371,14 @@ int InitSocketHandleForHttpRequest(
     const SSLConfig& ssl_config_for_proxy,
     const BoundNetLog& net_log,
     ClientSocketHandle* socket_handle,
+    const OnHostResolutionCallback& resolution_callback,
     const CompletionCallback& callback) {
   DCHECK(socket_handle);
   return InitSocketPoolHelper(
       request_url, request_extra_headers, request_load_flags, request_priority,
       session, proxy_info, force_spdy_over_ssl, want_spdy_over_npn,
       ssl_config_for_origin, ssl_config_for_proxy, false, net_log, 0,
-      socket_handle, callback);
+      socket_handle, resolution_callback, callback);
 }
 
 int InitSocketHandleForRawConnect(
@@ -396,7 +400,8 @@ int InitSocketHandleForRawConnect(
   return InitSocketPoolHelper(
       request_url, request_extra_headers, request_load_flags, request_priority,
       session, proxy_info, false, false, ssl_config_for_origin,
-      ssl_config_for_proxy, true, net_log, 0, socket_handle, callback);
+      ssl_config_for_proxy, true, net_log, 0, socket_handle,
+      OnHostResolutionCallback(), callback);
 }
 
 int PreconnectSocketsForHttpRequest(
@@ -416,7 +421,8 @@ int PreconnectSocketsForHttpRequest(
       request_url, request_extra_headers, request_load_flags, request_priority,
       session, proxy_info, force_spdy_over_ssl, want_spdy_over_npn,
       ssl_config_for_origin, ssl_config_for_proxy, false, net_log,
-      num_preconnect_streams, NULL, CompletionCallback());
+      num_preconnect_streams, NULL, OnHostResolutionCallback(),
+      CompletionCallback());
 }
 
 }  // namespace net
diff --git a/net/socket/client_socket_pool_manager.h b/net/socket/client_socket_pool_manager.h
index 5175de9..31f1f32 100644
--- a/net/socket/client_socket_pool_manager.h
+++ b/net/socket/client_socket_pool_manager.h
@@ -23,6 +23,9 @@ class Value;
 
 namespace net {
 
+typedef base::Callback<int(const AddressList&, const BoundNetLog& net_log)>
+OnHostResolutionCallback;
+
 class BoundNetLog;
 class ClientSocketHandle;
 class HostPortPair;
@@ -84,6 +87,9 @@ class NET_EXPORT_PRIVATE ClientSocketPoolManager {
 // ClientSocketHandle with the relevant socket pool. Use this method for
 // HTTP/HTTPS requests. |ssl_config_for_origin| is only used if the request
 // uses SSL and |ssl_config_for_proxy| is used if the proxy server is HTTPS.
+// |resolution_callback| will be invoked after the the hostname is
+// resolved.  If |resolution_callback| does not return OK, then the
+// connection will be aborted with that value.
 int InitSocketHandleForHttpRequest(
     const GURL& request_url,
     const HttpRequestHeaders& request_extra_headers,
@@ -97,6 +103,7 @@ int InitSocketHandleForHttpRequest(
     const SSLConfig& ssl_config_for_proxy,
     const BoundNetLog& net_log,
     ClientSocketHandle* socket_handle,
+    const OnHostResolutionCallback& resolution_callback,
     const CompletionCallback& callback);
 
 // A helper method that uses the passed in proxy information to initialize a
diff --git a/net/socket/deterministic_socket_data_unittest.cc b/net/socket/deterministic_socket_data_unittest.cc
index 92c6eef..99703be 100644
--- a/net/socket/deterministic_socket_data_unittest.cc
+++ b/net/socket/deterministic_socket_data_unittest.cc
@@ -69,7 +69,8 @@ DeterministicSocketDataTest::DeterministicSocketDataTest()
       tcp_params_(new TransportSocketParams(endpoint_,
                                             LOWEST,
                                             false,
-                                            false)),
+                                            false,
+                                            OnHostResolutionCallback())),
       histograms_(""),
       socket_pool_(10, 10, &histograms_, &socket_factory_) {
 }
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 0608ee2..8aeec46 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -117,6 +117,26 @@ void DumpMockRead(const MockRead& r) {
 
 }  // namespace
 
+MockConnect::MockConnect() : mode(ASYNC), result(OK) {
+  IPAddressNumber ip;
+  CHECK(ParseIPLiteralToNumber("192.0.2.33", &ip));
+  peer_addr = IPEndPoint(ip, 0);
+}
+
+MockConnect::MockConnect(IoMode io_mode, int r) : mode(io_mode), result(r) {
+  IPAddressNumber ip;
+  CHECK(ParseIPLiteralToNumber("192.0.2.33", &ip));
+  peer_addr = IPEndPoint(ip, 0);
+}
+
+MockConnect::MockConnect(IoMode io_mode, int r, IPEndPoint addr) :
+    mode(io_mode),
+    result(r),
+    peer_addr(addr) {
+}
+
+MockConnect::~MockConnect() {}
+
 StaticSocketDataProvider::StaticSocketDataProvider()
     : reads_(NULL),
       read_index_(0),
@@ -638,6 +658,9 @@ MockClientSocket::MockClientSocket(net::NetLog* net_log)
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       connected_(false),
       net_log_(BoundNetLog::Make(net_log, net::NetLog::SOURCE_NONE)) {
+  IPAddressNumber ip;
+  CHECK(ParseIPLiteralToNumber("192.0.2.33", &ip));
+  peer_addr_ = IPEndPoint(ip, 0);
 }
 
 bool MockClientSocket::SetReceiveBufferSize(int32 size) {
@@ -661,10 +684,7 @@ bool MockClientSocket::IsConnectedAndIdle() const {
 }
 
 int MockClientSocket::GetPeerAddress(IPEndPoint* address) const {
-  IPAddressNumber ip;
-  bool rv = ParseIPLiteralToNumber("192.0.2.33", &ip);
-  CHECK(rv);
-  *address = IPEndPoint(ip, 0);
+  *address = peer_addr_;
   return OK;
 }
 
@@ -739,6 +759,7 @@ MockTCPClientSocket::MockTCPClientSocket(const AddressList& addresses,
       pending_buf_len_(0),
       was_used_to_convey_data_(false) {
   DCHECK(data_);
+  peer_addr_ = data->connect_data().peer_addr;
   data_->Reset();
 }
 
@@ -825,8 +846,6 @@ bool MockTCPClientSocket::IsConnectedAndIdle() const {
 }
 
 int MockTCPClientSocket::GetPeerAddress(IPEndPoint* address) const {
-  if (!IsConnected())
-    return ERR_SOCKET_NOT_CONNECTED;
   return MockClientSocket::GetPeerAddress(address);
 }
 
@@ -1072,6 +1091,7 @@ MockSSLClientSocket::MockSSLClientSocket(
       is_protocol_negotiated_set_(false),
       protocol_negotiated_(kProtoUnknown) {
   DCHECK(data_);
+  peer_addr_ = data->connect.peer_addr;
   delete ssl_host_info;  // we take ownership but don't use it.
 }
 
@@ -1126,6 +1146,10 @@ int64 MockSSLClientSocket::NumBytesRead() const {
   return -1;
 }
 
+int MockSSLClientSocket::GetPeerAddress(IPEndPoint* address) const {
+  return transport_->socket()->GetPeerAddress(address);
+}
+
 base::TimeDelta MockSSLClientSocket::GetConnectTimeMicros() const {
   return base::TimeDelta::FromMicroseconds(-1);
 }
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index a75c904..22b0af2 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -60,11 +60,18 @@ enum IoMode {
 
 struct MockConnect {
   // Asynchronous connection success.
-  MockConnect() : mode(ASYNC), result(OK) { }
-  MockConnect(IoMode io_mode, int r) : mode(io_mode), result(r) { }
+  // Creates a MockConnect with |mode| ASYC, |result| OK, and
+  // |peer_addr| 192.0.2.33.
+  MockConnect();
+  // Creates a MockConnect with the specified mode and result, with
+  // |peer_addr| 192.0.2.33.
+  MockConnect(IoMode io_mode, int r);
+  MockConnect(IoMode io_mode, int r, IPEndPoint addr);
+  ~MockConnect();
 
   IoMode mode;
   int result;
+  IPEndPoint peer_addr;
 };
 
 struct MockRead {
@@ -618,6 +625,9 @@ class MockClientSocket : public SSLClientSocket {
   // True if Connect completed successfully and Disconnect hasn't been called.
   bool connected_;
 
+  // Address of the "remote" peer we're connected to.
+  IPEndPoint peer_addr_;
+
   BoundNetLog net_log_;
 };
 
@@ -744,6 +754,7 @@ class MockSSLClientSocket : public MockClientSocket, public AsyncSocket {
   virtual bool WasEverUsed() const OVERRIDE;
   virtual bool UsingTCPFastOpen() const OVERRIDE;
   virtual int64 NumBytesRead() const OVERRIDE;
+  virtual int GetPeerAddress(IPEndPoint* address) const OVERRIDE;
   virtual base::TimeDelta GetConnectTimeMicros() const OVERRIDE;
 
   // SSLClientSocket implementation.
diff --git a/net/socket/socks_client_socket_pool_unittest.cc b/net/socket/socks_client_socket_pool_unittest.cc
index 0bb8563..bb6b125 100644
--- a/net/socket/socks_client_socket_pool_unittest.cc
+++ b/net/socket/socks_client_socket_pool_unittest.cc
@@ -54,7 +54,8 @@ class SOCKSClientSocketPoolTest : public testing::Test {
 
   SOCKSClientSocketPoolTest()
       : ignored_transport_socket_params_(new TransportSocketParams(
-            HostPortPair("proxy", 80), MEDIUM, false, false)),
+          HostPortPair("proxy", 80), MEDIUM, false, false,
+          OnHostResolutionCallback())),
         transport_histograms_("MockTCP"),
         transport_socket_pool_(
             kMaxSockets, kMaxSocketsPerGroup,
diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index b841c02..a725438 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -48,7 +48,8 @@ class SSLClientSocketPoolTest : public testing::Test {
             &host_resolver_)),
         session_(CreateNetworkSession()),
         direct_transport_socket_params_(new TransportSocketParams(
-            HostPortPair("host", 443), MEDIUM, false, false)),
+            HostPortPair("host", 443), MEDIUM, false, false,
+            OnHostResolutionCallback())),
         transport_histograms_("MockTCP"),
         transport_socket_pool_(
             kMaxSockets,
@@ -56,7 +57,8 @@ class SSLClientSocketPoolTest : public testing::Test {
             &transport_histograms_,
             &socket_factory_),
         proxy_transport_socket_params_(new TransportSocketParams(
-            HostPortPair("proxy", 443), MEDIUM, false, false)),
+            HostPortPair("proxy", 443), MEDIUM, false, false,
+            OnHostResolutionCallback())),
         socks_socket_params_(new SOCKSSocketParams(
             proxy_transport_socket_params_, true,
             HostPortPair("sockshost", 443), MEDIUM)),
diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc
index b2ca683..dee8b57 100644
--- a/net/socket/transport_client_socket_pool.cc
+++ b/net/socket/transport_client_socket_pool.cc
@@ -49,8 +49,11 @@ TransportSocketParams::TransportSocketParams(
     const HostPortPair& host_port_pair,
     RequestPriority priority,
     bool disable_resolver_cache,
-    bool ignore_limits)
-    : destination_(host_port_pair), ignore_limits_(ignore_limits) {
+    bool ignore_limits,
+    const OnHostResolutionCallback& host_resolution_callback)
+    : destination_(host_port_pair),
+      ignore_limits_(ignore_limits),
+      host_resolution_callback_(host_resolution_callback) {
   Initialize(priority, disable_resolver_cache);
 }
 
@@ -166,8 +169,14 @@ int TransportConnectJob::DoResolveHost() {
 }
 
 int TransportConnectJob::DoResolveHostComplete(int result) {
-  if (result == OK)
-    next_state_ = STATE_TRANSPORT_CONNECT;
+  if (result == OK) {
+    // Invoke callback, and abort if it fails.
+    if (!params_->host_resolution_callback().is_null())
+      result = params_->host_resolution_callback().Run(addresses_, net_log());
+
+    if (result == OK)
+      next_state_ = STATE_TRANSPORT_CONNECT;
+  }
   return result;
 }
 
diff --git a/net/socket/transport_client_socket_pool.h b/net/socket/transport_client_socket_pool.h
index fc9ee22..06e669d 100644
--- a/net/socket/transport_client_socket_pool.h
+++ b/net/socket/transport_client_socket_pool.h
@@ -24,16 +24,27 @@ namespace net {
 
 class ClientSocketFactory;
 
+typedef base::Callback<int(const AddressList&, const BoundNetLog& net_log)>
+OnHostResolutionCallback;
+
 class NET_EXPORT_PRIVATE TransportSocketParams
     : public base::RefCounted<TransportSocketParams> {
  public:
-  TransportSocketParams(const HostPortPair& host_port_pair,
-                        RequestPriority priority,
-                        bool disable_resolver_cache,
-                        bool ignore_limits);
+  // |host_resolution_callback| will be invoked after the the hostname is
+  // resolved.  If |host_resolution_callback| does not return OK, then the
+  // connection will be aborted with that value.
+  TransportSocketParams(
+      const HostPortPair& host_port_pair,
+      RequestPriority priority,
+      bool disable_resolver_cache,
+      bool ignore_limits,
+      const OnHostResolutionCallback& host_resolution_callback);
 
   const HostResolver::RequestInfo& destination() const { return destination_; }
   bool ignore_limits() const { return ignore_limits_; }
+  const OnHostResolutionCallback& host_resolution_callback() const {
+    return host_resolution_callback_;
+  }
 
  private:
   friend class base::RefCounted<TransportSocketParams>;
@@ -43,6 +54,7 @@ class NET_EXPORT_PRIVATE TransportSocketParams
 
   HostResolver::RequestInfo destination_;
   bool ignore_limits_;
+  const OnHostResolutionCallback host_resolution_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(TransportSocketParams);
 };
diff --git a/net/socket/transport_client_socket_pool_unittest.cc b/net/socket/transport_client_socket_pool_unittest.cc
index b1fb87a..291640c 100644
--- a/net/socket/transport_client_socket_pool_unittest.cc
+++ b/net/socket/transport_client_socket_pool_unittest.cc
@@ -380,10 +380,12 @@ class TransportClientSocketPoolTest : public testing::Test {
             ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(true)),
         params_(
             new TransportSocketParams(HostPortPair("www.google.com", 80),
-                                     kDefaultPriority, false, false)),
+                                      kDefaultPriority, false, false,
+                                      OnHostResolutionCallback())),
         low_params_(
             new TransportSocketParams(HostPortPair("www.google.com", 80),
-                                      LOW, false, false)),
+                                      LOW, false, false,
+                                      OnHostResolutionCallback())),
         histograms_(new ClientSocketPoolHistograms("TCPUnitTest")),
         host_resolver_(new MockHostResolver),
         pool_(kMaxSockets,
@@ -401,7 +403,8 @@ class TransportClientSocketPoolTest : public testing::Test {
 
   int StartRequest(const std::string& group_name, RequestPriority priority) {
     scoped_refptr<TransportSocketParams> params(new TransportSocketParams(
-        HostPortPair("www.google.com", 80), MEDIUM, false, false));
+        HostPortPair("www.google.com", 80), MEDIUM, false, false,
+        OnHostResolutionCallback()));
     return test_base_.StartRequestUsingPool(
         &pool_, group_name, priority, params);
   }
@@ -524,7 +527,8 @@ TEST_F(TransportClientSocketPoolTest, InitHostResolutionFailure) {
   ClientSocketHandle handle;
   HostPortPair host_port_pair("unresolvable.host.name", 80);
   scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
-          host_port_pair, kDefaultPriority, false, false));
+      host_port_pair, kDefaultPriority, false, false,
+      OnHostResolutionCallback()));
   EXPECT_EQ(ERR_IO_PENDING,
             handle.Init("a", dest, kDefaultPriority, callback.callback(),
                         &pool_, BoundNetLog()));
@@ -798,7 +802,8 @@ class RequestSocketCallback : public TestCompletionCallbackBase {
       }
       within_callback_ = true;
       scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
-          HostPortPair("www.google.com", 80), LOWEST, false, false));
+          HostPortPair("www.google.com", 80), LOWEST, false, false,
+          OnHostResolutionCallback()));
       int rv = handle_->Init("a", dest, LOWEST, callback(), pool_,
                              BoundNetLog());
       EXPECT_EQ(OK, rv);
@@ -817,7 +822,8 @@ TEST_F(TransportClientSocketPoolTest, RequestTwice) {
   ClientSocketHandle handle;
   RequestSocketCallback callback(&handle, &pool_);
   scoped_refptr<TransportSocketParams> dest(new TransportSocketParams(
-      HostPortPair("www.google.com", 80), LOWEST, false, false));
+      HostPortPair("www.google.com", 80), LOWEST, false, false,
+      OnHostResolutionCallback()));
   int rv = handle.Init("a", dest, LOWEST, callback.callback(), &pool_,
                        BoundNetLog());
   ASSERT_EQ(ERR_IO_PENDING, rv);