Close idle H2 sockets when SpdySession is initialized.

net/socket/client_socket_pool_manager_impl.cc

Index: net/socket/client_socket_pool_manager_impl.cc
diff --git a/net/socket/client_socket_pool_manager_impl.cc b/net/socket/client_socket_pool_manager_impl.cc
index 26348cbb7afacbc70b9c524357087b3507ef97be..c1a5941d4f8c58d7ad8b326bc50f18375751f9bf 100644
--- a/net/socket/client_socket_pool_manager_impl.cc
+++ b/net/socket/client_socket_pool_manager_impl.cc
@@ -154,52 +154,60 @@ void ClientSocketPoolManagerImpl::FlushSocketPoolsWithError(int error) {
 void ClientSocketPoolManagerImpl::CloseIdleSockets() {
   // Close sockets in the highest level pools first, since higher level pools'
   // sockets may release stuff to the lower level pools.

[davidben, 2017/03/02 22:00:51]

Nit: it => pool or pair or something? (With a loop like this, these aren't
iterators anymore.)

[xunjieli, 2017/03/02 23:04:00]

Acknowledged. No longer apply :)

-  for (SSLSocketPoolMap::const_iterator it =
-       ssl_socket_pools_for_proxies_.begin();
-       it != ssl_socket_pools_for_proxies_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : ssl_socket_pools_for_proxies_)
+    it.second->CloseIdleSockets();
 
-  for (HTTPProxySocketPoolMap::const_iterator it =
-       http_proxy_socket_pools_.begin();
-       it != http_proxy_socket_pools_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : http_proxy_socket_pools_)
+    it.second->CloseIdleSockets();
 
-  for (SSLSocketPoolMap::const_iterator it =
-       ssl_socket_pools_for_https_proxies_.begin();
-       it != ssl_socket_pools_for_https_proxies_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : ssl_socket_pools_for_https_proxies_)
+    it.second->CloseIdleSockets();
 
-  for (TransportSocketPoolMap::const_iterator it =
-       transport_socket_pools_for_https_proxies_.begin();
-       it != transport_socket_pools_for_https_proxies_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : transport_socket_pools_for_https_proxies_)
+    it.second->CloseIdleSockets();
 
-  for (TransportSocketPoolMap::const_iterator it =
-       transport_socket_pools_for_http_proxies_.begin();
-       it != transport_socket_pools_for_http_proxies_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : transport_socket_pools_for_http_proxies_)
+    it.second->CloseIdleSockets();
 
-  for (SOCKSSocketPoolMap::const_iterator it =
-       socks_socket_pools_.begin();
-       it != socks_socket_pools_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : socks_socket_pools_)
+    it.second->CloseIdleSockets();
 
-  for (TransportSocketPoolMap::const_iterator it =
-       transport_socket_pools_for_socks_proxies_.begin();
-       it != transport_socket_pools_for_socks_proxies_.end();
-       ++it)
-    it->second->CloseIdleSockets();
+  for (const auto& it : transport_socket_pools_for_socks_proxies_)
+    it.second->CloseIdleSockets();
 
   ssl_socket_pool_->CloseIdleSockets();
   transport_socket_pool_->CloseIdleSockets();
 }
 
+void ClientSocketPoolManagerImpl::CloseIdleSocketsInGroup(
+    const std::string& group_name) {

[davidben, 2017/03/02 22:00:51]

This will close idle sockets with the same group name in all pools, rather than
just the one that the socket was made in. Is that quite right? I think group
names might only be unique within a single pool?

I see the SpdySession code is using ClientSocketHandle::group_name to get the
group name out. The ClientSocketHandle also has a pool_ pointer back to the
pool. Seems you could just do:

void ClientSocketHandle::CloseIdleSocketsInGroup() {
  if (pool_)
    pool_->CloseIdleSocketsInGroup(group_name_);
}

connection_->CloseIdleSocketsInGroup();

This would also save you from routing HttpNetworkSession into everything. :-)

[xunjieli, 2017/03/02 23:04:00]

That's very neat! Done. Thanks for the suggestion.

+  // Close sockets in the highest level pools first, since higher level pools'
+  // sockets may release stuff to the lower level pools.
+  for (const auto& it : ssl_socket_pools_for_proxies_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : http_proxy_socket_pools_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : ssl_socket_pools_for_https_proxies_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : transport_socket_pools_for_https_proxies_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : transport_socket_pools_for_http_proxies_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : socks_socket_pools_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  for (const auto& it : transport_socket_pools_for_socks_proxies_)
+    it.second->CloseIdleSocketsInGroup(group_name);
+
+  ssl_socket_pool_->CloseIdleSocketsInGroup(group_name);
+  transport_socket_pool_->CloseIdleSocketsInGroup(group_name);
+}
+
 TransportClientSocketPool*
 ClientSocketPoolManagerImpl::GetTransportSocketPool() {
   return transport_socket_pool_.get();