SpdySessionPool now identifies SpdySessions by both HostPortPair and proxy settings.

This ensures SpdySessions are not improperly reused.
TEST=none
BUG=49874


git-svn-id: svn://svn.chromium.org/chrome/branches/472/src@54784 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 87 insertions, 59 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index a2f4695..292af06 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -733,7 +733,8 @@ int HttpNetworkTransaction::DoInitConnection() {
 
   // Check first if we have a spdy session for this group.  If so, then go
   // straight to using that.
-  if (session_->spdy_session_pool()->HasSession(endpoint_)) {
+  HostPortProxyPair pair(endpoint_, proxy_info_.ToPacString());
+  if (session_->spdy_session_pool()->HasSession(pair)) {
     using_spdy_ = true;
     reused_socket_ = true;
     next_state_ = STATE_SPDY_GET_STREAM;
@@ -1292,15 +1293,16 @@ int HttpNetworkTransaction::DoSpdyGetStream() {
       session_->spdy_session_pool();
   scoped_refptr<SpdySession> spdy_session;
 
-  if (spdy_pool->HasSession(endpoint_)) {
-    spdy_session = spdy_pool->Get(endpoint_, session_, net_log_);
+  HostPortProxyPair pair(endpoint_, proxy_info_.ToPacString());
+  if (spdy_pool->HasSession(pair)) {
+    spdy_session = spdy_pool->Get(pair, session_, net_log_);
   } else {
     // SPDY is negotiated using the TLS next protocol negotiation (NPN)
     // extension, so |connection_| must contain an SSLClientSocket.
     DCHECK(using_ssl_);
     CHECK(connection_->socket());
     int error = spdy_pool->GetSpdySessionFromSSLSocket(
-        endpoint_, session_, connection_.release(), net_log_,
+        pair, session_, connection_.release(), net_log_,
         spdy_certificate_error_, &spdy_session);
     if (error != OK)
       return error;
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 92f109a..e70f6d0 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -5596,9 +5596,9 @@ TEST_F(HttpNetworkTransactionTest,
   EXPECT_EQ("hello world", response_data);
 
   // Set up an initial SpdySession in the pool to reuse.
+  HostPortProxyPair pair(HostPortPair("www.google.com", 443), "DIRECT");
   scoped_refptr<SpdySession> spdy_session =
-      session->spdy_session_pool()->Get(HostPortPair("www.google.com", 443),
-                                        session, BoundNetLog());
+      session->spdy_session_pool()->Get(pair, session, BoundNetLog());
   scoped_refptr<TCPSocketParams> tcp_params =
       new TCPSocketParams("www.google.com", 443, MEDIUM, GURL(), false);
   spdy_session->Connect("www.google.com:443", tcp_params, MEDIUM);
diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc
index 5fab0e2..b04a4e1 100644
--- a/net/spdy/spdy_http_stream_unittest.cc
+++ b/net/spdy/spdy_http_stream_unittest.cc
@@ -92,9 +92,10 @@ class SpdyHttpStreamTest : public testing::Test {
 
   scoped_refptr<SpdySession> CreateSpdySession() {
     HostPortPair host_port_pair("www.google.com", 80);
+    HostPortProxyPair pair(host_port_pair, "");
     scoped_refptr<SpdySession> session(
         session_->spdy_session_pool()->Get(
-            host_port_pair, session_, BoundNetLog()));
+            pair, session_, BoundNetLog()));
     return session;
   }
 
diff --git a/net/spdy/spdy_network_transaction.cc b/net/spdy/spdy_network_transaction.cc
index 7814553..0207b28 100644
--- a/net/spdy/spdy_network_transaction.cc
+++ b/net/spdy/spdy_network_transaction.cc
@@ -238,8 +238,9 @@ int SpdyNetworkTransaction::DoInitConnection() {
       new TCPSocketParams(host_port_pair, request_->priority,
                           request_->referrer, false);
 
+  HostPortProxyPair pair(host_port_pair, "");
   spdy_ = session_->spdy_session_pool()->Get(
-      host_port_pair, session_, net_log_);
+      pair, session_, net_log_);
   DCHECK(spdy_);
 
   return spdy_->Connect(
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 753db27..70652a0 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -133,7 +133,7 @@ class NetLogSpdySettingsParameter : public NetLog::EventParameters {
 // static
 bool SpdySession::use_ssl_ = true;
 
-SpdySession::SpdySession(const HostPortPair& host_port_pair,
+SpdySession::SpdySession(const HostPortProxyPair& host_port_proxy_pair,
                          HttpNetworkSession* session,
                          NetLog* net_log)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
@@ -144,7 +144,7 @@ SpdySession::SpdySession(const HostPortPair& host_port_pair,
           read_callback_(this, &SpdySession::OnReadComplete)),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           write_callback_(this, &SpdySession::OnWriteComplete)),
-      host_port_pair_(host_port_pair),
+      host_port_proxy_pair_(host_port_proxy_pair),
       session_(session),
       connection_(new ClientSocketHandle),
       read_buffer_(new IOBuffer(kReadBufferSize)),
@@ -168,7 +168,8 @@ SpdySession::SpdySession(const HostPortPair& host_port_pair,
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SPDY_SESSION)) {
   net_log_.BeginEvent(
       NetLog::TYPE_SPDY_SESSION,
-      new NetLogStringParameter("host_port", host_port_pair_.ToString()));
+      new NetLogStringParameter("host_port",
+                                host_port_proxy_pair_.first.ToString()));
 
   // TODO(mbelshe): consider randomization of the stream_hi_water_mark.
 
@@ -1187,7 +1188,7 @@ void SpdySession::OnSettings(const spdy::SpdySettingsControlFrame& frame) {
   if (spdy_framer_.ParseSettings(&frame, &settings)) {
     HandleSettings(settings);
     SpdySettingsStorage* settings_storage = session_->mutable_spdy_settings();
-    settings_storage->Set(host_port_pair_, settings);
+    settings_storage->Set(host_port_pair(), settings);
   }
 
   received_settings_ = true;
@@ -1199,7 +1200,7 @@ void SpdySession::OnSettings(const spdy::SpdySettingsControlFrame& frame) {
 
 void SpdySession::SendSettings() {
   const SpdySettingsStorage& settings_storage = session_->spdy_settings();
-  const spdy::SpdySettings& settings = settings_storage.Get(host_port_pair_);
+  const spdy::SpdySettings& settings = settings_storage.Get(host_port_pair());
   if (settings.empty())
     return;
   HandleSettings(settings);
@@ -1250,7 +1251,7 @@ void SpdySession::RecordHistograms() {
   if (received_settings_) {
     // Enumerate the saved settings, and set histograms for it.
     const SpdySettingsStorage& settings_storage = session_->spdy_settings();
-    const spdy::SpdySettings& settings = settings_storage.Get(host_port_pair_);
+    const spdy::SpdySettings& settings = settings_storage.Get(host_port_pair());
 
     spdy::SpdySettings::const_iterator it;
     for (it = settings.begin(); it != settings.end(); ++it) {
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index efe1cfe..3dde393 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -41,13 +41,20 @@ class SpdySession : public base::RefCounted<SpdySession>,
                     public spdy::SpdyFramerVisitorInterface {
  public:
   // Create a new SpdySession.
-  // |host_port_pair| is the host/port that this session connects to.
+  // |host_port_proxy_pair| is the host/port that this session connects to, and
+  // the proxy configuration settings that it's using.
   // |session| is the HttpNetworkSession.  |net_log| is the NetLog that we log
   // network events to.
-  SpdySession(const HostPortPair& host_port_pair, HttpNetworkSession* session,
+  SpdySession(const HostPortProxyPair& host_port_proxy_pair,
+              HttpNetworkSession* session,
               NetLog* net_log);
 
-  const HostPortPair& host_port_pair() const { return host_port_pair_; }
+  const HostPortPair& host_port_pair() const {
+    return host_port_proxy_pair_.first;
+  }
+  const HostPortProxyPair& host_port_proxy_pair() const {
+    return host_port_proxy_pair_;
+  }
 
   // Connect the Spdy Socket.
   // Returns net::Error::OK on success.
@@ -257,7 +264,7 @@ class SpdySession : public base::RefCounted<SpdySession>,
   CompletionCallbackImpl<SpdySession> write_callback_;
 
   // The domain this session is connected to.
-  const HostPortPair host_port_pair_;
+  const HostPortProxyPair host_port_proxy_pair_;
 
   SSLConfig ssl_config_;
 
diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc
index 8caa040..707f84c 100644
--- a/net/spdy/spdy_session_pool.cc
+++ b/net/spdy/spdy_session_pool.cc
@@ -25,22 +25,24 @@ SpdySessionPool::~SpdySessionPool() {
 }
 
 scoped_refptr<SpdySession> SpdySessionPool::Get(
-    const HostPortPair& host_port_pair, HttpNetworkSession* session,
+    const HostPortProxyPair& host_port_proxy_pair, HttpNetworkSession* session,
     const BoundNetLog& net_log) {
   scoped_refptr<SpdySession> spdy_session;
-  SpdySessionList* list = GetSessionList(host_port_pair);
+  SpdySessionList* list = GetSessionList(host_port_proxy_pair);
   if (list) {
     if (list->size() >= static_cast<unsigned int>(g_max_sessions_per_domain)) {
       spdy_session = list->front();
       list->pop_front();
     }
   } else {
-    list = AddSessionList(host_port_pair);
+    list = AddSessionList(host_port_proxy_pair);
   }
 
   DCHECK(list);
   if (!spdy_session)
-    spdy_session = new SpdySession(host_port_pair, session, net_log.net_log());
+    spdy_session = new SpdySession(host_port_proxy_pair,
+                                   session,
+                                   net_log.net_log());
 
   DCHECK(spdy_session);
   list->push_back(spdy_session);
@@ -49,17 +51,18 @@ scoped_refptr<SpdySession> SpdySessionPool::Get(
 }
 
 net::Error SpdySessionPool::GetSpdySessionFromSSLSocket(
-    const HostPortPair& host_port_pair,
+    const HostPortProxyPair& host_port_proxy_pair,
     HttpNetworkSession* session,
     ClientSocketHandle* connection,
     const BoundNetLog& net_log,
     int certificate_error_code,
     scoped_refptr<SpdySession>* spdy_session) {
   // Create the SPDY session and add it to the pool.
-  *spdy_session = new SpdySession(host_port_pair, session, net_log.net_log());
-  SpdySessionList* list = GetSessionList(host_port_pair);
+  *spdy_session = new SpdySession(host_port_proxy_pair,
+                                  session, net_log.net_log());
+  SpdySessionList* list = GetSessionList(host_port_proxy_pair);
   if (!list)
-    list = AddSessionList(host_port_pair);
+    list = AddSessionList(host_port_proxy_pair);
   DCHECK(list->empty());
   list->push_back(*spdy_session);
 
@@ -68,20 +71,21 @@ net::Error SpdySessionPool::GetSpdySessionFromSSLSocket(
                                                   certificate_error_code);
 }
 
-bool SpdySessionPool::HasSession(const HostPortPair& host_port_pair) const {
-  if (GetSessionList(host_port_pair))
+bool SpdySessionPool::HasSession(
+    const HostPortProxyPair& host_port_proxy_pair) const {
+  if (GetSessionList(host_port_proxy_pair))
     return true;
   return false;
 }
 
 void SpdySessionPool::Remove(const scoped_refptr<SpdySession>& session) {
-  SpdySessionList* list = GetSessionList(session->host_port_pair());
+  SpdySessionList* list = GetSessionList(session->host_port_proxy_pair());
   DCHECK(list);  // We really shouldn't remove if we've already been removed.
   if (!list)
     return;
   list->remove(session);
   if (list->empty())
-    RemoveSessionList(session->host_port_pair());
+    RemoveSessionList(session->host_port_proxy_pair());
 }
 
 void SpdySessionPool::OnIPAddressChanged() {
@@ -89,34 +93,38 @@ void SpdySessionPool::OnIPAddressChanged() {
 }
 
 SpdySessionPool::SpdySessionList*
-    SpdySessionPool::AddSessionList(const HostPortPair& host_port_pair) {
-  DCHECK(sessions_.find(host_port_pair) == sessions_.end());
+    SpdySessionPool::AddSessionList(
+        const HostPortProxyPair& host_port_proxy_pair) {
+  DCHECK(sessions_.find(host_port_proxy_pair) == sessions_.end());
   SpdySessionPool::SpdySessionList* list = new SpdySessionList();
-  sessions_[host_port_pair] = list;
+  sessions_[host_port_proxy_pair] = list;
   return list;
 }
 
 SpdySessionPool::SpdySessionList*
-    SpdySessionPool::GetSessionList(const HostPortPair& host_port_pair) {
-  SpdySessionsMap::iterator it = sessions_.find(host_port_pair);
+    SpdySessionPool::GetSessionList(
+        const HostPortProxyPair& host_port_proxy_pair) {
+  SpdySessionsMap::iterator it = sessions_.find(host_port_proxy_pair);
   if (it == sessions_.end())
     return NULL;
   return it->second;
 }
 
 const SpdySessionPool::SpdySessionList*
-    SpdySessionPool::GetSessionList(const HostPortPair& host_port_pair) const {
-  SpdySessionsMap::const_iterator it = sessions_.find(host_port_pair);
+    SpdySessionPool::GetSessionList(
+        const HostPortProxyPair& host_port_proxy_pair) const {
+  SpdySessionsMap::const_iterator it = sessions_.find(host_port_proxy_pair);
   if (it == sessions_.end())
     return NULL;
   return it->second;
 }
 
-void SpdySessionPool::RemoveSessionList(const HostPortPair& host_port_pair) {
-  SpdySessionList* list = GetSessionList(host_port_pair);
+void SpdySessionPool::RemoveSessionList(
+    const HostPortProxyPair& host_port_proxy_pair) {
+  SpdySessionList* list = GetSessionList(host_port_proxy_pair);
   if (list) {
     delete list;
-    sessions_.erase(host_port_pair);
+    sessions_.erase(host_port_proxy_pair);
   } else {
     DCHECK(false) << "removing orphaned session list";
   }
diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h
index 7574154..a99eb2d 100644
--- a/net/spdy/spdy_session_pool.h
+++ b/net/spdy/spdy_session_pool.h
@@ -15,8 +15,12 @@
 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier.h"
+#include "net/proxy/proxy_config.h"
 
 namespace net {
+// Sessions are uniquely identified by their HostPortPair and the PAC-style list
+// of valid proxy servers.
+typedef std::pair<HostPortPair, std::string> HostPortProxyPair;
 
 class BoundNetLog;
 class ClientSocketHandle;
@@ -34,7 +38,8 @@ class SpdySessionPool
   // Either returns an existing SpdySession or creates a new SpdySession for
   // use.
   scoped_refptr<SpdySession> Get(
-      const HostPortPair& host_port_pair, HttpNetworkSession* session,
+      const HostPortProxyPair& host_port_proxy_pair,
+      HttpNetworkSession* session,
       const BoundNetLog& net_log);
 
   // Set the maximum concurrent sessions per domain.
@@ -52,7 +57,7 @@ class SpdySessionPool
   // Returns OK on success, and the |spdy_session| will be provided.
   // Returns an error on failure, and |spdy_session| will be NULL.
   net::Error GetSpdySessionFromSSLSocket(
-      const HostPortPair& host_port_pair,
+      const HostPortProxyPair& host_port_proxy_pair,
       HttpNetworkSession* session,
       ClientSocketHandle* connection,
       const BoundNetLog& net_log,
@@ -61,7 +66,7 @@ class SpdySessionPool
 
   // TODO(willchan): Consider renaming to HasReusableSession, since perhaps we
   // should be creating a new session.
-  bool HasSession(const HostPortPair& host_port_pair)const;
+  bool HasSession(const HostPortProxyPair& host_port_proxy_pair) const;
 
   // Close all Spdy Sessions; used for debugging.
   void CloseAllSessions();
@@ -82,16 +87,18 @@ class SpdySessionPool
   friend class SpdyNetworkTransactionTest;  // For testing.
 
   typedef std::list<scoped_refptr<SpdySession> > SpdySessionList;
-  typedef std::map<HostPortPair, SpdySessionList*> SpdySessionsMap;
+  typedef std::map<HostPortProxyPair, SpdySessionList*> SpdySessionsMap;
 
   virtual ~SpdySessionPool();
 
   // Helper functions for manipulating the lists.
-  SpdySessionList* AddSessionList(const HostPortPair& host_port_pair);
-  SpdySessionList* GetSessionList(const HostPortPair& host_port_pair);
+  SpdySessionList* AddSessionList(
+      const HostPortProxyPair& host_port_proxy_pair);
+  SpdySessionList* GetSessionList(
+      const HostPortProxyPair& host_port_proxy_pair);
   const SpdySessionList* GetSessionList(
-      const HostPortPair& host_port_pair) const;
-  void RemoveSessionList(const HostPortPair& host_port_pair);
+      const HostPortProxyPair& host_port_proxy_pair) const;
+  void RemoveSessionList(const HostPortProxyPair& host_port_proxy_pair);
   // Releases the SpdySessionPool reference to all sessions.  Will result in all
   // idle sessions being deleted, and the active sessions from being reused, so
   // they will be deleted once all active streams belonging to that session go
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index 195fb10..6a031fa 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -123,14 +123,15 @@ TEST_F(SpdySessionTest, GoAway) {
   HostPortPair test_host_port_pair;
   test_host_port_pair.host = kTestHost;
   test_host_port_pair.port = kTestPort;
+  HostPortProxyPair pair(test_host_port_pair, "");
 
   scoped_refptr<SpdySessionPool> spdy_session_pool(
       http_session->spdy_session_pool());
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_host_port_pair));
+  EXPECT_FALSE(spdy_session_pool->HasSession(pair));
   scoped_refptr<SpdySession> session =
       spdy_session_pool->Get(
-          test_host_port_pair, http_session.get(), BoundNetLog());
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_host_port_pair));
+          pair, http_session.get(), BoundNetLog());
+  EXPECT_TRUE(spdy_session_pool->HasSession(pair));
 
   scoped_refptr<TCPSocketParams> tcp_params =
       new TCPSocketParams(kTestHost, kTestPort, MEDIUM, GURL(), false);
@@ -140,11 +141,10 @@ TEST_F(SpdySessionTest, GoAway) {
   // Flush the SpdySession::OnReadComplete() task.
   MessageLoop::current()->RunAllPending();
 
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_host_port_pair));
+  EXPECT_FALSE(spdy_session_pool->HasSession(pair));
 
   scoped_refptr<SpdySession> session2 =
-      spdy_session_pool->Get(
-          test_host_port_pair, http_session.get(), BoundNetLog());
+      spdy_session_pool->Get(pair, http_session.get(), BoundNetLog());
 
   // Delete the first session.
   session = NULL;
@@ -188,14 +188,15 @@ TEST_F(SpdySessionTest, GetActivePushStream) {
   HostPortPair test_host_port_pair;
   test_host_port_pair.host = kTestHost;
   test_host_port_pair.port = kTestPort;
+  HostPortProxyPair pair(test_host_port_pair, "");
 
   scoped_refptr<SpdySessionPool> spdy_session_pool(
       http_session->spdy_session_pool());
-  EXPECT_FALSE(spdy_session_pool->HasSession(test_host_port_pair));
+  EXPECT_FALSE(spdy_session_pool->HasSession(pair));
   scoped_refptr<SpdySession> session =
       spdy_session_pool->Get(
-          test_host_port_pair, http_session.get(), BoundNetLog());
-  EXPECT_TRUE(spdy_session_pool->HasSession(test_host_port_pair));
+          pair, http_session.get(), BoundNetLog());
+  EXPECT_TRUE(spdy_session_pool->HasSession(pair));
 
   // No push streams should exist in the beginning.
   std::string test_push_path = "/foo.js";
diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc
index 4105aa6..5faf219 100644
--- a/net/spdy/spdy_stream_unittest.cc
+++ b/net/spdy/spdy_stream_unittest.cc
@@ -172,9 +172,9 @@ class SpdyStreamTest : public testing::Test {
   scoped_refptr<SpdySession> CreateSpdySession() {
     spdy::SpdyFramer::set_enable_compression_default(false);
     HostPortPair host_port_pair("www.google.com", 80);
+    HostPortProxyPair pair(host_port_pair, "");
     scoped_refptr<SpdySession> session(
-        session_->spdy_session_pool()->Get(
-            host_port_pair, session_, BoundNetLog()));
+        session_->spdy_session_pool()->Get(pair, session_, BoundNetLog()));
     return session;
   }