Simplify HttpCache/HttpNetworkLayer/HttpNetworkSession interaction.

Eliminate lazy initialization of HttpNetworkSession in HttpNetworkLayer.
  * This eliminates the need to update parameters for HttpNetworkLayer, it just takes a HttpNetworkSession.
  * It is OK to eliminate lazy initialization since these variables are cheap.

BUG=none
TEST=none

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

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

5 files changed, 102 insertions, 263 deletions

diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 3ef5a7b7..1690e55 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -33,6 +33,7 @@
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_util.h"
+#include "net/socket/client_socket_factory.h"
 #include "net/socket/ssl_host_info.h"
 #include "net/spdy/spdy_session_pool.h"
 
@@ -278,7 +279,6 @@ class HttpCache::SSLHostInfoFactoryAdaptor : public SSLHostInfoFactory {
 };
 
 //-----------------------------------------------------------------------------
-
 HttpCache::HttpCache(HostResolver* host_resolver,
                      CertVerifier* cert_verifier,
                      DnsRRResolver* dnsrr_resolver,
@@ -295,21 +295,34 @@ HttpCache::HttpCache(HostResolver* host_resolver,
       mode_(NORMAL),
       ssl_host_info_factory_(new SSLHostInfoFactoryAdaptor(
             ALLOW_THIS_IN_INITIALIZER_LIST(this))),
-      network_layer_(HttpNetworkLayer::CreateFactory(host_resolver,
-          cert_verifier, dnsrr_resolver, dns_cert_checker_,
-          ssl_host_info_factory_.get(),
-          proxy_service, ssl_config_service,
-          http_auth_handler_factory, network_delegate, net_log)),
+      network_layer_(
+          new HttpNetworkLayer(
+              new HttpNetworkSession(
+                  host_resolver,
+                  cert_verifier,
+                  dnsrr_resolver,
+                  dns_cert_checker_,
+                  ssl_host_info_factory_.get(),
+                  proxy_service,
+                  ClientSocketFactory::GetDefaultFactory(),
+                  ssl_config_service,
+                  new SpdySessionPool(ssl_config_service),
+                  http_auth_handler_factory,
+                  network_delegate,
+                  net_log))),
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
 }
 
+
 HttpCache::HttpCache(HttpNetworkSession* session,
                      BackendFactory* backend_factory)
     : net_log_(session->net_log()),
       backend_factory_(backend_factory),
       building_backend_(false),
       mode_(NORMAL),
-      network_layer_(HttpNetworkLayer::CreateFactory(session)),
+      ssl_host_info_factory_(new SSLHostInfoFactoryAdaptor(
+            ALLOW_THIS_IN_INITIALIZER_LIST(this))),
+      network_layer_(new HttpNetworkLayer(session)),
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) {
 }
 
diff --git a/net/http/http_cache.h b/net/http/http_cache.h
index b431ee6..563ed17 100644
--- a/net/http/http_cache.h
+++ b/net/http/http_cache.h
@@ -131,7 +131,7 @@ class HttpCache : public HttpTransactionFactory,
   // The disk cache is initialized lazily (by CreateTransaction) in  this case.
   // Provide an existing HttpNetworkSession, the cache can construct a
   // network layer with a shared HttpNetworkSession in order for multiple
-  // network layers to share information (e.g. authenication data). The
+  // network layers to share information (e.g. authentication data). The
   // HttpCache takes ownership of the |backend_factory|.
   HttpCache(HttpNetworkSession* session, BackendFactory* backend_factory);
 
diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc
index 975e75c..293f8ec 100644
--- a/net/http/http_network_layer.cc
+++ b/net/http/http_network_layer.cc
@@ -18,81 +18,8 @@
 namespace net {
 
 //-----------------------------------------------------------------------------
-
-HttpNetworkLayer::HttpNetworkLayer(
-    ClientSocketFactory* socket_factory,
-    HostResolver* host_resolver,
-    CertVerifier* cert_verifier,
-    DnsRRResolver* dnsrr_resolver,
-    DnsCertProvenanceChecker* dns_cert_checker,
-    SSLHostInfoFactory* ssl_host_info_factory,
-    ProxyService* proxy_service,
-    SSLConfigService* ssl_config_service,
-    HttpAuthHandlerFactory* http_auth_handler_factory,
-    HttpNetworkDelegate* network_delegate,
-    NetLog* net_log)
-    : socket_factory_(socket_factory),
-      host_resolver_(host_resolver),
-      cert_verifier_(cert_verifier),
-      dnsrr_resolver_(dnsrr_resolver),
-      dns_cert_checker_(dns_cert_checker),
-      ssl_host_info_factory_(ssl_host_info_factory),
-      proxy_service_(proxy_service),
-      ssl_config_service_(ssl_config_service),
-      session_(NULL),
-      spdy_session_pool_(NULL),
-      http_auth_handler_factory_(http_auth_handler_factory),
-      network_delegate_(network_delegate),
-      net_log_(net_log),
-      suspended_(false) {
-  DCHECK(proxy_service_);
-  DCHECK(ssl_config_service_.get());
-}
-
-HttpNetworkLayer::HttpNetworkLayer(
-    ClientSocketFactory* socket_factory,
-    HostResolver* host_resolver,
-    CertVerifier* cert_verifier,
-    DnsRRResolver* dnsrr_resolver,
-    DnsCertProvenanceChecker* dns_cert_checker,
-    SSLHostInfoFactory* ssl_host_info_factory,
-    ProxyService* proxy_service,
-    SSLConfigService* ssl_config_service,
-    SpdySessionPool* spdy_session_pool,
-    HttpAuthHandlerFactory* http_auth_handler_factory,
-    HttpNetworkDelegate* network_delegate,
-    NetLog* net_log)
-    : socket_factory_(socket_factory),
-      host_resolver_(host_resolver),
-      cert_verifier_(cert_verifier),
-      dnsrr_resolver_(dnsrr_resolver),
-      dns_cert_checker_(dns_cert_checker),
-      ssl_host_info_factory_(ssl_host_info_factory),
-      proxy_service_(proxy_service),
-      ssl_config_service_(ssl_config_service),
-      session_(NULL),
-      spdy_session_pool_(spdy_session_pool),
-      http_auth_handler_factory_(http_auth_handler_factory),
-      network_delegate_(network_delegate),
-      net_log_(net_log),
-      suspended_(false) {
-  DCHECK(proxy_service_);
-  DCHECK(ssl_config_service_.get());
-}
-
 HttpNetworkLayer::HttpNetworkLayer(HttpNetworkSession* session)
-    : socket_factory_(ClientSocketFactory::GetDefaultFactory()),
-      host_resolver_(NULL),
-      cert_verifier_(NULL),
-      dnsrr_resolver_(NULL),
-      dns_cert_checker_(NULL),
-      ssl_host_info_factory_(NULL),
-      ssl_config_service_(NULL),
-      session_(session),
-      spdy_session_pool_(NULL),
-      http_auth_handler_factory_(NULL),
-      network_delegate_(NULL),
-      net_log_(NULL),
+    : session_(session),
       suspended_(false) {
   DCHECK(session_.get());
 }
@@ -104,29 +31,6 @@ HttpNetworkLayer::~HttpNetworkLayer() {
 
 // static
 HttpTransactionFactory* HttpNetworkLayer::CreateFactory(
-    HostResolver* host_resolver,
-    CertVerifier* cert_verifier,
-    DnsRRResolver* dnsrr_resolver,
-    DnsCertProvenanceChecker* dns_cert_checker,
-    SSLHostInfoFactory* ssl_host_info_factory,
-    ProxyService* proxy_service,
-    SSLConfigService* ssl_config_service,
-    HttpAuthHandlerFactory* http_auth_handler_factory,
-    HttpNetworkDelegate* network_delegate,
-    NetLog* net_log) {
-  DCHECK(proxy_service);
-
-  return new HttpNetworkLayer(ClientSocketFactory::GetDefaultFactory(),
-                              host_resolver, cert_verifier, dnsrr_resolver,
-                              dns_cert_checker,
-                              ssl_host_info_factory, proxy_service,
-                              ssl_config_service, http_auth_handler_factory,
-                              network_delegate,
-                              net_log);
-}
-
-// static
-HttpTransactionFactory* HttpNetworkLayer::CreateFactory(
     HttpNetworkSession* session) {
   DCHECK(session);
 
@@ -242,35 +146,6 @@ HttpCache* HttpNetworkLayer::GetCache() {
 }
 
 HttpNetworkSession* HttpNetworkLayer::GetSession() {
-  if (!session_) {
-    DCHECK(proxy_service_);
-    if (!spdy_session_pool_.get())
-      spdy_session_pool_.reset(new SpdySessionPool(ssl_config_service_));
-    session_ = new HttpNetworkSession(
-        host_resolver_,
-        cert_verifier_,
-        dnsrr_resolver_,
-        dns_cert_checker_,
-        ssl_host_info_factory_,
-        proxy_service_,
-        socket_factory_,
-        ssl_config_service_,
-        spdy_session_pool_.release(),
-        http_auth_handler_factory_,
-        network_delegate_,
-        net_log_);
-    // These were just temps for lazy-initializing HttpNetworkSession.
-    host_resolver_ = NULL;
-    cert_verifier_ = NULL;
-    dnsrr_resolver_ = NULL;
-    dns_cert_checker_ = NULL;
-    ssl_host_info_factory_ = NULL;
-    proxy_service_ = NULL;
-    socket_factory_ = NULL;
-    http_auth_handler_factory_ = NULL;
-    net_log_ = NULL;
-    network_delegate_ = NULL;
-  }
   return session_;
 }
 
diff --git a/net/http/http_network_layer.h b/net/http/http_network_layer.h
index 963ebee..da92761 100644
--- a/net/http/http_network_layer.h
+++ b/net/http/http_network_layer.h
@@ -32,52 +32,10 @@ class SSLHostInfoFactory;
 class HttpNetworkLayer : public HttpTransactionFactory,
                          public base::NonThreadSafe {
  public:
-  // |socket_factory|, |proxy_service|, |host_resolver|, etc. must remain
-  // valid for the lifetime of HttpNetworkLayer.
-  // TODO(wtc): we only need the next constructor.
-  HttpNetworkLayer(ClientSocketFactory* socket_factory,
-                   HostResolver* host_resolver,
-                   CertVerifier* cert_verifier,
-                   DnsRRResolver* dnsrr_resolver,
-                   DnsCertProvenanceChecker* dns_cert_checker,
-                   SSLHostInfoFactory* ssl_host_info_factory,
-                   ProxyService* proxy_service,
-                   SSLConfigService* ssl_config_service,
-                   HttpAuthHandlerFactory* http_auth_handler_factory,
-                   HttpNetworkDelegate* network_delegate,
-                   NetLog* net_log);
-  HttpNetworkLayer(
-      ClientSocketFactory* socket_factory,
-      HostResolver* host_resolver,
-      CertVerifier* cert_verifier,
-      DnsRRResolver* dnsrr_resolver,
-      DnsCertProvenanceChecker* dns_cert_checker,
-      SSLHostInfoFactory* ssl_host_info_factory,
-      ProxyService* proxy_service,
-      SSLConfigService* ssl_config_service,
-      SpdySessionPool* spdy_session_pool,
-      HttpAuthHandlerFactory* http_auth_handler_factory,
-      HttpNetworkDelegate* network_delegate,
-      NetLog* net_log);
-
   // Construct a HttpNetworkLayer with an existing HttpNetworkSession which
   // contains a valid ProxyService.
   explicit HttpNetworkLayer(HttpNetworkSession* session);
-  ~HttpNetworkLayer();
-
-  // This function hides the details of how a network layer gets instantiated
-  // and allows other implementations to be substituted.
-  static HttpTransactionFactory* CreateFactory(
-      HostResolver* host_resolver,
-      CertVerifier* cert_verifier,
-      DnsRRResolver* dnsrr_resolver,
-      DnsCertProvenanceChecker* dns_cert_checker,
-      SSLHostInfoFactory* ssl_host_info_factory,
-      ProxyService* proxy_service,
-      SSLConfigService* ssl_config_service,
-      HttpAuthHandlerFactory* http_auth_handler_factory,
-      HttpNetworkDelegate* network_delegate,
-      NetLog* net_log);
+  virtual ~HttpNetworkLayer();
 
   // Create a transaction factory that instantiate a network layer over an
   // existing network session. Network session contains some valuable
@@ -104,28 +62,7 @@ class HttpNetworkLayer : public HttpTransactionFactory,
   virtual void Suspend(bool suspend);
 
  private:
-  // The factory we will use to create network sockets.
-  ClientSocketFactory* socket_factory_;
-
-  // The host resolver, proxy service, etc. that will be used when lazily
-  // creating |session_|.
-  HostResolver* host_resolver_;
-  CertVerifier* cert_verifier_;
-  DnsRRResolver* dnsrr_resolver_;
-  DnsCertProvenanceChecker* dns_cert_checker_;
-  SSLHostInfoFactory* ssl_host_info_factory_;
-  scoped_refptr<ProxyService> proxy_service_;
-
-  // The SSL config service being used for the session.
-  scoped_refptr<SSLConfigService> ssl_config_service_;
-
-  scoped_refptr<HttpNetworkSession> session_;
-  scoped_ptr<SpdySessionPool> spdy_session_pool_;
-
-  HttpAuthHandlerFactory* http_auth_handler_factory_;
-  HttpNetworkDelegate* network_delegate_;
-  NetLog* net_log_;
-
+  const scoped_refptr<HttpNetworkSession> session_;
   bool suspended_;
 };
 
diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index 2720c10..f76ffbb 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -7,9 +7,11 @@
 #include "net/base/net_log.h"
 #include "net/base/ssl_config_service_defaults.h"
 #include "net/http/http_network_layer.h"
+#include "net/http/http_network_session.h"
 #include "net/http/http_transaction_unittest.h"
 #include "net/proxy/proxy_service.h"
 #include "net/socket/socket_test_util.h"
+#include "net/spdy/spdy_session_pool.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -21,115 +23,127 @@ class HttpNetworkLayerTest : public PlatformTest {
 };
 
 TEST_F(HttpNetworkLayerTest, CreateAndDestroy) {
+  MockClientSocketFactory mock_socket_factory;
   MockHostResolver host_resolver;
-  net::CertVerifier cert_verifier;
-  net::HttpNetworkLayer factory(
-      NULL,
-      &host_resolver,
-      &cert_verifier,
-      NULL /* dnsrr_resolver */,
-      NULL /* dns_cert_checker */,
-      NULL /* ssl_host_info_factory */,
-      net::ProxyService::CreateDirect(),
-      new net::SSLConfigServiceDefaults,
-      NULL,
-      NULL,
-      NULL);
-
-  scoped_ptr<net::HttpTransaction> trans;
+  CertVerifier cert_verifier;
+  scoped_refptr<HttpNetworkSession> network_session(
+      new HttpNetworkSession(
+          &host_resolver,
+          &cert_verifier,
+          NULL /* dnsrr_resolver */,
+          NULL /* dns_cert_checker */,
+          NULL /* ssl_host_info_factory */,
+          ProxyService::CreateDirect(),
+          &mock_socket_factory,
+          new SSLConfigServiceDefaults,
+          new SpdySessionPool(NULL),
+          NULL,
+          NULL,
+          NULL));
+
+  HttpNetworkLayer factory(network_session);
+
+  scoped_ptr<HttpTransaction> trans;
   int rv = factory.CreateTransaction(&trans);
-  EXPECT_EQ(net::OK, rv);
+  EXPECT_EQ(OK, rv);
   EXPECT_TRUE(trans.get() != NULL);
 }
 
 TEST_F(HttpNetworkLayerTest, Suspend) {
+  MockClientSocketFactory mock_socket_factory;
   MockHostResolver host_resolver;
-  net::CertVerifier cert_verifier;
-  net::HttpNetworkLayer factory(
-      NULL,
-      &host_resolver,
-      &cert_verifier,
-      NULL /* dnsrr_resolver */,
-      NULL /* dns_cert_checker */,
-      NULL /* ssl_host_info_factory */,
-      net::ProxyService::CreateDirect(),
-      new net::SSLConfigServiceDefaults,
-      NULL,
-      NULL,
-      NULL);
-
-  scoped_ptr<net::HttpTransaction> trans;
+  CertVerifier cert_verifier;
+  scoped_refptr<HttpNetworkSession> network_session(
+      new HttpNetworkSession(
+          &host_resolver,
+          &cert_verifier,
+          NULL /* dnsrr_resolver */,
+          NULL /* dns_cert_checker */,
+          NULL /* ssl_host_info_factory */,
+          ProxyService::CreateDirect(),
+          &mock_socket_factory,
+          new SSLConfigServiceDefaults,
+          new SpdySessionPool(NULL),
+          NULL,
+          NULL,
+          NULL));
+  HttpNetworkLayer factory(network_session);
+
+  scoped_ptr<HttpTransaction> trans;
   int rv = factory.CreateTransaction(&trans);
-  EXPECT_EQ(net::OK, rv);
+  EXPECT_EQ(OK, rv);
 
   trans.reset();
 
   factory.Suspend(true);
 
   rv = factory.CreateTransaction(&trans);
-  EXPECT_EQ(net::ERR_NETWORK_IO_SUSPENDED, rv);
+  EXPECT_EQ(ERR_NETWORK_IO_SUSPENDED, rv);
 
   ASSERT_TRUE(trans == NULL);
 
   factory.Suspend(false);
 
   rv = factory.CreateTransaction(&trans);
-  EXPECT_EQ(net::OK, rv);
+  EXPECT_EQ(OK, rv);
 }
 
 TEST_F(HttpNetworkLayerTest, GET) {
-  net::MockClientSocketFactory mock_socket_factory;
-  net::MockRead data_reads[] = {
-    net::MockRead("HTTP/1.0 200 OK\r\n\r\n"),
-    net::MockRead("hello world"),
-    net::MockRead(false, net::OK),
+  MockClientSocketFactory mock_socket_factory;
+  MockRead data_reads[] = {
+    MockRead("HTTP/1.0 200 OK\r\n\r\n"),
+    MockRead("hello world"),
+    MockRead(false, OK),
   };
-  net::MockWrite data_writes[] = {
-    net::MockWrite("GET / HTTP/1.1\r\n"
+  MockWrite data_writes[] = {
+    MockWrite("GET / HTTP/1.1\r\n"
                    "Host: www.google.com\r\n"
                    "Connection: keep-alive\r\n"
                    "User-Agent: Foo/1.0\r\n\r\n"),
   };
-  net::StaticSocketDataProvider data(data_reads, arraysize(data_reads),
+  StaticSocketDataProvider data(data_reads, arraysize(data_reads),
                                      data_writes, arraysize(data_writes));
   mock_socket_factory.AddSocketDataProvider(&data);
 
   MockHostResolver host_resolver;
-  net::CertVerifier cert_verifier;
-  net::HttpNetworkLayer factory(
-      &mock_socket_factory,
-      &host_resolver,
-      &cert_verifier,
-      NULL /* dnsrr_resolver */,
-      NULL /* dns_cert_checker */,
-      NULL /* ssl_host_info_factory */,
-      net::ProxyService::CreateDirect(),
-      new net::SSLConfigServiceDefaults,
-      NULL,
-      NULL,
-      NULL);
+  CertVerifier cert_verifier;
+  scoped_refptr<HttpNetworkSession> network_session(
+      new HttpNetworkSession(
+          &host_resolver,
+          &cert_verifier,
+          NULL /* dnsrr_resolver */,
+          NULL /* dns_cert_checker */,
+          NULL /* ssl_host_info_factory */,
+          ProxyService::CreateDirect(),
+          &mock_socket_factory,
+          new SSLConfigServiceDefaults,
+          new SpdySessionPool(NULL),
+          NULL,
+          NULL,
+          NULL));
+  HttpNetworkLayer factory(network_session);
 
   TestCompletionCallback callback;
 
-  scoped_ptr<net::HttpTransaction> trans;
+  scoped_ptr<HttpTransaction> trans;
   int rv = factory.CreateTransaction(&trans);
-  EXPECT_EQ(net::OK, rv);
+  EXPECT_EQ(OK, rv);
 
-  net::HttpRequestInfo request_info;
+  HttpRequestInfo request_info;
   request_info.url = GURL("http://www.google.com/");
   request_info.method = "GET";
-  request_info.extra_headers.SetHeader(net::HttpRequestHeaders::kUserAgent,
+  request_info.extra_headers.SetHeader(HttpRequestHeaders::kUserAgent,
                                        "Foo/1.0");
-  request_info.load_flags = net::LOAD_NORMAL;
+  request_info.load_flags = LOAD_NORMAL;
 
-  rv = trans->Start(&request_info, &callback, net::BoundNetLog());
-  if (rv == net::ERR_IO_PENDING)
+  rv = trans->Start(&request_info, &callback, BoundNetLog());
+  if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
-  ASSERT_EQ(net::OK, rv);
+  ASSERT_EQ(OK, rv);
 
   std::string contents;
   rv = ReadTransaction(trans.get(), &contents);
-  EXPECT_EQ(net::OK, rv);
+  EXPECT_EQ(OK, rv);
   EXPECT_EQ("hello world", contents);
 }