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 121 matching lines @@
 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_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());
   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());
   // 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_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(
       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());
   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),
@@ skipping 526 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();
 
   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 ||
       channel == version_info::Channel::DEV) {
     globals_->url_request_backoff_manager.reset(
         new net::URLRequestBackoffManager());
   }
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   // Start observing Keychain events. This needs to be done on the UI thread,
@@ skipping 212 matching lines @@
 }
 
 void IOThread::InitializeNetworkSessionParams(
     net::HttpNetworkSession::Params* params) {
   InitializeNetworkSessionParamsFromGlobals(*globals_, params);
 }
 
 // static
 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;
   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()));
 
+  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(
       globals_->http_server_properties->GetWeakPtr());
 }
 
 void IOThread::UpdateDnsClientEnabled() {
   globals()->host_resolver->SetDnsClientEnabled(*dns_client_enabled_);
 }
 
@@ skipping 347 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;
 }