[net/dns] Instrument DnsConfigService to measure performance and failures.

net/dns/dns_config_service_win.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/dns_config_service_win.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
 #include "base/win/windows_version.h"
 #include "googleurl/src/url_canon.h"
 #include "net/base/net_util.h"
 #include "net/base/network_change_notifier.h"
 #include "net/dns/dns_hosts.h"
 #include "net/dns/dns_protocol.h"
 #include "net/dns/serial_worker.h"
 
 #pragma comment(lib, "iphlpapi.lib")
 
 namespace net {
 
 namespace internal {
 
 namespace {
 
 const wchar_t* const kPrimaryDnsSuffixPath =
     L"SOFTWARE\\Policies\\Microsoft\\System\\DNSClient";
 
+enum ConfigParseWinResult {
+  CONFIG_PARSE_WIN_OK = 0,
+  CONFIG_PARSE_WIN_READ_IPHELPER,
+  CONFIG_PARSE_WIN_READ_POLICY_SEARCHLIST,
+  CONFIG_PARSE_WIN_READ_TCPIP_SEARCHLIST,
+  CONFIG_PARSE_WIN_READ_DOMAIN,
+  CONFIG_PARSE_WIN_READ_POLICY_DEVOLUTION,
+  CONFIG_PARSE_WIN_READ_DNSCACHE_DEVOLUTION,
+  CONFIG_PARSE_WIN_READ_TCPIP_DEVOLUTION,
+  CONFIG_PARSE_WIN_READ_APPEND_MULTILABEL,
+  CONFIG_PARSE_WIN_READ_PRIMARY_SUFFIX,
+  CONFIG_PARSE_WIN_BAD_ADDRESS,
+  CONFIG_PARSE_WIN_NO_NAMESERVERS,

[mmenke, 2012/06/18 17:19:50]

You should name these consistently - Currently some "Results" are what the
problem was, while others are the step that failed.  Should use the same naming
convention for them all.

[szym, 2012/06/18 20:53:08]

The naming scheme is meant to describe the problem with obtaining the config.
"READ_" denotes problems occurring in ReadSystemSettings.

+  CONFIG_PARSE_WIN_MAX  // Bounding values for enumeration.
+};
+
+static void RecordConfigParseResult(ConfigParseWinResult result) {
+  UMA_HISTOGRAM_ENUMERATION("AsyncDNS.ConfigParseWin",
+                            result, CONFIG_PARSE_WIN_MAX);
+}
+
+enum HostsParseWinResult {
+  HOSTS_PARSE_WIN_OK = 0,
+  HOSTS_PARSE_WIN_READ_HOSTS,
+  HOSTS_PARSE_WIN_READ_COMPUTER_NAME,
+  HOSTS_PARSE_WIN_READ_IPHELPER,
+  HOSTS_PARSE_WIN_BAD_ADDRESS,

[mmenke, 2012/06/18 17:19:50]

See above comment.

+  HOSTS_PARSE_WIN_MAX  // Bounding values for enumeration.
+};
+
+static void RecordHostsParseResult(HostsParseWinResult result) {
+  UMA_HISTOGRAM_ENUMERATION("AsyncDNS.HostsParseWin",
+                            result, HOSTS_PARSE_WIN_MAX);
+}
+
 // Convenience for reading values using RegKey.
 class RegistryReader : public base::NonThreadSafe {
  public:
   explicit RegistryReader(const wchar_t* key) {
     // Ignoring the result. |key_.Valid()| will catch failures.
     key_.Open(HKEY_LOCAL_MACHINE, key, KEY_QUERY_VALUE);
   }
 
   bool ReadString(const wchar_t* name,
                   DnsSystemSettings::RegString* out) const {
@@ skipping 26 matching lines @@
     }
     return (result == ERROR_FILE_NOT_FOUND);
   }
 
  private:
   base::win::RegKey key_;
 
   DISALLOW_COPY_AND_ASSIGN(RegistryReader);
 };
 
-// Returns NULL if failed.
+// Wrapper for GetAdaptersAddresses. Returns NULL if failed.
 scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> ReadIpHelper(ULONG flags) {
   base::ThreadRestrictions::AssertIOAllowed();
 
   scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> out;
   ULONG len = 15000;  // As recommended by MSDN for GetAdaptersAddresses.
   UINT rv = ERROR_BUFFER_OVERFLOW;
   // Try up to three times.
   for (unsigned tries = 0; (tries < 3) && (rv == ERROR_BUFFER_OVERFLOW);
        tries++) {
     out.reset(reinterpret_cast<PIP_ADAPTER_ADDRESSES>(malloc(len)));
@@ skipping 26 matching lines @@
 
   // |punycode_output| should now be ASCII; convert it to a std::string.
   // (We could use UTF16ToASCII() instead, but that requires an extra string
   // copy. Since ASCII is a subset of UTF8 the following is equivalent).
   bool success = UTF16ToUTF8(punycode.data(), punycode.length(), domain);
   DCHECK(success);
   DCHECK(IsStringASCII(*domain));
   return success && !domain->empty();
 }
 
+bool ReadDevolutionSetting(const RegistryReader& reader,
+                           DnsSystemSettings::DevolutionSetting* setting) {
+  return reader.ReadDword(L"UseDomainNameDevolution", &setting->enabled) &&
+         reader.ReadDword(L"DomainNameDevolutionLevel", &setting->level);
+}
+
+// Reads DnsSystemSettings from IpHelper and registry.
+bool ReadSystemSettings(DnsSystemSettings* settings) {

[mmenke, 2012/06/18 17:19:50]

Same suggestion as for the posix version - return a ConfigParseWinResult so you
don't have to ensure something is logged just before every return statement.

+  settings->addresses = ReadIpHelper(GAA_FLAG_SKIP_ANYCAST |
+                                     GAA_FLAG_SKIP_UNICAST |
+                                     GAA_FLAG_SKIP_MULTICAST |
+                                     GAA_FLAG_SKIP_FRIENDLY_NAME);
+  if (!settings->addresses.get()) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_IPHELPER);
+    return false;
+  }
+
+  RegistryReader tcpip_reader(kTcpipPath);
+  RegistryReader tcpip6_reader(kTcpip6Path);
+  RegistryReader dnscache_reader(kDnscachePath);
+  RegistryReader policy_reader(kPolicyPath);
+  RegistryReader primary_dns_suffix_reader(kPrimaryDnsSuffixPath);
+
+  if (!policy_reader.ReadString(L"SearchList",
+                                &settings->policy_search_list)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_POLICY_SEARCHLIST);
+    return false;
+  }
+
+  if (!tcpip_reader.ReadString(L"SearchList", &settings->tcpip_search_list)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_TCPIP_SEARCHLIST);
+    return false;
+  }
+
+  if (!tcpip_reader.ReadString(L"Domain", &settings->tcpip_domain)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_DOMAIN);
+    return false;
+  }
+
+  if (!ReadDevolutionSetting(policy_reader, &settings->policy_devolution)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_POLICY_DEVOLUTION);
+    return false;
+  }
+
+  if (!ReadDevolutionSetting(dnscache_reader,
+                             &settings->dnscache_devolution)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_DNSCACHE_DEVOLUTION);
+    return false;
+  }
+
+  if (!ReadDevolutionSetting(tcpip_reader, &settings->tcpip_devolution)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_TCPIP_DEVOLUTION);
+    return false;
+  }
+
+  if (!policy_reader.ReadDword(L"AppendToMultiLabelName",
+                               &settings->append_to_multi_label_name)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_APPEND_MULTILABEL);
+    return false;
+  }
+
+  if (!primary_dns_suffix_reader.ReadString(L"PrimaryDnsSuffix",
+                                            &settings->primary_dns_suffix)) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_READ_PRIMARY_SUFFIX);
+    return false;
+  }
+  return true;
+}
+
+// Default address of "localhost" and local computer name can be overridden
+// by the HOSTS file, but if it's not there, then we need to fill it in.
+bool AddLocalhostEntries(DnsHosts* phosts) {

[cbentzel, 2012/06/16 18:03:41]

Nit: We don't tend to use hungarian-style notation (phosts) - but I think this
is OK so the reference can be |hosts|.

[mmenke, 2012/06/18 17:19:50]

Why not just make the function take in DnsHosts& hosts?

[mmenke, 2012/06/18 17:19:50]

Again, suggest having this return a HostsParseWinResult.

[cbentzel, 2012/06/18 18:37:17]

Style-guide doesn't allow non-const references.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Argu...

[mmenke, 2012/06/18 18:46:26]

Ahh, right, good point.  Doing a quick search through net, I can't find this
pattern anywhere, except for a single instance in dns_hosts.h, so perhaps we
should just use it as a pointer everywhere.

+  DnsHosts& hosts = *phosts;
+  const unsigned char kIPv4Localhost[] = { 127, 0, 0, 1 };
+  const unsigned char kIPv6Localhost[] = { 0, 0, 0, 0, 0, 0, 0, 0,
+                                           0, 0, 0, 0, 0, 0, 0, 1 };
+  IPAddressNumber loopback_ipv4(kIPv4Localhost,
+                                kIPv4Localhost + arraysize(kIPv4Localhost));
+  IPAddressNumber loopback_ipv6(kIPv6Localhost,
+                                kIPv6Localhost + arraysize(kIPv6Localhost));
+
+  // This does not override any pre-existing entries from the HOSTS file.
+  hosts.insert(std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV4),
+                              loopback_ipv4));
+  hosts.insert(std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV6),
+                              loopback_ipv6));
+
+  WCHAR buffer[MAX_PATH];
+  DWORD size = MAX_PATH;
+  std::string localname;
+  if (!GetComputerNameExW(ComputerNameDnsHostname, buffer, &size) ||
+      !ParseDomainASCII(buffer, &localname)) {
+    LOG(ERROR) << "Failed to read local computer name";

[cbentzel, 2012/06/16 18:03:41]

Do you need the LOG(ERROR) now that we have the histogram?

+    RecordHostsParseResult(HOSTS_PARSE_WIN_READ_COMPUTER_NAME);
+    return false;
+  }
+  StringToLowerASCII(&localname);
+
+  bool have_ipv4 =
+      hosts.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)) > 0;
+  bool have_ipv6 =
+      hosts.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)) > 0;
+
+  if (have_ipv4 && have_ipv6)
+    return true;
+
+  scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> addresses =
+      ReadIpHelper(GAA_FLAG_SKIP_ANYCAST |
+                   GAA_FLAG_SKIP_DNS_SERVER |
+                   GAA_FLAG_SKIP_MULTICAST |
+                   GAA_FLAG_SKIP_FRIENDLY_NAME);
+  if (!addresses.get()) {
+    RecordHostsParseResult(HOSTS_PARSE_WIN_READ_IPHELPER);
+    return false;
+  }
+
+  // The order of adapters is the network binding order, so stick to the
+  // first good adapter for each family.
+  for (const IP_ADAPTER_ADDRESSES* adapter = addresses.get();
+       adapter != NULL && (!have_ipv4 || !have_ipv6);
+       adapter = adapter->Next) {
+    if (adapter->OperStatus != IfOperStatusUp)
+      continue;
+    if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK)
+      continue;
+
+    for (const IP_ADAPTER_UNICAST_ADDRESS* address =
+             adapter->FirstUnicastAddress;
+         address != NULL;
+         address = address->Next) {
+      IPEndPoint ipe;
+      if (!ipe.FromSockAddr(address->Address.lpSockaddr,
+                            address->Address.iSockaddrLength)) {
+        RecordHostsParseResult(HOSTS_PARSE_WIN_BAD_ADDRESS);
+        return false;
+      }
+      if (!have_ipv4 && (ipe.GetFamily() == AF_INET)) {
+        have_ipv4 = true;
+        hosts[DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)] = ipe.address();
+      } else if (!have_ipv6 && (ipe.GetFamily() == AF_INET6)) {
+        have_ipv6 = true;
+        hosts[DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)] = ipe.address();
+      }
+    }
+  }
+  RecordHostsParseResult(HOSTS_PARSE_WIN_OK);
+  return true;
+}
+
 }  // namespace
 
 FilePath GetHostsPath() {
   TCHAR buffer[MAX_PATH];
   UINT rc = GetSystemDirectory(buffer, MAX_PATH);
   DCHECK(0 < rc && rc < MAX_PATH);
   return FilePath(buffer).Append(FILE_PATH_LITERAL("drivers\\etc\\hosts"));
 }
 
 bool ParseSearchList(const string16& value, std::vector<std::string>* output) {
@@ skipping 42 matching lines @@
          address != NULL;
          address = address->Next) {
       IPEndPoint ipe;
       if (ipe.FromSockAddr(address->Address.lpSockaddr,
                            address->Address.iSockaddrLength)) {
         // Override unset port.
         if (!ipe.port())
           ipe = IPEndPoint(ipe.address(), dns_protocol::kDefaultPort);
         config->nameservers.push_back(ipe);
       } else {
+        RecordConfigParseResult(CONFIG_PARSE_WIN_BAD_ADDRESS);
         return false;
       }
     }
 
     // IP_ADAPTER_ADDRESSES in Vista+ has a search list at |FirstDnsSuffix|,
     // but it came up empty in all trials.
     // |DnsSuffix| stores the effective connection-specific suffix, which is
     // obtained via DHCP (regkey: Tcpip\Parameters\Interfaces\{XXX}\DhcpDomain)
     // or specified by the user (regkey: Tcpip\Parameters\Domain).
     std::string dns_suffix;
     if (ParseDomainASCII(adapter->DnsSuffix, &dns_suffix))
       config->search.push_back(dns_suffix);
   }
 
