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

net/spdy/spdy_session_pool.cc

Index: net/spdy/spdy_session_pool.cc
===================================================================
--- net/spdy/spdy_session_pool.cc	(revision 92397)
+++ net/spdy/spdy_session_pool.cc	(working copy)
@@ -150,6 +150,16 @@
       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);
@@ -302,16 +312,6 @@
   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;
 }
 
@@ -353,17 +353,6 @@
 
 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;