Change ClientSocketPoolManager::InitSocketHandleForHttpRequest and friends

net/socket/client_socket_pool_manager.h

 // Copyright (c) 2012 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.
 //
 // ClientSocketPoolManager manages access to all ClientSocketPools.  It's a
 // simple container for all of them.  Most importantly, it handles the lifetime
 // and destruction order properly.
 
 #ifndef NET_SOCKET_CLIENT_SOCKET_POOL_MANAGER_H_
 #define NET_SOCKET_CLIENT_SOCKET_POOL_MANAGER_H_
@@ skipping 26 matching lines @@
 class SSLClientSocketPool;
 
 struct SSLConfig;
 
 // This should rather be a simple constant but Windows shared libs doesn't
 // really offer much flexiblity in exporting contants.
 enum DefaultMaxValues { kDefaultMaxSocketsPerProxyServer = 32 };
 
 class NET_EXPORT_PRIVATE ClientSocketPoolManager {
  public:
+  enum SocketGroupType { NORMAL_GROUP, SSL_GROUP, FTP_GROUP };

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

It's not clear what a group type is, particular with force_spdy_over_ssl
muddying the waters.  Should document it.

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

Done. Hopefully this helps. now that the force_spdy wart has been removed, this
should hopefully be self explanatory?

+
+  // Returns the correct socket group type for |scheme|.
+  static SocketGroupType GroupTypeFromScheme(const std::string& scheme);
+
   ClientSocketPoolManager();
   virtual ~ClientSocketPoolManager();
 
   // The setter methods below affect only newly created socket pools after the
   // methods are called. Normally they should be called at program startup
   // before any ClientSocketPoolManagerImpl is created.
   static int max_sockets_per_pool(HttpNetworkSession::SocketPoolType pool_type);
   static void set_max_sockets_per_pool(
       HttpNetworkSession::SocketPoolType pool_type,
       int socket_count);
@@ skipping 26 matching lines @@
 };
 
 // A helper method that uses the passed in proxy information to initialize a
 // 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,
+    ClientSocketPoolManager::SocketGroupType group_type,
+    const HostPortPair& endpoint,
     const HttpRequestHeaders& request_extra_headers,
     int request_load_flags,
     RequestPriority request_priority,
     HttpNetworkSession* session,
     const ProxyInfo& proxy_info,
     bool force_spdy_over_ssl,
     bool want_spdy_over_npn,

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

Could you document these two parameters, while you're here?  I have no idea what
they mean.  Looks like if we "want_spdy_over_npn", but fail to negotiate SPDY,
we consider it a failed request?

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

Done. I'm not a fan of "want_spdy_over_npn" as a name. How about
"expect_spdy_over_npn" or "requires_" or "needs_"?

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

Maybe require_spdy_over_npn?  (The imperative seems a little clearer, and I
don't really know what "expect" means)

     const SSLConfig& ssl_config_for_origin,
     const SSLConfig& ssl_config_for_proxy,
     PrivacyMode privacy_mode,
     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
 // ClientSocketHandle with the relevant socket pool. Use this method for
 // HTTP/HTTPS requests for WebSocket handshake.
 // |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.
 // This function uses WEBSOCKET_SOCKET_POOL socket pools.
 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,
     HttpNetworkSession* session,
     const ProxyInfo& proxy_info,
     bool force_spdy_over_ssl,
     bool want_spdy_over_npn,
     const SSLConfig& ssl_config_for_origin,
     const SSLConfig& ssl_config_for_proxy,
     PrivacyMode privacy_mode,
@@ skipping 28 matching lines @@
     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);
 
 // Similar to InitSocketHandleForHttpRequest except that it initiates the
 // desired number of preconnect streams from the relevant socket pool.
 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,
     HttpNetworkSession* session,
     const ProxyInfo& proxy_info,
     bool force_spdy_over_ssl,
     bool want_spdy_over_npn,
     const SSLConfig& ssl_config_for_origin,
     const SSLConfig& ssl_config_for_proxy,
     PrivacyMode privacy_mode,
     const BoundNetLog& net_log,
     int num_preconnect_streams);
 
 }  // namespace net
 
 #endif  // NET_SOCKET_CLIENT_SOCKET_POOL_MANAGER_H_