DnsConfigService and a posix implementation

net/dns/dns_config_service_posix.cc

Index: net/dns/dns_config_service_posix.cc
diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc
new file mode 100644
index 0000000000000000000000000000000000000000..29973d73cd141c37f7b31f3121ccb1d78174998e
--- /dev/null
+++ b/net/dns/dns_config_service_posix.cc
@@ -0,0 +1,141 @@
+// 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 <resolv.h>
+
+#include "base/compiler_specific.h"
+#include "base/file_path.h"
+#include "base/files/file_path_watcher.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
+#include "base/scoped_ptr.h"
+#include "base/task.h"
+#include "base/threading/worker_pool.h"
+#include "net/base/ip_endpoint.h"
+#include "net/base/net_util.h"
+#include "net/dns/dns_config_service.h"
+
+namespace net {
+
+using base::files::FilePathWatcher;
+
+class DnsConfigReaderPosix : public DnsConfigReader {
+ public:
+  bool Read(DnsConfig* dns_config) OVERRIDE {
+    // res_ninit is available on Linux (glibc 2+), Mac OS X, and Android.
+    // TODO(szym): res_ninit is not thread-safe on BSD?
+    struct __res_state res;
+    if (res_ninit(&res) != 0)

[cbentzel, 2011/08/01 19:52:26]

Assuming that it's a newly constructed DnsConfig object and not having to clear
is fine, given how it's used.

[szym, 2011/08/01 20:22:01]

I'm not sure this is a safe general assumption. It seems some user could want to
re-use a DnsConfig it owns. Alternatively, we put DnsConfig in namespace {}. Can
it still be unit-tested in that case?

+      return false;
+
+    dns_config->nameservers.clear();
+    for (int i = 0; i < res.nscount; ++i) {
+      IPEndPoint ipe;
+      if (ipe.FromSockAddr(
+          reinterpret_cast<struct sockaddr*>(&res.nsaddr_list[i]),
+          sizeof res.nsaddr_list[i]))
+        dns_config->nameservers.push_back(ipe);
+    }
+
+    dns_config->search.clear();
+    for (int i = 0; (i < MAXDNSRCH) && *res.dnsrch[i]; ++i) {
+      dns_config->search.push_back(std::string(res.dnsrch[i]));
+    }
+
+    dns_config->domain = res.defdname;  // use only if |search| is empty
+    dns_config->ndots = res.ndots;
+    dns_config->timeout = res.retrans;
+    dns_config->attempts = res.retry;
+    dns_config->rotate = res.options & RES_ROTATE;
+    dns_config->edns0 = res.options & RES_USE_EDNS0;
+    return true;
+  }
+};
+
+// NOTE: this makes it ref counted thread-safe
+class DnsConfigServicePosix : public DnsConfigService,

[cbentzel, 2011/08/01 19:52:26]

Is this compiling? Doesn't it need to be derived from RefCounted to handle the
NewRunnableMethod call?

[szym, 2011/08/01 20:22:01]

It is compiling. Is that an error? Is it safe to add
RefCountedThreadSafe<DnsConfigServicePosix> to inheritance list? I forgot
whether in the case of duplicate definition order of classes in this list
matters.

+                              public FilePathWatcher::Delegate {
+ public:
+  DnsConfigServicePosix() {}
+  virtual ~DnsConfigServicePosix() {}
+  virtual bool Watch(DnsConfigService::Delegate* delegate) OVERRIDE;
+
+ private:
+  // TODO(szym): convert Task -> Closure?
+
+  // called on the worker thread
+  // reads DnsConfig and posts OnResultAvailable to |message_loop|
+  void ReadConfig(scoped_refptr<base::MessageLoopProxy> message_loop);
+
+  // called on the delegate's thread
+  // communicates result from DnsConfigReader to the delegate
+  void OnResultAvailable(const DnsConfig &config, bool error);
+
+  // FilePathWatcher::Delegate interface
+  virtual void OnFilePathChanged(const FilePath& path) OVERRIDE;
+
+  virtual void OnFilePathError(const FilePath& path) OVERRIDE {
+    delegate_->OnConfigError();
+  }
+
+  // message loop for the thread on which Watch is called
+  // this should be an IO loop
+  scoped_refptr<base::MessageLoopProxy> message_loop_;
+  scoped_ptr<FilePathWatcher> resolv_file_watcher_;
+  DnsConfigService::Delegate* delegate_;
+  DnsConfig last_config_;
+};
+
+bool DnsConfigServicePosix::Watch(DnsConfigService::Delegate* delegate) {
+  DCHECK(!resolv_file_watcher_.get());
+  DCHECK(MessageLoopForIO::current());
+

[cbentzel, 2011/08/01 19:52:26]

Should probably DCHECK that delegate_ is NULL.

[szym, 2011/08/01 20:22:01]

Done.

+  message_loop_ = base::MessageLoopProxy::CreateForCurrentThread();
+  delegate_ = delegate;
+
+  // start FilePathWatcher
+  resolv_file_watcher_.reset(new FilePathWatcher());
+  return resolv_file_watcher_->Watch(
+      FilePath(FILE_PATH_LITERAL("/etc/resolv.conf")),
+      this
+      );
+}
+
+void DnsConfigServicePosix::ReadConfig(
+    scoped_refptr<base::MessageLoopProxy> message_loop) {
+  // on the WorkerPool
+  DnsConfigReaderPosix reader;
+  DnsConfig config;
+  bool error = reader.Read(&config);
+  message_loop->PostTask(FROM_HERE, NewRunnableMethod(
+      this, &DnsConfigServicePosix::OnResultAvailable, config, error));
+}
+
+void DnsConfigServicePosix::OnResultAvailable(const DnsConfig &config,
+                                              bool error) {
+  if (error) {
+    delegate_->OnConfigError();
+  } else if (config != last_config_) {
+    delegate_->OnConfigChanged(config);
+    last_config_ = config;
+  }
+}
+
+// FilePathWatcher::Delegate interface
+void DnsConfigServicePosix::OnFilePathChanged(const FilePath& path) {
+  DCHECK(MessageLoopForIO::current());
+  // we should be on the same thread that called Watch so delegate should
+  // still exist?
+  if (!base::WorkerPool::PostTask(FROM_HERE, NewRunnableMethod(
+      this, &DnsConfigServicePosix::ReadConfig, message_loop_), false)) {
+    delegate_->OnConfigError();
+  }
+}
+
+DnsConfigService* DnsConfigService::CreateSystemService() {
+  return new DnsConfigServicePosix();
+}
+}
+