Prepare for per-context resolvers with new creation methods

Rewrote the standard HostResolver::Create...Resolver() methods. There
are now two general methods, one that takes a shared HostResolverManager
pointer, and one that creates a "standalone" resolver, matching current
behavior. Took the opportunity to combine the behaviors of CreateDefault
and CreateSystem since C++11 default args are a cleaner way to set
default options (at least for static methods like these).

Added TODOs to all the non-test and non-standalone tools that will need
to be converted to calling the creation with shared manager, but such
conversions are left to subsequent CLs.

TBR=seantopping@chromium.org,eugenebut@chromium.org,dimich@chromium.org,mmenke@chromium.org

Bug: 934402
Change-Id: I126b61db55b61ebd75b7b6f526b31ef0f0a02404
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531391
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#643385}

chrome/browser/io_thread.cc

@@ -153,8 +153,10 @@ std::unique_ptr<net::HostResolver> CreateGlobalHostResolver(
   std::string host_mapping_rules =
       command_line.GetSwitchValueASCII(network::switches::kHostResolverRules);
 
-  return net::HostResolver::CreateSystemResolver(net::HostResolver::Options(),
-                                                 net_log, host_mapping_rules);
+  // TODO(crbug.com/934402): Use a shared HostResolverManager instead of a
+  // global HostResolver.
+  return net::HostResolver::CreateStandaloneResolver(
+      net_log, net::HostResolver::Options(), host_mapping_rules);
 }
 
 }  // namespace

chromecast/browser/url_request_context_factory.cc

