Use common code to set HttpNetworkSession::Param pointers.

chrome/browser/io_thread.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/io_thread.h"
 
 #include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
@@ skipping 69 matching lines @@
 #include "net/spdy/spdy_session.h"
 #include "net/ssl/channel_id_service.h"
 #include "net/ssl/default_channel_id_store.h"
 #include "net/url_request/data_protocol_handler.h"
 #include "net/url_request/file_protocol_handler.h"
 #include "net/url_request/ftp_protocol_handler.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_backoff_manager.h"
 #include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "url/url_constants.h"
 
 #if defined(ENABLE_CONFIGURATION_POLICY)
 #include "policy/policy_constants.h"
 #endif
 
 #if defined(ENABLE_EXTENSIONS)
 #include "chrome/browser/extensions/event_router_forwarder.h"
@@ skipping 107 matching lines @@
   if (!command_line.HasSwitch(switches::kHostResolverRules))
     return global_host_resolver.Pass();
 
   scoped_ptr<net::MappedHostResolver> remapped_resolver(
       new net::MappedHostResolver(global_host_resolver.Pass()));
   remapped_resolver->SetRulesFromString(
       command_line.GetSwitchValueASCII(switches::kHostResolverRules));
   return remapped_resolver.Pass();
 }
 
-// TODO(willchan): Remove proxy script fetcher context since it's not necessary
-// now that I got rid of refcounting URLRequestContexts.
-// See IOThread::Globals for details.
-net::URLRequestContext*
-ConstructProxyScriptFetcherContext(IOThread::Globals* globals,
-                                   net::NetLog* net_log) {
-  net::URLRequestContext* context = new net::URLRequestContext;
-  context->set_net_log(net_log);
-  context->set_host_resolver(globals->host_resolver.get());
-  context->set_cert_verifier(globals->cert_verifier.get());
-  context->set_transport_security_state(
-      globals->transport_security_state.get());
-  context->set_cert_transparency_verifier(
-      globals->cert_transparency_verifier.get());
-  context->set_http_auth_handler_factory(
-      globals->http_auth_handler_factory.get());
-  context->set_proxy_service(globals->proxy_script_fetcher_proxy_service.get());
-  context->set_http_transaction_factory(
-      globals->proxy_script_fetcher_http_transaction_factory.get());
-  context->set_job_factory(
-      globals->proxy_script_fetcher_url_request_job_factory.get());
-  context->set_cookie_store(globals->system_cookie_store.get());
-  context->set_channel_id_service(
-      globals->system_channel_id_service.get());
-  context->set_network_delegate(globals->system_network_delegate.get());
-  context->set_http_user_agent_settings(
-      globals->http_user_agent_settings.get());
-  // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
-  // system URLRequestContext too. There's no reason this should be tied to a
-  // profile.
-  return context;
-}
-
-net::URLRequestContext*
-ConstructSystemRequestContext(IOThread::Globals* globals,
-                              net::NetLog* net_log) {
-  net::URLRequestContext* context = new SystemURLRequestContext;
-  context->set_net_log(net_log);
-  context->set_host_resolver(globals->host_resolver.get());
-  context->set_cert_verifier(globals->cert_verifier.get());
-  context->set_transport_security_state(
-      globals->transport_security_state.get());
-  context->set_cert_transparency_verifier(
-      globals->cert_transparency_verifier.get());
-  context->set_http_auth_handler_factory(
-      globals->http_auth_handler_factory.get());
-  context->set_proxy_service(globals->system_proxy_service.get());
-  context->set_http_transaction_factory(
-      globals->system_http_transaction_factory.get());
-  context->set_job_factory(globals->system_url_request_job_factory.get());
-  context->set_cookie_store(globals->system_cookie_store.get());
-  context->set_channel_id_service(
-      globals->system_channel_id_service.get());
-  context->set_network_delegate(globals->system_network_delegate.get());
-  context->set_http_user_agent_settings(
-      globals->http_user_agent_settings.get());
-  context->set_network_quality_estimator(
-      globals->network_quality_estimator.get());
-  context->set_backoff_manager(globals->url_request_backoff_manager.get());
-  return context;
-}
-
 int GetSwitchValueAsInt(const base::CommandLine& command_line,
                         const std::string& switch_name) {
   int value;
   if (!base::StringToInt(command_line.GetSwitchValueASCII(switch_name),
                          &value)) {
     return 0;
   }
   return value;
 }
 