-  if (config->nameservers.empty())
+  if (config->nameservers.empty()) {
+    RecordConfigParseResult(CONFIG_PARSE_WIN_NO_NAMESERVERS);
     return false;  // No point continuing.
+  }
 
   // Windows always tries a multi-label name "as is" before using suffixes.
   config->ndots = 1;
 
   if (!settings.append_to_multi_label_name.set) {
     // The default setting is true for XP, false for Vista+.
     if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
       config->append_to_multi_label_name = false;
     } else {
       config->append_to_multi_label_name = true;
@@ skipping 76 matching lines @@
   // behavior (see also ParseSearchList). If a suffix is not valid, it will be
   // discarded when the fully-qualified name is converted to DNS format.
 
   unsigned num_dots = std::count(primary_suffix.begin(),
                                  primary_suffix.end(), '.');
 
   for (size_t offset = 0; num_dots >= devolution.level.value; --num_dots) {
     offset = primary_suffix.find('.', offset + 1);
     config->search.push_back(primary_suffix.substr(offset + 1));
   }
-
+  RecordConfigParseResult(CONFIG_PARSE_WIN_OK);
   return true;
 }
 
-// Watches registry for changes and reads config from registry and IP helper.
-// Reading and opening of reg keys is always performed on WorkerPool. Setting
-// up watches requires IO loop.
+// Reads config from registry and IP helper. All work performed on WorkerPool.
 class DnsConfigServiceWin::ConfigReader : public SerialWorker {
  public:
   explicit ConfigReader(DnsConfigServiceWin* service)
       : service_(service),
         success_(false) {}
 
  private:
-  bool ReadDevolutionSetting(const RegistryReader& reader,
-                             DnsSystemSettings::DevolutionSetting& setting) {
-    return reader.ReadDword(L"UseDomainNameDevolution", &setting.enabled) &&
-           reader.ReadDword(L"DomainNameDevolutionLevel", &setting.level);
-  }
+  virtual ~ConfigReader() {}
 
   virtual void DoWork() OVERRIDE {
     // Should be called on WorkerPool.
-    success_ = false;
-
+    base::TimeTicks start_time = base::TimeTicks::Now();
     DnsSystemSettings settings = {};
-    settings.addresses = ReadIpHelper(GAA_FLAG_SKIP_ANYCAST |
-                                      GAA_FLAG_SKIP_UNICAST |
-                                      GAA_FLAG_SKIP_MULTICAST |
-                                      GAA_FLAG_SKIP_FRIENDLY_NAME);
-    if (!settings.addresses.get())
-      return;  // no point reading the rest
-
-    RegistryReader tcpip_reader(kTcpipPath);
-    RegistryReader tcpip6_reader(kTcpip6Path);
-    RegistryReader dnscache_reader(kDnscachePath);
-    RegistryReader policy_reader(kPolicyPath);
-    RegistryReader primary_dns_suffix_reader(kPrimaryDnsSuffixPath);
-
-    if (!policy_reader.ReadString(L"SearchList",
-                                  &settings.policy_search_list))
-      return;
-
-    if (!tcpip_reader.ReadString(L"SearchList", &settings.tcpip_search_list))
-      return;
-
-    if (!tcpip_reader.ReadString(L"Domain", &settings.tcpip_domain))
-      return;
-
-    if (!ReadDevolutionSetting(policy_reader, settings.policy_devolution))
-      return;
-
-    if (!ReadDevolutionSetting(dnscache_reader,
-                               settings.dnscache_devolution))
-      return;
-
-    if (!ReadDevolutionSetting(tcpip_reader, settings.tcpip_devolution))
-      return;
-
-    if (!policy_reader.ReadDword(L"AppendToMultiLabelName",
-                                 &settings.append_to_multi_label_name))
-      return;
-
-    if (!primary_dns_suffix_reader.ReadString(L"PrimaryDnsSuffix",
-                                              &settings.primary_dns_suffix))
-      return;
-
-    success_ = ConvertSettingsToDnsConfig(settings, &dns_config_);
+    success_ = ReadSystemSettings(&settings);
+    if (success_)
+      success_ = ConvertSettingsToDnsConfig(settings, &dns_config_);
+    UMA_HISTOGRAM_BOOLEAN("AsyncDNS.ConfigParseWinResult", success_);
+    UMA_HISTOGRAM_TIMES("AsyncDNS.ConfigParseDuration",
+                        base::TimeTicks::Now() - start_time);
   }
 
   virtual void OnWorkFinished() OVERRIDE {
     DCHECK(loop()->BelongsToCurrentThread());
     DCHECK(!IsCancelled());
     if (success_) {
       service_->OnConfigRead(dns_config_);
     } else {
       LOG(WARNING) << "Failed to read DnsConfig.";
     }
   }
 
   DnsConfigServiceWin* service_;
   // Written in DoRead(), read in OnReadFinished(). No locking required.
   DnsConfig dns_config_;
   bool success_;
 };
 
 // An extension for DnsHostsReader which also watches the HOSTS file,
 // reads local name from GetComputerNameEx, local IP from GetAdaptersAddresses,
 // and observes changes to local IP address.
 class DnsConfigServiceWin::HostsReader : public SerialWorker {
  public:
   explicit HostsReader(DnsConfigServiceWin* service)
       : path_(GetHostsPath()), service_(service) {
   }
 
  private:
+  virtual ~HostsReader() {}
+
   virtual void DoWork() OVERRIDE {
+    base::TimeTicks start_time = base::TimeTicks::Now();
     success_ = ParseHostsFile(path_, &hosts_);
-
-    if (!success_)
-      return;
-
-    success_ = false;
-
-    // Default address of "localhost" and local computer name can be overridden
-    // by the HOSTS file, but if it's not there, then we need to fill it in.
-
-    const unsigned char kIPv4Localhost[] = { 127, 0, 0, 1 };
-    const unsigned char kIPv6Localhost[] = { 0, 0, 0, 0, 0, 0, 0, 0,
-                                             0, 0, 0, 0, 0, 0, 0, 1 };
-    IPAddressNumber loopback_ipv4(kIPv4Localhost,
-                                  kIPv4Localhost + arraysize(kIPv4Localhost));
-    IPAddressNumber loopback_ipv6(kIPv6Localhost,
-                                  kIPv6Localhost + arraysize(kIPv6Localhost));
-
-    // This does not override any pre-existing entries from the HOSTS file.
-    hosts_.insert(std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV4),
-                                 loopback_ipv4));
-    hosts_.insert(std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV6),
-                                 loopback_ipv6));
-
-    WCHAR buffer[MAX_PATH];
-    DWORD size = MAX_PATH;
-    std::string localname;
-    if (!GetComputerNameExW(ComputerNameDnsHostname, buffer, &size) ||
-        !ParseDomainASCII(buffer, &localname)) {
-      LOG(ERROR) << "Failed to read local computer name";
-      return;
+    if (success_) {
+      success_ = AddLocalhostEntries(&hosts_);
+    } else {
+      RecordHostsParseResult(HOSTS_PARSE_WIN_READ_HOSTS);
     }
-    StringToLowerASCII(&localname);
-
-    bool have_ipv4 =
-        hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)) > 0;
-    bool have_ipv6 =
-        hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)) > 0;
-
-    if (have_ipv4 && have_ipv6) {
-      success_ = true;
-      return;
-    }
-
-    scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> addresses =
-        ReadIpHelper(GAA_FLAG_SKIP_ANYCAST |
-                     GAA_FLAG_SKIP_DNS_SERVER |
-                     GAA_FLAG_SKIP_MULTICAST |
-                     GAA_FLAG_SKIP_FRIENDLY_NAME);
-    if (!addresses.get())
-      return;
-
-    // The order of adapters is the network binding order, so stick to the
-    // first good adapter for each family.
-    for (const IP_ADAPTER_ADDRESSES* adapter = addresses.get();
-         adapter != NULL && (!have_ipv4 || !have_ipv6);
-         adapter = adapter->Next) {
-      if (adapter->OperStatus != IfOperStatusUp)
-        continue;
-      if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK)
-        continue;
-
-      for (const IP_ADAPTER_UNICAST_ADDRESS* address =
-               adapter->FirstUnicastAddress;
-           address != NULL;
-           address = address->Next) {
-        IPEndPoint ipe;
-        if (!ipe.FromSockAddr(address->Address.lpSockaddr,
-                              address->Address.iSockaddrLength)) {
-          return;
-        }
-        if (!have_ipv4 && (ipe.GetFamily() == AF_INET)) {
-          have_ipv4 = true;
-          hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)] = ipe.address();
-        } else if (!have_ipv6 && (ipe.GetFamily() == AF_INET6)) {
-          have_ipv6 = true;
-          hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)] = ipe.address();
-        }
-      }
-    }
-    success_ = true;
+    UMA_HISTOGRAM_BOOLEAN("AsyncDNS.HostParseResult", success_);
+    UMA_HISTOGRAM_TIMES("AsyncDNS.HostsParseDuration",
+                        base::TimeTicks::Now() - start_time);
   }
 
   virtual void OnWorkFinished() OVERRIDE {
     DCHECK(loop()->BelongsToCurrentThread());
     if (success_) {
       service_->OnHostsRead(hosts_);
     } else {
       LOG(WARNING) << "Failed to read DnsHosts.";
     }
   }
 
   const FilePath path_;
   DnsConfigServiceWin* service_;
   // Written in DoWork, read in OnWorkFinished, no locking necessary.
   DnsHosts hosts_;
   bool success_;
 
   DISALLOW_COPY_AND_ASSIGN(HostsReader);
 };
 
 DnsConfigServiceWin::DnsConfigServiceWin()
     : config_reader_(new ConfigReader(this)),
       hosts_reader_(new HostsReader(this)) {}
 
 DnsConfigServiceWin::~DnsConfigServiceWin() {
-  DCHECK(CalledOnValidThread());
   config_reader_->Cancel();
   hosts_reader_->Cancel();
   NetworkChangeNotifier::RemoveIPAddressObserver(this);
 }
 
 void DnsConfigServiceWin::Watch(const CallbackType& callback) {
   DnsConfigService::Watch(callback);
   // Also need to observe changes to local non-loopback IP for DnsHosts.
   NetworkChangeNotifier::AddIPAddressObserver(this);
 }
@@ skipping 27 matching lines @@
 
 }  // namespace internal
 
 // static
 scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() {
   return scoped_ptr<DnsConfigService>(new internal::DnsConfigServiceWin());
 }
 
 }  // namespace net