[SPDY] Refactor and clean up SpdySessionPool

Replace the map of keys to lists of sessions to a map
of keys to sessions, since we were already limiting
the lists to be of size 1 anyway.

Remove the functions from SpdySessionPool that were
used mostly by tests. Replace them with functions
in spdy_test_util_common.{h,cc}. Make a bunch of
tests use these functions.

Make the Close*Sessions() behave consistently.

Make SpdySession::CloseSessionOnError() always remove
the session from its pool.

Move the SpdySessionPool tests for spdy_session_unittest.cc
to a new file spdy_session_pool_unittest.cc. Add
a few more for the Close*Sessions() tests.

Remove --max-spdy-sessions-per-domain switch.

Parametrize HttpStreamFactoryTest on NextProto.

Misc. cleanup here and there.

BUG=255701
TBR=rch@chromium.org

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

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

chrome/browser/io_thread.cc

@@ -673,10 +673,6 @@ void IOThread::InitializeNetworkOptions(const CommandLine& command_line) {
     // Enable WebSocket over SPDY.
     net::WebSocketJob::set_websocket_over_spdy_enabled(true);
   }
-  if (command_line.HasSwitch(switches::kMaxSpdySessionsPerDomain)) {
-    globals_->max_spdy_sessions_per_domain.set(
-        GetSwitchValueAsInt(command_line, switches::kMaxSpdySessionsPerDomain));
-  }
   if (command_line.HasSwitch(switches::kMaxSpdyConcurrentStreams)) {
     globals_->max_spdy_concurrent_streams_limit.set(
         GetSwitchValueAsInt(command_line, switches::kMaxSpdyConcurrentStreams));
@@ -865,8 +861,6 @@ void IOThread::InitializeNetworkSessionParams(
   params->testing_fixed_http_port = globals_->testing_fixed_http_port;
   params->testing_fixed_https_port = globals_->testing_fixed_https_port;
 
-  globals_->max_spdy_sessions_per_domain.CopyToIfSet(
-      &params->max_spdy_sessions_per_domain);
   globals_->initial_max_spdy_concurrent_streams.CopyToIfSet(
       &params->spdy_initial_max_concurrent_streams);
   globals_->max_spdy_concurrent_streams_limit.CopyToIfSet(

chrome/browser/io_thread.h

@@ -154,7 +154,6 @@ class IOThread : public content::BrowserThreadDelegate {
     bool http_pipelining_enabled;
     uint16 testing_fixed_http_port;
     uint16 testing_fixed_https_port;
-    Optional<size_t> max_spdy_sessions_per_domain;
     Optional<size_t> initial_max_spdy_concurrent_streams;
     Optional<size_t> max_spdy_concurrent_streams_limit;
     Optional<bool> force_spdy_single_domain;

chrome/common/chrome_switches.cc

@@ -1424,9 +1424,6 @@ const char kUseSpdy[]                       = "use-spdy";
 // This will only work if asynchronous spell checking is not disabled.
 const char kUseSpellingSuggestions[]        = "use-spelling-suggestions";
 
-// Sets the maximum SPDY sessions per domain.
-const char kMaxSpdySessionsPerDomain[]      = "max-spdy-sessions-per-domain";
-
 // Sets the maximum concurrent streams over a SPDY session.
 const char kMaxSpdyConcurrentStreams[]      = "max-spdy-concurrent-streams";
 

chrome/common/chrome_switches.h

@@ -379,7 +379,6 @@ extern const char kUnlimitedStorage[];
 extern const char kUseSimpleCacheBackend[];
 extern const char kUseSpdy[];
 extern const char kUseSpellingSuggestions[];
-extern const char kMaxSpdySessionsPerDomain[];
 extern const char kMaxSpdyConcurrentStreams[];
 extern const char kUserDataDir[];
 extern const char kValidateCrx[];

net/http/http_network_session.cc

@@ -70,7 +70,6 @@ HttpNetworkSession::Params::Params()
       http_pipelining_enabled(false),
       testing_fixed_http_port(0),
       testing_fixed_https_port(0),
-      max_spdy_sessions_per_domain(0),
       force_spdy_single_domain(false),
       enable_spdy_ip_pooling(true),
       enable_spdy_credential_frames(false),
@@ -114,7 +113,6 @@ HttpNetworkSession::HttpNetworkSession(const Params& params)
       spdy_session_pool_(params.host_resolver,
                          params.ssl_config_service,
                          params.http_server_properties,
-                         params.max_spdy_sessions_per_domain,
                          params.force_spdy_single_domain,
                          params.enable_spdy_ip_pooling,
                          params.enable_spdy_credential_frames,
@@ -208,7 +206,7 @@ void HttpNetworkSession::CloseAllConnections() {
 void HttpNetworkSession::CloseIdleConnections() {
   normal_socket_pool_manager_->CloseIdleSockets();
   websocket_socket_pool_manager_->CloseIdleSockets();
-  spdy_session_pool_.CloseIdleSessions();
+  spdy_session_pool_.CloseCurrentIdleSessions();
 }
 
 ClientSocketPoolManager* HttpNetworkSession::GetSocketPoolManager(

net/http/http_network_session.h

@@ -73,7 +73,6 @@ class NET_EXPORT HttpNetworkSession
     bool http_pipelining_enabled;
     uint16 testing_fixed_http_port;
     uint16 testing_fixed_https_port;
-    size_t max_spdy_sessions_per_domain;
     bool force_spdy_single_domain;
     bool enable_spdy_ip_pooling;
     bool enable_spdy_credential_frames;

net/http/http_network_transaction_unittest.cc

@@ -8856,41 +8856,7 @@ TEST_P(HttpNetworkTransactionTest,
   SpdySessionKey key(host_port_pair, ProxyServer::Direct(),
                      kPrivacyModeDisabled);
   scoped_refptr<SpdySession> spdy_session =
-      session->spdy_session_pool()->Get(key, BoundNetLog());
-  scoped_refptr<TransportSocketParams> transport_params(
-      new TransportSocketParams(host_port_pair, MEDIUM, false, false,
-                                OnHostResolutionCallback()));
-
-  scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
-  EXPECT_EQ(ERR_IO_PENDING,
-            connection->Init(host_port_pair.ToString(),
-                             transport_params,
-                             LOWEST,
-                             callback.callback(),
-                             session->GetTransportSocketPool(
-                                 HttpNetworkSession::NORMAL_SOCKET_POOL),
-                             BoundNetLog()));
-  EXPECT_EQ(OK, callback.WaitForResult());
-
-  SSLConfig ssl_config;
-  session->ssl_config_service()->GetSSLConfig(&ssl_config);
-  scoped_ptr<ClientSocketHandle> ssl_connection(new ClientSocketHandle);
-  SSLClientSocketContext context;
-  context.cert_verifier = session_deps_.cert_verifier.get();
-  context.transport_security_state =
-      session_deps_.transport_security_state.get();
-  ssl_connection->set_socket(
-      session_deps_.socket_factory->CreateSSLClientSocket(
-          connection.release(),
-          HostPortPair(std::string(), 443),
-          ssl_config,
-          context));
-  EXPECT_EQ(ERR_IO_PENDING,
-            ssl_connection->socket()->Connect(callback.callback()));
-  EXPECT_EQ(OK, callback.WaitForResult());
-
-  EXPECT_EQ(OK, spdy_session->InitializeWithSocket(ssl_connection.release(),
-                                                   true, OK));
+      CreateSecureSpdySession(session, key, BoundNetLog());
 
   trans.reset(new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
 
@@ -10022,23 +9988,7 @@ TEST_P(HttpNetworkTransactionTest, PreconnectWithExistingSpdySession) {
   SpdySessionKey key(host_port_pair, ProxyServer::Direct(),
                      kPrivacyModeDisabled);
   scoped_refptr<SpdySession> spdy_session =
-      session->spdy_session_pool()->Get(key, BoundNetLog());
-  scoped_refptr<TransportSocketParams> transport_params(
-      new TransportSocketParams(host_port_pair, MEDIUM, false, false,
-                                OnHostResolutionCallback()));
-  TestCompletionCallback callback;
-
-  scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
-  EXPECT_EQ(ERR_IO_PENDING,
-            connection->Init(host_port_pair.ToString(),
-                             transport_params,
-                             LOWEST,
-                             callback.callback(),
-                             session->GetTransportSocketPool(
-                                 HttpNetworkSession::NORMAL_SOCKET_POOL),
-                             BoundNetLog()));
-  EXPECT_EQ(OK, callback.WaitForResult());
-  spdy_session->InitializeWithSocket(connection.release(), false, OK);
+      CreateInsecureSpdySession(session, key, BoundNetLog());
 
   HttpRequestInfo request;
   request.method = "GET";
@@ -10051,6 +10001,7 @@ TEST_P(HttpNetworkTransactionTest, PreconnectWithExistingSpdySession) {
   scoped_ptr<HttpTransaction> trans(
       new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
 
+  TestCompletionCallback callback;
   int rv = trans->Start(&request, callback.callback(), BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_EQ(OK, callback.WaitForResult());
@@ -11386,7 +11337,7 @@ TEST_P(HttpNetworkTransactionTest, CloseIdleSpdySessionToOpenNewOne) {
   SpdySessionKey spdy_session_key_a(
       host_port_pair_a, ProxyServer::Direct(), kPrivacyModeDisabled);
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_a));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_a));
 
   TestCompletionCallback callback;
   HttpRequestInfo request1;
@@ -11412,13 +11363,13 @@ TEST_P(HttpNetworkTransactionTest, CloseIdleSpdySessionToOpenNewOne) {
   EXPECT_EQ("hello!", response_data);
   trans.reset();
   EXPECT_TRUE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_a));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_a));
 
   HostPortPair host_port_pair_b("www.b.com", 443);
   SpdySessionKey spdy_session_key_b(
       host_port_pair_b, ProxyServer::Direct(), kPrivacyModeDisabled);
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_b));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_b));
   HttpRequestInfo request2;
   request2.method = "GET";
   request2.url = GURL("https://www.b.com/");
@@ -11438,15 +11389,15 @@ TEST_P(HttpNetworkTransactionTest, CloseIdleSpdySessionToOpenNewOne) {
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_a));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_a));
   EXPECT_TRUE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_b));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_b));
 
   HostPortPair host_port_pair_a1("www.a.com", 80);
   SpdySessionKey spdy_session_key_a1(
       host_port_pair_a1, ProxyServer::Direct(), kPrivacyModeDisabled);
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_a1));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_a1));
   HttpRequestInfo request3;
   request3.method = "GET";
   request3.url = GURL("http://www.a.com/");
