Use common code to set HttpNetworkSession::Param pointers.

chrome/browser/io_thread.cc

Index: chrome/browser/io_thread.cc
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 56b428dda57052d9ab1f40de7eb16e6ede85dd5a..8d2d5fd80e2e806ce1707c8f767e5a112cef32c9 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -87,6 +87,7 @@
 #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"
@@ -228,11 +229,10 @@ ConstructProxyScriptFetcherContext(IOThread::Globals* globals,
       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_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());
@@ -241,6 +241,8 @@ ConstructProxyScriptFetcherContext(IOThread::Globals* globals,
   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());
   // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
   // system URLRequestContext too. There's no reason this should be tied to a
   // profile.
@@ -258,11 +260,10 @@ ConstructSystemRequestContext(IOThread::Globals* globals,
       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());
-  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(
@@ -270,6 +271,8 @@ ConstructSystemRequestContext(IOThread::Globals* globals,
   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());
   context->set_network_quality_estimator(
       globals->network_quality_estimator.get());
   context->set_backoff_manager(globals->url_request_backoff_manager.get());
@@ -816,28 +819,6 @@ void IOThread::Init() {
           "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());
 
@@ -867,6 +848,30 @@ void IOThread::Init() {
   globals_->proxy_script_fetcher_context.reset(
       ConstructProxyScriptFetcherContext(globals_, net_log_));
 
+  net::HttpNetworkSession::Params session_params;
+  InitializeNetworkSessionParams(&session_params);
+  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
+      &session_params, globals_->proxy_script_fetcher_context.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()));
+  globals_->proxy_script_fetcher_context->set_http_transaction_factory(
+      globals_->proxy_script_fetcher_http_transaction_factory.get());
+  TRACE_EVENT_END0("startup", "IOThread::InitAsync:HttpNetworkSession");
+
   const version_info::Channel channel = chrome::GetChannel();
   if (channel == version_info::Channel::UNKNOWN ||
       channel == version_info::Channel::CANARY ||
@@ -1100,17 +1105,12 @@ void IOThread::InitializeNetworkSessionParams(
 void IOThread::InitializeNetworkSessionParamsFromGlobals(
     const IOThread::Globals& globals,
     net::HttpNetworkSession::Params* params) {

[mmenke, 2015/08/13 15:37:38]

Not for this CL, but may make sense to separate arguments that are just settings
the HttpNetworkSession accept, and arguments that are objects that actually do
stuff.  This would mostly set the latter.

Could also then reasonably make the settings object an optional thing to set
directly on the builder.  Then we'd just have to worry about how to handle
everything else.  Could even get rid of all these "CopyToIfSet" things, and just
have globals have a copy of this object, with settings overridden just according
to flags.

Anyhow, I'm just thinking out loud here.  Certainly fine to go in some other
direction.

-  params->host_resolver = globals.host_resolver.get();
-  params->cert_verifier = globals.cert_verifier.get();
-  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();
+  // TODO(wjmaclean): The next two properties of the params don't seem to be
+  // elements of URLRequestContext, so they must be set here.
+  // Is it OK that they are shared?
+  params->cert_policy_enforcer = globals.cert_policy_enforcer.get(); // WJM

[mmenke, 2015/08/13 15:37:38]

I have no idea what this does - should probably talk to the SSL people about
this (davidben, rsleevi)

[wjmaclean, 2015/08/14 13:29:19]

I've looped in davidben@ to comment on this.

+  params->host_mapping_rules = globals.host_mapping_rules.get(); // WJM

[mmenke, 2015/08/13 15:37:38]

host_mapping_rules is for command line options (Mostly used for debugging, I
guess?), so it should be fine.

[wjmaclean, 2015/08/14 13:29:19]

Acknowledged.

+
   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;
@@ -1221,18 +1221,22 @@ void IOThread::InitSystemRequestContextOnIOThread() {
           command_line,
           quick_check_enabled_.GetValue()));
 
+  globals_->system_url_request_job_factory.reset(
+      new net::URLRequestJobFactoryImpl());
+  globals_->system_request_context.reset(
+      ConstructSystemRequestContext(globals_, net_log_));

[mmenke, 2015/08/13 15:37:38]

Everything below this line may make more sense in ConstructSystemRequestContext
(And the system_url_request_job_factory construction line, too).  Does have a
bit more side effects that way, but that's also how the builder will work, when
we switch to it.

[wjmaclean, 2015/08/14 13:29:19]

Done.

+
   net::HttpNetworkSession::Params system_params;
   InitializeNetworkSessionParams(&system_params);
-  system_params.net_log = net_log_;
-  system_params.proxy_service = globals_->system_proxy_service.get();
+  net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
+      &system_params, globals_->system_request_context.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_http_transaction_factory(
+      globals_->system_http_transaction_factory.get());
+
   globals_->system_request_context->set_ssl_config_service(
       globals_->ssl_config_service.get());
   globals_->system_request_context->set_http_server_properties(