@@ skipping 520 matching lines @@
           switches::kEnableUserAlternateProtocolPorts)) {
     globals_->enable_user_alternate_protocol_ports = true;
   }
   // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
   // is fixed.
   tracked_objects::ScopedTracker tracking_profile13(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "466432 IOThread::InitAsync::InitializeNetworkOptions"));
   InitializeNetworkOptions(command_line);
 
-  net::HttpNetworkSession::Params session_params;
-  InitializeNetworkSessionParams(&session_params);
-  session_params.net_log = net_log_;
-  session_params.proxy_service =
-      globals_->proxy_script_fetcher_proxy_service.get();
-
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile14(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "466432 IOThread::InitAsync::HttpNetorkSession::Start"));
-  TRACE_EVENT_BEGIN0("startup", "IOThread::InitAsync:HttpNetworkSession");
-  scoped_refptr<net::HttpNetworkSession> network_session(
-      new net::HttpNetworkSession(session_params));
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile15(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "466432 IOThread::InitAsync::HttpNetorkSession::End"));
-  globals_->proxy_script_fetcher_http_transaction_factory
-      .reset(new net::HttpNetworkLayer(network_session.get()));
-  TRACE_EVENT_END0("startup", "IOThread::InitAsync:HttpNetworkSession");
-  scoped_ptr<net::URLRequestJobFactoryImpl> job_factory(
-      new net::URLRequestJobFactoryImpl());
-
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile16(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "466432 IOThread::InitAsync::SetProtocolHandler"));
-  job_factory->SetProtocolHandler(url::kDataScheme,
-                                  new net::DataProtocolHandler());
-  job_factory->SetProtocolHandler(
-      url::kFileScheme,
-      new net::FileProtocolHandler(
-          content::BrowserThread::GetBlockingPool()->
-              GetTaskRunnerWithShutdownBehavior(
-                  base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)));
-#if !defined(DISABLE_FTP_SUPPORT)
-  globals_->proxy_script_fetcher_ftp_transaction_factory.reset(
-      new net::FtpNetworkLayer(globals_->host_resolver.get()));
-  job_factory->SetProtocolHandler(
-      url::kFtpScheme,
-      new net::FtpProtocolHandler(
-          globals_->proxy_script_fetcher_ftp_transaction_factory.get()));
-#endif
-  globals_->proxy_script_fetcher_url_request_job_factory = job_factory.Pass();
-
+  TRACE_EVENT_BEGIN0("startup",
+                     "IOThread::Init:ProxyScriptFetcherRequestContext");
   globals_->proxy_script_fetcher_context.reset(
       ConstructProxyScriptFetcherContext(globals_, net_log_));
