Move more SystemURLRequestContext members into its ContextStorage

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 <utility>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 529 matching lines @@
       std::move(data_use_amortizer)));
 
 #if defined(OS_ANDROID)
   globals_->external_data_use_observer.reset(
       new chrome::android::ExternalDataUseObserver(
           globals_->data_use_aggregator.get(),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));
 #endif  // defined(OS_ANDROID)
 
-  globals_->host_resolver = CreateGlobalHostResolver(net_log_);
-
   std::map<std::string, std::string> network_quality_estimator_params;
   variations::GetVariationParams(kNetworkQualityEstimatorFieldTrialName,
                                  &network_quality_estimator_params);
 
   if (command_line.HasSwitch(switches::kForceEffectiveConnectionType)) {
     const std::string force_ect_value =
         base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
             switches::kForceEffectiveConnectionType);
 
     if (!force_ect_value.empty()) {
       // If the effective connection type is forced using command line switch,
       // it overrides the one set by field trial.
       network_quality_estimator_params[net::kForceEffectiveConnectionType] =
           force_ect_value;
     }
   }
 
   std::unique_ptr<net::ExternalEstimateProvider> external_estimate_provider;
 #if defined(OS_ANDROID)
   external_estimate_provider.reset(
       new chrome::android::ExternalEstimateProviderAndroid());
 #endif  // defined(OS_ANDROID)
   // Pass ownership.
   globals_->network_quality_estimator.reset(new net::NetworkQualityEstimator(
       std::move(external_estimate_provider), network_quality_estimator_params,
       net_log_));
   globals_->network_quality_observer = content::CreateNetworkQualityObserver(
       globals_->network_quality_estimator.get());
 
-  UpdateDnsClientEnabled();
-#if defined(OS_CHROMEOS)
-  // Creates a CertVerifyProc that doesn't allow any profile-provided certs.
-  globals_->cert_verifier = base::MakeUnique<net::CachingCertVerifier>(
-      base::MakeUnique<net::MultiThreadedCertVerifier>(
-          new chromeos::CertVerifyProcChromeOS()));
-#else
-  globals_->cert_verifier = IgnoreErrorsCertVerifier::MaybeWrapCertVerifier(
-      command_line, net::CertVerifier::CreateDefault());
-  UMA_HISTOGRAM_BOOLEAN(
-      "Net.Certificate.IgnoreCertificateErrorsSPKIListPresent",
-      command_line.HasSwitch(switches::kIgnoreCertificateErrorsSPKIList));
-#endif
-
   std::vector<scoped_refptr<const net::CTLogVerifier>> ct_logs(
       net::ct::CreateLogVerifiersForKnownLogs());
 
   globals_->ct_logs.assign(ct_logs.begin(), ct_logs.end());
 
-  net::MultiLogCTVerifier* ct_verifier = new net::MultiLogCTVerifier();
-  globals_->cert_transparency_verifier.reset(ct_verifier);
-  // Add built-in logs
-  ct_verifier->AddLogs(globals_->ct_logs);
-
   ct_tree_tracker_.reset(new certificate_transparency::TreeStateTracker(
       globals_->ct_logs, net_log_));
   // Register the ct_tree_tracker_ as observer for new STHs.
   RegisterSTHObserver(ct_tree_tracker_.get());

[Randy Smith (Not in Mondays), 2017/06/07 19:28:17]

Why not move this with the ct_verifier code move?  It looks to be a direct data
dependency; why not keep it clustered with that code?

[mmenke, 2017/06/07 21:06:03]

Because these aren't owned by (or pointed to by) the URLRequestContext, and I
want to share URLRequestContextBuilder code between this and ProfileIOData.  If
these remain shared, I'll either need special case code there for the first
URLRequestContext built, or I'll need to make all shared objects in a separate
function during set up (While the system URLRequestContext is created first,
once we switch to using a network service, that could change, if we construct
them on first use when using the NetworkService).  If I use a separate function,
it's more likely I'll move shared objects back here, and make this the separate
function.

That does perhaps argue I should be doing a design document up front to decide
some of this stuff, but there's just so much cruft that I've been reluctant to
do it, and the path is grungy enough that I'mt not sure how well I'd be able to
keep to the plan, anyways.

