[net/dns] Removes locking from DnsConfigServiceWin and adds local computer name.

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/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/utf_string_conversions.h"
 #include "base/win/object_watcher.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/dns/dns_protocol.h"
 #include "net/dns/file_path_watcher_wrapper.h"
 #include "net/dns/serial_worker.h"
 
 #pragma comment(lib, "iphlpapi.lib")
 
 namespace net {
 
 namespace internal {
 
 namespace {
 
-// Watches a single registry key for changes. Thread-safe.
-class RegistryWatcher : public base::win::ObjectWatcher::Delegate {
+// Registry key paths.
+const wchar_t* const kTcpipPath =
+    L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters";
+const wchar_t* const kTcpip6Path =
+    L"SYSTEM\\CurrentControlSet\\Services\\Tcpip6\\Parameters";
+const wchar_t* const kDnscachePath =
+    L"SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters";
+const wchar_t* const kPolicyPath =
+    L"SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient";
+
+// Convenience for reading values using RegKey.
+class RegistryReader : public base::NonThreadSafe {
  public:
-  typedef base::Callback<void(bool succeeded)> CallbackType;
-  RegistryWatcher() {}
-
-  bool Watch(const wchar_t* key, const CallbackType& callback) {
-    base::AutoLock lock(lock_);
-    DCHECK(!callback.is_null());
-    CancelLocked();
-    if (key_.Open(HKEY_LOCAL_MACHINE, key,
-                  KEY_NOTIFY | KEY_QUERY_VALUE) != ERROR_SUCCESS)
-      return false;
-    if (key_.StartWatching() != ERROR_SUCCESS)
-      return false;
-    if (!watcher_.StartWatching(key_.watch_event(), this))
-      return false;
-    callback_ = callback;
-    return true;
-  }
-
-  void Cancel() {
-    base::AutoLock lock(lock_);
-    CancelLocked();
-  }
-
-  virtual void OnObjectSignaled(HANDLE object) OVERRIDE {
-    base::AutoLock lock(lock_);
-    bool succeeded = (key_.StartWatching() == ERROR_SUCCESS) &&
-                     watcher_.StartWatching(key_.watch_event(), this);
-    if (!callback_.is_null())
-      callback_.Run(succeeded);
+  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 {
-    base::AutoLock lock(lock_);
+    DCHECK(CalledOnValidThread());
     out->set = false;
     if (!key_.Valid()) {
       // Assume that if the |key_| is invalid then the key is missing.
       return true;
     }
     LONG result = key_.ReadValue(name, &out->value);
     if (result == ERROR_SUCCESS) {
       out->set = true;
       return true;
     }
     return (result == ERROR_FILE_NOT_FOUND);
   }
 
   bool ReadDword(const wchar_t* name,
                  DnsSystemSettings::RegDword* out) const {
-    base::AutoLock lock(lock_);
+    DCHECK(CalledOnValidThread());
     out->set = false;
     if (!key_.Valid()) {
       // Assume that if the |key_| is invalid then the key is missing.
       return true;
     }
     LONG result = key_.ReadValueDW(name, &out->value);
     if (result == ERROR_SUCCESS) {
       out->set = true;
       return true;
     }
     return (result == ERROR_FILE_NOT_FOUND);
   }
 
  private:
-  void CancelLocked() {
-    lock_.AssertAcquired();
+  base::win::RegKey key_;
+
+  DISALLOW_COPY_AND_ASSIGN(RegistryReader);
+};
+
+
+// Watches a single registry key for changes.
+class RegistryWatcher : public base::win::ObjectWatcher::Delegate,
+                        public base::NonThreadSafe {
+ public:
+  typedef base::Callback<void(bool succeeded)> CallbackType;
+  RegistryWatcher() {}
+
+  bool Watch(const wchar_t* key, const CallbackType& callback) {
+    DCHECK(CalledOnValidThread());
+    DCHECK(!callback.is_null());
+    Cancel();
+    if (key_.Open(HKEY_LOCAL_MACHINE, key, KEY_NOTIFY) != ERROR_SUCCESS)
+      return false;
+    if (key_.StartWatching() != ERROR_SUCCESS)
+      return false;
+    if (!watcher_.StartWatching(key_.watch_event(), this))
+      return false;
+    callback_ = callback;
+    return true;
+  }
+
+  void Cancel() {
+    DCHECK(CalledOnValidThread());
     callback_.Reset();
     if (key_.Valid()) {
       watcher_.StopWatching();
       key_.StopWatching();
     }
   }
 
-  mutable base::Lock lock_;
+  virtual void OnObjectSignaled(HANDLE object) OVERRIDE {
+    DCHECK(CalledOnValidThread());
+    bool succeeded = (key_.StartWatching() == ERROR_SUCCESS) &&
+                     watcher_.StartWatching(key_.watch_event(), this);

[mmenke, 2012/03/19 15:18:01]

nit:  Think this should be indented one more space.

+    if (!callback_.is_null())
+      callback_.Run(succeeded);
+  }
+
+ private:
   CallbackType callback_;
   base::win::RegKey key_;
   base::win::ObjectWatcher watcher_;
 
   DISALLOW_COPY_AND_ASSIGN(RegistryWatcher);
 };
 
 // Converts a string16 domain name to ASCII, possibly using punycode.
 // Returns true if the conversion succeeds and output is not empty. In case of
 // failure, |domain| might become dirty.
@@ skipping 191 matching lines @@
       : service_(service),
         success_(false) {}
 
   bool Watch() {
     DCHECK(loop()->BelongsToCurrentThread());
 
     RegistryWatcher::CallbackType callback =
         base::Bind(&ConfigReader::OnChange, base::Unretained(this));
 
     // The Tcpip key must be present.
-    if (!tcpip_watcher_.Watch(
-        L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters",
-        callback))
+    if (!tcpip_watcher_.Watch(kTcpipPath, callback))
       return false;
 
     // Watch for IPv6 nameservers.
-    tcpip6_watcher_.Watch(
-        L"SYSTEM\\CurrentControlSet\\Services\\Tcpip6\\Parameters",
-        callback);
+    tcpip6_watcher_.Watch(kTcpip6Path, callback);
 
     // DNS suffix search list and devolution can be configured via group
     // policy which sets this registry key. If the key is missing, the policy
     // does not apply, and the DNS client uses Tcpip and Dnscache settings.
     // If a policy is installed, DnsConfigService will need to be restarted.
     // BUG=99509
 
-    dnscache_watcher_.Watch(
-        L"SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters",
-        callback);
-
-    policy_watcher_.Watch(
-        L"SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient",
-        callback);
+    dnscache_watcher_.Watch(kDnscachePath, callback);
+    policy_watcher_.Watch(kPolicyPath, callback);
 
     WorkNow();
     return true;
   }
 
   void Cancel() {
     DCHECK(loop()->BelongsToCurrentThread());
     SerialWorker::Cancel();
     policy_watcher_.Cancel();
     dnscache_watcher_.Cancel();
@@ skipping 32 matching lines @@
          (tries < 3) && (rv == ERROR_BUFFER_OVERFLOW);
          tries++) {
       settings->addresses.reset(
           reinterpret_cast<PIP_ADAPTER_ADDRESSES>(malloc(len)));
       rv = GetAdaptersAddresses(AF_UNSPEC, flags, NULL,
                                 settings->addresses.get(), &len);
     }
     return (rv == NO_ERROR);
   }
 
-  bool ReadDevolutionSetting(const RegistryWatcher& watcher,
+  bool ReadDevolutionSetting(const RegistryReader& reader,
                              DnsSystemSettings::DevolutionSetting& setting) {
-    return watcher.ReadDword(L"UseDomainNameDevolution", &setting.enabled) &&
-           watcher.ReadDword(L"DomainNameDevolutionLevel", &setting.level);
+    return reader.ReadDword(L"UseDomainNameDevolution", &setting.enabled) &&
+           reader.ReadDword(L"DomainNameDevolutionLevel", &setting.level);
   }
 
   virtual void DoWork() OVERRIDE {
     // Should be called on WorkerPool.
     success_ = false;
 
     DnsSystemSettings settings;
     memset(&settings, 0, sizeof(settings));
     if (!ReadIpHelper(&settings))
       return;  // no point reading the rest
 
-    if (!policy_watcher_.ReadString(L"SearchList",
-                                    &settings.policy_search_list))
+    RegistryReader tcpip_reader(kTcpipPath);
+    RegistryReader tcpip6_reader(kTcpip6Path);
+    RegistryReader dnscache_reader(kDnscachePath);
+    RegistryReader policy_reader(kPolicyPath);
+
+    if (!policy_reader.ReadString(L"SearchList",
+                                  &settings.policy_search_list))
       return;
 
-    if (!tcpip_watcher_.ReadString(L"SearchList", &settings.tcpip_search_list))
+    if (!tcpip_reader.ReadString(L"SearchList", &settings.tcpip_search_list))
       return;
 
-    if (!tcpip_watcher_.ReadString(L"Domain", &settings.tcpip_domain))
+    if (!tcpip_reader.ReadString(L"Domain", &settings.tcpip_domain))
       return;
 
-    if (!ReadDevolutionSetting(policy_watcher_, settings.policy_devolution))
+    if (!ReadDevolutionSetting(policy_reader, settings.policy_devolution))
       return;
 
-    if (!ReadDevolutionSetting(dnscache_watcher_,
+    if (!ReadDevolutionSetting(dnscache_reader,
                                settings.dnscache_devolution))
       return;
 
-    if (!ReadDevolutionSetting(tcpip_watcher_, settings.tcpip_devolution))
+    if (!ReadDevolutionSetting(tcpip_reader, settings.tcpip_devolution))
       return;
 
-    if (!policy_watcher_.ReadDword(L"AppendToMultiLabelName",
-                                   &settings.append_to_multi_label_name))
+    if (!policy_reader.ReadDword(L"AppendToMultiLabelName",
+                                 &settings.append_to_multi_label_name))
       return;
 
     success_ = ConvertSettingsToDnsConfig(settings, &dns_config_);
   }
 
   virtual void OnWorkFinished() OVERRIDE {
     DCHECK(loop()->BelongsToCurrentThread());
     DCHECK(!IsCancelled());
     if (success_) {
       service_->OnConfigRead(dns_config_);
     } else {
       LOG(WARNING) << "Failed to read config.";
     }
   }
 
   DnsConfigServiceWin* service_;
   // Written in DoRead(), read in OnReadFinished(). No locking required.
   DnsConfig dns_config_;
   bool success_;
 
   RegistryWatcher tcpip_watcher_;
   RegistryWatcher tcpip6_watcher_;
   RegistryWatcher dnscache_watcher_;
   RegistryWatcher policy_watcher_;
 };
 
+
+// An extension for DnsHostsReader which also reads local name from
+// GetComputerNameEx and watches registry for changes to local name.
+class DnsConfigServiceWin::HostsReader : public base::NonThreadSafe {
+ public:
+  explicit HostsReader(DnsConfigServiceWin* service) : service_(service) {
+    TCHAR buffer[MAX_PATH];
+    UINT rc = GetSystemDirectory(buffer, MAX_PATH);
+    DCHECK(0 < rc && rc < MAX_PATH);
+    FilePath hosts_path = FilePath(buffer).
+        Append(FILE_PATH_LITERAL("drivers\\etc\\hosts"));
+    hosts_reader_ = new DnsHostsReader(
+        hosts_path,
+        base::Bind(&HostsReader::OnHostsRead, base::Unretained(this)));
+  }
+
+  bool Watch() {
+    DCHECK(CalledOnValidThread());
+    if (!hosts_watcher_.Watch(hosts_reader_->path(),
+                              base::Bind(&HostsReader::OnHostsChanged,
+                                         base::Unretained(this)))) {
+      return false;
+    }
+    if (!registry_watcher_.Watch(kTcpipPath,
+                                 base::Bind(&HostsReader::OnNameChanged,
+                                 base::Unretained(this)))) {

[mmenke, 2012/03/19 15:18:01]

nit:  Indent this to the open paren, if it fits, so it's clearer that this is an
argument of bind.

+      return false;
+    }
+
+    hosts_reader_->WorkNow();
+    return true;
+  }
+
+  ~HostsReader() {
+    hosts_reader_->Cancel();
+  }
+
+ private:
+  void OnHostsChanged(bool succeeded) {
+    DCHECK(CalledOnValidThread());
+    service_->InvalidateHosts();
+    if (succeeded)
+      hosts_reader_->WorkNow();
+    else
+      LOG(ERROR) << "Failed to watch DNS hosts";
+  }
+
+  void OnHostsRead(const DnsHosts& hosts) {
+    DCHECK(CalledOnValidThread());
+    hosts_ = hosts;
+    UpdateService();
+  }
+
+  void OnNameChanged(bool succeeded) {
+    DCHECK(CalledOnValidThread());
+    if (!succeeded) {
+      LOG(ERROR) << "Failed to watch local computer name";
+      service_->InvalidateHosts();
+      return;
+    }
+    WCHAR buffer[MAX_PATH];
+    DWORD size = MAX_PATH;
+    std::string localname;
+    if (!GetComputerNameExW(ComputerNameDnsHostname, buffer, &size) ||
+        !ParseDomainASCII(buffer, &localname)) {
+      service_->InvalidateHosts();
+      LOG(ERROR) << "Failed to read local computer name";
+      return;
+    }
+
+    if (localname != localname_) {
+      localname_ = localname;
+      UpdateService();

[mmenke, 2012/03/19 15:18:01]

What if the last call to OnHostsChanged failed?

Same issue if this fails, but OnHostsChanged succeeds.

+    }
+  }
+
+  void UpdateService() {
+    DCHECK(CalledOnValidThread());
+    DnsHosts complete_hosts = hosts_;
+
+    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 };
+
+    complete_hosts[DnsHostsKey(localname_, ADDRESS_FAMILY_IPV4)] =
+        IPAddressNumber(kIPv4Localhost,
+                        kIPv4Localhost + arraysize(kIPv4Localhost));
+    complete_hosts[DnsHostsKey(localname_, ADDRESS_FAMILY_IPV6)] =
+        IPAddressNumber(kIPv6Localhost,
+                        kIPv6Localhost + arraysize(kIPv6Localhost));
+    service_->OnHostsRead(complete_hosts);
+  }
+
+  DnsConfigServiceWin* service_;
+  FilePathWatcherWrapper hosts_watcher_;
+  RegistryWatcher registry_watcher_;
+  scoped_refptr<DnsHostsReader> hosts_reader_;
+
+  // Contents of the HOSTS file.
+  DnsHosts hosts_;
+  // Local computer name in ASCII.
+  std::string localname_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostsReader);
+};
+
+
 DnsConfigServiceWin::DnsConfigServiceWin()
     : config_reader_(new ConfigReader(this)),
-      hosts_watcher_(new FilePathWatcherWrapper()) {}
+      hosts_reader_(new HostsReader(this)) {}
 
 DnsConfigServiceWin::~DnsConfigServiceWin() {
   DCHECK(CalledOnValidThread());
   config_reader_->Cancel();
-  if (hosts_reader_)
-    hosts_reader_->Cancel();
 }
 
 void DnsConfigServiceWin::Watch(const CallbackType& callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(!callback.is_null());
   set_callback(callback);
 
   // This is done only once per lifetime so open the keys and file watcher
   // handles on this thread.
   // TODO(szym): Should/can this be avoided? http://crbug.com/114223
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
   if (!config_reader_->Watch()) {
     LOG(ERROR) << "Failed to start watching DNS config";
     InvalidateConfig();
   }
 
-  TCHAR buffer[MAX_PATH];
-  UINT rc = GetSystemDirectory(buffer, MAX_PATH);
-  DCHECK(0 < rc && rc < MAX_PATH);
-  FilePath hosts_path = FilePath(buffer).
-      Append(FILE_PATH_LITERAL("drivers\\etc\\hosts"));
-  hosts_reader_ = new DnsHostsReader(
-      hosts_path,
-      base::Bind(&DnsConfigServiceWin::OnHostsRead, base::Unretained(this)));
-  if (hosts_watcher_->Watch(hosts_path,
-                            base::Bind(&DnsConfigServiceWin::OnHostsChanged,
-                                       base::Unretained(this)))) {
-    OnHostsChanged(true);
-  } else {
-    OnHostsChanged(false);
+  if (!hosts_reader_->Watch()) {
+    LOG(ERROR) << "Failed to start watching HOSTS";
+    InvalidateHosts();
   }
 }
 
-void DnsConfigServiceWin::OnHostsChanged(bool succeeded) {
-  InvalidateHosts();
-  if (succeeded)
-    hosts_reader_->WorkNow();
-  else
-    LOG(ERROR) << "Failed to watch DNS hosts";
-}
 

[mmenke, 2012/03/19 15:18:01]

nit:  Remove extra blank line.

 }  // namespace internal
 
 // static
 scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() {
   return scoped_ptr<DnsConfigService>(new internal::DnsConfigServiceWin());
 }
 
 }  // namespace net