SpdySessionPool closes down sessions accurately now.

This may or may not fix 50265. Regardless, the previous behavior was incorrect.
TEST=none
BUG=none

Review URL: http://codereview.chromium.org/3150023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56696 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 53 insertions, 33 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index ad72bea..a27a8bd 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -380,7 +380,7 @@ void HttpCache::CloseCurrentConnections() {
   if (session) {
     session->tcp_socket_pool()->Flush();
     if (session->spdy_session_pool())
-      session->spdy_session_pool()->CloseAllSessions();
+      session->spdy_session_pool()->CloseCurrentSessions();
   }
 }
 
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 112ade7..6c0973e 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -132,7 +132,7 @@ class SpdyNetworkTransactionTest
 
       output_.rv = callback.WaitForResult();
       if (output_.rv != OK) {
-        session_->spdy_session_pool()->ClearSessions();
+        session_->spdy_session_pool()->CloseCurrentSessions();
         return;
       }
 
@@ -1568,7 +1568,7 @@ TEST_P(SpdyNetworkTransactionTest, WindowUpdateOverflow) {
 
   ASSERT_TRUE(helper.session() != NULL);
   ASSERT_TRUE(helper.session()->spdy_session_pool() != NULL);
-  helper.session()->spdy_session_pool()->ClearSessions();
+  helper.session()->spdy_session_pool()->CloseAllSessions();
   helper.VerifyDataConsumed();
 
   SpdySession::SetFlowControl(false);
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 07de80f..c90bdd1 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -507,7 +507,7 @@ void SpdySession::OnTCPConnect(int result) {
 
   if (result != net::OK) {
     DCHECK_LT(result, 0);
-    CloseSessionOnError(static_cast<net::Error>(result));
+    CloseSessionOnError(static_cast<net::Error>(result), true);
     return;
   } else {
     UpdateConnectionTypeHistograms(CONNECTION_SPDY);
@@ -555,7 +555,7 @@ void SpdySession::OnSSLConnect(int result) {
     ReadSocket();
   } else {
     DCHECK_LT(result, 0);  // It should be an error, not a byte count.
-    CloseSessionOnError(static_cast<net::Error>(result));
+    CloseSessionOnError(static_cast<net::Error>(result), true);
   }
 }
 
@@ -576,7 +576,7 @@ void SpdySession::OnReadComplete(int bytes_read) {
           host_port_pair().ToString() << "].";
       error = ERR_CONNECTION_CLOSED;
     }
-    CloseSessionOnError(error);
+    CloseSessionOnError(error, true);
     return;
   }
 
@@ -650,7 +650,7 @@ void SpdySession::OnWriteComplete(int result) {
     in_flight_write_.release();
 
     // The stream is now errored.  Close it down.
-    CloseSessionOnError(static_cast<net::Error>(result));
+    CloseSessionOnError(static_cast<net::Error>(result), true);
   }
 }
 
