HostResolverImpl will return ERR_NAME_NOT_RESOLVED instead of Ok if address list is empty

net/dns/host_resolver_impl.cc

Index: net/dns/host_resolver_impl.cc
diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc
index 0aed70d6b75deb230261d520cd742f2f9bcb3ea1..55560fdc38e070373dcb3fdd1d7cc498d2b6ab8d 100644
--- a/net/dns/host_resolver_impl.cc
+++ b/net/dns/host_resolver_impl.cc
@@ -682,7 +682,9 @@ class HostResolverImpl::ProcTask
                         int error,
                         const int os_error) {
     DCHECK(origin_loop_->BelongsToCurrentThread());
-    DCHECK(error || !results.empty());

[szym, 2013/05/04 07:46:32]

Ok, hold on a second. If this DCHECK is here, then this means getaddrinfo never
gives us an empty address list and error == OK.

Now, DCHECKs are disabled in release builds, so maybe we're just not catching
it.

+    // empty results are not OK

[szym, 2013/05/04 07:46:32]

nit: Comments should be full sentences, with capital letter and full stop at
end.

[mef, 2013/05/06 16:49:11]

Done.

+    if (error == OK && results.empty())
+      error = ERR_NAME_NOT_RESOLVED;

[szym, 2013/05/04 07:46:32]

If you change |error| here then your |empty_list_on_ok| below will always be
false. (OnLookupComplete -> OnProcTaskComplete)

I'd suggest keeping it here, but also histogramming it here.

[mef, 2013/05/06 16:49:11]

Done.

 
     bool was_retry_attempt = attempt_number > 1;
 
@@ -1437,6 +1439,12 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job {
       }
     }
 
+    bool empty_list_on_ok = (net_error == OK && addr_list.empty());
+    UMA_HISTOGRAM_BOOLEAN("Net.DNS_EmptyListOnOk", empty_list_on_ok);

[szym, 2013/05/04 07:46:32]

I'd suggest putting the histogram in DNS.
Also, what do you want to count here? What fraction of all calls to getaddrinfo
returned OK and empty address list?

Then a name DNS.EmptyAddressListAndNoError would be appropriate.

[mef, 2013/05/06 16:49:11]

Done.

[mef, 2013/05/06 16:49:11]

Done.

+    if (empty_list_on_ok) {

[szym, 2013/05/04 07:46:32]

nit: no need for braces

[mef, 2013/05/06 16:49:11]

Done.

+      net_error = ERR_NAME_NOT_RESOLVED;
+    }
+
     if (dns_task_error_ != OK) {
       base::TimeDelta duration = base::TimeTicks::Now() - start_time;
       if (net_error == OK) {