[net/dns] Refactor DnsHostsReader to reduce unnecessary inheritance.

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


Review URL: https://chromiumcodereview.appspot.com/10536150

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

5 files changed, 76 insertions, 90 deletions

diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc
index a0571912..40c710a 100644
--- a/net/dns/dns_config_service_posix.cc
+++ b/net/dns/dns_config_service_posix.cc
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_util.h"
+#include "net/dns/dns_hosts.h"
 #include "net/dns/serial_worker.h"
 
 namespace net {
@@ -31,8 +32,7 @@ class ConfigReader : public SerialWorker {
  public:
   typedef base::Callback<void(const DnsConfig& config)> CallbackType;
   explicit ConfigReader(const CallbackType& callback)
-      : callback_(callback),
-        success_(false) {}
+      : callback_(callback), success_(false) {}
 
   void DoWork() OVERRIDE {
     success_ = false;
@@ -42,38 +42,72 @@ class ConfigReader : public SerialWorker {
     // Note: res_ninit in glibc always returns 0 and sets RES_INIT.
     // res_init behaves the same way.
     memset(&_res, 0, sizeof(_res));
-    if ((res_init() == 0) && (_res.options & RES_INIT)) {
+    if ((res_init() == 0) && (_res.options & RES_INIT))
       success_ = internal::ConvertResStateToDnsConfig(_res, &dns_config_);
-    }
 #else  // all other OS_POSIX
     struct __res_state res;
     memset(&res, 0, sizeof(res));
-    if ((res_ninit(&res) == 0) && (res.options & RES_INIT)) {
+    if ((res_ninit(&res) == 0) && (res.options & RES_INIT))
       success_ = internal::ConvertResStateToDnsConfig(res, &dns_config_);
-    }
+
     // Prefer res_ndestroy where available.
 #if defined(OS_MACOSX) || defined(OS_FREEBSD)
     res_ndestroy(&res);
 #else
     res_nclose(&res);
 #endif
-
 #endif
   }
 
   void OnWorkFinished() OVERRIDE {
     DCHECK(!IsCancelled());
-    if (success_)
+    if (success_) {
       callback_.Run(dns_config_);
+    } else {
+      LOG(WARNING) << "Failed to read DnsConfig.";
+    }
   }
 
  private:
   virtual ~ConfigReader() {}
 
-  CallbackType callback_;
+  const CallbackType callback_;
   // Written in DoWork, read in OnWorkFinished, no locking necessary.
   DnsConfig dns_config_;
   bool success_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConfigReader);
+};
+
+// A SerialWorker that reads the HOSTS file and runs Callback.
+class HostsReader : public SerialWorker {
+ public:
+  typedef base::Callback<void(const DnsHosts& hosts)> CallbackType;
+  explicit HostsReader(const CallbackType& callback)
+      : path_(kFilePathHosts), callback_(callback), success_(false) {}
+
+ private:
+  virtual ~HostsReader() {}
+
+  virtual void DoWork() OVERRIDE {
+    success_ = ParseHostsFile(path_, &hosts_);
+  }
+
+  virtual void OnWorkFinished() OVERRIDE {
+    if (success_) {
+      callback_.Run(hosts_);
+    } else {
+      LOG(WARNING) << "Failed to read DnsHosts.";
+    }
+  }
+
+  const FilePath path_;
+  const CallbackType callback_;
+  // Written in DoWork, read in OnWorkFinished, no locking necessary.
+  DnsHosts hosts_;
+  bool success_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostsReader);
 };
 
 }  // namespace
