Reland "Simplify in-process URLRequestContext creation with the NetworkService."

This reverts commit d6870941.

Reason for revert: The original CL wasn't the cause of the mac bots
failing.

Original change's description:
> Revert "Simplify in-process URLRequestContext creation with the NetworkService."
> 
> This reverts commit f34bba7c.
> 
> Reason for revert: A lot of tests are failing on Mac10.10 and Mac10.12.
> Looking at both blame[1][2] lists when the tests started failing, this
> seems like the most likely culprit.
> 
> [1] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.10%20Tests/33158
> [2] https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/13801
> 
> Original change's description:
> > Simplify in-process URLRequestContext creation with the NetworkService.
> >
> > The old code hooked up things to the URLRequestContext that hadn't been
> > ported over to work with the NetworkService yet, when the NetworkService
> > was enabled. This CL just does the minimum setup that's needed to not
> > crash. It also makes requests made with the in-process URLRequestContext
> > fail when the network service is enabled. These changes will help
> > identify code that still depends on the legacy path, and allow for some
> > cleanup of URLRequestContextBuilderMojo and NetworkContext.
> >
> > Cq-Include-Trybots: luci.chromium.try:linux_mojo
> > Change-Id: I4c3f40e6dc3c235844846bff7d1d43d0b1c986d0
> > Bug: 825242
> > Reviewed-on: https://chromium-review.googlesource.com/1096075
> > Commit-Queue: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#567879}
> 
> TBR=jam@chromium.org,mmenke@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 825242
> Change-Id: I36da9fb06fa89296e031c88d7c75c16e63f3b725
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Reviewed-on: https://chromium-review.googlesource.com/1103818
> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#567932}

TBR=jam@chromium.org,mmenke@chromium.org,ortuno@chromium.org

Change-Id: I4f379142535672553027802c10d0a678a5734aad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 825242
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1103858Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567948}

chrome/browser/BUILD.gn

