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

net/spdy/spdy_session_unittest.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.h"
 
 #include "net/spdy/spdy_io_buffer.h"
 #include "net/spdy/spdy_session_pool.h"
 #include "net/spdy/spdy_stream.h"
 #include "net/spdy/spdy_test_util.h"
 #include "testing/platform_test.h"
 
 namespace net {
 
 // TODO(cbentzel): Expose compression setter/getter in public SpdySession
 //                 interface rather than going through all these contortions.
 class SpdySessionTest : public PlatformTest {
  public:
   static void TurnOffCompression() {
     spdy::SpdyFramer::set_enable_compression_default(false);
   }
+
+  // This test has two variants, one for each style of closing the connection.

[willchan no longer on Chromium, 2011/07/17 01:23:45]

Did you just move this to gain the friend access? If so, please just use
SpdySessionPoolPeer.

Is this test changed in any way? Or is it just moved? It'd be easier for me to
see the real diff if it didn't get moved.

[ramant (doing other things), 2011/07/17 08:12:30]

Done.

+  // If |clean_via_close_current_sessions| is false, the sessions are closed
+  // manually, calling SpdySessionPool::Remove() directly.  If it is true,
+  // sessions are closed with SpdySessionPool::CloseCurrentSessions().
+  static void IPPoolingTest(bool clean_via_close_current_sessions) {
+    const int kTestPort = 80;
+    struct TestHosts {
+      std::string name;
+      std::string iplist;
+      HostPortProxyPair pair;
+    } test_hosts[] = {
+      { "www.foo.com",    "192.168.0.1,192.168.0.5" },
+      { "images.foo.com", "192.168.0.1,192.168.0.2,192.168.0.3" },
+      { "js.foo.com",     "192.168.0.4,192.168.0.3" },
+    };
+
+    SpdySessionDependencies session_deps;
+    session_deps.host_resolver->set_synchronous_mode(true);
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_hosts); i++) {
+      session_deps.host_resolver->rules()->AddIPLiteralRule(test_hosts[i].name,
+          test_hosts[i].iplist, "");
+
+      // This test requires that the HostResolver cache be populated.  Normal
+      // code would have done this already, but we do it manually.
+      HostResolver::RequestInfo info(
+          HostPortPair(test_hosts[i].name, kTestPort));
+      AddressList result;
+      session_deps.host_resolver->Resolve(
+          info, &result, NULL, NULL, BoundNetLog());
+
+      // Setup a HostPortProxyPair
+      test_hosts[i].pair = HostPortProxyPair(
+          HostPortPair(test_hosts[i].name, kTestPort), ProxyServer::Direct());
+    }
+
+    MockConnect connect_data(false, OK);
+    MockRead reads[] = {
+      MockRead(false, ERR_IO_PENDING)  // Stall forever.
+    };
+
+    StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
+    data.set_connect_data(connect_data);
+    session_deps.socket_factory->AddSocketDataProvider(&data);
+
+    SSLSocketDataProvider ssl(false, OK);
+    session_deps.socket_factory->AddSSLSocketDataProvider(&ssl);
+
+    scoped_refptr<HttpNetworkSession> http_session(
+        SpdySessionDependencies::SpdyCreateSession(&session_deps));
+
+    // Setup the first session to the first host.
+    SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool());
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[0].pair));
+    scoped_refptr<SpdySession> session =
+        spdy_session_pool->Get(test_hosts[0].pair, BoundNetLog());
+    EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[0].pair));
+
+    HostPortPair test_host_port_pair(test_hosts[0].name, kTestPort);
+    scoped_refptr<TransportSocketParams> transport_params(
+        new TransportSocketParams(test_host_port_pair,
+                            MEDIUM,
+                            GURL(),
+                            false,
+                            false));
+    scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
+    EXPECT_EQ(OK,
+              connection->Init(test_host_port_pair.ToString(),
+                               transport_params, MEDIUM,
+                               NULL, http_session->transport_socket_pool(),
+                               BoundNetLog()));
+    spdy_session_pool->AddAliases(test_hosts[0].pair);
+    EXPECT_EQ(OK,
+              session->InitializeWithSocket(connection.release(), false, OK));
+
+    // Flush the SpdySession::OnReadComplete() task.
+    MessageLoop::current()->RunAllPending();
+
+    // The third host has no overlap with the first, so it can't pool IPs.
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
+
+    // The second host overlaps with the first, and should IP pool.
+    EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[1].pair));
+
+    // Verify that the second host, through a proxy, won't share the IP.
+    HostPortProxyPair proxy_pair(test_hosts[1].pair.first,
+        ProxyServer::FromPacString("HTTP http://proxy.foo.com/"));
+    EXPECT_FALSE(spdy_session_pool->HasSession(proxy_pair));
+
+    // Overlap between 2 and 3 does is not transitive to 1.
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
+
+    // Create a new session to host 2.
+    scoped_refptr<SpdySession> session2 =
+        spdy_session_pool->Get(test_hosts[2].pair, BoundNetLog());
+
+    // Verify that we have sessions for everything.
+    EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[0].pair));
+    EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[1].pair));
+    EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[2].pair));
+
+    // Cleanup the sessions.
+    if (!clean_via_close_current_sessions) {
+      spdy_session_pool->Remove(session);
+      session = NULL;
+      spdy_session_pool->Remove(session2);
+      session2 = NULL;
+    } else {
+      spdy_session_pool->CloseCurrentSessions();
+    }
+
+    // Verify that the map is all cleaned up.
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[0].pair));
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[1].pair));
+    EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
+  }
+
 };
 