@@ -84,8 +118,7 @@ DnsConfigServicePosix::DnsConfigServicePosix() {
   config_reader_ = new ConfigReader(
       base::Bind(&DnsConfigServicePosix::OnConfigRead,
                  base::Unretained(this)));
-  hosts_reader_ = new DnsHostsReader(
-      FilePath(kFilePathHosts),
+  hosts_reader_ = new HostsReader(
       base::Bind(&DnsConfigServicePosix::OnHostsRead,
                  base::Unretained(this)));
 }
diff --git a/net/dns/dns_config_service_posix.h b/net/dns/dns_config_service_posix.h
index 61d7241..d66002a 100644
--- a/net/dns/dns_config_service_posix.h
+++ b/net/dns/dns_config_service_posix.h
@@ -16,6 +16,8 @@
 
 namespace net {
 
+class SerialWorker;
+
 // Use DnsConfigService::CreateSystemService to use it outside of tests.
 namespace internal {
 
diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc
index af609f0..0c2e2c1 100644
--- a/net/dns/dns_config_service_win.cc
+++ b/net/dns/dns_config_service_win.cc
@@ -24,6 +24,7 @@
 #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"
 
@@ -380,7 +381,7 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker {
     if (success_) {
       service_->OnConfigRead(dns_config_);
     } else {
-      LOG(WARNING) << "Failed to read config.";
+      LOG(WARNING) << "Failed to read DnsConfig.";
     }
   }
 
@@ -393,15 +394,15 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker {
 // 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 DnsHostsReader {
+class DnsConfigServiceWin::HostsReader : public SerialWorker {
  public:
   explicit HostsReader(DnsConfigServiceWin* service)
-      : DnsHostsReader(GetHostsPath()), service_(service) {
+      : path_(GetHostsPath()), service_(service) {
   }
 
  private:
   virtual void DoWork() OVERRIDE {
-    DnsHostsReader::DoWork();
+    success_ = ParseHostsFile(path_, &hosts_);
 
     if (!success_)
       return;
@@ -420,12 +421,10 @@ class DnsConfigServiceWin::HostsReader : public DnsHostsReader {
                                   kIPv6Localhost + arraysize(kIPv6Localhost));
 
     // This does not override any pre-existing entries from the HOSTS file.
-    dns_hosts_.insert(
-        std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV4),
-                       loopback_ipv4));
-    dns_hosts_.insert(
-        std::make_pair(DnsHostsKey("localhost", ADDRESS_FAMILY_IPV6),
-                       loopback_ipv6));
+    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;
@@ -438,9 +437,9 @@ class DnsConfigServiceWin::HostsReader : public DnsHostsReader {
     StringToLowerASCII(&localname);
 
     bool have_ipv4 =
-        dns_hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)) > 0;
+        hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)) > 0;
     bool have_ipv6 =
-        dns_hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)) > 0;
+        hosts_.count(DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)) > 0;
 
     if (have_ipv4 && have_ipv6) {
       success_ = true;
@@ -476,29 +475,30 @@ class DnsConfigServiceWin::HostsReader : public DnsHostsReader {
         }
         if (!have_ipv4 && (ipe.GetFamily() == AF_INET)) {
           have_ipv4 = true;
-          dns_hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)] =
-              ipe.address();
+          hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV4)] = ipe.address();
         } else if (!have_ipv6 && (ipe.GetFamily() == AF_INET6)) {
           have_ipv6 = true;
-          dns_hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)] =
-              ipe.address();
+          hosts_[DnsHostsKey(localname, ADDRESS_FAMILY_IPV6)] = ipe.address();
         }
       }
     }
-
     success_ = true;
   }
 
   virtual void OnWorkFinished() OVERRIDE {
     DCHECK(loop()->BelongsToCurrentThread());
     if (success_) {
-      service_->OnHostsRead(dns_hosts_);
+      service_->OnHostsRead(hosts_);
     } else {
-      LOG(WARNING) << "Failed to read hosts.";
+      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);
 };
diff --git a/net/dns/dns_hosts.cc b/net/dns/dns_hosts.cc
index bfd216c..668d202 100644
--- a/net/dns/dns_hosts.cc
+++ b/net/dns/dns_hosts.cc
@@ -47,19 +47,6 @@ void ParseHosts(const std::string& contents, DnsHosts* dns_hosts) {
   }
 }
 