-  // Register the ct_tree_tracker_ as observer for verified SCTs.
-  globals_->cert_transparency_verifier->SetObserver(ct_tree_tracker_.get());
 
-  CreateDefaultAuthHandlerFactory();
   globals_->dns_probe_service.reset(new chrome_browser_net::DnsProbeService());
   globals_->host_mapping_rules.reset(new net::HostMappingRules());
   if (command_line.HasSwitch(switches::kHostRules)) {
     TRACE_EVENT_BEGIN0("startup", "IOThread::InitAsync:SetRulesFromString");
     globals_->host_mapping_rules->SetRulesFromString(
         command_line.GetSwitchValueASCII(switches::kHostRules));
     TRACE_EVENT_END0("startup", "IOThread::InitAsync:SetRulesFromString");
   }
 
   session_params_.host_mapping_rules = *globals_->host_mapping_rules.get();
@@ skipping 24 matching lines @@
 #endif
 
 #if defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
   // Record how common CPUs with broken NEON units are. See
   // https://crbug.com/341598.
   crypto::EnsureOpenSSLInit();
   UMA_HISTOGRAM_BOOLEAN("Net.HasBrokenNEON", CRYPTO_has_broken_NEON());
 #endif
 
   ConstructSystemRequestContext();
+
+  UpdateDnsClientEnabled();
 }
 
 void IOThread::CleanUp() {
   base::debug::LeakTracker<SafeBrowsingURLRequestContext>::CheckForLeaks();
 
 #if defined(USE_NSS_CERTS)
   net::ShutdownNSSHttpIO();
 #endif
 
   system_url_request_context_getter_ = NULL;
 
   // Unlink the ct_tree_tracker_ from the global cert_transparency_verifier
   // and unregister it from new STH notifications so it will take no actions
   // on anything observed during CleanUp process.
   //
   // Null checks are just for tests that use TestingIOThreadState.
-  if (globals()->cert_transparency_verifier)
-    globals()->cert_transparency_verifier->SetObserver(nullptr);
+  if (globals()->system_request_context) {
+    globals()
+        ->system_request_context->cert_transparency_verifier()
+        ->SetObserver(nullptr);
+  }
   if (ct_tree_tracker_.get()) {
     UnregisterSTHObserver(ct_tree_tracker_.get());
     ct_tree_tracker_.reset();
   }
 
   if (globals_->system_request_context)
     globals_->system_request_context->proxy_service()->OnShutdown();
 
   // Release objects that the net::URLRequestContext could have been pointing
   // to.
@@ skipping 50 matching lines @@
 void IOThread::UpdateNegotiateDisableCnameLookup() {
   globals_->http_auth_preferences->set_negotiate_disable_cname_lookup(
       negotiate_disable_cname_lookup_.GetValue());
 }
 
 void IOThread::UpdateNegotiateEnablePort() {
   globals_->http_auth_preferences->set_negotiate_enable_port(
       negotiate_enable_port_.GetValue());
 }
 
-void IOThread::CreateDefaultAuthHandlerFactory() {
+std::unique_ptr<net::HttpAuthHandlerRegistryFactory>
+IOThread::CreateDefaultAuthHandlerFactory() {
   std::vector<std::string> supported_schemes = base::SplitString(
       auth_schemes_, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   globals_->http_auth_preferences.reset(new net::HttpAuthPreferences(
       supported_schemes
 #if defined(OS_POSIX) && !defined(OS_ANDROID)
       ,
       gssapi_library_name_
 #endif
 #if defined(OS_CHROMEOS)
       ,
       allow_gssapi_library_load_
 #endif
       ));
   UpdateServerWhitelist();
   UpdateDelegateWhitelist();
   UpdateNegotiateDisableCnameLookup();
   UpdateNegotiateEnablePort();
 #if defined(OS_ANDROID)
   UpdateAndroidAuthNegotiateAccountType();
 #endif
-  globals_->http_auth_handler_factory =
-      net::HttpAuthHandlerRegistryFactory::Create(
-          globals_->http_auth_preferences.get(), globals_->host_resolver.get());
+
+  return net::HttpAuthHandlerRegistryFactory::Create(
+      globals_->http_auth_preferences.get(),
+      globals_->system_request_context->host_resolver());
 }
 
 void IOThread::ClearHostCache(
     const base::Callback<bool(const std::string&)>& host_filter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  net::HostCache* host_cache = globals_->host_resolver->GetHostCache();
+  if (!globals_->system_request_context)
+    return;
+
+  net::HostCache* host_cache =
+      globals_->system_request_context->host_resolver()->GetHostCache();
   if (host_cache)
     host_cache->ClearForHosts(host_filter);
 }
 
 const net::HttpNetworkSession::Params& IOThread::NetworkSessionParams() const {
   return session_params_;
 }
 
 void IOThread::DisableQuic() {
   session_params_.enable_quic = false;
@@ skipping 13 matching lines @@
 
 void IOThread::ChangedToOnTheRecordOnIOThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Clear the host cache to avoid showing entries from the OTR session
   // in about:net-internals.
   ClearHostCache(base::Callback<bool(const std::string&)>());
 }
 
 void IOThread::UpdateDnsClientEnabled() {
-  globals()->host_resolver->SetDnsClientEnabled(*dns_client_enabled_);
+  globals()->system_request_context->host_resolver()->SetDnsClientEnabled(
+      *dns_client_enabled_);
 }
 
 void IOThread::RegisterSTHObserver(net::ct::STHObserver* observer) {
   chrome_browser_net::GetGlobalSTHDistributor()->RegisterObserver(observer);
 }
 
 void IOThread::UnregisterSTHObserver(net::ct::STHObserver* observer) {
   chrome_browser_net::GetGlobalSTHDistributor()->UnregisterObserver(observer);
 }
 
 bool IOThread::WpadQuickCheckEnabled() const {
   return quick_check_enabled_.GetValue();
 }
 
 bool IOThread::PacHttpsUrlStrippingEnabled() const {
   return pac_https_url_stripping_enabled_.GetValue();
 }
 
 void IOThread::ConstructSystemRequestContext() {
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+
   globals_->system_request_context =
       base::MakeUnique<SystemURLRequestContext>();
   net::URLRequestContext* context = globals_->system_request_context.get();
   globals_->system_request_context_storage =
       base::MakeUnique<net::URLRequestContextStorage>(context);
   net::URLRequestContextStorage* context_storage =
       globals_->system_request_context_storage.get();
 
   context->set_network_quality_estimator(
       globals_->network_quality_estimator.get());
   context->set_enable_brotli(globals_->enable_brotli);
   context->set_name("system");
 
   context_storage->set_http_user_agent_settings(
       base::MakeUnique<net::StaticHttpUserAgentSettings>(std::string(),
                                                          GetUserAgent()));
   std::unique_ptr<ChromeNetworkDelegate> chrome_network_delegate(
       new ChromeNetworkDelegate(extension_event_router_forwarder(),
                                 &system_enable_referrers_));
   // By default, data usage is considered off the record.
   chrome_network_delegate->set_data_use_aggregator(
       globals_->data_use_aggregator.get(),
       true /* is_data_usage_off_the_record */);
   context_storage->set_network_delegate(
       globals_->data_use_ascriber->CreateNetworkDelegate(
           std::move(chrome_network_delegate), GetMetricsDataUseForwarder()));
   context->set_net_log(net_log_);
-  context->set_host_resolver(globals_->host_resolver.get());
+  context_storage->set_host_resolver(CreateGlobalHostResolver(net_log_));
 
   context_storage->set_ssl_config_service(GetSSLConfigService());
-  context->set_http_auth_handler_factory(
-      globals_->http_auth_handler_factory.get());
+  context_storage->set_http_auth_handler_factory(
+      CreateDefaultAuthHandlerFactory());

[Randy Smith (Not in Mondays), 2017/06/07 19:28:17]

Would you be willing to either put in a comment that this call must occur after
the creation of the global host resolver above, or make it implicit by giving
the host resolver as an argument to the function?

[mmenke, 2017/06/07 21:06:03]

Did the latter (Was planning to do it in the next CL, anyway, when we won't even
have a URLRequestContext to steal things from)

 
   // In-memory cookie store.
   context_storage->set_cookie_store(
       content::CreateCookieStore(content::CookieStoreConfig()));
   // In-memory channel ID store.
   context_storage->set_channel_id_service(
       base::MakeUnique<net::ChannelIDService>(
           new net::DefaultChannelIDStore(nullptr)));
   context->cookie_store()->SetChannelIDServiceID(
       context->channel_id_service()->GetUniqueID());
 
   context_storage->set_transport_security_state(
       base::MakeUnique<net::TransportSecurityState>());
 
   context_storage->set_http_server_properties(
       base::MakeUnique<net::HttpServerPropertiesImpl>());
 
-  context->set_cert_verifier(globals_->cert_verifier.get());
-  context->set_cert_transparency_verifier(
-      globals_->cert_transparency_verifier.get());
+#if defined(OS_CHROMEOS)
+  // Creates a CertVerifyProc that doesn't allow any profile-provided certs.
+  context_storage->set_cert_verifier(base::MakeUnique<net::CachingCertVerifier>(
+      base::MakeUnique<net::MultiThreadedCertVerifier>(
+          new chromeos::CertVerifyProcChromeOS())));
+#else
+  context_storage->set_cert_verifier(
+      IgnoreErrorsCertVerifier::MaybeWrapCertVerifier(
+          command_line, net::CertVerifier::CreateDefault()));
+  UMA_HISTOGRAM_BOOLEAN(
+      "Net.Certificate.IgnoreCertificateErrorsSPKIListPresent",
+      command_line.HasSwitch(switches::kIgnoreCertificateErrorsSPKIList));
+#endif
+
+  std::unique_ptr<net::MultiLogCTVerifier> ct_verifier =
+      base::MakeUnique<net::MultiLogCTVerifier>();
+  // Add built-in logs
+  ct_verifier->AddLogs(globals_->ct_logs);
+
+  // Register the ct_tree_tracker_ as observer for verified SCTs.
+  ct_verifier->SetObserver(ct_tree_tracker_.get());
+
+  context_storage->set_cert_transparency_verifier(std::move(ct_verifier));
 
   context_storage->set_ct_policy_enforcer(
       base::MakeUnique<net::CTPolicyEnforcer>());
 
-  const base::CommandLine& command_line =
-      *base::CommandLine::ForCurrentProcess();
   context_storage->set_proxy_service(ProxyServiceFactory::CreateProxyService(
       net_log_, context, context->network_delegate(),
       std::move(system_proxy_config_service_), command_line,
       WpadQuickCheckEnabled(), PacHttpsUrlStrippingEnabled()));
 
   net::HttpNetworkSession::Context session_context;
   net::URLRequestContextBuilder::SetHttpNetworkSessionComponents(
       context, &session_context);
 
   context_storage->set_http_network_session(
@@ skipping 10 matching lines @@
                                   base::MakeUnique<net::DataProtocolHandler>());
   job_factory->SetProtocolHandler(
       url::kFileScheme,
       base::MakeUnique<net::FileProtocolHandler>(
           base::CreateTaskRunnerWithTraits(
               {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
                base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})));
 #if !BUILDFLAG(DISABLE_FTP_SUPPORT)
   job_factory->SetProtocolHandler(
       url::kFtpScheme,
-      net::FtpProtocolHandler::Create(globals_->host_resolver.get()));
+      net::FtpProtocolHandler::Create(context->host_resolver()));
 #endif
 
   context_storage->set_job_factory(std::move(job_factory));
 }
 
 // static
 void IOThread::ConfigureParamsFromFieldTrialsAndCommandLine(
     const base::CommandLine& command_line,
     bool is_quic_allowed_by_policy,
     bool http_09_on_non_default_ports_enabled,
@@ skipping 82 matching lines @@
         GetSwitchValueAsInt(command_line, switches::kTestingFixedHttpsPort);
   }
 
   params->http_09_on_non_default_ports_enabled =
       http_09_on_non_default_ports_enabled;
 }
 
 metrics::UpdateUsagePrefCallbackType IOThread::GetMetricsDataUseForwarder() {
   return base::Bind(&UpdateMetricsUsagePrefsOnUIThread);
 }