[net] Add Net.UnspecResolvedIPv6 to measure if getaddrinfo resolves IPv6 addresses.

net/base/host_resolver_impl.cc

Index: net/base/host_resolver_impl.cc
diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index 0a3e8640fbeb12727e6816d35aaaa0f86b7162eb..21f5c1e857be34c29f5cf92dc6c4247ce8e2dd23 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -1405,6 +1405,19 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
                           const AddressList& addr_list) {
     DCHECK(is_proc_running());
 
+    if (!resolver_->resolved_known_ipv6_hostname_ &&
+        key_.address_family == ADDRESS_FAMILY_UNSPECIFIED) {
+      if (key_.hostname == "www.google.com") {
+        resolver_->resolved_known_ipv6_hostname_ = true;

[mmenke, 2013/01/29 16:10:27]

This seems like the wrong way to track this.  I think we only want this to run
once per DNS config (Or maybe even only once per HostResolver).

With this code, if we think a client can look up IPv6 addresses and it can't, we
log a histogram entry every time we look up www.google.com.  On the other hand,
if a client can look up IPv6 addresses, then we only log one entry until the
config changes.

[mmenke, 2013/01/29 16:10:27]

If there's an error, should we even be logging a histogram?

[szym, 2013/01/29 16:55:12]

Good catch. I previously had this code in HostResolverImpl::CacheResult (no
errors), but forgot about the errors when I moved it here.

Done.

[szym, 2013/01/29 16:55:12]

We want it once per "network configuration". If we did it once per HostResolver
we would miss all the users who were disconnected when Chrome started.
Once we look up www.google.com, we set |resolved_known_ipv6_hostname_ = true|
and therefore we won't log again. That is until the next OnIPAddressChanged when
we reset it to false.

The reason why I do this in OnIPAddressChanged rather than OnDNSChanged is that
we really want to test what getaddrinfo does when IPv6ProbeJob thinks we have
IPv6. This should be independent of the DNS configuration.

[mmenke, 2013/01/29 17:00:46]

Good point, though that's at the cost of overrepresenting users who change
networks a lot (Which may be a good thing, actually), or who have false IP
address changed events trigger every second (hopefully rare).
Sorry, was misreading the code.  For some reason, was conflating setting
got_ipv6_address to true with setting resolved_known_ipv6_hostname_ to true.

+        bool got_ipv6_address = false;
+        for (size_t i = 0; i < addr_list.size(); ++i) {
+          if (addr_list[i].GetFamily() == ADDRESS_FAMILY_IPV6)
+            got_ipv6_address = true;
+        }
+        UMA_HISTOGRAM_BOOLEAN("Net.UnspecResolvedIPv6", got_ipv6_address);
+      }
+    }
+
     if (dns_task_error_ != OK) {
       base::TimeDelta duration = base::TimeTicks::Now() - start_time;
       if (net_error == OK) {
@@ -1659,14 +1672,15 @@ HostResolverImpl::HostResolverImpl(
       dispatcher_(job_limits),
       max_queued_jobs_(job_limits.total_jobs * 100u),
       proc_params_(proc_params),
+      net_log_(net_log),
       default_address_family_(ADDRESS_FAMILY_UNSPECIFIED),
       weak_ptr_factory_(this),
       probe_weak_ptr_factory_(this),
       received_dns_config_(false),
       num_dns_failures_(0),
       ipv6_probe_monitoring_(false),
-      additional_resolver_flags_(0),
-      net_log_(net_log) {
+      resolved_known_ipv6_hostname_(false),
+      additional_resolver_flags_(0) {
 
   DCHECK_GE(dispatcher_.num_priorities(), static_cast<size_t>(NUM_PRIORITIES));
 
@@ -2059,6 +2073,7 @@ void HostResolverImpl::TryServingAllJobsFromHosts() {
 }
 
 void HostResolverImpl::OnIPAddressChanged() {
+  resolved_known_ipv6_hostname_ = false;
   // Abandon all ProbeJobs.
   probe_weak_ptr_factory_.InvalidateWeakPtrs();
   if (cache_.get())