-DnsHostsReader::DnsHostsReader(const FilePath& path,
-                               const CallbackType& callback)
-    : path_(path),
-      callback_(callback),
-      success_(false) {
-  DCHECK(!callback.is_null());
-}
-
-DnsHostsReader::DnsHostsReader(const FilePath& path)
-    : path_(path),
-      success_(false) {
-}
-
 // Reads the contents of the file at |path| into |str| if the total length is
 // less than |max_size|.
 static bool ReadFile(const FilePath& path, int64 max_size, std::string* str) {
@@ -69,31 +56,20 @@ static bool ReadFile(const FilePath& path, int64 max_size, std::string* str) {
   return file_util::ReadFileToString(path, str);
 }
 
-void DnsHostsReader::DoWork() {
-  success_ = false;
-  dns_hosts_.clear();
-
+bool ParseHostsFile(const FilePath& path, DnsHosts* dns_hosts) {
+  dns_hosts->clear();
   // Missing file indicates empty HOSTS.
-  if (!file_util::PathExists(path_)) {
-    success_ = true;
-    return;
-  }
+  if (!file_util::PathExists(path))
+    return true;
 
   std::string contents;
   const int64 kMaxHostsSize = 1 << 16;
-  if (ReadFile(path_, kMaxHostsSize, &contents)) {
-    success_ = true;
-    ParseHosts(contents, &dns_hosts_);
-  }
-}
+  if (!ReadFile(path, kMaxHostsSize, &contents))
+    return false;
 
-void DnsHostsReader::OnWorkFinished() {
-  DCHECK(!IsCancelled());
-  if (success_)
-    callback_.Run(dns_hosts_);
+  ParseHosts(contents, dns_hosts);
+  return true;
 }
 
-DnsHostsReader::~DnsHostsReader() {}
-
 }  // namespace net
 
diff --git a/net/dns/dns_hosts.h b/net/dns/dns_hosts.h
index 5349ab5..9304a62 100644
--- a/net/dns/dns_hosts.h
+++ b/net/dns/dns_hosts.h
@@ -11,12 +11,10 @@
 #include <utility>
 #include <vector>
 
-#include "base/callback.h"
 #include "base/file_path.h"
 #include "net/base/address_family.h"
 #include "net/base/net_export.h"
 #include "net/base/net_util.h"  // can't forward-declare IPAddressNumber
-#include "net/dns/serial_worker.h"
 
 namespace net {
 
@@ -38,33 +36,10 @@ typedef std::map<DnsHostsKey, IPAddressNumber> DnsHosts;
 void NET_EXPORT_PRIVATE ParseHosts(const std::string& contents,
                                    DnsHosts* dns_hosts);
 
-// A SerialWorker that reads a HOSTS file and runs Callback.
-// Call WorkNow() to indicate file needs to be re-read.
-// Call Cancel() to disable the callback.
-class NET_EXPORT_PRIVATE DnsHostsReader
-    : NON_EXPORTED_BASE(public SerialWorker) {
- public:
-  typedef base::Callback<void(const DnsHosts& hosts)> CallbackType;
+// As above but reads the file pointed to by |path|.
+bool NET_EXPORT_PRIVATE ParseHostsFile(const FilePath& path,
+                                       DnsHosts* dns_hosts);
 
-  DnsHostsReader(const FilePath& path, const CallbackType& callback);
-
-  const FilePath& path() const { return path_; }
-
- protected:
-  explicit DnsHostsReader(const FilePath& path);
-  virtual ~DnsHostsReader();
-
-  virtual void DoWork() OVERRIDE;
-  virtual void OnWorkFinished() OVERRIDE;
-
-  FilePath path_;
-  CallbackType callback_;
-  // Written in DoWork, read in OnWorkFinished, no locking necessary.
-  DnsHosts dns_hosts_;
-  bool success_;
-
-  DISALLOW_COPY_AND_ASSIGN(DnsHostsReader);
-};
 
 
 }  // namespace net