+  TRACE_EVENT_END0("startup",
+                   "IOThread::Init:ProxyScriptFetcherRequestContext");
 
   const version_info::Channel channel = chrome::GetChannel();
   if (channel == version_info::Channel::UNKNOWN ||
       channel == version_info::Channel::CANARY ||
       channel == version_info::Channel::DEV) {
     globals_->url_request_backoff_manager.reset(
         new net::URLRequestBackoffManager());
   }
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
@@ skipping 214 matching lines @@
 
 void IOThread::InitializeNetworkSessionParams(
     net::HttpNetworkSession::Params* params) {
   InitializeNetworkSessionParamsFromGlobals(*globals_, params);
 }
 
 // static
 void IOThread::InitializeNetworkSessionParamsFromGlobals(
     const IOThread::Globals& globals,
     net::HttpNetworkSession::Params* params) {
-  params->host_resolver = globals.host_resolver.get();
-  params->cert_verifier = globals.cert_verifier.get();
+  //  The next two properties of the params don't seem to be
+  // elements of URLRequestContext, so they must be set here.
   params->cert_policy_enforcer = globals.cert_policy_enforcer.get();
-  params->channel_id_service = globals.system_channel_id_service.get();
-  params->transport_security_state = globals.transport_security_state.get();
-  params->ssl_config_service = globals.ssl_config_service.get();
-  params->http_auth_handler_factory = globals.http_auth_handler_factory.get();
-  params->http_server_properties =
-      globals.http_server_properties->GetWeakPtr();
-  params->network_delegate = globals.system_network_delegate.get();
   params->host_mapping_rules = globals.host_mapping_rules.get();
+
   params->ignore_certificate_errors = globals.ignore_certificate_errors;
   params->testing_fixed_http_port = globals.testing_fixed_http_port;
   params->testing_fixed_https_port = globals.testing_fixed_https_port;
   globals.enable_tcp_fast_open_for_ssl.CopyToIfSet(
       &params->enable_tcp_fast_open_for_ssl);
 
   globals.initial_max_spdy_concurrent_streams.CopyToIfSet(
       &params->spdy_initial_max_concurrent_streams);
   globals.enable_spdy_compression.CopyToIfSet(
       &params->enable_spdy_compression);
@@ skipping 90 matching lines @@
       *base::CommandLine::ForCurrentProcess();
   globals_->system_proxy_service.reset(
       ProxyServiceFactory::CreateProxyService(
           net_log_,
           globals_->proxy_script_fetcher_context.get(),
           globals_->system_network_delegate.get(),
           system_proxy_config_service_.release(),
           command_line,
           quick_check_enabled_.GetValue()));
 
-  net::HttpNetworkSession::Params system_params;
-  InitializeNetworkSessionParams(&system_params);
-  system_params.net_log = net_log_;
-  system_params.proxy_service = globals_->system_proxy_service.get();
-
-  globals_->system_http_transaction_factory.reset(
-      new net::HttpNetworkLayer(
-          new net::HttpNetworkSession(system_params)));
-  globals_->system_url_request_job_factory.reset(
-      new net::URLRequestJobFactoryImpl());
   globals_->system_request_context.reset(
       ConstructSystemRequestContext(globals_, net_log_));
-  globals_->system_request_context->set_ssl_config_service(
-      globals_->ssl_config_service.get());
-  globals_->system_request_context->set_http_server_properties(
-      globals_->http_server_properties->GetWeakPtr());
 }
 
 void IOThread::UpdateDnsClientEnabled() {
   globals()->host_resolver->SetDnsClientEnabled(*dns_client_enabled_);
 }
 
 void IOThread::ConfigureQuic(const base::CommandLine& command_line) {
   // Always fetch the field trial group to ensure it is reported correctly.
   // The command line flags will be associated with a group that is reported
   // so long as trial is actually queried.
@@ skipping 343 matching lines @@
   net::QuicVersionVector supported_versions = net::QuicSupportedVersions();
   for (size_t i = 0; i < supported_versions.size(); ++i) {
     net::QuicVersion version = supported_versions[i];
     if (net::QuicVersionToString(version) == quic_version) {
       return version;
     }
   }
 
   return net::QUIC_VERSION_UNSUPPORTED;
 }
+
+// static
+net::URLRequestContext*
+IOThread::ConstructSystemRequestContext(IOThread::Globals* globals,
+                              net::NetLog* net_log) {

[mmenke, 2015/08/18 18:30:32]

fix indent

[wjmaclean, 2015/08/18 18:55:37]

Done.

+  net::URLRequestContext* context = new SystemURLRequestContext;
+  context->set_net_log(net_log);
+  context->set_host_resolver(globals->host_resolver.get());
+  context->set_cert_verifier(globals->cert_verifier.get());
+  context->set_transport_security_state(
+      globals->transport_security_state.get());
+  context->set_cert_transparency_verifier(
+      globals->cert_transparency_verifier.get());
+  context->set_ssl_config_service(globals->ssl_config_service.get());
+  context->set_http_auth_handler_factory(
+      globals->http_auth_handler_factory.get());
+  context->set_proxy_service(globals->system_proxy_service.get());
+
+  globals->system_url_request_job_factory.reset(
+      new net::URLRequestJobFactoryImpl());
+  context->set_job_factory(globals->system_url_request_job_factory.get());
+
+  context->set_cookie_store(globals->system_cookie_store.get());
+  context->set_channel_id_service(
+      globals->system_channel_id_service.get());
+  context->set_network_delegate(globals->system_network_delegate.get());
+  context->set_http_user_agent_settings(
+      globals->http_user_agent_settings.get());
+  context->set_network_quality_estimator(
+      globals->network_quality_estimator.get());
+  context->set_backoff_manager(globals->url_request_backoff_manager.get());
+
+  context->set_ssl_config_service(
+      globals->ssl_config_service.get());

[mmenke, 2015/08/18 18:30:32]

You're doing this twice.

[wjmaclean, 2015/08/18 18:55:37]

Thanks for spotting this ... fixed!

+  context->set_http_server_properties(
+      globals->http_server_properties->GetWeakPtr());
+
+  net::HttpNetworkSession::Params system_params;
+  InitializeNetworkSessionParamsFromGlobals(*globals, &system_params);
+  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
+      context, &system_params);
+
+  globals->system_http_transaction_factory.reset(
+      new net::HttpNetworkLayer(new net::HttpNetworkSession(system_params)));
+  context->set_http_transaction_factory(
+      globals->system_http_transaction_factory.get());
+
+  return context;
+}
+
+// TODO(willchan): Remove proxy script fetcher context since it's not necessary
+// now that I got rid of refcounting URLRequestContexts.
+// See IOThread::Globals for details.