-namespace {
-
 // Test the SpdyIOBuffer class.
 TEST_F(SpdySessionTest, SpdyIOBuffer) {
   std::priority_queue<SpdyIOBuffer> queue_;
   const size_t kQueueSize = 100;
 
   // Insert 100 items; pri 100 to 1.
   for (size_t index = 0; index < kQueueSize; ++index) {
     SpdyIOBuffer buffer(new IOBuffer(), 0, kQueueSize - index, NULL);
     queue_.push(buffer);
   }
@@ skipping 60 matching lines @@
                                 MEDIUM,
                                 GURL(),
                                 false,
                                 false));
   scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
   EXPECT_EQ(OK,
             connection->Init(test_host_port_pair.ToString(),
                              transport_params, MEDIUM,
                              NULL, http_session->transport_socket_pool(),
                              BoundNetLog()));
+  spdy_session_pool->AddAliases(pair);

[willchan no longer on Chromium, 2011/07/17 01:23:45]

Why is this necessary?

[ramant (doing other things), 2011/07/17 08:12:30]

Done.

   EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
 
   // Flush the SpdySession::OnReadComplete() task.
   MessageLoop::current()->RunAllPending();
 
   EXPECT_FALSE(spdy_session_pool->HasSession(pair));
 
   scoped_refptr<SpdySession> session2 =
       spdy_session_pool->Get(pair, BoundNetLog());
 
@@ skipping 92 matching lines @@
                                 MEDIUM,
                                 GURL(),
                                 false,
                                 false));
   scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
   EXPECT_EQ(OK,
             connection->Init(test_host_port_pair.ToString(),
                              transport_params, MEDIUM,
                              NULL, http_session->transport_socket_pool(),
                              BoundNetLog()));
+  spdy_session_pool->AddAliases(pair);

[willchan no longer on Chromium, 2011/07/17 01:23:45]

Why is this necessary?

[ramant (doing other things), 2011/07/17 08:12:30]

Done.

   EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
 
   // Create 2 streams.  First will succeed.  Second will be pending.
   scoped_refptr<SpdyStream> spdy_stream1;
   TestCompletionCallback callback1;
   GURL url("http://www.google.com");
   EXPECT_EQ(OK,
             session->CreateStream(url,
                                   MEDIUM, /* priority, not important */
                                   &spdy_stream1,
@@ skipping 66 matching lines @@
                                 MEDIUM,
                                 GURL(),
                                 false,
                                 false));
   scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
   EXPECT_EQ(OK,
             connection->Init(test_host_port_pair.ToString(),
                              transport_params, MEDIUM,
                              NULL, http_session->transport_socket_pool(),
                              BoundNetLog()));
+  spdy_session_pool->AddAliases(pair);

[willchan no longer on Chromium, 2011/07/17 01:23:45]

Why is this necessary?

[ramant (doing other things), 2011/07/17 08:12:30]

Done.

   EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
 
   // Use scoped_ptr to let us invalidate the memory when we want to, to trigger
   // a valgrind error if the callback is invoked when it's not supposed to be.
   scoped_ptr<TestCompletionCallback> callback(new TestCompletionCallback);
 
   // Create 2 streams.  First will succeed.  Second will be pending.
   scoped_refptr<SpdyStream> spdy_stream1;
   GURL url("http://www.google.com");
   ASSERT_EQ(OK,
@@ skipping 80 matching lines @@
                                 MEDIUM,
                                 GURL(),
                                 false,
                                 false));
   scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
   EXPECT_EQ(OK,
             connection->Init(test_host_port_pair.ToString(),
                              transport_params, MEDIUM,
                              NULL, http_session->transport_socket_pool(),
                              BoundNetLog()));
+  spdy_session_pool->AddAliases(pair);

[willchan no longer on Chromium, 2011/07/17 01:23:45]

Ditto

[ramant (doing other things), 2011/07/17 08:12:30]

Done.

   EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(data.at_write_eof());
 }
 
