Change ClientSocketPoolManager::InitSocketHandleForHttpRequest and friends

net/socket/client_socket_pool_manager.cc

Index: net/socket/client_socket_pool_manager.cc
diff --git a/net/socket/client_socket_pool_manager.cc b/net/socket/client_socket_pool_manager.cc
index 94cfad49471c059adf80c9b8883af55bc1a2d930..eefa46a6dd2fc4300246e2d8e16cb860a6be5966 100644
--- a/net/socket/client_socket_pool_manager.cc
+++ b/net/socket/client_socket_pool_manager.cc
@@ -66,7 +66,8 @@ static_assert(arraysize(g_max_sockets_per_proxy_server) ==
 
 // The meat of the implementation for the InitSocketHandleForHttpRequest,
 // InitSocketHandleForRawConnect and PreconnectSocketsForHttpRequest methods.
-int InitSocketPoolHelper(const GURL& request_url,
+int InitSocketPoolHelper(ClientSocketPoolManager::SocketGroupType group_type,
+                         const HostPortPair endpoint,
                          const HttpRequestHeaders& request_extra_headers,
                          int request_load_flags,
                          RequestPriority request_priority,
@@ -88,10 +89,9 @@ int InitSocketPoolHelper(const GURL& request_url,
   scoped_refptr<SOCKSSocketParams> socks_params;
   scoped_ptr<HostPortPair> proxy_host_port;
 
-  bool using_ssl = request_url.SchemeIs("https") ||
-      request_url.SchemeIs("wss") || force_spdy_over_ssl;
-
-  HostPortPair origin_host_port = HostPortPair::FromURL(request_url);
+  bool using_ssl =
+      group_type == ClientSocketPoolManager::SSL_GROUP || force_spdy_over_ssl;

[mmenke, 2015/04/07 16:36:58]

This seems kinda weird.  We use the group type the caller passed to us...Unless
the caller also told us to override the group type?  Think it would make more
sense to pass in the real group type instead.

And then force_spdy_over_ssl would only be used for SPDY negotiation histograms,
I guess?  Do we even care about histograms in that case?

[Ryan Hamilton, 2015/04/07 19:07:30]

Oh goodness, this is hideous. You're right. I've pushed this logic up the stack.
If it's ok with you, I'm going to keep the logic down in the SSLClientSocketPool
for now, but I'll remove this whole crazy force_spdy logic completely in a
follow up CL (perhaps pending some stats). We added this stuff back in the
SPDY/1 days but that was like 5 year ago (before I started). It's LONG since
been useful.

[mmenke, 2015/04/07 19:24:16]

I'm fine with keeping it for now, think we should remove this stuff sooner
rather than later - I've scratched my head over all that spdy stuff multiple
times in the past, so there's a real cost in terms of hair loss over unused code
(Particularly weird legacy code).

+  HostPortPair origin_host_port = endpoint;
 
   if (!using_ssl && session->params().testing_fixed_http_port != 0) {
     origin_host_port.set_port(session->params().testing_fixed_http_port);
@@ -112,7 +112,7 @@ int InitSocketPoolHelper(const GURL& request_url,
   // Determine the host and port to connect to.
   std::string connection_group = origin_host_port.ToString();
   DCHECK(!connection_group.empty());
-  if (request_url.SchemeIs("ftp")) {
+  if (group_type == ClientSocketPoolManager::FTP_GROUP) {
     // Combining FTP with forced SPDY over SSL would be a "path to madness".
     // Make sure we never do that.
     DCHECK(!using_ssl);
@@ -328,6 +328,18 @@ int InitSocketPoolHelper(const GURL& request_url,
 
 }  // namespace
 
+// static
+ClientSocketPoolManager::SocketGroupType
+ClientSocketPoolManager::GroupTypeFromScheme(const std::string& scheme) {
+  if (scheme == "ftp")
+    return FTP_GROUP;
+
+  if (scheme == "https" || scheme == "wss")
+    return SSL_GROUP;
+
+  return NORMAL_GROUP;
+}
+
 ClientSocketPoolManager::ClientSocketPoolManager() {}
 ClientSocketPoolManager::~ClientSocketPoolManager() {}
 
@@ -394,7 +406,8 @@ void ClientSocketPoolManager::set_max_sockets_per_proxy_server(
 }
 
 int InitSocketHandleForHttpRequest(
-    const GURL& request_url,
+    ClientSocketPoolManager::SocketGroupType group_type,
+    const HostPortPair& endpoint,
     const HttpRequestHeaders& request_extra_headers,
     int request_load_flags,
     RequestPriority request_priority,
@@ -411,15 +424,16 @@ int InitSocketHandleForHttpRequest(
     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, privacy_mode, net_log,
-      0, socket_handle, HttpNetworkSession::NORMAL_SOCKET_POOL,
-      resolution_callback, callback);
+      group_type, endpoint, 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,
+      privacy_mode, net_log, 0, socket_handle,
+      HttpNetworkSession::NORMAL_SOCKET_POOL, resolution_callback, callback);
 }
 
 int InitSocketHandleForWebSocketRequest(
-    const GURL& request_url,
+    ClientSocketPoolManager::SocketGroupType group_type,
+    const HostPortPair& endpoint,
     const HttpRequestHeaders& request_extra_headers,
     int request_load_flags,
     RequestPriority request_priority,
@@ -436,11 +450,11 @@ int InitSocketHandleForWebSocketRequest(
     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, true, privacy_mode, net_log,
-      0, socket_handle, HttpNetworkSession::WEBSOCKET_SOCKET_POOL,
-      resolution_callback, callback);
+      group_type, endpoint, 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, true,
+      privacy_mode, net_log, 0, socket_handle,
+      HttpNetworkSession::WEBSOCKET_SOCKET_POOL, resolution_callback, callback);
 }
 
 int InitSocketHandleForRawConnect(
@@ -454,47 +468,42 @@ int InitSocketHandleForRawConnect(
     ClientSocketHandle* socket_handle,
     const CompletionCallback& callback) {
   DCHECK(socket_handle);
-  // Synthesize an HttpRequestInfo.
-  GURL request_url = GURL("http://" + host_port_pair.ToString());
   HttpRequestHeaders request_extra_headers;
   int request_load_flags = 0;
   RequestPriority request_priority = MEDIUM;
-
   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, privacy_mode, net_log, 0, socket_handle,
+      ClientSocketPoolManager::NORMAL_GROUP, host_port_pair,
+      request_extra_headers, request_load_flags, request_priority, session,
+      proxy_info, false, false, ssl_config_for_origin, ssl_config_for_proxy,
+      true, privacy_mode, net_log, 0, socket_handle,
       HttpNetworkSession::NORMAL_SOCKET_POOL, OnHostResolutionCallback(),
       callback);
 }
 
-int InitSocketHandleForTlsConnect(
-    const HostPortPair& host_port_pair,
-    HttpNetworkSession* session,
-    const ProxyInfo& proxy_info,
-    const SSLConfig& ssl_config_for_origin,
-    const SSLConfig& ssl_config_for_proxy,
-    PrivacyMode privacy_mode,
-    const BoundNetLog& net_log,
-    ClientSocketHandle* socket_handle,
-    const CompletionCallback& callback) {
+int InitSocketHandleForTlsConnect(const HostPortPair& endpoint,
+                                  HttpNetworkSession* session,
+                                  const ProxyInfo& proxy_info,
+                                  const SSLConfig& ssl_config_for_origin,
+                                  const SSLConfig& ssl_config_for_proxy,
+                                  PrivacyMode privacy_mode,
+                                  const BoundNetLog& net_log,
+                                  ClientSocketHandle* socket_handle,
+                                  const CompletionCallback& callback) {
   DCHECK(socket_handle);
-  // Synthesize an HttpRequestInfo.
-  GURL request_url = GURL("https://" + host_port_pair.ToString());
   HttpRequestHeaders request_extra_headers;
   int request_load_flags = 0;
   RequestPriority request_priority = MEDIUM;
-
   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, privacy_mode, net_log, 0, socket_handle,
-      HttpNetworkSession::NORMAL_SOCKET_POOL, OnHostResolutionCallback(),
-      callback);
+      ClientSocketPoolManager::SSL_GROUP, endpoint, request_extra_headers,
+      request_load_flags, request_priority, session, proxy_info, false, false,
+      ssl_config_for_origin, ssl_config_for_proxy, true, privacy_mode, net_log,
+      0, socket_handle, HttpNetworkSession::NORMAL_SOCKET_POOL,
+      OnHostResolutionCallback(), callback);
 }
 
 int PreconnectSocketsForHttpRequest(
-    const GURL& request_url,
+    ClientSocketPoolManager::SocketGroupType group_type,
+    const HostPortPair& endpoint,
     const HttpRequestHeaders& request_extra_headers,
     int request_load_flags,
     RequestPriority request_priority,
@@ -508,11 +517,12 @@ int PreconnectSocketsForHttpRequest(
     const BoundNetLog& net_log,
     int num_preconnect_streams) {
   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, privacy_mode, net_log,
-      num_preconnect_streams, NULL, HttpNetworkSession::NORMAL_SOCKET_POOL,
-      OnHostResolutionCallback(), CompletionCallback());
+      group_type, endpoint, 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,
+      privacy_mode, net_log, num_preconnect_streams, NULL,
+      HttpNetworkSession::NORMAL_SOCKET_POOL, OnHostResolutionCallback(),
+      CompletionCallback());
 }
 
 }  // namespace net