Changed SPDY's ip connection pooling logic to only add the used IP,

net/spdy/spdy_session_pool.cc

 // Copyright (c) 2011 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.
 
 #include "net/spdy/spdy_session_pool.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
 #include "net/base/address_list.h"
@@ skipping 132 matching lines @@
   if (!list)
     list = AddSessionList(host_port_proxy_pair);
   DCHECK(list->empty());
   list->push_back(*spdy_session);
 
   net_log.AddEvent(
       NetLog::TYPE_SPDY_SESSION_POOL_IMPORTED_SESSION_FROM_SOCKET,
       make_scoped_refptr(new NetLogSourceParameter(
           "session", (*spdy_session)->net_log().source())));
 
+  // We have a new session.  Lookup the IP addresses for this session so that

[willchan no longer on Chromium, 2011/07/14 12:35:39]

There should only be a single IP address. You should probably delete the
AddAliases() code and just do the AddressList=>IPEndPoint conversion here, and
assert that GetPeerAddress() returns only a single entry.

I need to think about proxies. I don't know if IP connection pooling works in
this case, because this GetPeerAddress() thing will report the proxy's address
instead of the origin server. Please add check to see if this is a direct
connection (it's part of the ProxyServer object in the HostPortProxyPair). The
condition should be added to the g_enable_ip_pooling condition check. This is an
additional bugfix.

[wtc, 2011/07/14 17:03:14]

I came to the same conclusion after I went home last night
that this fix won't work if a proxy is used.
I wonder if mbelshe's algorithm also intended to make IP
connection pooling work when proxies are used.  It is bad
that we're still resolving hostnames when a proxy is used.

[willchan no longer on Chromium, 2011/07/14 17:10:51]

We don't resolve the hostnames here. LookupAddresses() only uses cached entries.

+  // we can match future Sessions (potentially to different domains) which can
+  // potentially be pooled with this one.
+  if (g_enable_ip_pooling) {
+    // Get connected address from spdy_session.
+    AddressList addresses;
+    if (connection->socket()->GetPeerAddress(&addresses) == net::OK)
+      AddAliases(addresses, host_port_proxy_pair);
+  }
+
   // Now we can initialize the session with the SSL socket.
   return (*spdy_session)->InitializeWithSocket(connection, is_secure,
                                                certificate_error_code);
 }
 
 bool SpdySessionPool::HasSession(
     const HostPortProxyPair& host_port_proxy_pair) const {
   if (GetSessionList(host_port_proxy_pair))
     return true;
 
@@ skipping 132 matching lines @@
   return *single_domain_pair;
 }
 
 SpdySessionPool::SpdySessionList*
     SpdySessionPool::AddSessionList(
         const HostPortProxyPair& host_port_proxy_pair) {
   const HostPortProxyPair& pair = NormalizeListPair(host_port_proxy_pair);
   DCHECK(sessions_.find(pair) == sessions_.end());
   SpdySessionPool::SpdySessionList* list = new SpdySessionList();
   sessions_[pair] = list;
-
-  // We have a new session.  Lookup the IP addresses for this session so that
-  // we can match future Sessions (potentially to different domains) which can
-  // potentially be pooled with this one.
-  if (g_enable_ip_pooling) {
-    AddressList addresses;
-    if (LookupAddresses(host_port_proxy_pair, &addresses))
-      AddAliases(addresses, host_port_proxy_pair);
-  }
-
   return list;
 }
 
 SpdySessionPool::SpdySessionList*
     SpdySessionPool::GetSessionList(
         const HostPortProxyPair& host_port_proxy_pair) const {
   const HostPortProxyPair& pair = NormalizeListPair(host_port_proxy_pair);
   SpdySessionsMap::const_iterator it = sessions_.find(pair);
   if (it != sessions_.end())
     return it->second;
@@ skipping 21 matching lines @@
                               addresses,
                               NULL,
                               NULL,
                               net::BoundNetLog());
   DCHECK_NE(ERR_IO_PENDING, rv);
   return rv == OK;
 }
 
 void SpdySessionPool::AddAliases(const AddressList& addresses,
                                  const HostPortProxyPair& pair) {
-  // Note:  it is possible to think of strange overlapping sets of ip addresses
-  // for hosts such that a new session can override the alias for an IP
-  // address that was previously aliased to a different host.  This is probably
-  // undesirable, but seemingly unlikely and complicated to fix.
-  //    Example:
-  //      host1 = 1.1.1.1, 1.1.1.4
-  //      host2 = 1.1.1.4, 1.1.1.5
-  //      host3 = 1.1.1.3, 1.1.1.5
-  //      Creating session1 (to host1), creates an alias for host2 to host1.
-  //      Creating session2 (to host3), overrides the alias for host2 to host3.
-
   const addrinfo* address = addresses.head();
   while (address) {
     IPEndPoint endpoint;
     endpoint.FromSockAddr(address->ai_addr, address->ai_addrlen);
     aliases_[endpoint] = pair;
     address = address->ai_next;
   }
 }
 
 void SpdySessionPool::RemoveAliases(const HostPortProxyPair& pair) {
@@ skipping 45 matching lines @@
       delete list;
       RemoveAliases(old_map.begin()->first);
       old_map.erase(old_map.begin()->first);
     }
   }
   DCHECK(sessions_.empty());
   DCHECK(aliases_.empty());
 }
 
 }  // namespace net