Reland 61015 (unnecessary revert due to flaky build) - Stop refcounting SpdySessionPool.

BUG=57343
TEST=none

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

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/3541005

TBR=willchan@chromium.org

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

8 files changed, 28 insertions, 34 deletions

diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 5ddbd40..5645ad6 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "net/base/net_log_unittest.h"
+#include "net/http/http_network_session_peer.h"
 #include "net/http/http_transaction_unittest.h"
 #include "net/spdy/spdy_http_stream.h"
 #include "net/spdy/spdy_session.h"
@@ -363,7 +364,7 @@ class SpdyNetworkTransactionTest
     HostPortProxyPair pair(host_port_pair, ProxyServer::Direct());
     BoundNetLog log;
     const scoped_refptr<HttpNetworkSession>& session = helper.session();
-    scoped_refptr<SpdySessionPool> pool(session->spdy_session_pool());
+    SpdySessionPool* pool(session->spdy_session_pool());
     EXPECT_TRUE(pool->HasSession(pair));
     scoped_refptr<SpdySession> spdy_session(
         pool->Get(pair, session->mutable_spdy_settings(), log));
@@ -4243,8 +4244,7 @@ TEST_P(SpdyNetworkTransactionTest, DirectConnectProxyReconnect) {
   helper.SetSession(SpdySessionDependencies::SpdyCreateSession(
       helper.session_deps().get()));
 
-  scoped_refptr<SpdySessionPool> spdy_session_pool =
-      helper.session_deps()->spdy_session_pool;
+  SpdySessionPool* spdy_session_pool = helper.session()->spdy_session_pool();
   helper.RunPreTestSetup();
 
   // Construct and send a simple GET request.
@@ -4367,15 +4367,15 @@ TEST_P(SpdyNetworkTransactionTest, DirectConnectProxyReconnect) {
   request_proxy.method = "GET";
   request_proxy.url = GURL("http://www.google.com/foo.dat");
   request_proxy.load_flags = 0;
-  scoped_ptr<SpdySessionDependencies> ssd_proxy(
-      new SpdySessionDependencies(
-          ProxyService::CreateFixedFromPacResult("PROXY myproxy:70")));
+  scoped_ptr<SpdySessionDependencies> ssd_proxy(new SpdySessionDependencies());
   // Ensure that this transaction uses the same SpdySessionPool.
-  ssd_proxy->spdy_session_pool = spdy_session_pool;
   scoped_refptr<HttpNetworkSession> session_proxy =
       SpdySessionDependencies::SpdyCreateSession(ssd_proxy.get());
   NormalSpdyTransactionHelper helper_proxy(request_proxy,
                                            BoundNetLog(), GetParam());
+  HttpNetworkSessionPeer session_peer(session_proxy);
+  session_peer.SetProxyService(
+          ProxyService::CreateFixedFromPacResult("PROXY myproxy:70"));
   helper_proxy.session_deps().swap(ssd_proxy);
   helper_proxy.SetSession(session_proxy);
   helper_proxy.RunPreTestSetup();
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index ed3d3462..b00e938 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -246,7 +246,6 @@ SpdySession::SpdySession(const HostPortProxyPair& host_port_proxy_pair,
       frames_received_(0),
       sent_settings_(false),
       received_settings_(false),
-      in_session_pool_(true),
       initial_send_window_size_(spdy::kInitialWindowSize),
       initial_recv_window_size_(spdy::kInitialWindowSize),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SPDY_SESSION)) {
@@ -873,9 +872,9 @@ void SpdySession::DeleteStream(spdy::SpdyStreamId id, int status) {
 }
 
 void SpdySession::RemoveFromPool() {
-  if (in_session_pool_) {
+  if (spdy_session_pool_) {
     spdy_session_pool_->Remove(this);
-    in_session_pool_ = false;
+    spdy_session_pool_ = NULL;
   }
 }
 
diff --git a/net/spdy/spdy_session.h b/net/spdy/spdy_session.h
index 313440c..58a2710 100644
--- a/net/spdy/spdy_session.h
+++ b/net/spdy/spdy_session.h
@@ -51,6 +51,8 @@ class SpdySession : public base::RefCounted<SpdySession>,
   // Create a new SpdySession.
   // |host_port_proxy_pair| is the host/port that this session connects to, and
   // the proxy configuration settings that it's using.
+  // |spdy_session_pool| is the SpdySessionPool that owns us.  Its lifetime must
+  // strictly be greater than |this|.
   // |session| is the HttpNetworkSession.  |net_log| is the NetLog that we log
   // network events to.
   SpdySession(const HostPortProxyPair& host_port_proxy_pair,
@@ -159,7 +161,9 @@ class SpdySession : public base::RefCounted<SpdySession>,
     return frames_received_ > 0;
   }
 
-  void set_in_session_pool(bool val) { in_session_pool_ = val; }
+  void set_spdy_session_pool(SpdySessionPool* pool) {
+    spdy_session_pool_ = NULL;
+  }
 
   // Access to the number of active and pending streams.  These are primarily
   // available for testing and diagnostics.
@@ -294,7 +298,9 @@ class SpdySession : public base::RefCounted<SpdySession>,
   // The domain this session is connected to.
   const HostPortProxyPair host_port_proxy_pair_;
 
-  scoped_refptr<SpdySessionPool> spdy_session_pool_;
+  // |spdy_session_pool_| owns us, therefore its lifetime must exceed ours.  We
+  // set this to NULL after we are removed from the pool.
+  SpdySessionPool* spdy_session_pool_;
   SpdySettingsStorage* spdy_settings_;
 
   // The socket handle for this session.
@@ -361,8 +367,6 @@ class SpdySession : public base::RefCounted<SpdySession>,
   bool received_settings_;  // Did this session receive at least one settings
                             // frame.
 
-  bool in_session_pool_;  // True if the session is currently in the pool.
-
   // Initial send window size for the session; can be changed by an
   // arriving SETTINGS frame; newly created streams use this value for the
   // initial send window size.
diff --git a/net/spdy/spdy_session_pool.cc b/net/spdy/spdy_session_pool.cc
index 6c4ad52..086fa9f 100644
--- a/net/spdy/spdy_session_pool.cc
+++ b/net/spdy/spdy_session_pool.cc
@@ -176,7 +176,7 @@ void SpdySessionPool::CloseCurrentSessions() {
     CHECK(list);
     const scoped_refptr<SpdySession>& session = list->front();
     CHECK(session);
-    session->set_in_session_pool(false);
+    session->set_spdy_session_pool(NULL);
   }
 
   while (!old_map.empty()) {
diff --git a/net/spdy/spdy_session_pool.h b/net/spdy/spdy_session_pool.h
index 7dc5f27..49f9e45 100644
--- a/net/spdy/spdy_session_pool.h
+++ b/net/spdy/spdy_session_pool.h
@@ -35,11 +35,11 @@ class SpdySettingsStorage;
 // This is a very simple pool for open SpdySessions.
 // TODO(mbelshe): Make this production ready.
 class SpdySessionPool
-    : public base::RefCounted<SpdySessionPool>,
-      public NetworkChangeNotifier::Observer,
+    : public NetworkChangeNotifier::Observer,
       public SSLConfigService::Observer {
  public:
   explicit SpdySessionPool(SSLConfigService* ssl_config_service);
+  virtual ~SpdySessionPool();
 
   // Either returns an existing SpdySession or creates a new SpdySession for
   // use.
@@ -101,7 +101,6 @@ class SpdySessionPool
   virtual void OnSSLConfigChanged();
 
  private:
-  friend class base::RefCounted<SpdySessionPool>;
   friend class SpdySessionPoolPeer;  // For testing.
   friend class SpdyNetworkTransactionTest;  // For testing.
   FRIEND_TEST_ALL_PREFIXES(SpdyNetworkTransactionTest, WindowUpdateOverflow);
@@ -109,8 +108,6 @@ class SpdySessionPool
   typedef std::list<scoped_refptr<SpdySession> > SpdySessionList;
   typedef std::map<HostPortProxyPair, SpdySessionList*> SpdySessionsMap;
 
-  virtual ~SpdySessionPool();
-
   // Helper functions for manipulating the lists.
   SpdySessionList* AddSessionList(
       const HostPortProxyPair& host_port_proxy_pair);
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index d39a09e..38de37a 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -82,8 +82,7 @@ TEST_F(SpdySessionTest, GoAway) {
   HostPortPair test_host_port_pair(kTestHost, kTestPort);
   HostPortProxyPair pair(test_host_port_pair, ProxyServer::Direct());
 
-  scoped_refptr<SpdySessionPool> spdy_session_pool(
-      http_session->spdy_session_pool());
+  SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool());
   EXPECT_FALSE(spdy_session_pool->HasSession(pair));
   scoped_refptr<SpdySession> session =
       spdy_session_pool->Get(pair, http_session->mutable_spdy_settings(),
diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc
index 84d21d3..5ece945 100644
--- a/net/spdy/spdy_stream_unittest.cc
+++ b/net/spdy/spdy_stream_unittest.cc
@@ -13,7 +13,7 @@ namespace net {
 // TODO(ukai): factor out common part with spdy_http_stream_unittest.cc
 class SpdySessionPoolPeer {
  public:
-  explicit SpdySessionPoolPeer(const scoped_refptr<SpdySessionPool>& pool)
+  explicit SpdySessionPoolPeer(SpdySessionPool* pool)
       : pool_(pool) {}
 
   void RemoveSpdySession(const scoped_refptr<SpdySession>& session) {
@@ -21,7 +21,7 @@ class SpdySessionPoolPeer {
   }
 
  private:
-  const scoped_refptr<SpdySessionPool> pool_;
+  SpdySessionPool* const pool_;
 
   DISALLOW_COPY_AND_ASSIGN(SpdySessionPoolPeer);
 };
diff --git a/net/spdy/spdy_test_util.h b/net/spdy/spdy_test_util.h
index be2f7d0..13b3a2f 100644
--- a/net/spdy/spdy_test_util.h
+++ b/net/spdy/spdy_test_util.h
@@ -304,8 +304,7 @@ class SpdySessionDependencies {
         socket_factory(new MockClientSocketFactory),
         deterministic_socket_factory(new DeterministicMockClientSocketFactory),
         http_auth_handler_factory(
-            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
-        spdy_session_pool(new SpdySessionPool(NULL)) {
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)) {
           // Note: The CancelledTransaction test does cleanup by running all
           // tasks in the message loop (RunAllPending).  Unfortunately, that
           // doesn't clean up tasks on the host resolver thread; and
@@ -323,8 +322,7 @@ class SpdySessionDependencies {
         socket_factory(new MockClientSocketFactory),
         deterministic_socket_factory(new DeterministicMockClientSocketFactory),
         http_auth_handler_factory(
-            HttpAuthHandlerFactory::CreateDefault(host_resolver)),
-        spdy_session_pool(new SpdySessionPool(NULL)) {}
+            HttpAuthHandlerFactory::CreateDefault(host_resolver)) {}
 
   // NOTE: host_resolver must be ordered before http_auth_handler_factory.
   scoped_refptr<MockHostResolverBase> host_resolver;
@@ -333,7 +331,6 @@ class SpdySessionDependencies {
   scoped_ptr<MockClientSocketFactory> socket_factory;
   scoped_ptr<DeterministicMockClientSocketFactory> deterministic_socket_factory;
   scoped_ptr<HttpAuthHandlerFactory> http_auth_handler_factory;
-  scoped_refptr<SpdySessionPool> spdy_session_pool;
 
   static HttpNetworkSession* SpdyCreateSession(
       SpdySessionDependencies* session_deps) {
@@ -341,7 +338,7 @@ class SpdySessionDependencies {
                                   session_deps->proxy_service,
                                   session_deps->socket_factory.get(),
                                   session_deps->ssl_config_service,
-                                  session_deps->spdy_session_pool,
+                                  new SpdySessionPool(NULL),
                                   session_deps->http_auth_handler_factory.get(),
                                   NULL,
                                   NULL);
@@ -353,7 +350,7 @@ class SpdySessionDependencies {
                                   session_deps->
                                       deterministic_socket_factory.get(),
                                   session_deps->ssl_config_service,
-                                  session_deps->spdy_session_pool,
+                                  new SpdySessionPool(NULL),
                                   session_deps->http_auth_handler_factory.get(),
                                   NULL,
                                   NULL);
@@ -365,7 +362,6 @@ class SpdyURLRequestContext : public URLRequestContext {
   SpdyURLRequestContext() {
     host_resolver_ = new MockHostResolver;
     proxy_service_ = ProxyService::CreateDirect();
-    spdy_session_pool_ = new SpdySessionPool(NULL);
     ssl_config_service_ = new SSLConfigServiceDefaults;
     http_auth_handler_factory_ = HttpAuthHandlerFactory::CreateDefault(
         host_resolver_);
@@ -374,7 +370,7 @@ class SpdyURLRequestContext : public URLRequestContext {
                              host_resolver_,
                              proxy_service_,
                              ssl_config_service_,
-                             spdy_session_pool_.get(),
+                             new SpdySessionPool(NULL),
                              http_auth_handler_factory_,
                              network_delegate_,
                              NULL),
@@ -391,7 +387,6 @@ class SpdyURLRequestContext : public URLRequestContext {
 
  private:
   MockClientSocketFactory socket_factory_;
-  scoped_refptr<SpdySessionPool> spdy_session_pool_;
 };
 
 const SpdyHeaderInfo make_spdy_header(spdy::SpdyControlType type);