Create ContextHostResolver

Wraps HostResolverImpl and replaces almost all direct use of
HostResolverImpl. In subsequent CLs, this will become a per-context
object around the mostly-global HostResolverImpl, but until then, there
should be no functional difference from this CL alone. Just adding an
additional layer of indirection, generally invisible to callers.

In a slight deviation from a previous version of this work
(crrev.com/c/1418880), the new object is what implements the
HostResolver interface (not HostResolverImpl) to avoid including the
per-context parameters (especially HostCache) in the interface and keep
them implementation details between ContextHostResolver and
HostResolverImpl.

This also led to separating off ProcTaskParams
(to host_resolver_proc.h) as its no longer clearly owned by either
ContextHostResolver or HostResolverImpl (parameter to one, used by the
other) and it's now desirable to limit #includes of
host_resolver_impl.h.

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

components/cronet/stale_host_resolver.cc

@@ -17,6 +17,7 @@
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
+#include "net/dns/context_host_resolver.h"
 #include "net/dns/dns_util.h"
 #include "net/dns/host_resolver_source.h"
 #include "net/log/net_log_with_source.h"
@@ -422,7 +423,7 @@ StaleHostResolver::StaleOptions::StaleOptions()
       use_stale_on_name_not_resolved(false) {}
 
 StaleHostResolver::StaleHostResolver(
-    std::unique_ptr<net::HostResolverImpl> inner_resolver,
+    std::unique_ptr<net::ContextHostResolver> inner_resolver,
     const StaleOptions& stale_options)
     : inner_resolver_(std::move(inner_resolver)),
       options_(stale_options),

components/cronet/stale_host_resolver.h

@@ -12,18 +12,21 @@
 #include "base/time/default_tick_clock.h"
 #include "net/base/completion_once_callback.h"
 #include "net/dns/host_resolver.h"
-#include "net/dns/host_resolver_impl.h"
 
 namespace base {
 class TickClock;
 }  // namespace base
 
