Implementation of DnsConfigService for win (+ Bind migration).

DnsConfigServiceWin watches Windows registry for changes and asks IPHelper for effective DNS server addresses. To support group policy, suffix search list is read directly from registry. (Alternative to use WMI is too cumbersome.)

BUG=90881
TEST=./net_unittests --gtest_filter=DnsConfigService*


Review URL: http://codereview.chromium.org/8142013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104738 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 899 insertions, 304 deletions

diff --git a/net/dns/dns_config_service.cc b/net/dns/dns_config_service.cc
index ed3f1d2..dcf58bd 100644
--- a/net/dns/dns_config_service.cc
+++ b/net/dns/dns_config_service.cc
@@ -55,9 +55,9 @@ void DnsConfigService::RemoveObserver(Observer* observer) {
 void DnsConfigService::OnConfigRead(const DnsConfig& config) {
   DCHECK(CalledOnValidThread());
   if (!config.EqualsIgnoreHosts(dns_config_)) {
-    DnsConfig copy = config;
-    copy.hosts.swap(dns_config_.hosts);
-    dns_config_ = copy;
+    DnsHosts current_hosts = dns_config_.hosts;
+    dns_config_ = config;
+    dns_config_.hosts.swap(current_hosts);
     have_config_ = true;
     if (have_hosts_) {
       FOR_EACH_OBSERVER(Observer, observers_, OnConfigChanged(dns_config_));
@@ -76,8 +76,9 @@ void DnsConfigService::OnHostsRead(const DnsHosts& hosts) {
   }
 }
 
-DnsHostsReader::DnsHostsReader(DnsConfigService* service)
-  : service_(service),
+DnsHostsReader::DnsHostsReader(const FilePath& path, DnsConfigService* service)
+  : path_(path),
+    service_(service),
     success_(false) {
   DCHECK(service);
 }
@@ -91,17 +92,18 @@ static bool ReadFile(const FilePath& path, int64 size, std::string* str) {
   return file_util::ReadFileToString(path, str);
 }
 
-void DnsHostsReader::DoRead() {
+void DnsHostsReader::DoWork() {
   success_ = false;
   std::string contents;
   const int64 kMaxHostsSize = 1 << 16;
-  if (ReadFile(get_path(), kMaxHostsSize, &contents)) {
+  if (ReadFile(path_, kMaxHostsSize, &contents)) {
     success_ = true;
     ParseHosts(contents, &dns_hosts_);
   }
 }
 
-void DnsHostsReader::OnReadFinished() {
+void DnsHostsReader::OnWorkFinished() {
+  DCHECK(!IsCancelled());
   if (success_)
     service_->OnHostsRead(dns_hosts_);
 }
diff --git a/net/dns/dns_config_service.h b/net/dns/dns_config_service.h
index 6ceeaa6..655ea78 100644
--- a/net/dns/dns_config_service.h
+++ b/net/dns/dns_config_service.h
@@ -10,13 +10,15 @@
 #include <string>
 #include <vector>
 
+#include "base/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/observer_list.h"
+#include "base/threading/non_thread_safe.h"
 #include "base/time.h"
 #include "net/base/ip_endpoint.h"  // win requires size of IPEndPoint
 #include "net/base/net_export.h"
 #include "net/dns/dns_hosts.h"
-#include "net/dns/watching_file_reader.h"
+#include "net/dns/serial_worker.h"
 
 namespace net {
 
@@ -110,16 +112,19 @@ class NET_EXPORT_PRIVATE DnsConfigService
 
 // A WatchingFileReader that reads a HOSTS file and notifies
 // DnsConfigService::OnHostsRead().
-class DnsHostsReader : public WatchingFileReader {
+// Client should call Cancel() when |service| is going away.
+class NET_EXPORT_PRIVATE DnsHostsReader
+  : NON_EXPORTED_BASE(public SerialWorker) {
  public:
-  explicit DnsHostsReader(DnsConfigService* service);
+  DnsHostsReader(const FilePath& path, DnsConfigService* service);
 
-  virtual void DoRead() OVERRIDE;
-  virtual void OnReadFinished() OVERRIDE;
+  virtual void DoWork() OVERRIDE;
+  virtual void OnWorkFinished() OVERRIDE;
 
  private:
   virtual ~DnsHostsReader();
 
+  FilePath path_;
   DnsConfigService* service_;
   // Written in DoRead, read in OnReadFinished, no locking necessary.
   DnsHosts dns_hosts_;
diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc
index b05b164..0224c39 100644
--- a/net/dns/dns_config_service_posix.cc
+++ b/net/dns/dns_config_service_posix.cc
@@ -12,6 +12,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_util.h"
+#include "net/dns/serial_worker.h"
+#include "net/dns/watching_file_reader.h"
 
 #ifndef _PATH_RESCONF  // Normally defined in <resolv.h>
 #define _PATH_RESCONF "/etc/resolv.conf"
@@ -19,15 +21,15 @@
 
 namespace net {
 
-// A WatcherDelegate that uses ResolverLib to initialize res_state and converts
+// A SerialWorker that uses ResolverLib to initialize res_state and converts
 // it to DnsConfig.
-class DnsConfigServicePosix::DnsConfigReader : public WatchingFileReader {
+class DnsConfigServicePosix::ConfigReader : public SerialWorker {
  public:
-  explicit DnsConfigReader(DnsConfigServicePosix* service)
+  explicit ConfigReader(DnsConfigServicePosix* service)
     : service_(service),
       success_(false) {}
 
-  void DoRead() OVERRIDE {
+  void DoWork() OVERRIDE {
     struct __res_state res;
     if ((res_ninit(&res) == 0) && (res.options & RES_INIT)) {
       success_ = ConvertResToConfig(res, &dns_config_);
@@ -42,34 +44,33 @@ class DnsConfigServicePosix::DnsConfigReader : public WatchingFileReader {
 #endif
   }
 
-  void OnReadFinished() OVERRIDE {
+  void OnWorkFinished() OVERRIDE {
+    DCHECK(!IsCancelled());
     if (success_)
       service_->OnConfigRead(dns_config_);
   }
 
  private:
-  virtual ~DnsConfigReader() {}
+  virtual ~ConfigReader() {}
 
   DnsConfigServicePosix* service_;
-  // Written in DoRead, read in OnReadFinished, no locking necessary.
+  // Written in DoWork, read in OnWorkFinished, no locking necessary.
   DnsConfig dns_config_;
   bool success_;
 };
 
 DnsConfigServicePosix::DnsConfigServicePosix()
-  : config_reader_(new DnsConfigReader(this)),
-    hosts_reader_(new DnsHostsReader(this)) {}
+  : config_watcher_(new WatchingFileReader()),
+    hosts_watcher_(new WatchingFileReader()) {}
 
-DnsConfigServicePosix::~DnsConfigServicePosix() {
-  DCHECK(CalledOnValidThread());
-  config_reader_->Cancel();
-  hosts_reader_->Cancel();
-}
+DnsConfigServicePosix::~DnsConfigServicePosix() {}
 
 void DnsConfigServicePosix::Watch() {
   DCHECK(CalledOnValidThread());
-  config_reader_->StartWatch(FilePath(FILE_PATH_LITERAL(_PATH_RESCONF)));
-  hosts_reader_->StartWatch(FilePath(FILE_PATH_LITERAL("/etc/hosts")));
+  config_watcher_->StartWatch(FilePath(FILE_PATH_LITERAL(_PATH_RESCONF)),
+                              new ConfigReader(this));
+  FilePath hosts_path(FILE_PATH_LITERAL("/etc/hosts"));
+  hosts_watcher_->StartWatch(hosts_path, new DnsHostsReader(hosts_path, this));
 }
 
 // static
diff --git a/net/dns/dns_config_service_posix.h b/net/dns/dns_config_service_posix.h
index 5349adf..91e03f4 100644
--- a/net/dns/dns_config_service_posix.h
+++ b/net/dns/dns_config_service_posix.h
@@ -9,25 +9,26 @@
 #include <resolv.h>
 
 #include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "net/base/net_export.h"
 #include "net/dns/dns_config_service.h"
 
 namespace net {
 
+class WatchingFileReader;
+
 class NET_EXPORT_PRIVATE DnsConfigServicePosix
   : NON_EXPORTED_BASE(public DnsConfigService) {
  public:
-  class DnsConfigReader;
-
   DnsConfigServicePosix();
   virtual ~DnsConfigServicePosix();
 
   virtual void Watch() OVERRIDE;
 
  private:
-  scoped_refptr<WatchingFileReader> config_reader_;
-  scoped_refptr<WatchingFileReader> hosts_reader_;
+  class ConfigReader;
+  scoped_ptr<WatchingFileReader> config_watcher_;
+  scoped_ptr<WatchingFileReader> hosts_watcher_;
 
   DISALLOW_COPY_AND_ASSIGN(DnsConfigServicePosix);
 };
diff --git a/net/dns/dns_config_service_unittest.cc b/net/dns/dns_config_service_unittest.cc
index 0a86dca..9692f33 100644
--- a/net/dns/dns_config_service_unittest.cc
+++ b/net/dns/dns_config_service_unittest.cc
@@ -4,9 +4,10 @@
 
 #include "net/dns/dns_config_service.h"
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
-#include "base/task.h"
 #include "base/test/test_timeouts.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -81,18 +82,17 @@ TEST_F(DnsConfigServiceTest, NotifyOnChange) {
   EXPECT_TRUE(last_config_.Equals(complete_config));
 }
 
-#if defined(OS_POSIX)
-// TODO(szym): enable OS_WIN once ready
+#if defined(OS_POSIX) || defined(OS_WIN)
 // This is really an integration test.
 TEST_F(DnsConfigServiceTest, GetSystemConfig) {
   scoped_ptr<DnsConfigService> service(DnsConfigService::CreateSystemService());
 
   // Quit the loop after timeout unless cancelled
   const int64 kTimeout = TestTimeouts::action_timeout_ms();
-  ScopedRunnableMethodFactory<DnsConfigServiceTest> factory_(this);
+  base::WeakPtrFactory<DnsConfigServiceTest> factory_(this);
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      factory_.NewRunnableMethod(&DnsConfigServiceTest::Timeout),
+      base::Bind(&DnsConfigServiceTest::Timeout, factory_.GetWeakPtr()),
       kTimeout);
 
   service->AddObserver(this);
@@ -102,7 +102,7 @@ TEST_F(DnsConfigServiceTest, GetSystemConfig) {
   ASSERT_TRUE(last_config_.IsValid()) << "Did not receive DnsConfig in " <<
       kTimeout << "ms";
 }
-#endif  // OS_POSIX
+#endif  // OS_POSIX || OS_WIN
 
 }  // namespace net
 
diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc
new file mode 100644
index 0000000..cd58bb8
--- /dev/null
+++ b/net/dns/dns_config_service_win.cc
@@ -0,0 +1,293 @@
+// Copyright (c) 2011 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 <iphlpapi.h>
+#include <windows.h>
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/string_split.h"
+#include "base/string_util.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 "googleurl/src/url_canon.h"
+#include "net/base/net_util.h"
+#include "net/dns/serial_worker.h"
+#include "net/dns/watching_file_reader.h"
+
+namespace net {
+
+// Converts a string16 domain name to ASCII, possibly using punycode.
+// Returns true if the conversion succeeds.
+bool ParseDomainASCII(const string16& widestr, std::string* domain) {
+  DCHECK(domain);
+  // Check if already ASCII.
+  if (IsStringASCII(widestr)) {
+    *domain = UTF16ToASCII(widestr);
+    return true;
+  }
+
+  // Otherwise try to convert it from IDN to punycode.
+  const int kInitialBufferSize = 256;
+  url_canon::RawCanonOutputT<char16, kInitialBufferSize> punycode;
+  if (!url_canon::IDNToASCII(widestr.data(),
+                             widestr.length(),
+                             &punycode)) {
+    return false;
+  }
+
+  // |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;
+}
+
+bool ParseSearchList(const string16& value, std::vector<std::string>* out) {
+  DCHECK(out);
+  if (value.empty())
+    return false;
+
+  // If the list includes an empty hostname (",," or ", ,"), it is terminated.
+  // Although nslookup and network connection property tab ignore such
+  // fragments ("a,b,,c" becomes ["a", "b", "c"]), our reference is getaddrinfo
+  // (which sees ["a", "b"]). WMI queries also return a matching search list.
+  std::vector<string16> woutput;
+  base::SplitString(value, ',', &woutput);
+  std::vector<std::string> output;
+  for (size_t i = 0; i < woutput.size(); ++i) {
+    // Convert non-ascii to punycode, although getaddrinfo does not properly
+    // handle such suffixes.
+    const string16& t = woutput[i];
+    std::string parsed;
+    if (t.empty() || !ParseDomainASCII(t, &parsed))
+      break;
+    output.push_back(parsed);
+  }
+  if (output.empty())
+    return false;
+  out->swap(output);
+  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.
+class DnsConfigServiceWin::ConfigReader : public SerialWorker {
+ public:
+  // Watches a single registry key for changes.
+  class RegistryWatcher : public base::win::ObjectWatcher::Delegate {
+   public:
+    explicit RegistryWatcher(ConfigReader* reader) : reader_(reader) {}
+
+    bool StartWatch(const wchar_t* key) {
+      DCHECK(!key_.IsWatching());
+      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;
+      return true;
+    }
+
+    void Cancel() {
+      reader_ = NULL;
+      key_.StopWatching();
+      watcher_.StopWatching();
+    }
+
+    void OnObjectSignaled(HANDLE object) OVERRIDE {
+      if (reader_)
+        reader_->WorkNow();
+      // TODO(szym): handle errors
+      key_.StartWatching();
+      watcher_.StartWatching(key_.watch_event(), this);
+    }
+
+    bool Read(const wchar_t* name, string16* value) const {
+      return key_.ReadValue(name, value) == ERROR_SUCCESS;
+    }
+
+   private:
+    ConfigReader* reader_;
+    base::win::RegKey key_;
+    base::win::ObjectWatcher watcher_;
+  };
+
+  explicit ConfigReader(DnsConfigServiceWin* service)
+    : policy_watcher_(this),
+      tcpip_watcher_(this),
+      service_(service) {}
+
+  bool StartWatch() {
+    DCHECK(loop()->BelongsToCurrentThread());
+
+    // This is done only once per lifetime so open the keys on this thread.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+    if (!tcpip_watcher_.StartWatch(
+        L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters"))
+      return false;
+
+    // DNS suffix search list can be installed via group policy which sets
+    // this registry key. If the key is missing, the policy does not apply,
+    // and the DNS client uses DHCP and manual settings.
+    // If a policy is installed, DnsConfigService will need to be restarted.
+    // BUG=99509
+    policy_watcher_.StartWatch(
+        L"Software\\Policies\\Microsoft\\Windows NT\\DNSClient");
+    WorkNow();
+    return true;
+  }
+
+  void Cancel() {
+    DCHECK(loop()->BelongsToCurrentThread());
+    SerialWorker::Cancel();
+    policy_watcher_.Cancel();
+    tcpip_watcher_.Cancel();
+  }
+
+  // Delay between calls to WorkerPool::PostTask
+  static const int kWorkerPoolRetryDelayMs = 100;
+
+ private:
+  friend class RegistryWatcher;
+  virtual ~ConfigReader() {}
+
+  // Uses GetAdapterAddresses to get effective DNS server order and default
+  // DNS suffix.
+  bool ReadIpHelper(DnsConfig* config) {
+    base::ThreadRestrictions::AssertIOAllowed();
+
+    ULONG flags = GAA_FLAG_SKIP_ANYCAST | GAA_FLAG_SKIP_UNICAST |
+                  GAA_FLAG_SKIP_MULTICAST | GAA_FLAG_SKIP_FRIENDLY_NAME;
+    IP_ADAPTER_ADDRESSES buf;
+    IP_ADAPTER_ADDRESSES* adapters = &buf;
+    ULONG len = sizeof(buf);
+    scoped_array<char> extra_buf;
+
+    UINT rv = GetAdaptersAddresses(AF_UNSPEC, flags, NULL, adapters, &len);
+    if (rv == ERROR_BUFFER_OVERFLOW) {
+      extra_buf.reset(new char[len]);
+      adapters = reinterpret_cast<IP_ADAPTER_ADDRESSES*>(extra_buf.get());
+      rv = GetAdaptersAddresses(AF_UNSPEC, flags, NULL, adapters, &len);
+    }
+    if (rv != NO_ERROR)
+      return false;
+
+    config->search.clear();
+    config->nameservers.clear();
+
+    IP_ADAPTER_ADDRESSES* adapter;
+    for (adapter = adapters; adapter != NULL; adapter = adapter->Next) {
+      if (adapter->OperStatus != IfOperStatusUp)
+        continue;
+      if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK)
+        continue;
+
+      // IP_ADAPTER_ADDRESSES in Vista+ has a search list at FirstDnsSuffix,
+      // but it's only the per-interface search list. First initialize with
+      // DnsSuffix, then override from registry.
+      std::string dns_suffix;
+      if (ParseDomainASCII(adapter->DnsSuffix, &dns_suffix)) {
+        config->search.push_back(dns_suffix);
+      }
+      IP_ADAPTER_DNS_SERVER_ADDRESS* address;
+      for (address = adapter->FirstDnsServerAddress; address != NULL;
+           address = address->Next) {
+        IPEndPoint ipe;
+        if (ipe.FromSockAddr(address->Address.lpSockaddr,
+                             address->Address.iSockaddrLength)) {
+          config->nameservers.push_back(ipe);
+        } else {
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
+  // Read and parse SearchList from registry. Only accept the value if all
+  // elements of the search list are non-empty and can be converted to UTF-8.
+  bool ReadSearchList(const RegistryWatcher& watcher, DnsConfig* config) {
+    string16 value;
+    if (!watcher.Read(L"SearchList", &value))
+      return false;
+    return ParseSearchList(value, &(config->search));
+  }
+
+  void DoWork() OVERRIDE {
+    // Should be called on WorkerPool.
+    dns_config_ = DnsConfig();
+    if (!ReadIpHelper(&dns_config_))
+      return;  // no point reading the rest
+
+    // check global registry overrides
+    if (!ReadSearchList(policy_watcher_, &dns_config_))
+      ReadSearchList(tcpip_watcher_, &dns_config_);
+
+    // TODO(szym): add support for DNS suffix devolution BUG=99510
+  }
+
+  void OnWorkFinished() OVERRIDE {
+    DCHECK(loop()->BelongsToCurrentThread());
+    DCHECK(!IsCancelled());
+    if (dns_config_.IsValid()) {
+      service_->OnConfigRead(dns_config_);
+    } else {
+      LOG(WARNING) << "Failed to read name servers from registry";
+    }
+  }
+
+  // Written in DoRead(), read in OnReadFinished(). No locking required.
+  DnsConfig dns_config_;
+
+  RegistryWatcher policy_watcher_;
+  RegistryWatcher tcpip_watcher_;
+
+  DnsConfigServiceWin* service_;
+};
+
+DnsConfigServiceWin::DnsConfigServiceWin()
+  : config_reader_(new ConfigReader(this)),
+    hosts_watcher_(new WatchingFileReader()) {}
+
+DnsConfigServiceWin::~DnsConfigServiceWin() {
+  DCHECK(CalledOnValidThread());
+  config_reader_->Cancel();
+}
+
+void DnsConfigServiceWin::Watch() {
+  DCHECK(CalledOnValidThread());
+  bool started = config_reader_->StartWatch();
+  // TODO(szym): handle possible failure
+  DCHECK(started);
+
+  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_watcher_->StartWatch(hosts_path, new DnsHostsReader(hosts_path, this));
+}
+
+// static
+DnsConfigService* DnsConfigService::CreateSystemService() {
+  return new DnsConfigServiceWin();
+}
+
+}  // namespace net
+
diff --git a/net/dns/dns_config_service_win.h b/net/dns/dns_config_service_win.h
new file mode 100644
index 0000000..97dcf6c
--- /dev/null
+++ b/net/dns/dns_config_service_win.h
@@ -0,0 +1,48 @@
+// Copyright (c) 2011 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.
+
+#ifndef NET_DNS_DNS_CONFIG_SERVICE_WIN_H_
+#define NET_DNS_DNS_CONFIG_SERVICE_WIN_H_
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/string16.h"
+#include "net/base/net_export.h"
+#include "net/dns/dns_config_service.h"
+
+namespace net {
+
+class WatchingFileReader;
+
+class NET_EXPORT_PRIVATE DnsConfigServiceWin
+  : NON_EXPORTED_BASE(public DnsConfigService) {
+ public:
+  DnsConfigServiceWin();
+  virtual ~DnsConfigServiceWin();
+
+  virtual void Watch() OVERRIDE;
+
+ private:
+  class ConfigReader;
+  scoped_refptr<ConfigReader> config_reader_;
+  scoped_ptr<WatchingFileReader> hosts_watcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsConfigServiceWin);
+};
+
+// Parses |value| as search list (comma-delimited list of domain names) from
+// a registry key and stores it in |out|. Returns true on success.
+// Empty entries (e.g., "chromium.org,,org") terminate the list.
+// Non-ascii hostnames are converted to punycode.
+bool NET_EXPORT_PRIVATE ParseSearchList(const string16& value,
+                                        std::vector<std::string>* out);
+
+}  // namespace net
+
+#endif  // NET_DNS_DNS_CONFIG_SERVICE_WIN_H_
+
diff --git a/net/dns/dns_config_service_win_unittest.cc b/net/dns/dns_config_service_win_unittest.cc
new file mode 100644
index 0000000..6397c32
--- /dev/null
+++ b/net/dns/dns_config_service_win_unittest.cc
@@ -0,0 +1,54 @@
+// Copyright (c) 2011 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 "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+TEST(DnsConfigServiceWinTest, ParseDomain) {
+  const struct TestCase {
+    const wchar_t* input;
+    const char* output[4];  // NULL-terminated, empty if expected false
+  } cases[] = {
+    { L"chromium.org", { "chromium.org", NULL } },
+    { L"chromium.org,org", { "chromium.org", "org", NULL } },
+    // Empty suffixes terminate the list
+    { L"crbug.com,com,,org", { "crbug.com", "com", NULL } },
+    // IDN are converted to punycode
+    { L"\u017c\xf3\u0142ta.pi\u0119\u015b\u0107.pl,pl",
+        { "xn--ta-4ja03asj.xn--pi-wla5e0q.pl", "pl", NULL } },
+    // Empty search list is invalid
+    { L"", { NULL } },
+    { L",,", { NULL } },
+    { NULL, { NULL } },
+  };
+
+  std::vector<std::string> actual_output, expected_output;
+  for (const TestCase* t = cases; t->input; ++t) {
+    actual_output.clear();
+    actual_output.push_back("UNSET");
+    expected_output.clear();
+    for (const char* const* output = t->output; *output; ++output) {
+      expected_output.push_back(*output);
+    }
+    bool result = ParseSearchList(t->input, &actual_output);
+    if (!expected_output.empty()) {
+      EXPECT_TRUE(result);
+      EXPECT_EQ(expected_output, actual_output);
+    } else {
+      EXPECT_FALSE(result) << "Unexpected parse success on " << t->input;
+      expected_output.push_back("UNSET");
+      EXPECT_EQ(expected_output, actual_output);
+    }
+  }
+}
+
+}  // namespace
+
+}  // namespace net
+
diff --git a/net/dns/serial_worker.cc b/net/dns/serial_worker.cc
new file mode 100644
index 0000000..d2e42c0
--- /dev/null
+++ b/net/dns/serial_worker.cc
@@ -0,0 +1,99 @@
+// Copyright (c) 2011 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/serial_worker.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/message_loop_proxy.h"
+#include "base/threading/worker_pool.h"
+
+namespace net {
+
+SerialWorker::SerialWorker()
+  : message_loop_(base::MessageLoopProxy::current()),
+    state_(IDLE) {}
+
+SerialWorker::~SerialWorker() {}
+
+void SerialWorker::WorkNow() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  switch (state_) {
+    case IDLE:
+      if (!base::WorkerPool::PostTask(FROM_HERE, base::Bind(
+          &SerialWorker::DoWorkJob, this), false)) {
+#if defined(OS_POSIX)
+        // See worker_pool_posix.cc.
+        NOTREACHED() << "WorkerPool::PostTask is not expected to fail on posix";
+#else
+        LOG(WARNING) << "Failed to WorkerPool::PostTask, will retry later";
+        message_loop_->PostDelayedTask(
+            FROM_HERE,
+            base::Bind(&SerialWorker::RetryWork, this),
+            kWorkerPoolRetryDelayMs);
+        state_ = WAITING;
+        return;
+#endif
+      }
+      state_ = WORKING;
+      return;
+    case WORKING:
+      // Remember to re-read after |DoRead| finishes.
+      state_ = PENDING;
+      return;
+    case CANCELLED:
+    case PENDING:
+    case WAITING:
+      return;
+    default:
+      NOTREACHED() << "Unexpected state " << state_;
+  }
+}
+
+void SerialWorker::Cancel() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  state_ = CANCELLED;
+}
+
+void SerialWorker::DoWorkJob() {
+  this->DoWork();
+  // If this fails, the loop is gone, so there is no point retrying.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &SerialWorker::OnWorkJobFinished, this));
+}
+
+void SerialWorker::OnWorkJobFinished() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  switch (state_) {
+    case CANCELLED:
+      return;
+    case WORKING:
+      state_ = IDLE;
+      this->OnWorkFinished();
+      return;
+    case PENDING:
+      state_ = IDLE;
+      WorkNow();
+      return;
+    default:
+      NOTREACHED() << "Unexpected state " << state_;
+  }
+}
+
+void SerialWorker::RetryWork() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  switch (state_) {
+    case CANCELLED:
+      return;
+    case WAITING:
+      state_ = IDLE;
+      WorkNow();
+      return;
+    default:
+      NOTREACHED() << "Unexpected state " << state_;
+  }
+}
+
+}  // namespace net
+
diff --git a/net/dns/serial_worker.h b/net/dns/serial_worker.h
new file mode 100644
index 0000000..4d6f8fc
--- /dev/null
+++ b/net/dns/serial_worker.h
@@ -0,0 +1,102 @@
+// Copyright (c) 2011 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.
+
+#ifndef NET_DNS_SERIAL_WORKER_H_
+#define NET_DNS_SERIAL_WORKER_H_
+#pragma once
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "net/base/net_export.h"
+
+// Forward declaration
+namespace base {
+class MessageLoopProxy;
+}
+
+namespace net {
+
+// SerialWorker executes a job on WorkerPool serially -- **once at a time**.
+// On |WorkNow|, a call to |DoWork| is scheduled on the WorkerPool. Once it
+// completes, |OnWorkFinished| is called on the origin thread.
+// If |WorkNow| is called (1 or more times) while |DoWork| is already under way,
+// |DoWork| will be called once: after current |DoWork| completes, before a
+// call to |OnWorkFinished|.
+//
+// This behavior is designed for updating a result after some trigger, for
+// example reading a file once FilePathWatcher indicates it changed.
+//
+// Derived classes should store results of work done in |DoWork| in dedicated
+// fields and read them in |OnWorkFinished| which is executed on the origin
+// thread. This avoids the need to template this class.
+//
+// This implementation avoids locking by using the |state_| member to ensure
+// that |DoWork| and |OnWorkFinished| cannot execute in parallel.
+//
+// TODO(szym): update to WorkerPool::PostTaskAndReply once available.
+class NET_EXPORT_PRIVATE SerialWorker
+  : NON_EXPORTED_BASE(public base::RefCountedThreadSafe<SerialWorker>) {
+ public:
+  SerialWorker();
+
+  // Unless already scheduled, post |DoWork| to WorkerPool.
+  // Made virtual to allow mocking.
+  virtual void WorkNow();
+
+  // Stop scheduling read jobs.
+  void Cancel();
+
+  bool IsCancelled() const { return state_ == CANCELLED; }
+
+  // Delay between calls to WorkerPool::PostTask
+  static const int kWorkerPoolRetryDelayMs = 100;
+
+ protected:
+  friend class base::RefCountedThreadSafe<SerialWorker>;
+  // protected to allow sub-classing, but prevent deleting
+  virtual ~SerialWorker();
+
+  // Executed on WorkerPool, at most once at a time.
+  virtual void DoWork() = 0;
+
+  // Executed on origin thread after |DoRead| completes.
+  virtual void OnWorkFinished() = 0;
+
+  base::MessageLoopProxy* loop() {
+    return message_loop_;
+  }
+
+ private:
+  enum State {
+    CANCELLED = -1,
+    IDLE = 0,
+    WORKING,  // |DoWorkJob| posted on WorkerPool, until |OnWorkJobFinished|
+    PENDING,  // |WorkNow| while WORKING, must re-do work
+    WAITING,  // WorkerPool is busy, |RetryWork| is posted
+  };
+
+  // Called on the worker thread, executes |DoWork| and notifies the origin
+  // thread.
+  void DoWorkJob();
+
+  // Called on the the origin thread after |DoWork| completes.
+  void OnWorkJobFinished();
+
+  // Posted to message loop in case WorkerPool is busy. (state == WAITING)
+  void RetryWork();
+
+  // Message loop for the thread of origin.
+  scoped_refptr<base::MessageLoopProxy> message_loop_;
+
+  State state_;
+
+  DISALLOW_COPY_AND_ASSIGN(SerialWorker);
+};
+
+}  // namespace net
+
+#endif  // NET_DNS_SERIAL_WORKER_H_
+
diff --git a/net/dns/serial_worker_unittest.cc b/net/dns/serial_worker_unittest.cc
new file mode 100644
index 0000000..4062c2a
--- /dev/null
+++ b/net/dns/serial_worker_unittest.cc
@@ -0,0 +1,170 @@
+// Copyright (c) 2011 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/serial_worker.h"
+
+#include "base/message_loop.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+class SerialWorkerTest : public testing::Test {
+ public:
+  // The class under test
+  class TestSerialWorker : public SerialWorker {
+   public:
+    explicit TestSerialWorker(SerialWorkerTest* t)
+      : test_(t) {}
+    virtual void DoWork() OVERRIDE {
+      ASSERT_TRUE(test_);
+      test_->OnWork();
+    }
+    virtual void OnWorkFinished() OVERRIDE {
+      ASSERT_TRUE(test_);
+      test_->OnWorkFinished();
+    }
+   private:
+    virtual ~TestSerialWorker() {}
+    SerialWorkerTest* test_;
+  };
+
+  // Mocks
+
+  void OnWork() {
+    { // Check that OnWork is executed serially.
+      base::AutoLock lock(work_lock_);
+      EXPECT_FALSE(work_running_) << "DoRead is not called serially!";
+      work_running_ = true;
+    }
+    BreakNow("OnWork");
+    work_allowed_.Wait();
+    // Calling from WorkerPool, but protected by work_allowed_/work_called_.
+    output_value_ = input_value_;
+
+    { // This lock might be destroyed after work_called_ is signalled.
+      base::AutoLock lock(work_lock_);
+      work_running_ = false;
+    }
+    work_called_.Signal();
+  }
+
+  void OnWorkFinished() {
+    EXPECT_TRUE(message_loop_ == MessageLoop::current());
+    EXPECT_EQ(output_value_, input_value_);
+    BreakNow("OnWorkFinished");
+  }
+
+ protected:
+  friend class BreakTask;
+  class BreakTask : public Task {
+   public:
+    BreakTask(SerialWorkerTest* test, std::string breakpoint)
+      : test_(test), breakpoint_(breakpoint) {}
+    virtual ~BreakTask() {}
+    virtual void Run() OVERRIDE {
+      test_->breakpoint_ = breakpoint_;
+      MessageLoop::current()->QuitNow();
+    }
+   private:
+    SerialWorkerTest* test_;
+    std::string breakpoint_;
+  };
+
+  void BreakNow(std::string b) {
+    message_loop_->PostTask(FROM_HERE, new BreakTask(this, b));
+  }
+
+  void RunUntilBreak(std::string b) {
+    message_loop_->Run();
+    ASSERT_EQ(breakpoint_, b);
+  }
+
+  SerialWorkerTest()
+      : input_value_(0),
+        output_value_(-1),
+        work_allowed_(false, false),
+        work_called_(false, false),
+        work_running_(false) {
+  }
+
+  // Helpers for tests.
+
+  // Lets OnWork run and waits for it to complete. Can only return if OnWork is
+  // executed on a concurrent thread.
+  void WaitForWork() {
+    RunUntilBreak("OnWork");
+    work_allowed_.Signal();
+    work_called_.Wait();
+  }
+
+  // test::Test methods
+  virtual void SetUp() OVERRIDE {
+    message_loop_ = MessageLoop::current();
+    worker_ = new TestSerialWorker(this);
+  }
+
+  virtual void TearDown() OVERRIDE {
+    // Cancel the worker to catch if it makes a late DoWork call.
+    worker_->Cancel();
+    // Check if OnWork is stalled.
+    EXPECT_FALSE(work_running_) << "OnWork should be done by TearDown";
+    // Release it for cleanliness.
+    if (work_running_) {
+      WaitForWork();
+    }
+  }
+
+  // Input value read on WorkerPool.
+  int input_value_;
+  // Output value written on WorkerPool.
+  int output_value_;
+
+  // read is called on WorkerPool so we need to synchronize with it.
+  base::WaitableEvent work_allowed_;
+  base::WaitableEvent work_called_;
+
+  // Protected by read_lock_. Used to verify that read calls are serialized.
+  bool work_running_;
+  base::Lock work_lock_;
+
+  // Loop for this thread.
+  MessageLoop* message_loop_;
+
+  // WatcherDelegate under test.
+  scoped_refptr<TestSerialWorker> worker_;
+
+  std::string breakpoint_;
+};
+
+TEST_F(SerialWorkerTest, ExecuteAndSerializeReads) {
+  for (int i = 0; i < 3; ++i) {
+    ++input_value_;
+    worker_->WorkNow();
+    WaitForWork();
+    RunUntilBreak("OnWorkFinished");
+
+    message_loop_->AssertIdle();
+  }
+
+  // Schedule two calls. OnWork checks if it is called serially.
+  ++input_value_;
+  worker_->WorkNow();
+  // read is blocked, so this will have to induce re-work
+  worker_->WorkNow();
+  WaitForWork();
+  WaitForWork();
+  RunUntilBreak("OnWorkFinished");
+
+  // No more tasks should remain.
+  message_loop_->AssertIdle();
+}
+
+}  // namespace
+
+}  // namespace net
+
diff --git a/net/dns/watching_file_reader.cc b/net/dns/watching_file_reader.cc
index 2a550bd..869da37 100644
--- a/net/dns/watching_file_reader.cc
+++ b/net/dns/watching_file_reader.cc
@@ -6,7 +6,7 @@
 
 #include "base/bind.h"
 #include "base/location.h"
-#include "base/message_loop_proxy.h"
+#include "base/message_loop.h"
 #include "base/threading/worker_pool.h"
 
 namespace net {
@@ -28,58 +28,69 @@ FilePathWatcherFactory::CreateFilePathWatcher() {
   return new FilePathWatcherShim();
 }
 
+// A FilePathWatcher::Delegate that forwards calls to the WatchingFileReader.
+// This is separated out to keep WatchingFileReader strictly single-threaded.
+class WatchingFileReader::WatcherDelegate
+  : public base::files::FilePathWatcher::Delegate {
+ public:
+  explicit WatcherDelegate(base::WeakPtr<WatchingFileReader> reader)
+    : reader_(reader) {}
+  void OnFilePathChanged(const FilePath& path) OVERRIDE {
+    if (reader_) reader_->OnFilePathChanged(path);
+  }
+  void OnFilePathError(const FilePath& path) OVERRIDE {
+    if (reader_) reader_->OnFilePathError(path);
+  }
+ private:
+  virtual ~WatcherDelegate() {}
+  base::WeakPtr<WatchingFileReader> reader_;
+};
+
 WatchingFileReader::WatchingFileReader()
-  : message_loop_(base::MessageLoopProxy::current()),
-    factory_(new FilePathWatcherFactory()),
-    reading_(false),
-    read_pending_(false),
-    cancelled_(false) {}
+  : watcher_factory_(new FilePathWatcherFactory()) {}
 
-void WatchingFileReader::StartWatch(const FilePath& path) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+WatchingFileReader::~WatchingFileReader() {
+  if (reader_)
+    reader_->Cancel();
+}
+
+void WatchingFileReader::StartWatch(const FilePath& path,
+                                    SerialWorker* reader) {
+  DCHECK(CalledOnValidThread());
   DCHECK(!watcher_.get());
+  DCHECK(!watcher_delegate_.get());
   DCHECK(path_.empty());
   path_ = path;
+  watcher_delegate_ = new WatcherDelegate(AsWeakPtr());
+  reader_ = reader;
   RestartWatch();
 }
 
-void WatchingFileReader::Cancel() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  cancelled_ = true;
-  // Let go of the watcher to break the reference cycle.
-  watcher_.reset();
-  // Destroy the non-thread-safe factory now, since dtor is non-thread-safe.
-  factory_.reset();
-}
-
 void WatchingFileReader::OnFilePathChanged(const FilePath& path) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  ReadNow();
+  DCHECK(CalledOnValidThread());
+  reader_->WorkNow();
 }
 
 void WatchingFileReader::OnFilePathError(const FilePath& path) {
-  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(CalledOnValidThread());
   RestartWatch();
 }
 
-WatchingFileReader::~WatchingFileReader() {
-  DCHECK(cancelled_);
-}
-
 void WatchingFileReader::RescheduleWatch() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  message_loop_->PostDelayedTask(
+  DCHECK(CalledOnValidThread());
+  MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&WatchingFileReader::RestartWatch, this),
+      base::Bind(&WatchingFileReader::RestartWatch, AsWeakPtr()),
       kWatchRetryDelayMs);
 }
 
 void WatchingFileReader::RestartWatch() {
-  if (cancelled_)
+  DCHECK(CalledOnValidThread());
+  if (reader_->IsCancelled())
     return;
-  watcher_.reset(factory_->CreateFilePathWatcher());
-  if (watcher_->Watch(path_, this)) {
-    ReadNow();
+  watcher_.reset(watcher_factory_->CreateFilePathWatcher());
+  if (watcher_->Watch(path_, watcher_delegate_)) {
+    reader_->WorkNow();
   } else {
     LOG(WARNING) << "Watch on " <<
                     path_.LossyDisplayName() <<
@@ -88,50 +99,4 @@ void WatchingFileReader::RestartWatch() {
   }
 }
 
-void WatchingFileReader::ReadNow() {
-  if (cancelled_)
-    return;
-  if (reading_) {
-    // Remember to re-read after DoRead posts results.
-    read_pending_ = true;
-  } else {
-    if (!base::WorkerPool::PostTask(FROM_HERE, base::Bind(
-        &WatchingFileReader::DoReadJob, this), false)) {
-#if defined(OS_POSIX)
-      // See worker_pool_posix.cc.
-      NOTREACHED() << "WorkerPool::PostTask is not expected to fail on posix";
-#else
-      LOG(WARNING) << "Failed to WorkerPool::PostTask, will retry later";
-      message_loop_->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&WatchingFileReader::ReadNow, this),
-          kWorkerPoolRetryDelayMs);
-      return;
-#endif
-    }
-    reading_ = true;
-    read_pending_ = false;
-  }
-}
-
-void WatchingFileReader::DoReadJob() {
-  this->DoRead();
-  // If this fails, the loop is gone, so there is no point retrying.
-  message_loop_->PostTask(FROM_HERE, base::Bind(
-      &WatchingFileReader::OnReadJobFinished, this));
-}
-
-void WatchingFileReader::OnReadJobFinished() {
-  DCHECK(message_loop_->BelongsToCurrentThread());
-  if (cancelled_)
-    return;
-  reading_ = false;
-  if (read_pending_) {
-    // Discard this result and re-read.
-    ReadNow();
-    return;
-  }
-  this->OnReadFinished();
-}
-
 }  // namespace net
diff --git a/net/dns/watching_file_reader.h b/net/dns/watching_file_reader.h
index 15237ea..e93004e 100644
--- a/net/dns/watching_file_reader.h
+++ b/net/dns/watching_file_reader.h
@@ -11,8 +11,10 @@
 #include "base/compiler_specific.h"
 #include "base/files/file_path_watcher.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "net/base/net_export.h"
+#include "net/dns/serial_worker.h"
 
 // Forward declaration
 namespace base {
@@ -27,7 +29,6 @@ class NET_EXPORT_PRIVATE FilePathWatcherShim
  public:
   FilePathWatcherShim();
   virtual ~FilePathWatcherShim();
-
   virtual bool Watch(
       const FilePath& path,
       base::files::FilePathWatcher::Delegate* delegate) WARN_UNUSED_RESULT;
@@ -46,37 +47,22 @@ class NET_EXPORT_PRIVATE FilePathWatcherFactory
   DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFactory);
 };
 
-// A FilePathWatcher::Delegate that restarts Watch on failures and executes
-// DoRead on WorkerPool (one at a time) in response to FilePathChanged.
-// Derived classes should store results of the work done in DoRead in dedicated
-// fields and read them in OnReadFinished.
-//
-// This implementation avoids locking by using the reading_ flag to ensure that
-// DoRead and OnReadFinished cannot execute in parallel.
-//
-// There's no need to call Cancel() on destruction. On the contrary, a ref-cycle
-// between the WatchingFileReader and a FilePathWatcher will prevent destruction
-// until Cancel() is called.
-//
-// TODO(szym): update to WorkerPool::PostTaskAndReply once available.
+// Convenience wrapper which maintains a FilePathWatcher::Delegate, restarts
+// Watch on failures and calls PooledReader::ReadNow when OnFilePathChanged.
+// Cancels the PooledReader upon destruction.
 class NET_EXPORT_PRIVATE WatchingFileReader
-  : NON_EXPORTED_BASE(public base::files::FilePathWatcher::Delegate) {
+  : NON_EXPORTED_BASE(public base::NonThreadSafe),
+    NON_EXPORTED_BASE(public base::SupportsWeakPtr<WatchingFileReader>) {
  public:
   WatchingFileReader();
+  virtual ~WatchingFileReader();
 
   // Must be called at most once. Made virtual to allow mocking.
-  virtual void StartWatch(const FilePath& path);
-
-  // Unless already scheduled, post DoRead to WorkerPool.
-  void ReadNow();
-
-  // Do not perform any further work, except perhaps a latent DoRead().
-  // Always call it once done with the delegate to ensure destruction.
-  void Cancel();
+  virtual void StartWatch(const FilePath& path, SerialWorker* reader);
 
   void set_watcher_factory(FilePathWatcherFactory* factory) {
     DCHECK(!watcher_.get());
-    factory_.reset(factory);
+    watcher_factory_.reset(factory);
   }
 
   FilePath get_path() const {
@@ -86,43 +72,20 @@ class NET_EXPORT_PRIVATE WatchingFileReader
   // Delay between calls to FilePathWatcher::Watch.
   static const int kWatchRetryDelayMs = 100;
 
-  // Delay between calls to WorkerPool::PostTask
-  static const int kWorkerPoolRetryDelayMs = 100;
-
- protected:
-  // protected to allow sub-classing, but prevent deleting
-  virtual ~WatchingFileReader();
-
-  // Executed on WorkerPool, at most once at a time.
-  virtual void DoRead() = 0;
-  // Executed on origin thread after DoRead() completes.
-  virtual void OnReadFinished() = 0;
-
  private:
-  // FilePathWatcher::Delegate interface
-  virtual void OnFilePathChanged(const FilePath& path) OVERRIDE;
-  virtual void OnFilePathError(const FilePath& path) OVERRIDE;
+  class WatcherDelegate;
+  // FilePathWatcher::Delegate interface exported via WatcherDelegate
+  virtual void OnFilePathChanged(const FilePath& path);
+  virtual void OnFilePathError(const FilePath& path);
 
   void RescheduleWatch();
   void RestartWatch();
 
-  // Called on the worker thread, executes DoRead and notifies the origin
-  // thread.
-  void DoReadJob();
-
-  // Called on the the origin thread after DoRead completes.
-  void OnReadJobFinished();
-
-  // Message loop for the thread on which watcher is used (of TYPE_IO).
-  scoped_refptr<base::MessageLoopProxy> message_loop_;
   FilePath path_;
-  scoped_ptr<FilePathWatcherFactory> factory_;
+  scoped_ptr<FilePathWatcherFactory> watcher_factory_;
   scoped_ptr<FilePathWatcherShim> watcher_;
-  // True after DoRead before OnResultsAvailable.
-  bool reading_;
-  // True after OnFilePathChanged fires while |reading_| is true.
-  bool read_pending_;
-  bool cancelled_;
+  scoped_refptr<WatcherDelegate> watcher_delegate_;
+  scoped_refptr<SerialWorker> reader_;
 
   DISALLOW_COPY_AND_ASSIGN(WatchingFileReader);
 };
diff --git a/net/dns/watching_file_reader_unittest.cc b/net/dns/watching_file_reader_unittest.cc
index 24021f0..510398c 100644
--- a/net/dns/watching_file_reader_unittest.cc
+++ b/net/dns/watching_file_reader_unittest.cc
@@ -16,34 +16,21 @@ namespace {
 class WatchingFileReaderTest : public testing::Test {
  public:
 
-  // The class under test
-  class TestWatchingFileReader : public WatchingFileReader {
+  // Mocks
+
+  class MockWorker : public SerialWorker {
    public:
-    explicit TestWatchingFileReader(WatchingFileReaderTest* t)
-      : test_(t) {}
-    virtual void DoRead() OVERRIDE {
-      ASSERT_TRUE(test_);
-      test_->OnRead();
-    }
-    virtual void OnReadFinished() OVERRIDE {
-      ASSERT_TRUE(test_);
-      test_->OnReadFinished();
-    }
-    void TestCancel() {
-      // Could execute concurrently with DoRead()
-      test_ = NULL;
+    explicit MockWorker(WatchingFileReaderTest* t) : test_(t) {}
+    virtual void WorkNow() OVERRIDE {
+      test_->OnWork();
     }
+    virtual void DoWork() OVERRIDE {}
+    virtual void OnWorkFinished() OVERRIDE {}
    private:
-    virtual ~TestWatchingFileReader() {}
+    virtual ~MockWorker() {}
     WatchingFileReaderTest* test_;
-    base::Lock lock_;
   };
 
-  // Mocks
-
-  // WatcherDelegate owns the FilePathWatcherFactory it gets, so use simple
-  // proxies to call the test fixture.
-
   class MockFilePathWatcherShim
     : public FilePathWatcherShim {
    public:
@@ -112,28 +99,9 @@ class WatchingFileReaderTest : public testing::Test {
     return !fail_on_watch_;
   }
 
-  void OnRead() {
-    { // Check that OnRead is executed serially.
-      base::AutoLock lock(read_lock_);
-      EXPECT_FALSE(read_running_) << "DoRead is not called serially!";
-      read_running_ = true;
-    }
-    BreakNow("OnRead");
-    read_allowed_.Wait();
-    // Calling from WorkerPool, but protected by read_allowed_/read_called_.
-    output_value_ = input_value_;
-
-    { // This lock might be destroyed after read_called_ is signalled.
-      base::AutoLock lock(read_lock_);
-      read_running_ = false;
-    }
-    read_called_.Signal();
-  }
-
-  void OnReadFinished() {
+  void OnWork() {
     EXPECT_TRUE(message_loop_ == MessageLoop::current());
-    EXPECT_EQ(output_value_, input_value_);
-    BreakNow("OnReadFinished");
+    BreakNow("OnWork");
   }
 
  protected:
@@ -164,122 +132,40 @@ class WatchingFileReaderTest : public testing::Test {
   WatchingFileReaderTest()
       : watcher_shim_(NULL),
         fail_on_watch_(false),
-        input_value_(0),
-        output_value_(-1),
-        read_allowed_(false, false),
-        read_called_(false, false),
-        read_running_(false) {
-  }
-
-  // Helpers for tests.
-
-  // Lets OnRead run and waits for it to complete. Can only return if OnRead is
-  // executed on a concurrent thread.
-  void WaitForRead() {
-    RunUntilBreak("OnRead");
-    read_allowed_.Signal();
-    read_called_.Wait();
-  }
-
-  // test::Test methods
-  virtual void SetUp() OVERRIDE {
-    message_loop_ = MessageLoop::current();
-    watcher_ = new TestWatchingFileReader(this);
+        message_loop_(MessageLoop::current()),
+        watcher_(new WatchingFileReader()) {
     watcher_->set_watcher_factory(new MockFilePathWatcherFactory(this));
   }
 
-  virtual void TearDown() OVERRIDE {
-    // Cancel the watcher to catch if it makes a late DoRead call.
-    watcher_->TestCancel();
-    watcher_->Cancel();
-    // Check if OnRead is stalled.
-    EXPECT_FALSE(read_running_) << "OnRead should be done by TearDown";
-    // Release it for cleanliness.
-    if (read_running_) {
-      WaitForRead();
-    }
-  }
-
   MockFilePathWatcherShim* watcher_shim_;
-
   bool fail_on_watch_;
-
-  // Input value read on WorkerPool.
-  int input_value_;
-  // Output value written on WorkerPool.
-  int output_value_;
-
-  // read is called on WorkerPool so we need to synchronize with it.
-  base::WaitableEvent read_allowed_;
-  base::WaitableEvent read_called_;
-
-  // Protected by read_lock_. Used to verify that read calls are serialized.
-  bool read_running_;
-  base::Lock read_lock_;
-
-  // Loop for this thread.
   MessageLoop* message_loop_;
-
-  // WatcherDelegate under test.
-  scoped_refptr<TestWatchingFileReader> watcher_;
+  scoped_ptr<WatchingFileReader> watcher_;
 
   std::string breakpoint_;
 };
 
 TEST_F(WatchingFileReaderTest, FilePathWatcherFailures) {
   fail_on_watch_ = true;
-  watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name")));
+  watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name")),
+                       new MockWorker(this));
   RunUntilBreak("OnWatch");
 
   fail_on_watch_ = false;
   RunUntilBreak("OnWatch");  // Due to backoff this will take 100ms.
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
+  RunUntilBreak("OnWork");
 
   ASSERT_TRUE(watcher_shim_);
   watcher_shim_->PathError();
   RunUntilBreak("OnWatch");
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
-
-  message_loop_->AssertIdle();
-}
-
-TEST_F(WatchingFileReaderTest, ExecuteAndSerializeReads) {
-  watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name")));
-  RunUntilBreak("OnWatch");
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
-
-  message_loop_->AssertIdle();
-
-  ++input_value_;
-  ASSERT_TRUE(watcher_shim_);
-  watcher_shim_->PathChanged();
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
-
-  message_loop_->AssertIdle();
-
-  ++input_value_;
-  ASSERT_TRUE(watcher_shim_);
-  watcher_shim_->PathChanged();
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
+  RunUntilBreak("OnWork");
 
   message_loop_->AssertIdle();
 
-  // Schedule two calls. OnRead checks if it is called serially.
-  ++input_value_;
   ASSERT_TRUE(watcher_shim_);
   watcher_shim_->PathChanged();
-  // read is blocked, so this will have to induce read_pending
-  watcher_shim_->PathChanged();
-  WaitForRead();
-  WaitForRead();
-  RunUntilBreak("OnReadFinished");
+  RunUntilBreak("OnWork");
 
-  // No more tasks should remain.
   message_loop_->AssertIdle();
 }
 
diff --git a/net/net.gyp b/net/net.gyp
index 34f1965..9021035 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -312,6 +312,8 @@
         'dns/dns_config_service.h',
         'dns/dns_config_service_posix.cc',
         'dns/dns_config_service_posix.h',
+        'dns/dns_config_service_win.cc',
+        'dns/dns_config_service_win.h',
         'dns/dns_hosts.cc',
         'dns/dns_hosts.h',
         'dns/dns_query.cc',
@@ -320,6 +322,8 @@
         'dns/dns_response.h',
         'dns/dns_transaction.cc',
         'dns/dns_transaction.h',
+        'dns/serial_worker.cc',
+        'dns/serial_worker.h',
         'dns/watching_file_reader.cc',
         'dns/watching_file_reader.h',
         'ftp/ftp_auth_cache.cc',
@@ -982,10 +986,12 @@
         'dns/async_host_resolver_unittest.cc',
         'dns/dns_config_service_posix_unittest.cc',
         'dns/dns_config_service_unittest.cc',
+        'dns/dns_config_service_win_unittest.cc',
         'dns/dns_hosts_unittest.cc',
         'dns/dns_query_unittest.cc',
         'dns/dns_response_unittest.cc',
         'dns/dns_transaction_unittest.cc',
+        'dns/serial_worker_unittest.cc',
         'dns/watching_file_reader_unittest.cc',
         'ftp/ftp_auth_cache_unittest.cc',
         'ftp/ftp_ctrl_response_buffer_unittest.cc',