Reland "Generalize for non-address results in HostResolverImpl"

This reverts commit e369ab31.

Reason for revert: The CL in question did not cause issue 869227, which is a pre-existing infra problem.

Original change's description:
> Revert "Generalize for non-address results in HostResolverImpl"
> 
> This reverts commit 07ee5f0f.
> 
> Reason for revert: StartTestServer(SpawnedTestServer::SSLOptions())
> started failing after this CL
> 
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/17143
> 
> [ RUN      ] SSLClientSocketReadTest.Read_WithSynchronousError/0
> [56283:775:1128/182253.758810:39802574526192:ERROR:local_test_server_posix.cc(79)] poll() timed out; bytes_read=0
> [56283:775:1128/182253.758860:39802574568901:ERROR:local_test_server_posix.cc(167)] Could not read server_data_len
> [56283:775:1128/182253.760826:39802576544202:ERROR:ssl_client_socket_unittest.cc(830)] Could not start SpawnedTestServer
> ../../net/socket/ssl_client_socket_unittest.cc:1532: Failure
> Value of: StartTestServer(SpawnedTestServer::SSLOptions())
>   Actual: false
> Expected: true
> Stack trace:
> 0   net_unittests                       0x000000010f41750b testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop(int) + 91
> 1   net_unittests                       0x000000010f416ec9 testing::internal::AssertHelper::operator=(testing::Message const&) const + 89
> 2   net_unittests                       0x000000010e7d9239 net::SSLClientSocketReadTest_Read_WithSynchronousError_Test::TestBody() + 585
> 
> [  FAILED  ] SSLClientSocketReadTest.Read_WithSynchronousError/0, where GetParam() = false (10083 ms)
> 
> Original change's description:
> > Generalize for non-address results in HostResolverImpl
> > 
> > Now extensively using HostCache::Entry internally (but only internally
> > to preserve flexibility of HostCache) as a generalized results
> > container within HostResolverImpl. Added some needed setter/merge
> > functionality to Entry for that purpose.  Many methods within the
> > resolver changed to use Entry for consistency even if they only ever
> > deal with address results, eg ResolveAsIP().
> > 
> > Slightly modified results port-setting functionality to only set port
> > to an input port (from |host.port()|) if the port in the results is 0
> > (where before it would set it whenever it wasn't equal to the input
> > port).  This will be necessary eg for SRV support where DNS provides a
> > specific port that needs to be maintained in the results. Also cleaned
> > up the port setting to consistently only ever happen just before
> > setting results on the RequestImpl and having 0 or a DNS-provided port
> > until then.
> > 
> > Bug: 846423
> > Change-Id: I679c0ac915e0f81b49adb5ee769f250be49c9c90
> > Reviewed-on: https://chromium-review.googlesource.com/c/1340835
> > Reviewed-by: Matt Menke <mmenke@chromium.org>
> > Commit-Queue: Eric Orth <ericorth@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#611963}
> 
> TBR=mmenke@chromium.org,ericorth@chromium.org
> 
> Change-Id: Icc3121b6f6de985fb64ed9b2bee4951faf5378a3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 846423
> Reviewed-on: https://chromium-review.googlesource.com/c/1354746
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612052}

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

Change-Id: Ie2f80d423a5370f976918357b32b8eb772c4f57c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 846423
Reviewed-on: https://chromium-review.googlesource.com/c/1354267Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612164}

net/dns/host_cache.cc

@@ -6,16 +6,15 @@
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/default_tick_clock.h"
 #include "base/trace_event/trace_event.h"
-#include "base/values.h"
 #include "net/base/net_errors.h"
 #include "net/base/trace_constants.h"
-#include "net/dns/dns_util.h"
 #include "net/dns/host_resolver.h"
 #include "net/log/net_log.h"
 
@@ -67,6 +66,16 @@ bool AddressListFromListValue(const base::ListValue* value,
   return true;
 }
 
+template <typename T>
+void MergeLists(base::Optional<T>* target, const base::Optional<T>& source) {
+  if (target->has_value() && source) {
+    target->value().insert(target->value().end(), source.value().begin(),
+                           source.value().end());
+  } else if (source) {
+    *target = source;
+  }
+}
+
 }  // namespace
 
 // Used in histograms; do not modify existing values.
