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

net/dns/host_resolver_impl.cc

 // Copyright (c) 2012 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/host_resolver_impl.h"
 
 #if defined(OS_WIN)
 #include <Winsock2.h>
 #elif defined(OS_POSIX)
 #include <netdb.h>
@@ skipping 664 matching lines @@
     StartLookupAttempt();
   }
 
   // Callback for when DoLookup() completes (runs on origin thread).
   void OnLookupComplete(const AddressList& results,
                         const base::TimeTicks& start_time,
                         const uint32 attempt_number,
                         int error,
                         const int os_error) {
     DCHECK(origin_loop_->BelongsToCurrentThread());
-    DCHECK(error || !results.empty());
+    // If results are empty, we should return an error.
+    bool empty_list_on_ok = (error == OK && results.empty());
+    UMA_HISTOGRAM_BOOLEAN("DNS.EmptyAddressListAndNoError1", empty_list_on_ok);
+    if (empty_list_on_ok)
+      error = ERR_NAME_NOT_RESOLVED;
 
     bool was_retry_attempt = attempt_number > 1;
 
     // Ideally the following code would be part of host_resolver_proc.cc,
     // however it isn't safe to call NetworkChangeNotifier from worker threads.
     // So we do it here on the IO thread instead.
     if (error != OK && NetworkChangeNotifier::IsOffline())
       error = ERR_INTERNET_DISCONNECTED;
 
     // If this is the first attempt that is finishing later, then record data
@@ skipping 734 matching lines @@
         resolver_->resolved_known_ipv6_hostname_ = 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);
       }
     }
 
+    bool empty_list_on_ok = (net_error == OK && addr_list.empty());

[szym, 2013/05/06 20:07:02]

As I pointed out before, I don't think this can ever be true.

In line 689 you overwrite |error|. OnProcTaskComplete is actually called in line
745 (via callback_.Run) with the overwritten value.

My suggestion was to remove it from here entirely and leave it in
OnLookupComplete only.

[mef, 2013/05/07 14:53:40]

Done. I left it here because my previous experiment with unit test showed it to
be effective, but I guess it was done after I removed DCHECK but before I've
added the same logic to OnLookupComplete.

On 2013/05/06 20:07:02, szym wrote:

+    UMA_HISTOGRAM_BOOLEAN("DNS.EmptyAddressListAndNoError2", empty_list_on_ok);
+    if (empty_list_on_ok)
+      net_error = ERR_NAME_NOT_RESOLVED;
+
     if (dns_task_error_ != OK) {
       base::TimeDelta duration = base::TimeTicks::Now() - start_time;
       if (net_error == OK) {
         DNS_HISTOGRAM("AsyncDNS.FallbackSuccess", duration);
         if ((dns_task_error_ == ERR_NAME_NOT_RESOLVED) &&
             ResemblesNetBIOSName(key_.hostname)) {
           UmaAsyncDnsResolveStatus(RESOLVE_STATUS_SUSPECT_NETBIOS);
         } else {
           UmaAsyncDnsResolveStatus(RESOLVE_STATUS_PROC_SUCCESS);
         }
@@ skipping 779 matching lines @@
   }
   DnsConfig dns_config;
   NetworkChangeNotifier::GetDnsConfig(&dns_config);
   dns_client_->SetConfig(dns_config);
   num_dns_failures_ = 0;
   if (dns_config.IsValid())
     UMA_HISTOGRAM_BOOLEAN("AsyncDNS.DnsClientEnabled", true);
 }
 
 }  // namespace net