@@ -11466,9 +11417,9 @@ TEST_P(HttpNetworkTransactionTest, CloseIdleSpdySessionToOpenNewOne) {
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_a));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_a));
   EXPECT_FALSE(
-      session->spdy_session_pool()->HasSession(spdy_session_key_b));
+      HasSpdySession(session->spdy_session_pool(), spdy_session_key_b));
 
   HttpStreamFactory::SetNextProtos(std::vector<std::string>());
 }

net/http/http_proxy_client_socket_pool.cc

@@ -204,7 +204,7 @@ int HttpProxyConnectJob::DoSSLConnect() {
     SpdySessionKey key(params_->destination().host_port_pair(),
                        ProxyServer::Direct(),
                        kPrivacyModeDisabled);
-    if (params_->spdy_session_pool()->HasSession(key)) {
+    if (params_->spdy_session_pool()->GetIfExists(key, net_log())) {
       using_spdy_ = true;
       next_state_ = STATE_SPDY_PROXY_CREATE_STREAM;
       return OK;
@@ -302,19 +302,19 @@ int HttpProxyConnectJob::DoSpdyProxyCreateStream() {
                      ProxyServer::Direct(),
                      kPrivacyModeDisabled);
   SpdySessionPool* spdy_pool = params_->spdy_session_pool();
-  scoped_refptr<SpdySession> spdy_session;
+  scoped_refptr<SpdySession> spdy_session =
+      spdy_pool->GetIfExists(key, net_log());
   // It's possible that a session to the proxy has recently been created
-  if (spdy_pool->HasSession(key)) {
+  if (spdy_session) {
     if (transport_socket_handle_.get()) {
       if (transport_socket_handle_->socket())
         transport_socket_handle_->socket()->Disconnect();
       transport_socket_handle_->Reset();
     }
-    spdy_session = spdy_pool->Get(key, net_log());
   } else {
     // Create a session direct to the proxy itself
     int rv = spdy_pool->GetSpdySessionFromSocket(
-        key, transport_socket_handle_.release(),
+        key, transport_socket_handle_.Pass(),
         net_log(), OK, &spdy_session, /*using_ssl_*/ true);
     if (rv < 0)
       return rv;

net/http/http_stream_factory_impl_job.cc

@@ -1099,7 +1099,7 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
     spdy_session = spdy_pool->GetIfExists(spdy_session_key, net_log_);
     if (!spdy_session.get()) {
       int error = spdy_pool->GetSpdySessionFromSocket(spdy_session_key,
-                                                      connection_.release(),
+                                                      connection_.Pass(),
                                                       net_log_,
                                                       spdy_certificate_error_,
                                                       &new_spdy_session_,