+namespace net {
+class ContextHostResolver;
+}  // namespace net
+
 namespace cronet {
 namespace {
 class StaleHostResolverTest;
 }  // namespace
 
-// A HostResolver that wraps a HostResolverImpl and uses it to make requests,
+// A HostResolver that wraps a ContextHostResolver and uses it to make requests,
 // but "impatiently" returns stale data (if available and usable) after a delay,
 // to reduce DNS latency at the expense of accuracy.
 class StaleHostResolver : public net::HostResolver {
@@ -60,7 +63,7 @@ class StaleHostResolver : public net::HostResolver {
   // Creates a StaleHostResolver that uses |inner_resolver| for actual
   // resolution, but potentially returns stale data according to
   // |stale_options|.
-  StaleHostResolver(std::unique_ptr<net::HostResolverImpl> inner_resolver,
+  StaleHostResolver(std::unique_ptr<net::ContextHostResolver> inner_resolver,
                     const StaleOptions& stale_options);
 
   ~StaleHostResolver() override;
@@ -107,9 +110,9 @@ class StaleHostResolver : public net::HostResolver {
   // Set |tick_clock_| for testing. Must be set before issuing any requests.
   void SetTickClockForTesting(const base::TickClock* tick_clock);
 
-  // The underlying HostResolverImpl that will be used to make cache and network
-  // requests.
-  std::unique_ptr<net::HostResolverImpl> inner_resolver_;
+  // The underlying ContextHostResolver that will be used to make cache and
+  // network requests.
+  std::unique_ptr<net::ContextHostResolver> inner_resolver_;
 
   // Shared instance of tick clock, overridden for testing.
   const base::TickClock* tick_clock_ = base::DefaultTickClock::GetInstance();

components/cronet/stale_host_resolver_unittest.cc

@@ -30,6 +30,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier.h"
 #include "net/cert/cert_verifier.h"
+#include "net/dns/context_host_resolver.h"
 #include "net/dns/dns_config.h"
 #include "net/dns/dns_hosts.h"
 #include "net/dns/dns_test_util.h"
@@ -171,14 +172,15 @@ class StaleHostResolverTest : public testing::Test {
     mock_proc_ = new MockHostResolverProc(result);
   }
 
-  std::unique_ptr<net::HostResolverImpl> CreateMockInnerResolverWithDnsClient(
+  std::unique_ptr<net::ContextHostResolver>
+  CreateMockInnerResolverWithDnsClient(
       std::unique_ptr<net::DnsClient> dns_client) {
-    std::unique_ptr<net::HostResolverImpl> inner_resolver(
+    std::unique_ptr<net::ContextHostResolver> inner_resolver(
         net::HostResolver::CreateDefaultResolverImpl(nullptr));
 
-    net::HostResolverImpl::ProcTaskParams proc_params(mock_proc_.get(), 1u);
-    inner_resolver->set_proc_params_for_test(proc_params);
-    inner_resolver->SetDnsClient(std::move(dns_client));
+    net::ProcTaskParams proc_params(mock_proc_.get(), 1u);
+    inner_resolver->SetProcParamsForTesting(proc_params);
+    inner_resolver->SetDnsClientForTesting(std::move(dns_client));
     return inner_resolver;
   }
 

components/cronet/url_request_context_config.cc

@@ -24,6 +24,7 @@
 #include "net/cert/ct_policy_status.h"
 #include "net/cert/do_nothing_ct_verifier.h"
 #include "net/cert/multi_threaded_cert_verifier.h"
+#include "net/dns/context_host_resolver.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/mapped_host_resolver.h"
 #include "net/http/http_network_session.h"

net/dns/BUILD.gn

@@ -31,6 +31,8 @@ source_set("dns") {
     sources += [
       "address_sorter.h",
       "address_sorter_win.cc",
+      "context_host_resolver.cc",
+      "context_host_resolver.h",
       "dns_config.cc",
       "dns_config_overrides.cc",
       "dns_config_service.cc",
@@ -433,8 +435,8 @@ source_set("test_support") {
 source_set("fuzzer_test_support") {
   testonly = true
   sources = [
-    "fuzzed_host_resolver.cc",
-    "fuzzed_host_resolver.h",
+    "fuzzed_context_host_resolver.cc",
+    "fuzzed_context_host_resolver.h",
   ]
   deps = [
     "//base",

net/dns/context_host_resolver.cc

@@ -0,0 +1,107 @@
+// Copyright (c) 2019 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 "net/dns/context_host_resolver.h"
+
+#include <utility>
+
+#include "base/strings/string_piece.h"
+#include "base/time/tick_clock.h"
+#include "net/dns/dns_client.h"
+#include "net/dns/dns_config.h"
+#include "net/dns/host_resolver_impl.h"
+#include "net/dns/host_resolver_proc.h"
+
+namespace net {
+
+ContextHostResolver::ContextHostResolver(std::unique_ptr<HostResolverImpl> impl)
+    : impl_(std::move(impl)) {}
+
+ContextHostResolver::~ContextHostResolver() = default;
+
+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);
+}
+
+std::unique_ptr<HostResolver::MdnsListener>
+ContextHostResolver::CreateMdnsListener(const HostPortPair& host,
+                                        DnsQueryType query_type) {
+  return impl_->CreateMdnsListener(host, query_type);
+}
+
+void ContextHostResolver::SetDnsClientEnabled(bool enabled) {
+  impl_->SetDnsClientEnabled(enabled);
+}
+
+HostCache* ContextHostResolver::GetHostCache() {
+  return impl_->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);
+}
+
+std::unique_ptr<base::Value> ContextHostResolver::GetDnsConfigAsValue() const {
+  return impl_->GetDnsConfigAsValue();
+}
+
+void ContextHostResolver::SetNoIPv6OnWifi(bool no_ipv6_on_wifi) {
+  impl_->SetNoIPv6OnWifi(no_ipv6_on_wifi);
+}
+
+bool ContextHostResolver::GetNoIPv6OnWifi() {
+  return impl_->GetNoIPv6OnWifi();
+}
+
+void ContextHostResolver::SetDnsConfigOverrides(
+    const DnsConfigOverrides& overrides) {
+  impl_->SetDnsConfigOverrides(overrides);
+}
+
+void ContextHostResolver::SetRequestContext(
+    URLRequestContext* request_context) {
+  impl_->SetRequestContext(request_context);
+}
+
+const std::vector<DnsConfig::DnsOverHttpsServerConfig>*
+ContextHostResolver::GetDnsOverHttpsServersForTesting() const {
+  return impl_->GetDnsOverHttpsServersForTesting();
+}
+
+size_t ContextHostResolver::LastRestoredCacheSize() const {
+  return impl_->LastRestoredCacheSize();
+}
+
+size_t ContextHostResolver::CacheSize() const {
+  return impl_->CacheSize();
+}
+
+void ContextHostResolver::SetProcParamsForTesting(
+    const ProcTaskParams& proc_params) {
+  impl_->set_proc_params_for_test(proc_params);
+}
+
+void ContextHostResolver::SetDnsClientForTesting(
+    std::unique_ptr<DnsClient> dns_client) {
+  impl_->SetDnsClient(std::move(dns_client));
+}
+
+void ContextHostResolver::SetBaseDnsConfigForTesting(
+    const DnsConfig& base_config) {
+  impl_->SetBaseDnsConfigForTesting(base_config);
+}
+
+void ContextHostResolver::SetTickClockForTesting(
+    const base::TickClock* tick_clock) {
+  impl_->SetTickClockForTesting(tick_clock);
+}
+
+}  // namespace net

net/dns/context_host_resolver.h

@@ -0,0 +1,82 @@
+// Copyright (c) 2019 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 NET_DNS_CONTEXT_HOST_RESOLVER_H_
+#define NET_DNS_CONTEXT_HOST_RESOLVER_H_
+
+#include <memory>
+#include <vector>
+
+#include "net/base/net_export.h"
+#include "net/dns/host_resolver.h"
+
+namespace base {
+class TickClock;
+}  // namespace base
+
+namespace net {
+
+class DnsClient;
+struct DnsConfig;
+class HostResolverImpl;
+struct ProcTaskParams;
+
+// Wrapper for HostResolverImpl that sets per-context parameters for created
+// requests. Except for tests, typically only interacted with through the
+// HostResolver interface.
+//
+// See HostResolver::Create[...]() methods for construction.
+//
+// TODO(crbug.com/934402): Construct individually for each URLRequestContext
+// rather than using this as the singleton shared resolver.
+class NET_EXPORT ContextHostResolver : public HostResolver {
+ public:
+  // Creates a ContextHostResolver that forwards all of its requests through
+  // |impl|.
+  explicit ContextHostResolver(std::unique_ptr<HostResolverImpl> impl);
+  ~ContextHostResolver() override;
+
+  // HostResolver methods:
+  std::unique_ptr<ResolveHostRequest> CreateRequest(
+      const HostPortPair& host,
+      const NetLogWithSource& net_log,
+      const base::Optional<ResolveHostParameters>& optional_parameters)
+      override;
+  std::unique_ptr<MdnsListener> CreateMdnsListener(
+      const HostPortPair& host,
+      DnsQueryType query_type) override;
+  void SetDnsClientEnabled(bool enabled) override;
+  HostCache* GetHostCache() override;
+  bool HasCached(base::StringPiece hostname,
+                 HostCache::Entry::Source* source_out,
+                 HostCache::EntryStaleness* stale_out,
+                 bool* secure_out) const override;
+  std::unique_ptr<base::Value> GetDnsConfigAsValue() const override;
+  void SetNoIPv6OnWifi(bool no_ipv6_on_wifi) override;
+  bool GetNoIPv6OnWifi() override;
+  void SetDnsConfigOverrides(const DnsConfigOverrides& overrides) override;
+  void SetRequestContext(URLRequestContext* request_context) override;
+  const std::vector<DnsConfig::DnsOverHttpsServerConfig>*
+  GetDnsOverHttpsServersForTesting() const override;
+
+  // Returns the number of host cache entries that were restored, or 0 if there
+  // is no cache.
+  size_t LastRestoredCacheSize() const;
+  // Returns the number of entries in the host cache, or 0 if there is no cache.
+  size_t CacheSize() const;
+
+  void SetProcParamsForTesting(const ProcTaskParams& proc_params);
+  void SetDnsClientForTesting(std::unique_ptr<DnsClient> dns_client);
+  void SetBaseDnsConfigForTesting(const DnsConfig& base_config);
+  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<HostResolverImpl> impl_;
+};
+
+}  // namespace net
+
+#endif  // NET_DNS_CONTEXT_HOST_RESOLVER_H_