@@ -784,6 +784,8 @@ jumbo_split_static_library("browser") {
     "net/dns_probe_runner.h",
     "net/dns_probe_service.cc",
     "net/dns_probe_service.h",
+    "net/failing_url_request_interceptor.cc",
+    "net/failing_url_request_interceptor.h",
     "net/file_downloader.cc",
     "net/file_downloader.h",
     "net/net_error_diagnostics_dialog.h",

chrome/browser/io_thread.cc

@@ -36,6 +36,7 @@
 #include "chrome/browser/data_use_measurement/chrome_data_use_ascriber.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/net/dns_probe_service.h"
+#include "chrome/browser/net/failing_url_request_interceptor.h"
 #include "chrome/browser/net/proxy_service_factory.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_features.h"
@@ -457,10 +458,33 @@ void IOThread::SetUpProxyService(
 }
 
 void IOThread::ConstructSystemRequestContext() {
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    globals_->deprecated_network_quality_estimator =
+        std::make_unique<net::NetworkQualityEstimator>(
+            std::make_unique<net::NetworkQualityEstimatorParams>(
+                std::map<std::string, std::string>()),
+            net_log_);
+    net::URLRequestContextBuilder builder;
+    std::vector<std::unique_ptr<net::URLRequestInterceptor>>
+        url_request_interceptors;
+    url_request_interceptors.emplace_back(
+        std::make_unique<FailingURLRequestInterceptor>());
+    builder.SetInterceptors(std::move(url_request_interceptors));
+    builder.set_network_quality_estimator(
+        globals_->deprecated_network_quality_estimator.get());
+    builder.SetCertVerifier(
+        std::make_unique<WrappedCertVerifierForIOThreadTesting>());
+    builder.set_proxy_resolution_service(
+        net::ProxyResolutionService::CreateDirect());
+    globals_->system_request_context_owner =
+        network::URLRequestContextOwner(nullptr, builder.Build());
+    globals_->system_request_context =
+        globals_->system_request_context_owner.url_request_context.get();
+    network_context_params_.reset();
+  } else {
     std::unique_ptr<network::URLRequestContextBuilderMojo> builder =
         std::make_unique<network::URLRequestContextBuilderMojo>();
 
-  if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
     auto chrome_network_delegate = std::make_unique<ChromeNetworkDelegate>(
         extension_event_router_forwarder(), &system_enable_referrers_);
     // By default, data usage is considered off the record.
@@ -470,9 +494,6 @@ void IOThread::ConstructSystemRequestContext() {
     builder->set_network_delegate(
         globals_->data_use_ascriber->CreateNetworkDelegate(
             std::move(chrome_network_delegate), GetMetricsDataUseForwarder()));
-  }
-  std::unique_ptr<net::HostResolver> host_resolver(
-      CreateGlobalHostResolver(net_log_));
 
     std::unique_ptr<net::CertVerifier> cert_verifier;
     if (g_cert_verifier_for_io_thread_testing) {
@@ -504,22 +525,8 @@ void IOThread::ConstructSystemRequestContext() {
     if (!is_quic_allowed_on_init_)
       globals_->quic_disabled = true;
 
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
-    globals_->deprecated_network_quality_estimator =
-        std::make_unique<net::NetworkQualityEstimator>(
-            std::make_unique<net::NetworkQualityEstimatorParams>(
-                std::map<std::string, std::string>()),
-            net_log_);
-    globals_->deprecated_host_resolver = std::move(host_resolver);
-    globals_->system_request_context_owner = std::move(builder)->Create(
-        std::move(network_context_params_).get(), !is_quic_allowed_on_init_,
-        net_log_, globals_->deprecated_host_resolver.get(),
-        globals_->deprecated_network_quality_estimator.get());
-    globals_->system_request_context =
-        globals_->system_request_context_owner.url_request_context.get();
-  } else {
     network::NetworkService* network_service = content::GetNetworkServiceImpl();
-    network_service->SetHostResolver(std::move(host_resolver));
+    network_service->SetHostResolver(CreateGlobalHostResolver(net_log_));
 
     // These must be done after the SetHostResolver call.
     network_service->SetUpHttpAuth(std::move(http_auth_static_params_));

chrome/browser/net/failing_url_request_interceptor.cc

+// Copyright 2018 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/net/failing_url_request_interceptor.h"
+
+#include "net/base/net_errors.h"
+#include "net/url_request/url_request_error_job.h"
+
+FailingURLRequestInterceptor::FailingURLRequestInterceptor() {}
+
+FailingURLRequestInterceptor::~FailingURLRequestInterceptor() {}
+
+net::URLRequestJob* FailingURLRequestInterceptor::MaybeInterceptRequest(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) const {
+  return new net::URLRequestErrorJob(request, network_delegate,
+                                     net::ERR_NOT_IMPLEMENTED);
+}

chrome/browser/net/failing_url_request_interceptor.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_NET_FAILING_URL_REQUEST_INTERCEPTOR_H_
+#define CHROME_BROWSER_NET_FAILING_URL_REQUEST_INTERCEPTOR_H_
+
+#include "net/url_request/url_request_interceptor.h"
+
+// A URLRequestInterceptor that fails all network requests.
+class FailingURLRequestInterceptor : public net::URLRequestInterceptor {
+ public:
+  FailingURLRequestInterceptor();
+  ~FailingURLRequestInterceptor() override;
+
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FailingURLRequestInterceptor);
+};
+
+#endif  // CHROME_BROWSER_NET_FAILING_URL_REQUEST_INTERCEPTOR_H_

chrome/browser/profiles/profile_io_data.cc

@@ -34,6 +34,7 @@
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/net/chrome_url_request_context_getter.h"
+#include "chrome/browser/net/failing_url_request_interceptor.h"
 #include "chrome/browser/net/profile_network_context_service.h"
 #include "chrome/browser/net/profile_network_context_service_factory.h"
 #include "chrome/browser/policy/cloud/policy_header_service_factory.h"
@@ -528,7 +529,8 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
   network_prediction_options_.MoveToThread(io_task_runner);
 
 #if defined(OS_CHROMEOS)
-  if (!g_cert_verifier_for_profile_io_data_testing) {
+  if (!g_cert_verifier_for_profile_io_data_testing &&
+      !base::FeatureList::IsEnabled(network::features::kNetworkService)) {
     profile_params_->policy_cert_verifier =
         policy::PolicyCertServiceFactory::CreateForProfile(profile);
   }
@@ -1032,6 +1034,47 @@ void ProfileIOData::Init(
   extensions_request_context_.reset(new net::URLRequestContext());
   extensions_request_context_->set_name("extensions");
 
+  // Take ownership over these parameters.
+  cookie_settings_ = profile_params_->cookie_settings;
+  host_content_settings_map_ = profile_params_->host_content_settings_map;
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  extension_info_map_ = profile_params_->extension_info_map;
+#endif
+
+#if defined(OS_CHROMEOS)
+  username_hash_ = profile_params_->username_hash;
+  system_key_slot_use_type_ = profile_params_->system_key_slot_use_type;
+  // If we're using the system slot for certificate management, we also must
+  // have access to the user's slots.
+  DCHECK(!(username_hash_.empty() &&
+           system_key_slot_use_type_ ==
+               SystemKeySlotUseType::kUseForClientAuthAndCertManagement));
+  if (system_key_slot_use_type_ ==
+      SystemKeySlotUseType::kUseForClientAuthAndCertManagement) {
+    EnableNSSSystemKeySlotForResourceContext(resource_context_.get());
+  }
+
+  certificate_provider_ = std::move(profile_params_->certificate_provider);
+#endif
+
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    net::URLRequestContextBuilder builder;
+    std::vector<std::unique_ptr<net::URLRequestInterceptor>>
+        url_request_interceptors;
+    url_request_interceptors.emplace_back(
+        std::make_unique<FailingURLRequestInterceptor>());
+    builder.SetInterceptors(std::move(url_request_interceptors));
+    builder.set_network_quality_estimator(
+        io_thread_globals->deprecated_network_quality_estimator.get());
+    builder.set_proxy_resolution_service(
+        net::ProxyResolutionService::CreateDirect());
+    builder.SetCertVerifier(
+        std::make_unique<WrappedCertVerifierForProfileIODataTesting>());
+    main_request_context_owner_ =
+        network::URLRequestContextOwner(nullptr, builder.Build());
+    main_request_context_ =
+        main_request_context_owner_.url_request_context.get();
+  } else {
     // Create the main request context.
     std::unique_ptr<network::URLRequestContextBuilderMojo> builder =
         std::make_unique<network::URLRequestContextBuilderMojo>();
@@ -1083,29 +1126,6 @@ void ProfileIOData::Init(
 
     io_thread->SetUpProxyService(builder.get());
 
-  // Take ownership over these parameters.
-  cookie_settings_ = profile_params_->cookie_settings;
-  host_content_settings_map_ = profile_params_->host_content_settings_map;
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-  extension_info_map_ = profile_params_->extension_info_map;
-#endif
-
-#if defined(OS_CHROMEOS)
-  username_hash_ = profile_params_->username_hash;
-  system_key_slot_use_type_ = profile_params_->system_key_slot_use_type;
-  // If we're using the system slot for certificate management, we also must
-  // have access to the user's slots.
-  DCHECK(!(username_hash_.empty() &&
-           system_key_slot_use_type_ ==
-               SystemKeySlotUseType::kUseForClientAuthAndCertManagement));
-  if (system_key_slot_use_type_ ==
-      SystemKeySlotUseType::kUseForClientAuthAndCertManagement) {
-    EnableNSSSystemKeySlotForResourceContext(resource_context_.get());
-  }
-
-  certificate_provider_ = std::move(profile_params_->certificate_provider);
-#endif
-
     if (g_cert_verifier_for_profile_io_data_testing) {
       builder->SetCertVerifier(
           std::make_unique<WrappedCertVerifierForProfileIODataTesting>());
@@ -1114,16 +1134,18 @@ void ProfileIOData::Init(
 #if defined(OS_CHROMEOS)
       crypto::ScopedPK11Slot public_slot =
           crypto::GetPublicSlotForChromeOSUser(username_hash_);
-    // The private slot won't be ready by this point. It shouldn't be necessary
-    // for cert trust purposes anyway.
+      // The private slot won't be ready by this point. It shouldn't be
+      // necessary for cert trust purposes anyway.
       scoped_refptr<net::CertVerifyProc> verify_proc(
           new chromeos::CertVerifyProcChromeOS(std::move(public_slot)));
       if (profile_params_->policy_cert_verifier) {
-      profile_params_->policy_cert_verifier->InitializeOnIOThread(verify_proc);
+        profile_params_->policy_cert_verifier->InitializeOnIOThread(
+            verify_proc);
         cert_verifier = std::move(profile_params_->policy_cert_verifier);
       } else {
         cert_verifier = std::make_unique<net::CachingCertVerifier>(
-          std::make_unique<net::MultiThreadedCertVerifier>(verify_proc.get()));
+            std::make_unique<net::MultiThreadedCertVerifier>(
+                verify_proc.get()));
       }
 #elif defined(OS_LINUX) || defined(OS_MACOSX)
       cert_verifier = std::make_unique<net::CachingCertVerifier>(
@@ -1159,21 +1181,11 @@ void ProfileIOData::Init(
     builder->SetCreateHttpTransactionFactoryCallback(
         base::BindOnce(&content::CreateDevToolsNetworkTransactionFactory));
 
-  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
-    main_request_context_owner_ = std::move(builder)->Create(
-        std::move(profile_params_->main_network_context_params).get(),
-        io_thread_globals->quic_disabled, io_thread->net_log(),
-        io_thread_globals->deprecated_host_resolver.get(),
-        io_thread_globals->deprecated_network_quality_estimator.get());
-    main_request_context_ =
-        main_request_context_owner_.url_request_context.get();
-  } else {
     main_network_context_ =
         content::GetNetworkServiceImpl()->CreateNetworkContextWithBuilder(
             std::move(profile_params_->main_network_context_request),
             std::move(profile_params_->main_network_context_params),
             std::move(builder), &main_request_context_);
-  }
 
     if (!base::FeatureList::IsEnabled(network::features::kNetworkService) &&
         chrome_network_delegate_unowned->domain_reliability_monitor()) {
@@ -1201,6 +1213,7 @@ void ProfileIOData::Init(
     resource_context_->host_resolver_ =
         io_thread_globals->system_request_context->host_resolver();
     resource_context_->request_context_ = main_request_context_;
+  }
 
   OnMainRequestContextCreated(profile_params_.get());