@@ -114,9 +123,78 @@ HostCache::Key::Key(const std::string& hostname,
 HostCache::Key::Key()
     : Key("", DnsQueryType::UNSPECIFIED, 0, HostResolverSource::ANY) {}
 
+HostCache::Entry::Entry(int error, Source source, base::TimeDelta ttl)
+    : error_(error), source_(source), ttl_(ttl) {
+  DCHECK_GE(ttl_, base::TimeDelta());
+  DCHECK_NE(OK, error_);
+}
+
+HostCache::Entry::Entry(int error, Source source)
+    : error_(error), source_(source), ttl_(base::TimeDelta::FromSeconds(-1)) {
+  DCHECK_NE(OK, error_);
+}
+
+HostCache::Entry::Entry(const Entry& entry) = default;
+
+HostCache::Entry::Entry(Entry&& entry) = default;
+
 HostCache::Entry::~Entry() = default;
 
-HostCache::Entry::Entry(HostCache::Entry&& entry) = default;
+base::Optional<base::TimeDelta> HostCache::Entry::GetOptionalTtl() const {
+  if (has_ttl())
+    return ttl();
+  else
+    return base::nullopt;
+}
+
+// static
+HostCache::Entry HostCache::Entry::MergeEntries(Entry front, Entry back) {
+  // Only expected to merge OK or ERR_NAME_NOT_RESOLVED results.
+  DCHECK(front.error() == OK || front.error() == ERR_NAME_NOT_RESOLVED);
+  DCHECK(back.error() == OK || back.error() == ERR_NAME_NOT_RESOLVED);
+
+  // Build results in |front| to preserve unmerged fields.
+
+  front.error_ =
+      front.error() == OK || back.error() == OK ? OK : ERR_NAME_NOT_RESOLVED;
+
+  MergeLists(&front.addresses_, back.addresses());
+  MergeLists(&front.text_records_, back.text_records());
+  MergeLists(&front.hostnames_, back.hostnames());
+
+  // Use canonical name from |back| iff empty in |front|.
+  if (front.addresses() && front.addresses().value().canonical_name().empty() &&
+      back.addresses()) {
+    front.addresses_.value().set_canonical_name(
+        back.addresses().value().canonical_name());
+  }
+
+  // Only expected to merge entries from same source.
+  DCHECK_EQ(front.source(), back.source());
+
+  if (front.has_ttl() && back.has_ttl()) {
+    front.ttl_ = std::min(front.ttl(), back.ttl());
+  } else if (back.has_ttl()) {
+    front.ttl_ = back.ttl();
+  }
+
+  front.expires_ = std::min(front.expires(), back.expires());
+  front.network_changes_ =
+      std::max(front.network_changes(), back.network_changes());
+  front.total_hits_ = front.total_hits() + back.total_hits();
+  front.stale_hits_ = front.stale_hits() + back.stale_hits();
+
+  return front;
+}
+
+NetLogParametersCallback HostCache::Entry::CreateNetLogCallback() const {
+  return base::BindRepeating(&HostCache::Entry::NetLogCallback,
+                             base::Unretained(this));
+}
+
+HostCache::Entry& HostCache::Entry::operator=(const Entry& entry) = default;
+
+HostCache::Entry& HostCache::Entry::operator=(Entry&& entry) = default;
 
 HostCache::Entry::Entry(const HostCache::Entry& entry,
                         base::TimeTicks now,
@@ -174,6 +252,69 @@ void HostCache::Entry::GetStaleness(base::TimeTicks now,
   out->stale_hits = stale_hits_;
 }
 
+std::unique_ptr<base::Value> HostCache::Entry::NetLogCallback(
+    NetLogCaptureMode capture_mode) const {
+  return std::make_unique<base::Value>(
+      GetAsValue(false /* include_staleness */));
+}
+
+base::DictionaryValue HostCache::Entry::GetAsValue(
+    bool include_staleness) const {
+  base::DictionaryValue entry_dict;
+
+  if (include_staleness) {
+    // The kExpirationKey value is using TimeTicks instead of Time used if
+    // |include_staleness| is false, so it cannot be used to deserialize.
+    // This is ok as it is used only for netlog.
+    entry_dict.SetString(kExpirationKey, NetLog::TickCountToString(expires()));
+    entry_dict.SetInteger(kTtlKey, ttl().InMilliseconds());
+    entry_dict.SetInteger(kNetworkChangesKey, network_changes());
+  } else {
+    // Convert expiration time in TimeTicks to Time for serialization, using a
+    // string because base::Value doesn't handle 64-bit integers.
+    base::Time expiration_time =
+        base::Time::Now() - (base::TimeTicks::Now() - expires());
+    entry_dict.SetString(
+        kExpirationKey, base::Int64ToString(expiration_time.ToInternalValue()));
+  }
+
+  if (error() != OK) {
+    entry_dict.SetInteger(kErrorKey, error());
+  } else {
+    if (addresses()) {
+      // Append all of the resolved addresses.
+      base::ListValue addresses_value;
+      for (const IPEndPoint& address : addresses().value()) {
+        addresses_value.GetList().emplace_back(address.ToStringWithoutPort());
+      }
+      entry_dict.SetKey(kAddressesKey, std::move(addresses_value));
+    }
+
+    if (text_records()) {
+      // Append all resolved text records.
+      base::ListValue text_list_value;
+      for (const std::string& text_record : text_records().value()) {
+        text_list_value.GetList().emplace_back(text_record);
+      }
+      entry_dict.SetKey(kTextRecordsKey, std::move(text_list_value));
+    }
+
+    if (hostnames()) {
+      // Append all the resolved hostnames.
+      base::ListValue hostnames_value;
+      base::ListValue host_ports_value;
+      for (const HostPortPair& hostname : hostnames().value()) {
+        hostnames_value.GetList().emplace_back(hostname.host());
+        host_ports_value.GetList().emplace_back(hostname.port());
+      }
+      entry_dict.SetKey(kHostnameResultsKey, std::move(hostnames_value));
+      entry_dict.SetKey(kHostPortsKey, std::move(host_ports_value));
+    }
+  }
+
+  return entry_dict;
+}
+
 HostCache::HostCache(size_t max_entries)
     : max_entries_(max_entries),
       network_changes_(0),
@@ -368,8 +509,8 @@ void HostCache::GetAsListValue(base::ListValue* entry_list,
     const Key& key = pair.first;
     const Entry& entry = pair.second;
 
-    std::unique_ptr<base::DictionaryValue> entry_dict(
-        new base::DictionaryValue());
+    auto entry_dict = std::make_unique<base::DictionaryValue>(
+        entry.GetAsValue(include_staleness));
 
     entry_dict->SetString(kHostnameKey, key.hostname);
     entry_dict->SetInteger(kDnsQueryTypeKey,
@@ -378,58 +519,6 @@ void HostCache::GetAsListValue(base::ListValue* entry_list,
     entry_dict->SetInteger(kHostResolverSourceKey,
                            static_cast<int>(key.host_resolver_source));
 
-    if (include_staleness) {
-      // The kExpirationKey value is using TimeTicks instead of Time used if
-      // |include_staleness| is false, so it cannot be used to deserialize.
-      // This is ok as it is used only for netlog.
-      entry_dict->SetString(kExpirationKey,
-                            NetLog::TickCountToString(entry.expires()));
-      entry_dict->SetInteger(kTtlKey, entry.ttl().InMilliseconds());
-      entry_dict->SetInteger(kNetworkChangesKey, entry.network_changes());
-    } else {
-      // Convert expiration time in TimeTicks to Time for serialization, using a
-      // string because base::Value doesn't handle 64-bit integers.
-      base::Time expiration_time =
-          base::Time::Now() - (tick_clock_->NowTicks() - entry.expires());
-      entry_dict->SetString(
-          kExpirationKey,
-          base::Int64ToString(expiration_time.ToInternalValue()));
-    }
-
-    if (entry.error() != OK) {
-      entry_dict->SetInteger(kErrorKey, entry.error());
-    } else {
-      if (entry.addresses()) {
-        // Append all of the resolved addresses.
-        base::ListValue addresses_value;
-        for (const IPEndPoint& address : entry.addresses().value()) {
-          addresses_value.GetList().emplace_back(address.ToStringWithoutPort());
-        }
-        entry_dict->SetKey(kAddressesKey, std::move(addresses_value));
-      }
-
-      if (entry.text_records()) {
-        // Append all resolved text records.
-        base::ListValue text_list_value;
-        for (const std::string& text_record : entry.text_records().value()) {
-          text_list_value.GetList().emplace_back(text_record);
-        }
-        entry_dict->SetKey(kTextRecordsKey, std::move(text_list_value));
-      }
-
-      if (entry.hostnames()) {
-        // Append all the resolved hostnames.
-        base::ListValue hostnames_value;
-        base::ListValue host_ports_value;
-        for (const HostPortPair& hostname : entry.hostnames().value()) {
-          hostnames_value.GetList().emplace_back(hostname.host());
-          host_ports_value.GetList().emplace_back(hostname.port());
-        }
-        entry_dict->SetKey(kHostnameResultsKey, std::move(hostnames_value));
-        entry_dict->SetKey(kHostPortsKey, std::move(host_ports_value));
-      }
-    }
-
     entry_list->Append(std::move(entry_dict));
   }
 }

net/dns/host_cache.h

@@ -21,6 +21,7 @@
 #include "base/optional.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
+#include "base/values.h"
 #include "net/base/address_family.h"
 #include "net/base/address_list.h"
 #include "net/base/expiring_cache.h"
@@ -29,6 +30,8 @@
 #include "net/dns/dns_util.h"
 #include "net/dns/host_resolver_source.h"
 #include "net/dns/public/dns_query_type.h"
+#include "net/log/net_log_capture_mode.h"
+#include "net/log/net_log_parameters_callback.h"
 
 namespace base {
 class ListValue;
@@ -110,26 +113,58 @@ class NET_EXPORT HostCache {
       SetResult(std::forward<T>(results));
     }
 
+    // For errors with no |results|.
+    Entry(int error, Source source, base::TimeDelta ttl);
+    Entry(int error, Source source);
+
+    Entry(const Entry& entry);
     Entry(Entry&& entry);
     ~Entry();
 
+    Entry& operator=(const Entry& entry);
+    Entry& operator=(Entry&& entry);
+
     int error() const { return error_; }
+    void set_error(int error) { error_ = error; }
     const base::Optional<AddressList>& addresses() const { return addresses_; }
+    void set_addresses(const base::Optional<AddressList>& addresses) {
+      addresses_ = addresses;
+    }
     const base::Optional<std::vector<std::string>>& text_records() const {
       return text_records_;
     }
+    void set_text_records(
+        base::Optional<std::vector<std::string>> text_records) {
+      text_records_ = std::move(text_records);
+    }
     const base::Optional<std::vector<HostPortPair>>& hostnames() const {
       return hostnames_;
     }
+    void set_hostnames(base::Optional<std::vector<HostPortPair>> hostnames) {
+      hostnames_ = std::move(hostnames);
+    }
     Source source() const { return source_; }
     bool has_ttl() const { return ttl_ >= base::TimeDelta(); }
     base::TimeDelta ttl() const { return ttl_; }
+    base::Optional<base::TimeDelta> GetOptionalTtl() const;
+    void set_ttl(base::TimeDelta ttl) { ttl_ = ttl; }
 
     base::TimeTicks expires() const { return expires_; }
 
     // Public for the net-internals UI.
     int network_changes() const { return network_changes_; }
 
+    // Merge |front| and |back|, representing results from multiple
+    // transactions for the same overal host resolution query. On merging result
+    // lists, result elements from |front| will be merged in front of elements
+    // from |back|. Fields that cannot be merged take precedence from |front|.
+    static Entry MergeEntries(Entry front, Entry back);
+
+    // Creates a callback for use with the NetLog that returns a Value
+    // representation of the entry.  The callback must be destroyed before
+    // |this| is.
+    NetLogParametersCallback CreateNetLogCallback() const;
+
    private:
     friend class HostCache;
 
@@ -163,6 +198,10 @@ class NET_EXPORT HostCache {
                       int network_changes,
                       EntryStaleness* out) const;
 
+    std::unique_ptr<base::Value> NetLogCallback(
+        NetLogCaptureMode capture_mode) const;
+    base::DictionaryValue GetAsValue(bool include_staleness) const;
+
     // The resolve results for this entry.
     int error_;
     base::Optional<AddressList> addresses_;

net/dns/host_cache_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -868,4 +869,162 @@ TEST(HostCacheTest, PersistenceDelegate) {
   EXPECT_EQ(4, delegate.num_changes());
 }
 
+TEST(HostCacheTest, MergeEntries) {
+  const IPAddress kAddressFront(1, 2, 3, 4);
+  const IPEndPoint kEndpointFront(kAddressFront, 0);
+  HostCache::Entry front(OK, AddressList(kEndpointFront),
+                         HostCache::Entry::SOURCE_DNS);
+  front.set_text_records(std::vector<std::string>{"text1"});
+  const HostPortPair kHostnameFront("host", 1);
+  front.set_hostnames(std::vector<HostPortPair>{kHostnameFront});
+
+  const IPAddress kAddressBack(0x20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                               0);
+  const IPEndPoint kEndpointBack(kAddressBack, 0);
+  HostCache::Entry back(OK, AddressList(kEndpointBack),
+                        HostCache::Entry::SOURCE_DNS);
+  back.set_text_records(std::vector<std::string>{"text2"});
+  const HostPortPair kHostnameBack("host", 2);
+  back.set_hostnames(std::vector<HostPortPair>{kHostnameBack});
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  EXPECT_EQ(OK, result.error());
+  EXPECT_EQ(HostCache::Entry::SOURCE_DNS, result.source());
+
+  ASSERT_TRUE(result.addresses());
+  EXPECT_THAT(result.addresses().value().endpoints(),
+              testing::ElementsAre(kEndpointFront, kEndpointBack));
+  EXPECT_THAT(result.text_records(),
+              testing::Optional(testing::ElementsAre("text1", "text2")));
+  EXPECT_THAT(result.hostnames(), testing::Optional(testing::ElementsAre(
+                                      kHostnameFront, kHostnameBack)));
+}
+
+TEST(HostCacheTest, MergeEntries_frontEmpty) {
+  HostCache::Entry front(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS);
+
+  const IPAddress kAddressBack(0x20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+                               0);
+  const IPEndPoint kEndpointBack(kAddressBack, 0);
+  HostCache::Entry back(OK, AddressList(kEndpointBack),
+                        HostCache::Entry::SOURCE_DNS,
+                        base::TimeDelta::FromHours(4));
+  back.set_text_records(std::vector<std::string>{"text2"});
+  const HostPortPair kHostnameBack("host", 2);
+  back.set_hostnames(std::vector<HostPortPair>{kHostnameBack});
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  EXPECT_EQ(OK, result.error());
+  EXPECT_EQ(HostCache::Entry::SOURCE_DNS, result.source());
+
+  ASSERT_TRUE(result.addresses());
+  EXPECT_THAT(result.addresses().value().endpoints(),
+              testing::ElementsAre(kEndpointBack));
+  EXPECT_THAT(result.text_records(),
+              testing::Optional(testing::ElementsAre("text2")));
+  EXPECT_THAT(result.hostnames(),
+              testing::Optional(testing::ElementsAre(kHostnameBack)));
+
+  EXPECT_EQ(base::TimeDelta::FromHours(4), result.ttl());
+}
+
+TEST(HostCacheTest, MergeEntries_backEmpty) {
+  const IPAddress kAddressFront(1, 2, 3, 4);
+  const IPEndPoint kEndpointFront(kAddressFront, 0);
+  HostCache::Entry front(OK, AddressList(kEndpointFront),
+                         HostCache::Entry::SOURCE_DNS,
+                         base::TimeDelta::FromMinutes(5));
+  front.set_text_records(std::vector<std::string>{"text1"});
+  const HostPortPair kHostnameFront("host", 1);
+  front.set_hostnames(std::vector<HostPortPair>{kHostnameFront});
+
+  HostCache::Entry back(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS);
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  EXPECT_EQ(OK, result.error());
+  EXPECT_EQ(HostCache::Entry::SOURCE_DNS, result.source());
+
+  ASSERT_TRUE(result.addresses());
+  EXPECT_THAT(result.addresses().value().endpoints(),
+              testing::ElementsAre(kEndpointFront));
+  EXPECT_THAT(result.text_records(),
+              testing::Optional(testing::ElementsAre("text1")));
+  EXPECT_THAT(result.hostnames(),
+              testing::Optional(testing::ElementsAre(kHostnameFront)));
+
+  EXPECT_EQ(base::TimeDelta::FromMinutes(5), result.ttl());
+}
+
+TEST(HostCacheTest, MergeEntries_bothEmpty) {
+  HostCache::Entry front(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS);
+  HostCache::Entry back(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS);
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, result.error());
+  EXPECT_EQ(HostCache::Entry::SOURCE_DNS, result.source());
+
+  EXPECT_FALSE(result.addresses());
+  EXPECT_FALSE(result.text_records());
+  EXPECT_FALSE(result.hostnames());
+
+  EXPECT_FALSE(result.has_ttl());
+}
+
+TEST(HostCacheTest, MergeEntries_differentTtl) {
+  HostCache::Entry front(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS,
+                         base::TimeDelta::FromDays(12));
+  HostCache::Entry back(ERR_NAME_NOT_RESOLVED, HostCache::Entry::SOURCE_DNS,
+                        base::TimeDelta::FromSeconds(42));
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  EXPECT_EQ(base::TimeDelta::FromSeconds(42), result.ttl());
+}
+
+TEST(HostCacheTest, MergeEntries_FrontCannonnamePreserved) {
+  AddressList addresses_front;
+  const std::string kCanonicalNameFront = "name1";
+  addresses_front.set_canonical_name(kCanonicalNameFront);
+  HostCache::Entry front(OK, addresses_front, HostCache::Entry::SOURCE_DNS);
+
+  AddressList addresses_back;
+  const std::string kCanonicalNameBack = "name2";
+  addresses_back.set_canonical_name(kCanonicalNameBack);
+  HostCache::Entry back(OK, addresses_back, HostCache::Entry::SOURCE_DNS);
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  ASSERT_TRUE(result.addresses());
+  EXPECT_EQ(kCanonicalNameFront, result.addresses().value().canonical_name());
+}
+
+// Test that the back canonname can be used if there is no front cannonname.
+TEST(HostCacheTest, MergeEntries_BackCannonnameUsable) {
+  AddressList addresses_front;
+  const std::string kCanonicalNameFront = "";
+  addresses_front.set_canonical_name(kCanonicalNameFront);
+  HostCache::Entry front(OK, addresses_front, HostCache::Entry::SOURCE_DNS);
+
+  AddressList addresses_back;
+  const std::string kCanonicalNameBack = "name2";
+  addresses_back.set_canonical_name(kCanonicalNameBack);
+  HostCache::Entry back(OK, addresses_back, HostCache::Entry::SOURCE_DNS);
+
+  HostCache::Entry result =
+      HostCache::Entry::MergeEntries(std::move(front), std::move(back));
+
+  ASSERT_TRUE(result.addresses());
+  EXPECT_EQ(kCanonicalNameBack, result.addresses().value().canonical_name());
+}
+
 }  // namespace net

net/dns/host_resolver.h

@@ -92,10 +92,23 @@ class NET_EXPORT HostResolver {
     // Address record (A or AAAA) results of the request. Should only be called
     // after Start() signals completion, either by invoking the callback or by
     // returning a result other than |ERR_IO_PENDING|.
-    //
-    // TODO(crbug.com/821021): Implement other GetResults() methods for requests
-    // that return other data (eg DNS TXT requests).
     virtual const base::Optional<AddressList>& GetAddressResults() const = 0;
+
+    // Text record (TXT) results of the request. Should only be called after
+    // Start() signals completion, either by invoking the callback or by
+    // returning a result other than |ERR_IO_PENDING|.
+    virtual const base::Optional<std::vector<std::string>>& GetTextResults()
+        const = 0;
+
+    // Hostname record (SRV or PTR) results of the request. For SRV results,
+    // hostnames are ordered acording to their priorities and weights. See RFC
+    // 2782.
+    //
+    // Should only be called after Start() signals completion, either by
+    // invoking the callback or by returning a result other than
+    // |ERR_IO_PENDING|.
+    virtual const base::Optional<std::vector<HostPortPair>>&
+    GetHostnameResults() const = 0;
   };
 
   // |max_concurrent_resolves| is how many resolve requests will be allowed to

net/dns/host_resolver_impl.h

@@ -241,11 +241,10 @@ class NET_EXPORT HostResolverImpl
   int Resolve(RequestImpl* request);
 
   // Attempts host resolution using fast local sources: IP literal resolution,
-  // cache lookup, HOSTS lookup (if enabled), and localhost. Returns OK if
-  // successful, ERR_NAME_NOT_RESOLVED if input is invalid, or
-  // ERR_DNS_CACHE_MISS if the host could not be resolved using local sources.
-  //
-  // On success, the resulting addresses are written to |addresses|.
+  // cache lookup, HOSTS lookup (if enabled), and localhost. Returns results
+  // with error() OK if successful, ERR_NAME_NOT_RESOLVED if input is invalid,
+  // or ERR_DNS_CACHE_MISS if the host could not be resolved using local
+  // sources.
   //
   // On ERR_DNS_CACHE_MISS and OK, the cache key for the request is written to
   // |key|. On other errors, it may not be.
@@ -256,33 +255,27 @@ class NET_EXPORT HostResolverImpl
   //
   // If |allow_stale| is false, then stale cache entries will not be returned,
   // and |stale_info| must be null.
-  int ResolveLocally(const HostPortPair& host,
-                     DnsQueryType requested_address_family,
-                     HostResolverSource source,
-                     HostResolverFlags flags,
-                     bool allow_cache,
-                     bool allow_stale,
-                     HostCache::EntryStaleness* stale_info,
-                     const NetLogWithSource& request_net_log,
-                     AddressList* addresses,
-                     Key* key);
+  HostCache::Entry ResolveLocally(const std::string& hostname,
+                                  DnsQueryType requested_address_family,
+                                  HostResolverSource source,
+                                  HostResolverFlags flags,
+                                  bool allow_cache,
+                                  bool allow_stale,
+                                  HostCache::EntryStaleness* stale_info,
+                                  const NetLogWithSource& request_net_log,
+                                  Key* out_key);
 
   // Attempts to create and start a Job to asynchronously attempt to resolve
   // |key|. On success, returns ERR_IO_PENDING and attaches the Job to
   // |request|. On error, marks |request| completed and returns the error.
   int CreateAndStartJob(const Key& key, RequestImpl* request);
 
-  // Tries to resolve |key| as an IP, returns true and sets |net_error| if
-  // succeeds, returns false otherwise.
-  bool ResolveAsIP(const Key& key,
-                   uint16_t host_port,
-                   const IPAddress* ip_address,
-                   int* net_error,
-                   AddressList* addresses);
-
-  // If |key| is not found in cache returns false, otherwise returns
-  // true, sets |net_error| to the cached error code and fills |addresses|
-  // if it is a positive entry.
+  // Tries to resolve |key| and its possible IP address representation,
+  // |ip_address|. Returns a results entry iff the input can be resolved.
+  base::Optional<HostCache::Entry> ResolveAsIP(const Key& key,
+                                               const IPAddress* ip_address);
+
+  // Returns the result iff a positive match is found for |key| in the cache.
   //
   // If |allow_stale| is true, then stale cache entries can be returned.
   // |stale_info| must be non-null, and will be filled in with details of the
@@ -290,24 +283,18 @@ class NET_EXPORT HostResolverImpl
   //
   // If |allow_stale| is false, then stale cache entries will not be returned,
   // and |stale_info| must be null.
-  bool ServeFromCache(const Key& key,
-                      uint16_t host_port,
-                      int* net_error,
-                      AddressList* addresses,
-                      bool allow_stale,
-                      HostCache::EntryStaleness* stale_info);
-
-  // If we have a DnsClient with a valid DnsConfig, and |key| is found in the
-  // HOSTS file, returns true and fills |addresses|. Otherwise returns false.
-  bool ServeFromHosts(const Key& key,
-                      uint16_t host_port,
-                      AddressList* addresses);
-
-  // If |key| is for a localhost name (RFC 6761), returns true and fills
-  // |addresses| with the loopback IP. Otherwise returns false.
-  bool ServeLocalhost(const Key& key,
-                      uint16_t host_port,
-                      AddressList* addresses);
+  base::Optional<HostCache::Entry> ServeFromCache(
+      const Key& key,
+      bool allow_stale,
+      HostCache::EntryStaleness* stale_info);
+
+  // Iff we have a DnsClient with a valid DnsConfig, and |key| can be resolved
+  // from the HOSTS file, return the results.
+  base::Optional<HostCache::Entry> ServeFromHosts(const Key& key);
+
+  // Iff |key| is for a localhost name (RFC 6761) and address DNS query type,
+  // returns a results entry with the loopback IP.
+  base::Optional<HostCache::Entry> ServeLocalhost(const Key& key);
 
   // Returns the (hostname, address_family) key to use for |info|, choosing an
   // "effective" address family by inheriting the resolver's default address
@@ -480,7 +467,7 @@ class NET_EXPORT HostResolverImpl
 };
 
 // Resolves a local hostname (such as "localhost" or "localhost6") into
-// IP endpoints with the given port. Returns true if |host| is a local
+// IP endpoints (with port 0). Returns true if |host| is a local
 // hostname and false otherwise. Special IPv6 names (e.g. "localhost6")
 // will resolve to an IPv6 address only, whereas other names will
 // resolve to both IPv4 and IPv6.
@@ -488,7 +475,6 @@ class NET_EXPORT HostResolverImpl
 // TODO(tfarina): It would be better to change the tests so this function
 // gets exercised indirectly through HostResolverImpl.
 NET_EXPORT_PRIVATE bool ResolveLocalHostname(base::StringPiece host,
-                                             uint16_t port,
                                              AddressList* address_list);
 
 }  // namespace net