[mmenke, 2015/08/18 18:30:32]

Method level comments should go with the definition, not the declaration.

[wjmaclean, 2015/08/18 18:55:37]

Done.

+// static

[mmenke, 2015/08/18 18:30:32]

optional nit:  The latest google style guide says nothing about adding static
comments, it was from an earlier version, so could just remove all the "//
statics" from this file...Fine to leave them in, though.  Up to you.

[wjmaclean, 2015/08/18 18:55:37]

Done. (Removed throughout this file.)

+net::URLRequestContext* IOThread::ConstructProxyScriptFetcherContext(
+    IOThread::Globals* globals,
+    net::NetLog* net_log) {
+  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
+  // is fixed.
+  tracked_objects::ScopedTracker tracking_profile16(
+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "466432 IOThread::ConstructProxyScriptFetcherContext"));
+  net::URLRequestContext* context = new net::URLRequestContext;
+  context->set_net_log(net_log);
+  context->set_host_resolver(globals->host_resolver.get());
+  context->set_cert_verifier(globals->cert_verifier.get());
+  context->set_transport_security_state(
+      globals->transport_security_state.get());
+  context->set_cert_transparency_verifier(
+      globals->cert_transparency_verifier.get());
+  context->set_ssl_config_service(globals->ssl_config_service.get());
+  context->set_http_auth_handler_factory(
+      globals->http_auth_handler_factory.get());
+  context->set_proxy_service(globals->proxy_script_fetcher_proxy_service.get());
+
+  context->set_cookie_store(globals->system_cookie_store.get());
+  context->set_channel_id_service(
+      globals->system_channel_id_service.get());
+  context->set_network_delegate(globals->system_network_delegate.get());
+  context->set_http_user_agent_settings(
+      globals->http_user_agent_settings.get());
+  context->set_http_server_properties(
+      globals->http_server_properties->GetWeakPtr());
+
+  net::HttpNetworkSession::Params session_params;
+  InitializeNetworkSessionParamsFromGlobals(*globals, &session_params);
+  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
+      context, &session_params);
+
+  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
+  // is fixed.
+  tracked_objects::ScopedTracker tracking_profile14(
+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "466432 IOThread::ConstructProxyScriptFetcherContext2"));
+  scoped_refptr<net::HttpNetworkSession> network_session(
+      new net::HttpNetworkSession(session_params));
+  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/466432
+  // is fixed.
+  tracked_objects::ScopedTracker tracking_profile15(

[mmenke, 2015/08/18 18:30:32]

nit:  Should probably renumber these to match their strings (1/2/3 instead of
6/4/5)

[wjmaclean, 2015/08/18 18:55:37]

Done.

+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "466432 IOThread::InitAsync::HttpNetorkSession::End"));

[mmenke, 2015/08/18 18:30:32]

IOThread::ConstructProxyScriptFetcherContext3?

[wjmaclean, 2015/08/18 18:55:37]

Done.

+  globals->proxy_script_fetcher_http_transaction_factory
+      .reset(new net::HttpNetworkLayer(network_session.get()));
+  context->set_http_transaction_factory(
+      globals->proxy_script_fetcher_http_transaction_factory.get());
+
+  scoped_ptr<net::URLRequestJobFactoryImpl> job_factory(
+      new net::URLRequestJobFactoryImpl());
+
+  job_factory->SetProtocolHandler(url::kDataScheme,
+                                  new net::DataProtocolHandler());
+  job_factory->SetProtocolHandler(
+      url::kFileScheme,
+      new net::FileProtocolHandler(
+          content::BrowserThread::GetBlockingPool()->
+              GetTaskRunnerWithShutdownBehavior(
+                  base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)));
+#if !defined(DISABLE_FTP_SUPPORT)
+  globals->proxy_script_fetcher_ftp_transaction_factory.reset(
+      new net::FtpNetworkLayer(globals->host_resolver.get()));
+  job_factory->SetProtocolHandler(
+      url::kFtpScheme,
+      new net::FtpProtocolHandler(
+          globals->proxy_script_fetcher_ftp_transaction_factory.get()));
+#endif
+  globals->proxy_script_fetcher_url_request_job_factory = job_factory.Pass();
+
+  context->set_job_factory(
+      globals->proxy_script_fetcher_url_request_job_factory.get());
+
+  // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
+  // system URLRequestContext too. There's no reason this should be tied to a
+  // profile.
+  return context;
+}