@@ -671,7 +671,7 @@ net::Error SpdySession::ReadSocket() {
   switch (bytes_read) {
     case 0:
       // Socket is closed!
-      CloseSessionOnError(ERR_CONNECTION_CLOSED);
+      CloseSessionOnError(ERR_CONNECTION_CLOSED, true);
       return ERR_CONNECTION_CLOSED;
     case net::ERR_IO_PENDING:
       // Waiting for data.  Nothing to do now.
@@ -732,7 +732,7 @@ void SpdySession::WriteSocket() {
             spdy_framer_.CompressFrame(uncompressed_frame));
         if (!compressed_frame.get()) {
           LOG(ERROR) << "SPDY Compression failure";
-          CloseSessionOnError(net::ERR_SPDY_PROTOCOL_ERROR);
+          CloseSessionOnError(net::ERR_SPDY_PROTOCOL_ERROR, true);
           return;
         }
 
@@ -824,7 +824,7 @@ void SpdySession::QueueFrame(spdy::SpdyFrame* frame,
   WriteSocketLater();
 }
 
-void SpdySession::CloseSessionOnError(net::Error err) {
+void SpdySession::CloseSessionOnError(net::Error err, bool remove_from_pool) {
   // Closing all streams can have a side-effect of dropping the last reference
   // to |this|.  Hold a reference through this function.
   scoped_refptr<SpdySession> self(this);
@@ -839,7 +839,8 @@ void SpdySession::CloseSessionOnError(net::Error err) {
   if (state_ != CLOSED) {
     state_ = CLOSED;
     error_ = err;
-    RemoveFromPool();
+    if (remove_from_pool)
+      RemoveFromPool();
     CloseAllStreams(err);
   }
 }
@@ -929,7 +930,7 @@ bool SpdySession::GetSSLCertRequestInfo(
 
 void SpdySession::OnError(spdy::SpdyFramer* framer) {
   LOG(ERROR) << "SpdySession error: " << framer->error_code();
-  CloseSessionOnError(net::ERR_SPDY_PROTOCOL_ERROR);
+  CloseSessionOnError(net::ERR_SPDY_PROTOCOL_ERROR, true);
 }
 
 void SpdySession::OnStreamFrameData(spdy::SpdyStreamId stream_id,
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 4230289..6fe8d37 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -150,7 +150,9 @@ class SpdySession : public base::RefCounted<SpdySession>,
   // the session as permanently closed.
   // |err| should not be OK; this function is intended to be called on
   // error.
-  void CloseSessionOnError(net::Error err);
+  // |remove_from_pool| indicates whether to also remove the session from the
+  // session pool.
+  void CloseSessionOnError(net::Error err, bool remove_from_pool);
 
   // Indicates whether the session is being reused after having successfully
   // used to send/receive data in the past.
@@ -158,6 +160,8 @@ class SpdySession : public base::RefCounted<SpdySession>,
     return frames_received_ > 0;
   }
 
+  void set_in_session_pool(bool val) { in_session_pool_ = val; }
+
  private:
   friend class base::RefCounted<SpdySession>;
   FRIEND_TEST_ALL_PREFIXES(SpdySessionTest, GetActivePushStream);
diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc
index 7bccd5c43..4eb70e6 100644
--- a/net/spdy/spdy_session_pool.cc
+++ b/net/spdy/spdy_session_pool.cc
@@ -90,7 +90,7 @@ void SpdySessionPool::Remove(const scoped_refptr<SpdySession>& session) {
 }
 
 void SpdySessionPool::OnIPAddressChanged() {
-  ClearSessions();
+  CloseCurrentSessions();
 }
 
 SpdySessionPool::SpdySessionList*
@@ -131,25 +131,41 @@ void SpdySessionPool::RemoveSessionList(
   }
 }
 
-void SpdySessionPool::ClearSessions() {
+void SpdySessionPool::CloseAllSessions() {
   while (!sessions_.empty()) {
     SpdySessionList* list = sessions_.begin()->second;
-    DCHECK(list);
-    sessions_.erase(sessions_.begin()->first);
-    while (list->size()) {
-      list->pop_front();
-    }
-    delete list;
+    CHECK(list);
+    const scoped_refptr<SpdySession>& session = list->front();
+    CHECK(session);
+    // This call takes care of removing the session from the pool, as well as
+    // removing the session list if the list is empty.
+    session->CloseSessionOnError(net::ERR_ABORTED, true);
   }
 }
 
-void SpdySessionPool::CloseAllSessions() {
-  while (!sessions_.empty()) {
-    SpdySessionList* list = sessions_.begin()->second;
-    DCHECK(list);
+void SpdySessionPool::CloseCurrentSessions() {
+  SpdySessionsMap old_map;
+  old_map.swap(sessions_);
+  for (SpdySessionsMap::const_iterator it = old_map.begin();
+       it != old_map.end(); ++it) {
+    SpdySessionList* list = it->second;
+    CHECK(list);
+    const scoped_refptr<SpdySession>& session = list->front();
+    CHECK(session);
+    session->set_in_session_pool(false);
+  }
+
+  while (!old_map.empty()) {
+    SpdySessionList* list = old_map.begin()->second;
+    CHECK(list);
     const scoped_refptr<SpdySession>& session = list->front();
-    DCHECK(session);
-    session->CloseSessionOnError(net::ERR_ABORTED);
+    CHECK(session);
+    session->CloseSessionOnError(net::ERR_ABORTED, false);
+    list->pop_front();
+    if (list->empty()) {
+      delete list;
+      old_map.erase(old_map.begin()->first);
+    }
   }
 }
 
diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h
index 3bdd2b3..7efb400 100644
--- a/net/spdy/spdy_session_pool.h
+++ b/net/spdy/spdy_session_pool.h
@@ -73,8 +73,12 @@ class SpdySessionPool
   // should be creating a new session.
   bool HasSession(const HostPortProxyPair& host_port_proxy_pair) const;
 
-  // Close all Spdy Sessions; used for debugging.
+  // Close all SpdySessions, including any new ones created in the process of
+  // closing the current ones.
   void CloseAllSessions();
+  // Close only the currently existing SpdySessions. Let any new ones created
+  // continue to live.
+  void CloseCurrentSessions();
 
   // Removes a SpdySession from the SpdySessionPool. This should only be called
   // by SpdySession, because otherwise session->state_ is not set to CLOSED.
@@ -106,11 +110,6 @@ class SpdySessionPool
   const SpdySessionList* GetSessionList(
       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
-  // away.
-  void ClearSessions();
 
   // This is our weak session pool - one session per domain.
   SpdySessionsMap sessions_;