-// This test has two variants, one for each style of closing the connection.
-// If |clean_via_close_current_sessions| is false, the sessions are closed
-// manually, calling SpdySessionPool::Remove() directly.  If it is true,
-// sessions are closed with SpdySessionPool::CloseCurrentSessions().
-void IPPoolingTest(bool clean_via_close_current_sessions) {
-  const int kTestPort = 80;
-  struct TestHosts {
-    std::string name;
-    std::string iplist;
-    HostPortProxyPair pair;
-  } test_hosts[] = {
-    { "www.foo.com",    "192.168.0.1,192.168.0.5" },
-    { "images.foo.com", "192.168.0.2,192.168.0.3,192.168.0.5" },
-    { "js.foo.com",     "192.168.0.4,192.168.0.3" },
-  };
-
-  SpdySessionDependencies session_deps;
-  session_deps.host_resolver->set_synchronous_mode(true);
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_hosts); i++) {
-    session_deps.host_resolver->rules()->AddIPLiteralRule(test_hosts[i].name,
-        test_hosts[i].iplist, "");
-
-    // This test requires that the HostResolver cache be populated.  Normal
-    // code would have done this already, but we do it manually.
-    HostResolver::RequestInfo info(HostPortPair(test_hosts[i].name, kTestPort));
-    AddressList result;
-    session_deps.host_resolver->Resolve(
-        info, &result, NULL, NULL, BoundNetLog());
-
-    // Setup a HostPortProxyPair
-    test_hosts[i].pair = HostPortProxyPair(
-        HostPortPair(test_hosts[i].name, kTestPort), ProxyServer::Direct());
-  }
-
-  MockConnect connect_data(false, OK);
-  MockRead reads[] = {
-    MockRead(false, ERR_IO_PENDING)  // Stall forever.
-  };
-
-  StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
-  data.set_connect_data(connect_data);
-  session_deps.socket_factory->AddSocketDataProvider(&data);
-
-  SSLSocketDataProvider ssl(false, OK);
-  session_deps.socket_factory->AddSSLSocketDataProvider(&ssl);
-
-  scoped_refptr<HttpNetworkSession> http_session(
-      SpdySessionDependencies::SpdyCreateSession(&session_deps));
-
-  // Setup the first session to the first host.
-  SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool());
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[0].pair));
-  scoped_refptr<SpdySession> session =
-      spdy_session_pool->Get(test_hosts[0].pair, BoundNetLog());
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[0].pair));
-
-  HostPortPair test_host_port_pair(test_hosts[0].name, kTestPort);
-  scoped_refptr<TransportSocketParams> transport_params(
-      new TransportSocketParams(test_host_port_pair,
-                          MEDIUM,
-                          GURL(),
-                          false,
-                          false));
-  scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
-  EXPECT_EQ(OK,
-            connection->Init(test_host_port_pair.ToString(),
-                             transport_params, MEDIUM,
-                             NULL, http_session->transport_socket_pool(),
-                             BoundNetLog()));
-  EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
-
-  // Flush the SpdySession::OnReadComplete() task.
-  MessageLoop::current()->RunAllPending();
-
-  // The third host has no overlap with the first, so it can't pool IPs.
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
-
-  // The second host overlaps with the first, and should IP pool.
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[1].pair));
-
-  // Verify that the second host, through a proxy, won't share the IP.
-  HostPortProxyPair proxy_pair(test_hosts[1].pair.first,
-      ProxyServer::FromPacString("HTTP http://proxy.foo.com/"));
-  EXPECT_FALSE(spdy_session_pool->HasSession(proxy_pair));
-
-  // Overlap between 2 and 3 does is not transitive to 1.
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
-
-  // Create a new session to host 2.
-  scoped_refptr<SpdySession> session2 =
-      spdy_session_pool->Get(test_hosts[2].pair, BoundNetLog());
-
-  // Verify that we have sessions for everything.
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[0].pair));
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[1].pair));
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_hosts[2].pair));
-
-  // Cleanup the sessions.
-  if (!clean_via_close_current_sessions) {
-    spdy_session_pool->Remove(session);
-    session = NULL;
-    spdy_session_pool->Remove(session2);
-    session2 = NULL;
-  } else {
-    spdy_session_pool->CloseCurrentSessions();
-  }
-
-  // Verify that the map is all cleaned up.
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[0].pair));
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[1].pair));
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_hosts[2].pair));
-}
-
 TEST_F(SpdySessionTest, IPPooling) {
-  IPPoolingTest(false);
+  SpdySessionTest::IPPoolingTest(false);
 }
 
 TEST_F(SpdySessionTest, IPPoolingCloseCurrentSessions) {
-  IPPoolingTest(true);
+  SpdySessionTest::IPPoolingTest(true);
 }
 
 TEST_F(SpdySessionTest, ClearSettingsStorage) {
   SpdySettingsStorage settings_storage;
   const std::string kTestHost("www.foo.com");
   const int kTestPort = 80;
   HostPortPair test_host_port_pair(kTestHost, kTestPort);
   spdy::SpdySettings test_settings;
   spdy::SettingsFlagsAndId id(0);
   id.set_id(spdy::SETTINGS_MAX_CONCURRENT_STREAMS);
@@ skipping 25 matching lines @@
   const size_t max_concurrent_streams = 2;
   spdy::SpdySettings test_settings;
   test_settings.push_back(spdy::SpdySetting(id, max_concurrent_streams));
 
   test_settings_storage->Set(test_host_port_pair, test_settings);
   EXPECT_NE(0u, test_settings_storage->Get(test_host_port_pair).size());
   spdy_session_pool->OnIPAddressChanged();
   EXPECT_EQ(0u, test_settings_storage->Get(test_host_port_pair).size());
 }
 
-}  // namespace
-
 }  // namespace net