Clean up SSLSocketParams

Remove ProxyServer::Scheme parameter from SSLSocketParams. Replace it with
a ConnectionType enum.

Rework some of the parameter/function names.

BUG=166689
R=mmenke@chromium.org

Review URL: https://codereview.chromium.org/23232002

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

7 files changed, 115 insertions, 93 deletions

diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc
index c75df6f..b9cdbd2 100644
--- a/net/http/http_proxy_client_socket_pool.cc
+++ b/net/http/http_proxy_client_socket_pool.cc
@@ -58,7 +58,7 @@ HttpProxySocketParams::HttpProxySocketParams(
 
 const HostResolver::RequestInfo& HttpProxySocketParams::destination() const {
   if (transport_params_.get() == NULL) {
-    return ssl_params_->transport_params()->destination();
+    return ssl_params_->GetDirectConnectionParams()->destination();
   } else {
     return transport_params_->destination();
   }
@@ -212,10 +212,11 @@ int HttpProxyConnectJob::DoSSLConnect() {
   }
   next_state_ = STATE_SSL_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
+  const scoped_refptr<SSLSocketParams>& ssl_params = params_->ssl_params();
+  RequestPriority priority =
+      ssl_params->GetDirectConnectionParams()->destination().priority();
   return transport_socket_handle_->Init(
-      group_name(), params_->ssl_params(),
-      params_->ssl_params()->transport_params()->destination().priority(),
-      callback_, ssl_pool_, net_log());
+      group_name(), ssl_params, priority, callback_, ssl_pool_, net_log());
 }
 
 int HttpProxyConnectJob::DoSSLConnectComplete(int result) {
diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc
index 2e54fdf..0f1f8b6 100644
--- a/net/http/http_proxy_client_socket_pool_unittest.cc
+++ b/net/http/http_proxy_client_socket_pool_unittest.cc
@@ -73,7 +73,6 @@ class HttpProxyClientSocketPoolTest
             new SSLSocketParams(ignored_transport_socket_params_,
                                 NULL,
                                 NULL,
-                                ProxyServer::SCHEME_DIRECT,
                                 HostPortPair("www.google.com", 443),
                                 ssl_config_,
                                 kPrivacyModeDisabled,
diff --git a/net/socket/client_socket_pool_manager.cc b/net/socket/client_socket_pool_manager.cc
index 71496d2..cb49e08 100644
--- a/net/socket/client_socket_pool_manager.cc
+++ b/net/socket/client_socket_pool_manager.cc
@@ -182,7 +182,6 @@ int InitSocketPoolHelper(const GURL& request_url,
         ssl_params = new SSLSocketParams(proxy_tcp_params,
                                          NULL,
                                          NULL,
-                                         ProxyServer::SCHEME_DIRECT,
                                          *proxy_host_port.get(),
                                          ssl_config_for_proxy,
                                          kPrivacyModeDisabled,
@@ -229,7 +228,6 @@ int InitSocketPoolHelper(const GURL& request_url,
         new SSLSocketParams(tcp_params,
                             socks_params,
                             http_proxy_params,
-                            proxy_info.proxy_server().scheme(),
                             origin_host_port,
                             ssl_config_for_origin,
                             privacy_mode,
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index d07c76f..a0f9d29 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -26,20 +26,18 @@
 namespace net {
 
 SSLSocketParams::SSLSocketParams(
-    const scoped_refptr<TransportSocketParams>& transport_params,
-    const scoped_refptr<SOCKSSocketParams>& socks_params,
+    const scoped_refptr<TransportSocketParams>& direct_params,
+    const scoped_refptr<SOCKSSocketParams>& socks_proxy_params,
     const scoped_refptr<HttpProxySocketParams>& http_proxy_params,
-    ProxyServer::Scheme proxy,
     const HostPortPair& host_and_port,
     const SSLConfig& ssl_config,
     PrivacyMode privacy_mode,
     int load_flags,
     bool force_spdy_over_ssl,
     bool want_spdy_over_npn)
-    : transport_params_(transport_params),
+    : direct_params_(direct_params),
+      socks_proxy_params_(socks_proxy_params),
       http_proxy_params_(http_proxy_params),
-      socks_params_(socks_params),
-      proxy_(proxy),
       host_and_port_(host_and_port),
       ssl_config_(ssl_config),
       privacy_mode_(privacy_mode),
@@ -47,35 +45,55 @@ SSLSocketParams::SSLSocketParams(
       force_spdy_over_ssl_(force_spdy_over_ssl),
       want_spdy_over_npn_(want_spdy_over_npn),
       ignore_limits_(false) {
-  switch (proxy_) {
-    case ProxyServer::SCHEME_DIRECT:
-      DCHECK(transport_params_.get() != NULL);
-      DCHECK(http_proxy_params_.get() == NULL);
-      DCHECK(socks_params_.get() == NULL);
-      ignore_limits_ = transport_params_->ignore_limits();
-      break;
-    case ProxyServer::SCHEME_HTTP:
-    case ProxyServer::SCHEME_HTTPS:
-      DCHECK(transport_params_.get() == NULL);
-      DCHECK(http_proxy_params_.get() != NULL);
-      DCHECK(socks_params_.get() == NULL);
-      ignore_limits_ = http_proxy_params_->ignore_limits();
-      break;
-    case ProxyServer::SCHEME_SOCKS4:
-    case ProxyServer::SCHEME_SOCKS5:
-      DCHECK(transport_params_.get() == NULL);
-      DCHECK(http_proxy_params_.get() == NULL);
-      DCHECK(socks_params_.get() != NULL);
-      ignore_limits_ = socks_params_->ignore_limits();
-      break;
-    default:
-      LOG(DFATAL) << "unknown proxy type";
-      break;
+  if (direct_params_) {
+    DCHECK(!socks_proxy_params_);
+    DCHECK(!http_proxy_params_);
+    ignore_limits_ = direct_params_->ignore_limits();
+  } else if (socks_proxy_params_) {
+    DCHECK(!http_proxy_params_);
+    ignore_limits_ = socks_proxy_params_->ignore_limits();
+  } else {
+    DCHECK(http_proxy_params_);
+    ignore_limits_ = http_proxy_params_->ignore_limits();
   }
 }
 
 SSLSocketParams::~SSLSocketParams() {}
 
+SSLSocketParams::ConnectionType SSLSocketParams::GetConnectionType() const {
+  if (direct_params_) {
+    DCHECK(!socks_proxy_params_);
+    DCHECK(!http_proxy_params_);
+    return DIRECT;
+  }
+
+  if (socks_proxy_params_) {
+    DCHECK(!http_proxy_params_);
+    return SOCKS_PROXY;
+  }
+
+  DCHECK(http_proxy_params_);
+  return HTTP_PROXY;
+}
+
+const scoped_refptr<TransportSocketParams>&
+SSLSocketParams::GetDirectConnectionParams() const {
+  DCHECK_EQ(GetConnectionType(), DIRECT);
+  return direct_params_;
+}
+
+const scoped_refptr<SOCKSSocketParams>&
+SSLSocketParams::GetSocksProxyConnectionParams() const {
+  DCHECK_EQ(GetConnectionType(), SOCKS_PROXY);
+  return socks_proxy_params_;
+}
+
+const scoped_refptr<HttpProxySocketParams>&
+SSLSocketParams::GetHttpProxyConnectionParams() const {
+  DCHECK_EQ(GetConnectionType(), HTTP_PROXY);
+  return http_proxy_params_;
+}
+
 // Timeout for the SSL handshake portion of the connect.
 static const int kSSLHandshakeTimeoutInSeconds = 30;
 
@@ -201,11 +219,11 @@ int SSLConnectJob::DoTransportConnect() {
 
   next_state_ = STATE_TRANSPORT_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
-  scoped_refptr<TransportSocketParams> transport_params =
-      params_->transport_params();
+  scoped_refptr<TransportSocketParams> direct_params =
+      params_->GetDirectConnectionParams();
   return transport_socket_handle_->Init(
-      group_name(), transport_params,
-      transport_params->destination().priority(), callback_, transport_pool_,
+      group_name(), direct_params,
+      direct_params->destination().priority(), callback_, transport_pool_,
       net_log());
 }
 
@@ -220,9 +238,11 @@ int SSLConnectJob::DoSOCKSConnect() {
   DCHECK(socks_pool_);
   next_state_ = STATE_SOCKS_CONNECT_COMPLETE;
   transport_socket_handle_.reset(new ClientSocketHandle());
-  scoped_refptr<SOCKSSocketParams> socks_params = params_->socks_params();
+  scoped_refptr<SOCKSSocketParams> socks_proxy_params =
+      params_->GetSocksProxyConnectionParams();
   return transport_socket_handle_->Init(
-      group_name(), socks_params, socks_params->destination().priority(),
+      group_name(), socks_proxy_params,
+      socks_proxy_params->destination().priority(),
       callback_, socks_pool_, net_log());
 }
 
@@ -239,7 +259,7 @@ int SSLConnectJob::DoTunnelConnect() {
 
   transport_socket_handle_.reset(new ClientSocketHandle());
   scoped_refptr<HttpProxySocketParams> http_proxy_params =
-      params_->http_proxy_params();
+      params_->GetHttpProxyConnectionParams();
   return transport_socket_handle_->Init(
       group_name(), http_proxy_params,
       http_proxy_params->destination().priority(), callback_, http_proxy_pool_,
@@ -420,23 +440,22 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
   return result;
 }
 
-int SSLConnectJob::ConnectInternal() {
-  switch (params_->proxy()) {
-    case ProxyServer::SCHEME_DIRECT:
-      next_state_ = STATE_TRANSPORT_CONNECT;
-      break;
-    case ProxyServer::SCHEME_HTTP:
-    case ProxyServer::SCHEME_HTTPS:
-      next_state_ = STATE_TUNNEL_CONNECT;
-      break;
-    case ProxyServer::SCHEME_SOCKS4:
-    case ProxyServer::SCHEME_SOCKS5:
-      next_state_ = STATE_SOCKS_CONNECT;
-      break;
-    default:
-      NOTREACHED() << "unknown proxy type";
-      break;
+SSLConnectJob::State SSLConnectJob::GetInitialState(
+    SSLSocketParams::ConnectionType connection_type) {
+  switch (connection_type) {
+    case SSLSocketParams::DIRECT:
+      return STATE_TRANSPORT_CONNECT;
+    case SSLSocketParams::HTTP_PROXY:
+      return STATE_TUNNEL_CONNECT;
+    case SSLSocketParams::SOCKS_PROXY:
+      return STATE_SOCKS_CONNECT;
   }
+  NOTREACHED();
+  return STATE_NONE;
+}
+
+int SSLConnectJob::ConnectInternal() {
+  next_state_ = GetInitialState(params_->GetConnectionType());
   return DoLoop(OK);
 }
 
diff --git a/net/socket/ssl_client_socket_pool.h b/net/socket/ssl_client_socket_pool.h
index 273029b..47551f1 100644
--- a/net/socket/ssl_client_socket_pool.h
+++ b/net/socket/ssl_client_socket_pool.h
@@ -13,7 +13,6 @@
 #include "net/base/privacy_mode.h"
 #include "net/dns/host_resolver.h"
 #include "net/http/http_response_info.h"
-#include "net/proxy/proxy_server.h"
 #include "net/socket/client_socket_pool.h"
 #include "net/socket/client_socket_pool_base.h"
 #include "net/socket/client_socket_pool_histograms.h"
@@ -35,32 +34,39 @@ class TransportClientSocketPool;
 class TransportSecurityState;
 class TransportSocketParams;
 
-// SSLSocketParams only needs the socket params for the transport socket
-// that will be used (denoted by |proxy|).
 class NET_EXPORT_PRIVATE SSLSocketParams
     : public base::RefCounted<SSLSocketParams> {
  public:
-  SSLSocketParams(const scoped_refptr<TransportSocketParams>& transport_params,
-                  const scoped_refptr<SOCKSSocketParams>& socks_params,
-                  const scoped_refptr<HttpProxySocketParams>& http_proxy_params,
-                  ProxyServer::Scheme proxy,
-                  const HostPortPair& host_and_port,
-                  const SSLConfig& ssl_config,
-                  PrivacyMode privacy_mode,
-                  int load_flags,
-                  bool force_spdy_over_ssl,
-                  bool want_spdy_over_npn);
-
-  const scoped_refptr<TransportSocketParams>& transport_params() {
-      return transport_params_;
-  }
-  const scoped_refptr<HttpProxySocketParams>& http_proxy_params() {
-    return http_proxy_params_;
-  }
-  const scoped_refptr<SOCKSSocketParams>& socks_params() {
-    return socks_params_;
-  }
-  ProxyServer::Scheme proxy() const { return proxy_; }
+  enum ConnectionType { DIRECT, SOCKS_PROXY, HTTP_PROXY };
+
+  // Exactly one of |direct_params|, |socks_proxy_params|, and
+  // |http_proxy_params| must be non-NULL.
+  SSLSocketParams(
+      const scoped_refptr<TransportSocketParams>& direct_params,
+      const scoped_refptr<SOCKSSocketParams>& socks_proxy_params,
+      const scoped_refptr<HttpProxySocketParams>& http_proxy_params,
+      const HostPortPair& host_and_port,
+      const SSLConfig& ssl_config,
+      PrivacyMode privacy_mode,
+      int load_flags,
+      bool force_spdy_over_ssl,
+      bool want_spdy_over_npn);
+
+  // Returns the type of the underlying connection.
+  ConnectionType GetConnectionType() const;
+
+  // Must be called only when GetConnectionType() returns DIRECT.
+  const scoped_refptr<TransportSocketParams>&
+      GetDirectConnectionParams() const;
+
+  // Must be called only when GetConnectionType() returns SOCKS_PROXY.
+  const scoped_refptr<SOCKSSocketParams>&
+      GetSocksProxyConnectionParams() const;
+
+  // Must be called only when GetConnectionType() returns HTTP_PROXY.
+  const scoped_refptr<HttpProxySocketParams>&
+      GetHttpProxyConnectionParams() const;
+
   const HostPortPair& host_and_port() const { return host_and_port_; }
   const SSLConfig& ssl_config() const { return ssl_config_; }
   PrivacyMode privacy_mode() const { return privacy_mode_; }
@@ -73,10 +79,9 @@ class NET_EXPORT_PRIVATE SSLSocketParams
   friend class base::RefCounted<SSLSocketParams>;
   ~SSLSocketParams();
 
-  const scoped_refptr<TransportSocketParams> transport_params_;
+  const scoped_refptr<TransportSocketParams> direct_params_;
+  const scoped_refptr<SOCKSSocketParams> socks_proxy_params_;
   const scoped_refptr<HttpProxySocketParams> http_proxy_params_;
-  const scoped_refptr<SOCKSSocketParams> socks_params_;
-  const ProxyServer::Scheme proxy_;
   const HostPortPair host_and_port_;
   const SSLConfig ssl_config_;
   const PrivacyMode privacy_mode_;
@@ -138,6 +143,10 @@ class SSLConnectJob : public ConnectJob {
   int DoSSLConnect();
   int DoSSLConnectComplete(int result);
 
+  // Returns the initial state for the state machine based on the
+  // |connection_type|.
+  static State GetInitialState(SSLSocketParams::ConnectionType connection_type);
+
   // Starts the SSL connection process.  Returns OK on success and
   // ERR_IO_PENDING if it cannot immediately service the request.
   // Otherwise, it returns a net error code.
diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc
index 280f6e7..8310441 100644
--- a/net/socket/ssl_client_socket_pool_unittest.cc
+++ b/net/socket/ssl_client_socket_pool_unittest.cc
@@ -159,7 +159,6 @@ class SSLClientSocketPoolTest
                                             : NULL,
         proxy == ProxyServer::SCHEME_SOCKS5 ? socks_socket_params_ : NULL,
         proxy == ProxyServer::SCHEME_HTTP ? http_proxy_socket_params_ : NULL,
-        proxy,
         HostPortPair("host", 443),
         ssl_config_,
         kPrivacyModeDisabled,
diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc
index 443779b..af3b337 100644
--- a/net/spdy/spdy_test_util_common.cc
+++ b/net/spdy/spdy_test_util_common.cc
@@ -509,13 +509,10 @@ base::WeakPtr<SpdySession> CreateSpdySessionHelper(
   int rv = ERR_UNEXPECTED;
   if (is_secure) {
     SSLConfig ssl_config;
-    scoped_refptr<SOCKSSocketParams> socks_params;
-    scoped_refptr<HttpProxySocketParams> http_proxy_params;
     scoped_refptr<SSLSocketParams> ssl_params(
         new SSLSocketParams(transport_params,
-                            socks_params,
-                            http_proxy_params,
-                            ProxyServer::SCHEME_DIRECT,
+                            NULL,
+                            NULL,
                             key.host_port_pair(),
                             ssl_config,
                             key.privacy_mode(),