@@ -233,7 +233,9 @@ void URLRequestContextFactory::InitializeSystemContextDependencies() {
   if (system_dependencies_initialized_)
     return;
 
-  host_resolver_ = net::HostResolver::CreateDefaultResolver(NULL);
+  // TODO(crbug.com/934402): Use a shared HostResolverManager instead of a
+  // global HostResolver.
+  host_resolver_ = net::HostResolver::CreateStandaloneResolver(nullptr);
   cert_verifier_ = net::CertVerifier::CreateDefault();
   ssl_config_service_.reset(new net::SSLConfigServiceDefaults);
   transport_security_state_.reset(new net::TransportSecurityState());

components/cronet/ios/cronet_environment.mm

@@ -359,9 +359,11 @@ bool IsNetLogPathValid(const base::FilePath& path) {
   effective_experimental_options_ =
       std::move(config->effective_experimental_options);
 
+  // TODO(crbug.com/934402): Use a shared HostResolverManager instead of a
+  // global HostResolver.
   std::unique_ptr<net::MappedHostResolver> mapped_host_resolver(
       new net::MappedHostResolver(
-          net::HostResolver::CreateDefaultResolver(nullptr)));
+          net::HostResolver::CreateStandaloneResolver(nullptr)));
 
   if (!config->storage_path.empty()) {
     cronet_prefs_manager_ = std::make_unique<CronetPrefsManager>(

components/cronet/stale_host_resolver_unittest.cc

@@ -176,7 +176,7 @@ class StaleHostResolverTest : public testing::Test {
   CreateMockInnerResolverWithDnsClient(
       std::unique_ptr<net::DnsClient> dns_client) {
     std::unique_ptr<net::ContextHostResolver> inner_resolver(
-        net::HostResolver::CreateDefaultResolverImpl(nullptr));
+        net::HostResolver::CreateStandaloneContextResolver(nullptr));
 
     net::ProcTaskParams proc_params(mock_proc_.get(), 1u);
     inner_resolver->SetProcParamsForTesting(proc_params);

components/cronet/url_request_context_config.cc

@@ -672,13 +672,15 @@ void URLRequestContextConfig::ParseAndSetExperimentalOptions(
       disable_ipv6_on_wifi) {
     CHECK(net_log) << "All DNS-related experiments require NetLog.";
     std::unique_ptr<net::HostResolver> host_resolver;
+    // TODO(crbug.com/934402): Consider using a shared HostResolverManager for
+    // Cronet HostResolvers.
     if (stale_dns_enable) {
       DCHECK(!disable_ipv6_on_wifi);
       host_resolver.reset(new StaleHostResolver(
-          net::HostResolver::CreateDefaultResolverImpl(net_log),
+          net::HostResolver::CreateStandaloneContextResolver(net_log),
           stale_dns_options));
     } else {
-      host_resolver = net::HostResolver::CreateDefaultResolver(net_log);
+      host_resolver = net::HostResolver::CreateStandaloneResolver(net_log);
     }
     if (disable_ipv6_on_wifi)
       host_resolver->SetNoIPv6OnWifi(true);

google_apis/gcm/tools/mcs_probe.cc

@@ -212,7 +212,6 @@ class MCSProbe {
   std::unique_ptr<net::URLRequestContext> url_request_context_;
   net::NetLog net_log_;
   std::unique_ptr<net::FileNetLogObserver> logger_;
-  std::unique_ptr<net::HostResolver> host_resolver_;
   MCSProbeAuthPreferences http_auth_preferences_;
   std::unique_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory_;
 
@@ -339,13 +338,13 @@ void MCSProbe::InitializeNetworkState() {
     logger_->StartObserving(&net_log_, capture_mode);
   }
 
-  host_resolver_ = net::HostResolver::CreateDefaultResolver(&net_log_);
   http_auth_handler_factory_ = net::HttpAuthHandlerRegistryFactory::Create(
       &http_auth_preferences_, std::vector<std::string>{net::kBasicAuthScheme});
 
   net::URLRequestContextBuilder builder;
   builder.set_net_log(&net_log_);
-  builder.set_shared_host_resolver(host_resolver_.get());
+  builder.set_host_resolver(
+      net::HostResolver::CreateStandaloneResolver(&net_log_));
   builder.set_shared_http_auth_handler_factory(
       http_auth_handler_factory_.get());
   builder.set_proxy_resolution_service(

ios/components/io_thread/ios_io_thread.mm

@@ -99,9 +99,10 @@
     net::NetLog* net_log) {
   TRACE_EVENT0("startup", "IOSIOThread::CreateGlobalHostResolver");
 
+  // TODO(crbug.com/934402): Use a shared HostResolverManager instead of a
+  // single global HostResolver for iOS.
   std::unique_ptr<net::HostResolver> global_host_resolver =
-      net::HostResolver::CreateSystemResolver(net::HostResolver::Options(),
-                                              net_log);
+      net::HostResolver::CreateStandaloneResolver(net_log);
 
   return global_host_resolver;
 }

ios/web/shell/shell_url_request_context_getter.mm

@@ -118,7 +118,7 @@
             new net::HttpServerPropertiesImpl()));
 
     std::unique_ptr<net::HostResolver> host_resolver(
-        net::HostResolver::CreateDefaultResolver(
+        net::HostResolver::CreateStandaloneResolver(
             url_request_context_->net_log()));
     storage_->set_http_auth_handler_factory(
         net::HttpAuthHandlerFactory::CreateDefault());

ios/web_view/internal/web_view_url_request_context_getter.mm

@@ -122,7 +122,7 @@
             new net::HttpServerPropertiesImpl()));
 
     std::unique_ptr<net::HostResolver> host_resolver(
-        net::HostResolver::CreateDefaultResolver(
+        net::HostResolver::CreateStandaloneResolver(
             url_request_context_->net_log()));
     storage_->set_http_auth_handler_factory(
         net::HttpAuthHandlerFactory::CreateDefault());

net/dns/context_host_resolver.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/logging.h"
 #include "base/strings/string_piece.h"
 #include "base/time/tick_clock.h"
 #include "net/dns/dns_client.h"
@@ -15,94 +16,104 @@
 
 namespace net {
 
+ContextHostResolver::ContextHostResolver(HostResolverManager* manager)
+    : manager_(manager) {
+  DCHECK(manager_);
+}
+
 ContextHostResolver::ContextHostResolver(
-    std::unique_ptr<HostResolverManager> impl)
-    : impl_(std::move(impl)) {}
+    std::unique_ptr<HostResolverManager> owned_manager)
+    : manager_(owned_manager.get()), owned_manager_(std::move(owned_manager)) {
+  DCHECK(manager_);
+}
 
-ContextHostResolver::~ContextHostResolver() = default;
+ContextHostResolver::~ContextHostResolver() {
+  if (owned_manager_)
+    DCHECK_EQ(owned_manager_.get(), manager_);
+}
 
 std::unique_ptr<HostResolver::ResolveHostRequest>
 ContextHostResolver::CreateRequest(
     const HostPortPair& host,
     const NetLogWithSource& source_net_log,
     const base::Optional<ResolveHostParameters>& optional_parameters) {
-  return impl_->CreateRequest(host, source_net_log, optional_parameters);
+  return manager_->CreateRequest(host, source_net_log, optional_parameters);
 }
 
 std::unique_ptr<HostResolver::MdnsListener>
 ContextHostResolver::CreateMdnsListener(const HostPortPair& host,
                                         DnsQueryType query_type) {
-  return impl_->CreateMdnsListener(host, query_type);
+  return manager_->CreateMdnsListener(host, query_type);
 }
 
 void ContextHostResolver::SetDnsClientEnabled(bool enabled) {
-  impl_->SetDnsClientEnabled(enabled);
+  manager_->SetDnsClientEnabled(enabled);
 }
 
 HostCache* ContextHostResolver::GetHostCache() {
-  return impl_->GetHostCache();
+  return manager_->GetHostCache();
 }
 
 bool ContextHostResolver::HasCached(base::StringPiece hostname,
                                     HostCache::Entry::Source* source_out,
                                     HostCache::EntryStaleness* stale_out,
                                     bool* secure_out) const {
-  return impl_->HasCached(hostname, source_out, stale_out, secure_out);
+  return manager_->HasCached(hostname, source_out, stale_out, secure_out);
 }
 
 std::unique_ptr<base::Value> ContextHostResolver::GetDnsConfigAsValue() const {
-  return impl_->GetDnsConfigAsValue();
+  return manager_->GetDnsConfigAsValue();
 }
 
 void ContextHostResolver::SetNoIPv6OnWifi(bool no_ipv6_on_wifi) {
-  impl_->SetNoIPv6OnWifi(no_ipv6_on_wifi);
+  manager_->SetNoIPv6OnWifi(no_ipv6_on_wifi);
 }
 
 bool ContextHostResolver::GetNoIPv6OnWifi() {
-  return impl_->GetNoIPv6OnWifi();
+  return manager_->GetNoIPv6OnWifi();
 }
 
 void ContextHostResolver::SetDnsConfigOverrides(
     const DnsConfigOverrides& overrides) {
-  impl_->SetDnsConfigOverrides(overrides);
+  manager_->SetDnsConfigOverrides(overrides);
 }
 
 void ContextHostResolver::SetRequestContext(
     URLRequestContext* request_context) {
-  impl_->SetRequestContext(request_context);
+  manager_->SetRequestContext(request_context);
 }
 
 const std::vector<DnsConfig::DnsOverHttpsServerConfig>*
 ContextHostResolver::GetDnsOverHttpsServersForTesting() const {
-  return impl_->GetDnsOverHttpsServersForTesting();
+  return manager_->GetDnsOverHttpsServersForTesting();
 }
 
 size_t ContextHostResolver::LastRestoredCacheSize() const {
-  return impl_->LastRestoredCacheSize();
+  return manager_->LastRestoredCacheSize();
 }
 
 size_t ContextHostResolver::CacheSize() const {
-  return impl_->CacheSize();
+  return manager_->CacheSize();
 }
 
 void ContextHostResolver::SetProcParamsForTesting(
     const ProcTaskParams& proc_params) {
-  impl_->set_proc_params_for_test(proc_params);
+  manager_->set_proc_params_for_test(proc_params);
 }
 
 void ContextHostResolver::SetDnsClientForTesting(
     std::unique_ptr<DnsClient> dns_client) {
-  impl_->SetDnsClient(std::move(dns_client));
+  manager_->SetDnsClient(std::move(dns_client));
 }
 
 void ContextHostResolver::SetBaseDnsConfigForTesting(
     const DnsConfig& base_config) {
-  impl_->SetBaseDnsConfigForTesting(base_config);
+  manager_->SetBaseDnsConfigForTesting(base_config);
 }
 
 void ContextHostResolver::SetTickClockForTesting(
     const base::TickClock* tick_clock) {
-  impl_->SetTickClockForTesting(tick_clock);
+  manager_->SetTickClockForTesting(tick_clock);
 }
 
 }  // namespace net

net/dns/context_host_resolver.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/macros.h"
 #include "net/base/net_export.h"
 #include "net/dns/host_resolver.h"
 
@@ -33,8 +34,11 @@ struct ProcTaskParams;
 class NET_EXPORT ContextHostResolver : public HostResolver {
  public:
   // Creates a ContextHostResolver that forwards all of its requests through
-  // |impl|.
-  explicit ContextHostResolver(std::unique_ptr<HostResolverManager> impl);
+  // |manager|.
+  explicit ContextHostResolver(HostResolverManager* manager);
+  // Same except the created resolver will own its own HostResolverManager.
+  explicit ContextHostResolver(
+      std::unique_ptr<HostResolverManager> owned_manager);
   ~ContextHostResolver() override;
 
   // HostResolver methods:
@@ -72,9 +76,10 @@ class NET_EXPORT ContextHostResolver : public HostResolver {
   void SetTickClockForTesting(const base::TickClock* tick_clock);
 
  private:
-  // TODO(crbug.com/934402): Make this a non-owned pointer to the singleton
-  // resolver.
-  std::unique_ptr<HostResolverManager> impl_;
+  HostResolverManager* const manager_;
+  std::unique_ptr<HostResolverManager> owned_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContextHostResolver);
 };
 
 }  // namespace net

net/dns/host_resolver.cc

@@ -97,9 +97,17 @@ HostResolver::Options::Options()
       enable_caching(true) {}
 
 std::unique_ptr<HostResolver> HostResolver::Factory::CreateResolver(
+    HostResolverManager* manager,
+    base::StringPiece host_mapping_rules) {
+  return HostResolver::CreateResolver(manager, host_mapping_rules);
+}
+
+std::unique_ptr<HostResolver> HostResolver::Factory::CreateStandaloneResolver(
+    NetLog* net_log,
     const Options& options,
-    NetLog* net_log) {
-  return CreateSystemResolver(options, net_log);
+    base::StringPiece host_mapping_rules) {
+  return HostResolver::CreateStandaloneResolver(net_log, options,
+                                                host_mapping_rules);
 }
 
 HostResolver::~HostResolver() = default;
@@ -143,12 +151,12 @@ HostResolver::GetDnsOverHttpsServersForTesting() const {
 }
 
 // static
-std::unique_ptr<HostResolver> HostResolver::CreateSystemResolver(
-    const Options& options,
-    NetLog* net_log,
+std::unique_ptr<HostResolver> HostResolver::CreateResolver(
+    HostResolverManager* manager,
     base::StringPiece host_mapping_rules) {
-  std::unique_ptr<ContextHostResolver> resolver =
-      CreateSystemResolverImpl(options, net_log);
+  DCHECK(manager);
+
+  auto resolver = std::make_unique<ContextHostResolver>(manager);
 
   if (host_mapping_rules.empty())
     return resolver;
@@ -159,23 +167,29 @@ std::unique_ptr<HostResolver> HostResolver::CreateSystemResolver(
 }
 
 // static
-std::unique_ptr<ContextHostResolver> HostResolver::CreateSystemResolverImpl(
-    const Options& options,
-    NetLog* net_log) {
-  return std::make_unique<ContextHostResolver>(
-      std::make_unique<HostResolverManager>(options, net_log));
-}
+std::unique_ptr<HostResolver> HostResolver::CreateStandaloneResolver(
+    NetLog* net_log,
+    base::Optional<Options> options,
+    base::StringPiece host_mapping_rules) {
+  auto resolver = std::make_unique<ContextHostResolver>(
+      std::make_unique<HostResolverManager>(
+          std::move(options).value_or(Options()), net_log));
 
-// static
-std::unique_ptr<HostResolver> HostResolver::CreateDefaultResolver(
-    NetLog* net_log) {
-  return CreateSystemResolver(Options(), net_log);
+  if (host_mapping_rules.empty())
+    return resolver;
+  auto remapped_resolver =
+      std::make_unique<MappedHostResolver>(std::move(resolver));
+  remapped_resolver->SetRulesFromString(host_mapping_rules);
+  return remapped_resolver;
 }
 
 // static
-std::unique_ptr<ContextHostResolver> HostResolver::CreateDefaultResolverImpl(
-    NetLog* net_log) {
-  return CreateSystemResolverImpl(Options(), net_log);
+std::unique_ptr<ContextHostResolver>
+HostResolver::CreateStandaloneContextResolver(NetLog* net_log,
+                                              base::Optional<Options> options) {
+  return std::make_unique<ContextHostResolver>(
+      std::make_unique<HostResolverManager>(
+          std::move(options).value_or(Options()), net_log));
 }
 
 // static