net/dns/host_resolver_impl_unittest.cc

@@ -568,34 +568,27 @@ class TestHostResolverImpl : public HostResolverImpl {
   }
 };
 
-const uint16_t kLocalhostLookupPort = 80;
-
-bool HasEndpoint(const IPEndPoint& endpoint, const AddressList& addresses) {
+bool HasAddress(const IPAddress& search_address, const AddressList& addresses) {
   for (const auto& address : addresses) {
-    if (endpoint == address)
+    if (search_address == address.address())
       return true;
   }
   return false;
 }
 
 void TestBothLoopbackIPs(const std::string& host) {
-  IPEndPoint localhost_ipv4(IPAddress::IPv4Localhost(), kLocalhostLookupPort);
-  IPEndPoint localhost_ipv6(IPAddress::IPv6Localhost(), kLocalhostLookupPort);
-
   AddressList addresses;
-  EXPECT_TRUE(ResolveLocalHostname(host, kLocalhostLookupPort, &addresses));
+  EXPECT_TRUE(ResolveLocalHostname(host, &addresses));
   EXPECT_EQ(2u, addresses.size());
-  EXPECT_TRUE(HasEndpoint(localhost_ipv4, addresses));
-  EXPECT_TRUE(HasEndpoint(localhost_ipv6, addresses));
+  EXPECT_TRUE(HasAddress(IPAddress::IPv4Localhost(), addresses));
+  EXPECT_TRUE(HasAddress(IPAddress::IPv6Localhost(), addresses));
 }
 
 void TestIPv6LoopbackOnly(const std::string& host) {
-  IPEndPoint localhost_ipv6(IPAddress::IPv6Localhost(), kLocalhostLookupPort);
-
   AddressList addresses;
-  EXPECT_TRUE(ResolveLocalHostname(host, kLocalhostLookupPort, &addresses));
+  EXPECT_TRUE(ResolveLocalHostname(host, &addresses));
   EXPECT_EQ(1u, addresses.size());
-  EXPECT_TRUE(HasEndpoint(localhost_ipv6, addresses));
+  EXPECT_TRUE(HasAddress(IPAddress::IPv6Localhost(), addresses));
 }
 
 // Used to bind the unique_ptr<Request>* into callbacks.
@@ -5628,7 +5621,9 @@ TEST_F(HostResolverImplDnsTest, NoCanonicalName) {
   EXPECT_THAT(request->Resolve(), IsError(ERR_IO_PENDING));
   ASSERT_THAT(request->WaitForResult(), IsOk());
 
-  EXPECT_TRUE(request->list().canonical_name().empty());
+  // HostResolver may still give name, but if so, it must be correct.
+  std::string result_name = request->list().canonical_name();
+  EXPECT_TRUE(result_name.empty() || result_name == "canonical");
 }
 
 TEST_F(HostResolverImplDnsTest, NoCanonicalName_CreateRequest) {
@@ -5645,8 +5640,10 @@ TEST_F(HostResolverImplDnsTest, NoCanonicalName_CreateRequest) {
       HostPortPair("alias", 80), NetLogWithSource(), base::nullopt));
   ASSERT_THAT(response.result_error(), IsOk());
 
-  EXPECT_TRUE(
-      response.request()->GetAddressResults().value().canonical_name().empty());
+  // HostResolver may still give name, but if so, it must be correct.
+  std::string result_name =
+      response.request()->GetAddressResults().value().canonical_name();
+  EXPECT_TRUE(result_name.empty() || result_name == "canonical");
 }
 
 TEST_F(HostResolverImplDnsTest, CanonicalName_CreateRequest) {
@@ -5730,40 +5727,25 @@ TEST_F(HostResolverImplTest, ResolveLocalHostname) {
   TestIPv6LoopbackOnly("localhost6.localdomain6");
   TestIPv6LoopbackOnly("localhost6.localdomain6.");
 
-  EXPECT_FALSE(
-      ResolveLocalHostname("127.0.0.1", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("::1", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:0:0:0:1", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname("localhostx", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname("localhost.x", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("foo.localdomain", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("foo.localdomain.x", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname("localhost6x", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("localhost.localdomain6",
-                                    kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("localhost6.localdomain",
-                                    kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname("127.0.0.1.1", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname(".127.0.0.255", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("::2", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("::1:1", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:1:0:0:1", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("::1:1", kLocalhostLookupPort, &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:0:0:0:0:1", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(ResolveLocalHostname("foo.localhost.com", kLocalhostLookupPort,
-                                    &addresses));
-  EXPECT_FALSE(
-      ResolveLocalHostname("foo.localhoste", kLocalhostLookupPort, &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("127.0.0.1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("::1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:0:0:0:1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("localhostx", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("localhost.x", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("foo.localdomain", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("foo.localdomain.x", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("localhost6x", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("localhost.localdomain6", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("localhost6.localdomain", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("127.0.0.1.1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname(".127.0.0.255", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("::2", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("::1:1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:1:0:0:1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("::1:1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("0:0:0:0:0:0:0:0:1", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("foo.localhost.com", &addresses));
+  EXPECT_FALSE(ResolveLocalHostname("foo.localhoste", &addresses));
 }
 
 TEST_F(HostResolverImplDnsTest, AddDnsOverHttpsServerAfterConfig) {

net/dns/host_resolver_mdns_task.cc

@@ -19,13 +19,15 @@ namespace net {
 class HostResolverMdnsTask::Transaction {
  public:
   Transaction(DnsQueryType query_type, HostResolverMdnsTask* task)
-      : query_type_(query_type), result_(ERR_IO_PENDING), task_(task) {}
+      : query_type_(query_type),
+        results_(ERR_IO_PENDING, HostCache::Entry::SOURCE_UNKNOWN),
+        task_(task) {}
 
   void Start() {
     DCHECK_CALLED_ON_VALID_SEQUENCE(task_->sequence_checker_);
 
     // Should not be completed or running yet.
-    DCHECK_EQ(ERR_IO_PENDING, result_);
+    DCHECK_EQ(ERR_IO_PENDING, results_.error());
     DCHECK(!async_transaction_);
 
     // TODO(crbug.com/846423): Use |allow_cached_response| to set the
@@ -44,57 +46,67 @@ class HostResolverMdnsTask::Transaction {
     bool start_result = inner_transaction->Start();
 
     if (!start_result)
-      task_->CompleteWithResult(ERR_FAILED, true /* post_needed */);
-    else if (result_ == ERR_IO_PENDING)
+      task_->Complete(true /* post_needed */);
+    else if (results_.error() == ERR_IO_PENDING)
       async_transaction_ = std::move(inner_transaction);
   }
 
-  bool IsDone() const { return result_ != ERR_IO_PENDING; }
+  bool IsDone() const { return results_.error() != ERR_IO_PENDING; }
   bool IsError() const {
-    return IsDone() && result_ != OK && result_ != ERR_NAME_NOT_RESOLVED;
+    return IsDone() && results_.error() != OK &&
+           results_.error() != ERR_NAME_NOT_RESOLVED;
   }
-  int result() const { return result_; }
+  const HostCache::Entry& results() const { return results_; }
 
   void Cancel() {
     DCHECK_CALLED_ON_VALID_SEQUENCE(task_->sequence_checker_);
-    DCHECK_EQ(ERR_IO_PENDING, result_);
+    DCHECK_EQ(ERR_IO_PENDING, results_.error());
 
-    result_ = ERR_FAILED;
+    results_ = HostCache::Entry(ERR_FAILED, HostCache::Entry::SOURCE_UNKNOWN);
     async_transaction_ = nullptr;
   }
 
  private:
   void OnComplete(MDnsTransaction::Result result, const RecordParsed* parsed) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(task_->sequence_checker_);
-    DCHECK_EQ(ERR_IO_PENDING, result_);
+    DCHECK_EQ(ERR_IO_PENDING, results_.error());
 
+    int error = ERR_UNEXPECTED;
     switch (result) {
       case MDnsTransaction::RESULT_RECORD:
-        result_ = OK;
+        error = OK;
         break;
       case MDnsTransaction::RESULT_NO_RESULTS:
       case MDnsTransaction::RESULT_NSEC:
-        result_ = ERR_NAME_NOT_RESOLVED;
+        error = ERR_NAME_NOT_RESOLVED;
         break;
       default:
         // No other results should be possible with the request flags used.
         NOTREACHED();
     }
 
-    if (result_ == net::OK) {
+    if (error == net::OK) {
       switch (query_type_) {
         case DnsQueryType::A:
-          task_->result_addresses_.push_back(
-              IPEndPoint(parsed->rdata<net::ARecordRdata>()->address(), 0));
+          results_ = HostCache::Entry(
+              OK,
+              AddressList(
+                  IPEndPoint(parsed->rdata<net::ARecordRdata>()->address(), 0)),
+              HostCache::Entry::SOURCE_UNKNOWN);
           break;
         case DnsQueryType::AAAA:
-          task_->result_addresses_.push_back(
-              IPEndPoint(parsed->rdata<net::AAAARecordRdata>()->address(), 0));
+          results_ = HostCache::Entry(
+              OK,
+              AddressList(IPEndPoint(
+                  parsed->rdata<net::AAAARecordRdata>()->address(), 0)),
+              HostCache::Entry::SOURCE_UNKNOWN);
           break;
         default:
           // TODO(crbug.com/846423): Add result parsing for non-address types.
           NOTIMPLEMENTED();
       }
+    } else {
+      results_ = HostCache::Entry(error, HostCache::Entry::SOURCE_UNKNOWN);
     }
 
     // If we don't have a saved async_transaction, it means OnComplete was
@@ -106,7 +118,7 @@ class HostResolverMdnsTask::Transaction {
   const DnsQueryType query_type_;
 
   // ERR_IO_PENDING until transaction completes (or is cancelled).
-  int result_;
+  HostCache::Entry results_;
 
   // Not saved until MDnsTransaction::Start completes to differentiate inline
   // completion.
@@ -132,11 +144,11 @@ HostResolverMdnsTask::~HostResolverMdnsTask() {
   transactions_.clear();
 }
 
-void HostResolverMdnsTask::Start(CompletionOnceCallback completion_callback) {
+void HostResolverMdnsTask::Start(base::OnceClosure completion_closure) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(!completion_callback_);
+  DCHECK(!completion_closure_);
 
-  completion_callback_ = std::move(completion_callback);
+  completion_closure_ = std::move(completion_closure);
 
   for (auto& transaction : transactions_) {
     // Only start transaction if it is not already marked done. A transaction
@@ -147,29 +159,47 @@ void HostResolverMdnsTask::Start(CompletionOnceCallback completion_callback) {
   }
 }
 
-void HostResolverMdnsTask::CheckCompletion(bool post_needed) {
+HostCache::Entry HostResolverMdnsTask::GetResults() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!transactions_.empty());
+  DCHECK(!completion_closure_);
+  DCHECK(std::all_of(transactions_.begin(), transactions_.end(),
+                     [](const Transaction& t) { return t.IsDone(); }));
 
-  // Finish immediately if any transactions completed with an error.
   auto found_error =
       std::find_if(transactions_.begin(), transactions_.end(),
                    [](const Transaction& t) { return t.IsError(); });
   if (found_error != transactions_.end()) {
-    CompleteWithResult(found_error->result(), post_needed);
+    return found_error->results();
+  }
+
+  HostCache::Entry combined_results = transactions_.front().results();
+  for (auto it = ++transactions_.begin(); it != transactions_.end(); ++it) {
+    combined_results = HostCache::Entry::MergeEntries(
+        std::move(combined_results), it->results());
+  }
+
+  return combined_results;
+}
+
+void HostResolverMdnsTask::CheckCompletion(bool post_needed) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Finish immediately if any transactions completed with an error.
+  if (std::any_of(transactions_.begin(), transactions_.end(),
+                  [](const Transaction& t) { return t.IsError(); })) {
+    Complete(post_needed);
     return;
   }
 
   if (std::all_of(transactions_.begin(), transactions_.end(),
                   [](const Transaction& t) { return t.IsDone(); })) {
-    // Task is overall successful if any of the transactions found results.
-    int result = result_addresses_.empty() ? ERR_NAME_NOT_RESOLVED : OK;
-
-    CompleteWithResult(result, post_needed);
+    Complete(post_needed);
     return;
   }
 }
 
-void HostResolverMdnsTask::CompleteWithResult(int result, bool post_needed) {
+void HostResolverMdnsTask::Complete(bool post_needed) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Cancel any incomplete async transactions.
@@ -180,15 +210,14 @@ void HostResolverMdnsTask::CompleteWithResult(int result, bool post_needed) {
 
   if (post_needed) {
     base::SequencedTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(
-            [](base::WeakPtr<HostResolverMdnsTask> task, int result) {
-              if (task)
-                std::move(task->completion_callback_).Run(result);
-            },
-            weak_ptr_factory_.GetWeakPtr(), result));
+        FROM_HERE, base::BindOnce(
+                       [](base::WeakPtr<HostResolverMdnsTask> task) {
+                         if (task)
+                           std::move(task->completion_closure_).Run();
+                       },
+                       weak_ptr_factory_.GetWeakPtr()));
   } else {
-    std::move(completion_callback_).Run(result);
+    std::move(completion_closure_).Run();
   }
 }
 

net/dns/host_resolver_mdns_task.h

@@ -9,11 +9,12 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "base/containers/unique_ptr_adapters.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequence_checker.h"
-#include "net/base/completion_once_callback.h"
+#include "net/dns/host_cache.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/mdns_client.h"
 #include "net/dns/public/dns_query_type.h"
@@ -31,28 +32,27 @@ class HostResolverMdnsTask {
                        const std::vector<DnsQueryType>& query_types);
   ~HostResolverMdnsTask();
 
-  // Starts the task. |completion_callback| will be called asynchronously with
-  // results.
+  // Starts the task. |completion_closure| will be called asynchronously.
   //
   // Should only be called once.
-  void Start(CompletionOnceCallback completion_callback);
+  void Start(base::OnceClosure completion_closure);
 
-  const AddressList& result_addresses() { return result_addresses_; }
+  // Results only available after invocation of the completion closure.
+  HostCache::Entry GetResults() const;
 
  private:
   class Transaction;
 
   void CheckCompletion(bool post_needed);
-  void CompleteWithResult(int result, bool post_needed);
+  void Complete(bool post_needed);
 
   MDnsClient* const mdns_client_;
 
   const std::string hostname_;
 
-  AddressList result_addresses_;
   std::vector<Transaction> transactions_;
 
-  CompletionOnceCallback completion_callback_;
+  base::OnceClosure completion_closure_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

net/dns/mapped_host_resolver.cc

@@ -22,8 +22,22 @@ class MappedHostResolver::AlwaysErrorRequestImpl
   int Start(CompletionOnceCallback callback) override { return error_; }
 
   const base::Optional<AddressList>& GetAddressResults() const override {
-    static base::NoDestructor<base::Optional<AddressList>> nullopt_address_list;
-    return *nullopt_address_list;
+    static base::NoDestructor<base::Optional<AddressList>> nullopt_result;
+    return *nullopt_result;
+  }
+
+  const base::Optional<std::vector<std::string>>& GetTextResults()
+      const override {
+    static const base::NoDestructor<base::Optional<std::vector<std::string>>>
+        nullopt_result;
+    return *nullopt_result;
+  }
+
+  const base::Optional<std::vector<HostPortPair>>& GetHostnameResults()
+      const override {
+    static const base::NoDestructor<base::Optional<std::vector<HostPortPair>>>
+        nullopt_result;
+    return *nullopt_result;
   }
 
  private:

net/dns/mock_host_resolver.cc

@@ -12,6 +12,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
+#include "base/no_destructor.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/pattern.h"
@@ -110,6 +111,22 @@ class MockHostResolverBase::RequestImpl
     return address_results_;
   }
 
+  const base::Optional<std::vector<std::string>>& GetTextResults()
+      const override {
+    DCHECK(complete_);
+    static const base::NoDestructor<base::Optional<std::vector<std::string>>>
+        nullopt_result;
+    return *nullopt_result;
+  }
+
+  const base::Optional<std::vector<HostPortPair>>& GetHostnameResults()
+      const override {
+    DCHECK(complete_);
+    static const base::NoDestructor<base::Optional<std::vector<HostPortPair>>>
+        nullopt_result;
+    return *nullopt_result;
+  }
+
   void set_address_results(const AddressList& address_results) {
     // Should only be called at most once and before request is marked
     // completed.
@@ -741,6 +758,16 @@ class HangingHostResolver::RequestImpl
     IMMEDIATE_CRASH();
   }
 
+  const base::Optional<std::vector<std::string>>& GetTextResults()
+      const override {
+    IMMEDIATE_CRASH();
+  }
+
+  const base::Optional<std::vector<HostPortPair>>& GetHostnameResults()
+      const override {
+    IMMEDIATE_CRASH();
+  }
+
   void ChangeRequestPriority(RequestPriority priority) override {}
 
  private: