Flip: FlipSessionPool changes.

(1) Move it into the HttpNetworkSession so that HttpNetworkTransaction can access it.
This is in anticipation of switching HTTP/HTTPS connections over to FLIP.
(2) Add some more functionality to FlipSessionPool, allowing HttpNetworkTransactions to check for
the existence of other, reusable FlipSessions.

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

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

11 files changed, 130 insertions, 90 deletions

diff --git a/net/flip/flip_network_transaction.cc b/net/flip/flip_network_transaction.cc
index f0433ab..b664826 100644
--- a/net/flip/flip_network_transaction.cc
+++ b/net/flip/flip_network_transaction.cc
@@ -302,7 +302,7 @@ int FlipNetworkTransaction::DoInitConnection() {
 
   HostResolver::RequestInfo resolve_info(host, port);
 
-  flip_ = FlipSession::GetFlipSession(resolve_info, session_);
+  flip_ = session_->flip_session_pool()->Get(resolve_info, session_);
   DCHECK(flip_);
 
   int rv = flip_->Connect(connection_group, resolve_info, request_->priority);
diff --git a/net/flip/flip_network_transaction_unittest.cc b/net/flip/flip_network_transaction_unittest.cc
index 4f41063..be2c41e 100644
--- a/net/flip/flip_network_transaction_unittest.cc
+++ b/net/flip/flip_network_transaction_unittest.cc
@@ -25,11 +25,13 @@
 
 //-----------------------------------------------------------------------------
 
+namespace net {
+
 namespace {
 
 // Create a proxy service which fails on all requests (falls back to direct).
-net::ProxyService* CreateNullProxyService() {
-  return net::ProxyService::CreateNull();
+ProxyService* CreateNullProxyService() {
+  return ProxyService::CreateNull();
 }
 
 // Helper to manage the lifetimes of the dependencies for a
@@ -38,40 +40,42 @@ class SessionDependencies {
  public:
   // Default set of dependencies -- "null" proxy service.
   SessionDependencies()
-      : host_resolver(new net::MockHostResolver),
+      : host_resolver(new MockHostResolver),
         proxy_service(CreateNullProxyService()),
-        ssl_config_service(new net::SSLConfigServiceDefaults) {}
+        ssl_config_service(new SSLConfigServiceDefaults),
+        flip_session_pool(new FlipSessionPool) {}
 
   // Custom proxy service dependency.
-  explicit SessionDependencies(net::ProxyService* proxy_service)
-      : host_resolver(new net::MockHostResolver),
+  explicit SessionDependencies(ProxyService* proxy_service)
+      : host_resolver(new MockHostResolver),
         proxy_service(proxy_service),
-        ssl_config_service(new net::SSLConfigServiceDefaults) {}
-
-  scoped_refptr<net::MockHostResolverBase> host_resolver;
-  scoped_refptr<net::ProxyService> proxy_service;
-  scoped_refptr<net::SSLConfigService> ssl_config_service;
-  net::MockClientSocketFactory socket_factory;
+        ssl_config_service(new SSLConfigServiceDefaults),
+        flip_session_pool(new FlipSessionPool) {}
+
+  scoped_refptr<MockHostResolverBase> host_resolver;
+  scoped_refptr<ProxyService> proxy_service;
+  scoped_refptr<SSLConfigService> ssl_config_service;
+  MockClientSocketFactory socket_factory;
+  scoped_refptr<FlipSessionPool> flip_session_pool;
 };
 
-net::ProxyService* CreateFixedProxyService(const std::string& proxy) {
-  net::ProxyConfig proxy_config;
+ProxyService* CreateFixedProxyService(const std::string& proxy) {
+  ProxyConfig proxy_config;
   proxy_config.proxy_rules.ParseFromString(proxy);
-  return net::ProxyService::CreateFixed(proxy_config);
+  return ProxyService::CreateFixed(proxy_config);
 }
 
 
-net::HttpNetworkSession* CreateSession(SessionDependencies* session_deps) {
-  return new net::HttpNetworkSession(session_deps->host_resolver,
-                                     session_deps->proxy_service,
-                                     &session_deps->socket_factory,
-                                     session_deps->ssl_config_service);
+HttpNetworkSession* CreateSession(SessionDependencies* session_deps) {
+  return new HttpNetworkSession(session_deps->host_resolver,
+                                session_deps->proxy_service,
+                                &session_deps->socket_factory,
+                                session_deps->ssl_config_service,
+                                session_deps->flip_session_pool);
 }
 
 }  // namespace
 
-namespace net {
-
 class FlipNetworkTransactionTest : public PlatformTest {
  public:
   virtual void SetUp() {
@@ -323,4 +327,3 @@ TEST_F(FlipNetworkTransactionTest, ResponseWithoutSynReply) {
 
 
 }  // namespace net
-
diff --git a/net/flip/flip_session.cc b/net/flip/flip_session.cc
index cb0da56..f2ff88a 100644
--- a/net/flip/flip_session.cc
+++ b/net/flip/flip_session.cc
@@ -26,17 +26,8 @@
 namespace net {
 
 // static
-scoped_ptr<FlipSessionPool> FlipSession::session_pool_;
 bool FlipSession::use_ssl_ = true;
 
-FlipSession* FlipSession::GetFlipSession(
-    const HostResolver::RequestInfo& info,
-    HttpNetworkSession* session) {
-  if (!session_pool_.get())
-    session_pool_.reset(new FlipSessionPool());
-  return session_pool_->Get(info, session);
-}
-
 FlipSession::FlipSession(std::string host, HttpNetworkSession* session)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
           connect_callback_(this, &FlipSession::OnTCPConnect)),
@@ -70,8 +61,8 @@ FlipSession::~FlipSession() {
     // With Flip we can't recycle sockets.
     connection_.socket()->Disconnect();
   }
-  if (session_pool_.get())
-    session_pool_->Remove(this);
+
+  session_->flip_session_pool()->Remove(this);
 }
 
 net::Error FlipSession::Connect(const std::string& group_name,
@@ -239,7 +230,7 @@ bool FlipSession::CancelStream(int id) {
   return true;
 }
 
-bool FlipSession::IsStreamActive(int id) {
+bool FlipSession::IsStreamActive(int id) const {
   return active_streams_.find(id) != active_streams_.end();
 }
 
diff --git a/net/flip/flip_session.h b/net/flip/flip_session.h
index ea895da..176dd29 100644
--- a/net/flip/flip_session.h
+++ b/net/flip/flip_session.h
@@ -90,14 +90,10 @@ class FlipDelegate {
 class FlipSession : public base::RefCounted<FlipSession>,
                     public flip::FlipFramerVisitorInterface {
  public:
-  // Factory for finding open sessions.
-  // TODO(mbelshe): Break this out into a connection pool class?
-  static FlipSession* GetFlipSession(const HostResolver::RequestInfo&,
-                                     HttpNetworkSession* session);
   virtual ~FlipSession();
 
   // Get the domain for this FlipSession.
-  std::string domain() { return domain_; }
+  const std::string& domain() const { return domain_; }
 
   // Connect the FLIP Socket.
   // Returns net::Error::OK on success.
@@ -117,11 +113,12 @@ class FlipSession : public base::RefCounted<FlipSession>,
   bool CancelStream(int id);
 
   // Check if a stream is active.
-  bool IsStreamActive(int id);
+  bool IsStreamActive(int id) const;
 
   // The LoadState is used for informing the user of the current network
   // status, such as "resolving host", "connecting", etc.
   LoadState GetLoadState() const;
+
  protected:
   friend class FlipNetworkTransactionTest;
   friend class FlipSessionPool;
@@ -231,12 +228,9 @@ class FlipSession : public base::RefCounted<FlipSession>,
   // Flip Frame state.
   flip::FlipFramer flip_framer_;
 
-  // This is our weak session pool - one session per domain.
-  static scoped_ptr<FlipSessionPool> session_pool_;
   static bool use_ssl_;
 };
 
 }  // namespace net
 
 #endif  // NET_FLIP_FLIP_SESSION_H_
-
diff --git a/net/flip/flip_session_pool.cc b/net/flip/flip_session_pool.cc
index 274ebf1..386fddc 100644
--- a/net/flip/flip_session_pool.cc
+++ b/net/flip/flip_session_pool.cc
@@ -10,16 +10,16 @@
 namespace net {
 
 // The maximum number of sessions to open to a single domain.
-const size_t kMaxSessionsPerDomain = 1;
+static const size_t kMaxSessionsPerDomain = 1;
 
-scoped_ptr<FlipSessionPool::FlipSessionsMap> FlipSessionPool::sessions_;
+FlipSessionPool::FlipSessionPool() {}
+FlipSessionPool::~FlipSessionPool() {
+  CloseAllSessions();
+}
 
 FlipSession* FlipSessionPool::Get(const HostResolver::RequestInfo& info,
                                   HttpNetworkSession* session) {
-  if (!sessions_.get())
-    sessions_.reset(new FlipSessionsMap());
-
-  const std::string domain = info.hostname();
+  const std::string& domain = info.hostname();
   FlipSession* flip_session = NULL;
   FlipSessionList* list = GetSessionList(domain);
   if (list) {
@@ -43,48 +43,60 @@ FlipSession* FlipSessionPool::Get(const HostResolver::RequestInfo& info,
   return flip_session;
 }
 
+bool FlipSessionPool::HasSession(const HostResolver::RequestInfo& info) const {
+  const std::string& domain = info.hostname();
+  if (GetSessionList(domain))
+    return true;
+  return false;
+}
+
 void FlipSessionPool::Remove(FlipSession* session) {
   std::string domain = session->domain();
   FlipSessionList* list = GetSessionList(domain);
   if (list == NULL)
     return;
   list->remove(session);
-  if (!list->size())
+  if (list->empty())
     RemoveSessionList(domain);
 }
 
 FlipSessionPool::FlipSessionList*
-    FlipSessionPool::AddSessionList(std::string domain) {
-  DCHECK(sessions_->find(domain) == sessions_->end());
-  return (*sessions_)[domain] = new FlipSessionList();
+    FlipSessionPool::AddSessionList(const std::string& domain) {
+  DCHECK(sessions_.find(domain) == sessions_.end());
+  return sessions_[domain] = new FlipSessionList();
 }
 
-// static
 FlipSessionPool::FlipSessionList*
-    FlipSessionPool::GetSessionList(std::string domain) {
-  FlipSessionsMap::iterator it = sessions_->find(domain);
-  if (it == sessions_->end())
+    FlipSessionPool::GetSessionList(const std::string& domain) {
+  FlipSessionsMap::iterator it = sessions_.find(domain);
+  if (it == sessions_.end())
     return NULL;
   return it->second;
 }
 
-// static
-void FlipSessionPool::RemoveSessionList(std::string domain) {
+const FlipSessionPool::FlipSessionList*
+    FlipSessionPool::GetSessionList(const std::string& domain) const {
+  FlipSessionsMap::const_iterator it = sessions_.find(domain);
+  if (it == sessions_.end())
+    return NULL;
+  return it->second;
+}
+
+void FlipSessionPool::RemoveSessionList(const std::string& domain) {
   FlipSessionList* list = GetSessionList(domain);
   if (list) {
     delete list;
-    sessions_->erase(domain);
+    sessions_.erase(domain);
   } else {
     DCHECK(false) << "removing orphaned session list";
   }
 }
 
-// static
 void FlipSessionPool::CloseAllSessions() {
-  while (sessions_.get() && sessions_->size()) {
-    FlipSessionList* list = sessions_->begin()->second;
+  while (sessions_.size()) {
+    FlipSessionList* list = sessions_.begin()->second;
     DCHECK(list);
-    sessions_->erase(sessions_->begin()->first);
+    sessions_.erase(sessions_.begin()->first);
     while (list->size()) {
       FlipSession* session = list->front();
       list->pop_front();
@@ -96,4 +108,3 @@ void FlipSessionPool::CloseAllSessions() {
 }
 
 }  // namespace net
-
diff --git a/net/flip/flip_session_pool.h b/net/flip/flip_session_pool.h
index 9562413..e9c01a3 100644
--- a/net/flip/flip_session_pool.h
+++ b/net/flip/flip_session_pool.h
@@ -15,43 +15,60 @@
 
 namespace net {
 
+class ClientSocket;
 class FlipSession;
 class HttpNetworkSession;
 
 // This is a very simple pool for open FlipSessions.
 // TODO(mbelshe): Make this production ready.
-class FlipSessionPool {
+class FlipSessionPool : public base::RefCounted<FlipSessionPool> {
  public:
-  FlipSessionPool() {}
-  virtual ~FlipSessionPool() {}
+  FlipSessionPool();
+  virtual ~FlipSessionPool();
 
-  // Factory for finding open sessions.
+  // Either returns an existing FlipSession or creates a new FlipSession for
+  // use.
   FlipSession* Get(const HostResolver::RequestInfo& info,
                    HttpNetworkSession* session);
 
-  // Close all Flip Sessions; used for debugging.
-  static void CloseAllSessions();
+  // Builds a FlipSession from an existing socket.
+  FlipSession* GetFlipSessionFromSocket(
+      const HostResolver::RequestInfo& info,
+      HttpNetworkSession* session,
+      ClientSocket* socket) {
+    // TODO(willchan): Implement this to allow a HttpNetworkTransaction to
+    // upgrade a TCP connection from HTTP to FLIP.
+    return NULL;
+  }
 
- protected:
-  friend class FlipSession;
+  // TODO(willchan): Consider renaming to HasReusableSession, since perhaps we
+  // should be creating a new session.
+  bool HasSession(const HostResolver::RequestInfo& info) const;
 
-  // Return a FlipSession to the pool.
-  void Remove(FlipSession* session);
+  // Close all Flip Sessions; used for debugging.
+  void CloseAllSessions();
 
  private:
+  friend class FlipSession;  // Needed for Remove().
+
   typedef std::list<FlipSession*> FlipSessionList;
   typedef std::map<std::string, FlipSessionList*> FlipSessionsMap;
 
+  // Return a FlipSession to the pool.
+  void Remove(FlipSession* session);
+
   // Helper functions for manipulating the lists.
-  FlipSessionList* AddSessionList(std::string domain);
-  static FlipSessionList* GetSessionList(std::string domain);
-  static void RemoveSessionList(std::string domain);
+  FlipSessionList* AddSessionList(const std::string& domain);
+  FlipSessionList* GetSessionList(const std::string& domain);
+  const FlipSessionList* GetSessionList(const std::string& domain) const;
+  void RemoveSessionList(const std::string& domain);
 
   // This is our weak session pool - one session per domain.
-  static scoped_ptr<FlipSessionsMap> sessions_;
+  FlipSessionsMap sessions_;
+
+  DISALLOW_COPY_AND_ASSIGN(FlipSessionPool);
 };
 
 }  // namespace net
 
 #endif  // NET_FLIP_FLIP_SESSION_POOL_H_
-
diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc
index 1857e52..74cd548 100644
--- a/net/http/http_network_layer.cc
+++ b/net/http/http_network_layer.cc
@@ -8,6 +8,7 @@
 #include "net/flip/flip_framer.h"
 #include "net/flip/flip_network_transaction.h"
 #include "net/flip/flip_session.h"
+#include "net/flip/flip_session_pool.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_network_transaction.h"
 #include "net/socket/client_socket_factory.h"
@@ -47,6 +48,7 @@ HttpNetworkLayer::HttpNetworkLayer(ClientSocketFactory* socket_factory,
       proxy_service_(proxy_service),
       ssl_config_service_(ssl_config_service),
       session_(NULL),
+      flip_session_pool_(NULL),
       suspended_(false) {
   DCHECK(proxy_service_);
   DCHECK(ssl_config_service_.get());
@@ -56,6 +58,7 @@ HttpNetworkLayer::HttpNetworkLayer(HttpNetworkSession* session)
     : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
       ssl_config_service_(NULL),
       session_(session),
+      flip_session_pool_(session->flip_session_pool()),
       suspended_(false) {
   DCHECK(session_.get());
 }
@@ -88,8 +91,10 @@ void HttpNetworkLayer::Suspend(bool suspend) {
 HttpNetworkSession* HttpNetworkLayer::GetSession() {
   if (!session_) {
     DCHECK(proxy_service_);
-    session_ = new HttpNetworkSession(host_resolver_, proxy_service_,
-                                      socket_factory_, ssl_config_service_);
+    FlipSessionPool* flip_pool = enable_flip_ ? new FlipSessionPool : NULL;
+    session_ = new HttpNetworkSession(
+        host_resolver_, proxy_service_, socket_factory_,
+        ssl_config_service_, flip_pool);
     // These were just temps for lazy-initializing HttpNetworkSession.
     host_resolver_ = NULL;
     proxy_service_ = NULL;
diff --git a/net/http/http_network_layer.h b/net/http/http_network_layer.h
index 84f966d..e40be8c 100644
--- a/net/http/http_network_layer.h
+++ b/net/http/http_network_layer.h
@@ -14,6 +14,7 @@
 namespace net {
 
 class ClientSocketFactory;
+class FlipSessionPool;
 class HostResolver;
 class HttpNetworkSession;
 class ProxyInfo;
@@ -73,6 +74,8 @@ class HttpNetworkLayer : public HttpTransactionFactory {
   scoped_refptr<SSLConfigService> ssl_config_service_;
 
   scoped_refptr<HttpNetworkSession> session_;
+  scoped_refptr<FlipSessionPool> flip_session_pool_;
+
   bool suspended_;
   static bool enable_flip_;
 };
diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc
index bd543f0..f372292 100644
--- a/net/http/http_network_session.cc
+++ b/net/http/http_network_session.cc
@@ -5,6 +5,7 @@
 #include "net/http/http_network_session.h"
 
 #include "base/logging.h"
+#include "net/flip/flip_session_pool.h"
 
 namespace net {
 
@@ -18,18 +19,23 @@ HttpNetworkSession::HttpNetworkSession(
     HostResolver* host_resolver,
     ProxyService* proxy_service,
     ClientSocketFactory* client_socket_factory,
-    SSLConfigService* ssl_config_service)
+    SSLConfigService* ssl_config_service,
+    FlipSessionPool* flip_session_pool)
     : tcp_socket_pool_(new TCPClientSocketPool(
           max_sockets_, max_sockets_per_group_, host_resolver,
           client_socket_factory)),
       socket_factory_(client_socket_factory),
       host_resolver_(host_resolver),
       proxy_service_(proxy_service),
-      ssl_config_service_(ssl_config_service) {
+      ssl_config_service_(ssl_config_service),
+      flip_session_pool_(flip_session_pool) {
   DCHECK(proxy_service);
   DCHECK(ssl_config_service);
 }
 
+HttpNetworkSession::~HttpNetworkSession() {
+}
+
 // static
 void HttpNetworkSession::set_max_sockets_per_group(int socket_count) {
   DCHECK(0 < socket_count);
diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h
index 664d277..b5786de 100644
--- a/net/http/http_network_session.h
+++ b/net/http/http_network_session.h
@@ -16,13 +16,16 @@
 namespace net {
 
 class ClientSocketFactory;
+class FlipSessionPool;
 
 // This class holds session objects used by HttpNetworkTransaction objects.
 class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> {
  public:
   HttpNetworkSession(HostResolver* host_resolver, ProxyService* proxy_service,
                      ClientSocketFactory* client_socket_factory,
-                     SSLConfigService* ssl_config_service);
+                     SSLConfigService* ssl_config_service,
+                     FlipSessionPool* flip_session_pool);
+  ~HttpNetworkSession();
 
   HttpAuthCache* auth_cache() { return &auth_cache_; }
   SSLClientAuthCache* ssl_client_auth_cache() {
@@ -36,6 +39,7 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> {
   HostResolver* host_resolver() { return host_resolver_; }
   ProxyService* proxy_service() { return proxy_service_; }
   SSLConfigService* ssl_config_service() { return ssl_config_service_; }
+  FlipSessionPool* flip_session_pool() { return flip_session_pool_; }
 
   static void set_max_sockets_per_group(int socket_count);
 
@@ -57,6 +61,7 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> {
   scoped_refptr<HostResolver> host_resolver_;
   scoped_refptr<ProxyService> proxy_service_;
   scoped_refptr<SSLConfigService> ssl_config_service_;
+  scoped_refptr<FlipSessionPool> flip_session_pool_;
 };
 
 }  // namespace net
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index b47e34a..d929075 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -11,6 +11,7 @@
 #include "net/base/ssl_info.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/upload_data.h"
+#include "net/flip/flip_session_pool.h"
 #include "net/http/http_auth_handler_ntlm.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_network_session.h"
@@ -41,18 +42,21 @@ class SessionDependencies {
   SessionDependencies()
       : host_resolver(new MockHostResolver),
         proxy_service(CreateNullProxyService()),
-        ssl_config_service(new SSLConfigServiceDefaults) {}
+        ssl_config_service(new SSLConfigServiceDefaults),
+        flip_session_pool(new FlipSessionPool) {}
 
   // Custom proxy service dependency.
   explicit SessionDependencies(ProxyService* proxy_service)
       : host_resolver(new MockHostResolver),
         proxy_service(proxy_service),
-        ssl_config_service(new SSLConfigServiceDefaults) {}
+        ssl_config_service(new SSLConfigServiceDefaults),
+        flip_session_pool(new FlipSessionPool) {}
 
   scoped_refptr<MockHostResolverBase> host_resolver;
   scoped_refptr<ProxyService> proxy_service;
   scoped_refptr<SSLConfigService> ssl_config_service;
   MockClientSocketFactory socket_factory;
+  scoped_refptr<FlipSessionPool> flip_session_pool;
 };
 
 ProxyService* CreateFixedProxyService(const std::string& proxy) {
@@ -66,7 +70,8 @@ HttpNetworkSession* CreateSession(SessionDependencies* session_deps) {
   return new HttpNetworkSession(session_deps->host_resolver,
                                 session_deps->proxy_service,
                                 &session_deps->socket_factory,
-                                session_deps->ssl_config_service);
+                                session_deps->ssl_config_service,
+                                session_deps->flip_session_pool);
 }
 
 class HttpNetworkTransactionTest : public PlatformTest {