Massively simplify the NetworkChangeNotifier infrastructure:

  * Use a process-wide object (singleton pattern)
  * Create/destroy this object on the main thread, make it outlive all consumers
  * Make observer-related functions threadsafe

As a result, the notifier can now be used by any thread (eliminating things like NetworkChangeObserverProxy and NetworkChangeNotifierProxy, and expanding its usefulness); its creation and inner workings are much simplified (eliminating implementation-specific classes); and it is simpler to access (eliminating things like NetworkChangeNotifierThread and a LOT of passing pointers around).

BUG=none
TEST=Unittests; network changes still trigger notifications
Review URL: http://codereview.chromium.org/2802015

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

1 files changed, 65 insertions, 70 deletions

diff --git a/net/base/network_change_notifier_linux.cc b/net/base/network_change_notifier_linux.cc
index 9821be5..aa58c14 100644
--- a/net/base/network_change_notifier_linux.cc
+++ b/net/base/network_change_notifier_linux.cc
@@ -7,13 +7,15 @@
 #include <errno.h>
 #include <sys/socket.h>
 
-#include "base/basictypes.h"
 #include "base/eintr_wrapper.h"
-#include "base/logging.h"
-#include "base/message_loop.h"
+#include "base/task.h"
+#include "base/thread.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier_netlink_linux.h"
 
+// We only post tasks to a child thread we own, so we don't need refcounting.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(net::NetworkChangeNotifierLinux);
+
 namespace net {
 
 namespace {
@@ -23,122 +25,115 @@ const int kInvalidSocket = -1;
 }  // namespace
 
 NetworkChangeNotifierLinux::NetworkChangeNotifierLinux()
-    : netlink_fd_(kInvalidSocket),
-#if defined(OS_CHROMEOS)
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
-#endif
-      loop_(MessageLoopForIO::current()) {
-  netlink_fd_ = InitializeNetlinkSocket();
-  if (netlink_fd_ < 0) {
-    netlink_fd_ = kInvalidSocket;
-    return;
-  }
-
-  ListenForNotifications();
-  loop_->AddDestructionObserver(this);
+    : notifier_thread_(new base::Thread("NetworkChangeNotifier")),
+      netlink_fd_(kInvalidSocket) {
+  // We create this notifier thread because the notification implementation
+  // needs a MessageLoopForIO, and there's no guarantee that
+  // MessageLoop::current() meets that criterion.
+  base::Thread::Options thread_options(MessageLoop::TYPE_IO, 0);
+  notifier_thread_->StartWithOptions(thread_options);
+  notifier_thread_->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &NetworkChangeNotifierLinux::Init));
 }
 
 NetworkChangeNotifierLinux::~NetworkChangeNotifierLinux() {
-  DCHECK(CalledOnValidThread());
-  StopWatching();
-
-  if (loop_)
-    loop_->RemoveDestructionObserver(this);
+  // We don't need to explicitly Stop(), but doing so allows us to sanity-
+  // check that the notifier thread shut down properly.
+  notifier_thread_->Stop();
+  DCHECK_EQ(kInvalidSocket, netlink_fd_);
 }
 
-void NetworkChangeNotifierLinux::AddObserver(Observer* observer) {
-  DCHECK(CalledOnValidThread());
-  observers_.AddObserver(observer);
-}
+void NetworkChangeNotifierLinux::WillDestroyCurrentMessageLoop() {
+  DCHECK(notifier_thread_ != NULL);
+  // We can't check the notifier_thread_'s message_loop(), as it's now 0.
+  // DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
 
-void NetworkChangeNotifierLinux::RemoveObserver(Observer* observer) {
-  DCHECK(CalledOnValidThread());
-  observers_.RemoveObserver(observer);
+  if (netlink_fd_ != kInvalidSocket) {
+    if (HANDLE_EINTR(close(netlink_fd_)) != 0)
+      PLOG(ERROR) << "Failed to close socket";
+    netlink_fd_ = kInvalidSocket;
+    netlink_watcher_.StopWatchingFileDescriptor();
+  }
 }
 
 void NetworkChangeNotifierLinux::OnFileCanReadWithoutBlocking(int fd) {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(fd, netlink_fd_);
+  DCHECK(notifier_thread_ != NULL);
+  DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
 
+  DCHECK_EQ(fd, netlink_fd_);
   ListenForNotifications();
 }
 
 void NetworkChangeNotifierLinux::OnFileCanWriteWithoutBlocking(int /* fd */) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(notifier_thread_ != NULL);
+  DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
+
   NOTREACHED();
 }
 
-void NetworkChangeNotifierLinux::WillDestroyCurrentMessageLoop() {
-  DCHECK(CalledOnValidThread());
-  StopWatching();
-  loop_ = NULL;
+void NetworkChangeNotifierLinux::Init() {
+  DCHECK(notifier_thread_ != NULL);
+  DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
+
+  netlink_fd_ = InitializeNetlinkSocket();
+  if (netlink_fd_ < 0) {
+    netlink_fd_ = kInvalidSocket;
+    return;
+  }
+  MessageLoop::current()->AddDestructionObserver(this);
+  ListenForNotifications();
 }
 
 void NetworkChangeNotifierLinux::ListenForNotifications() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(notifier_thread_ != NULL);
+  DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
+
   char buf[4096];
   int rv = ReadNotificationMessage(buf, arraysize(buf));
-  while (rv > 0 ) {
+  while (rv > 0) {
     if (HandleNetlinkMessage(buf, rv)) {
       LOG(INFO) << "Detected IP address changes.";
-
 #if defined(OS_CHROMEOS)
       // TODO(zelidrag): chromium-os:3996 - introduced artificial delay to
       // work around the issue of proxy initialization before name resolving
       // is functional in ChromeOS. This should be removed once this bug
       // is properly fixed.
-      MessageLoop::current()->PostDelayedTask(
-          FROM_HERE,
-          factory_.NewRunnableMethod(
-              &NetworkChangeNotifierLinux::NotifyObserversIPAddressChanged),
-          500);
+      const int kObserverNotificationDelayMS = 500;
+      MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableMethod(
+          &NetworkChangeNotifier::NotifyObserversOfIPAddressChange),
+          kObserverNotificationDelayMS);
 #else
-      NotifyObserversIPAddressChanged();
+      NotifyObserversOfIPAddressChange();
 #endif
     }
     rv = ReadNotificationMessage(buf, arraysize(buf));
   }
 
   if (rv == ERR_IO_PENDING) {
-    rv = loop_->WatchFileDescriptor(
-        netlink_fd_, false, MessageLoopForIO::WATCH_READ, &netlink_watcher_,
-        this);
+    rv = MessageLoopForIO::current()->WatchFileDescriptor(netlink_fd_, false,
+        MessageLoopForIO::WATCH_READ, &netlink_watcher_, this);
     LOG_IF(ERROR, !rv) << "Failed to watch netlink socket: " << netlink_fd_;
   }
 }
 
-void NetworkChangeNotifierLinux::NotifyObserversIPAddressChanged() {
-  FOR_EACH_OBSERVER(Observer, observers_, OnIPAddressChanged());
-}
-
 int NetworkChangeNotifierLinux::ReadNotificationMessage(char* buf, size_t len) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(notifier_thread_ != NULL);
+  DCHECK_EQ(notifier_thread_->message_loop(), MessageLoop::current());
+
   DCHECK_NE(len, 0u);
   DCHECK(buf);
-
   memset(buf, 0, sizeof(buf));
   int rv = recv(netlink_fd_, buf, len, 0);
-  if (rv > 0) {
+  if (rv > 0)
     return rv;
-  } else {
-    DCHECK_NE(rv, 0);
-    if (errno != EAGAIN && errno != EWOULDBLOCK) {
-      PLOG(DFATAL) << "recv";
-      return ERR_FAILED;
-    }
 
-    return ERR_IO_PENDING;
+  DCHECK_NE(rv, 0);
+  if (errno != EAGAIN && errno != EWOULDBLOCK) {
+    PLOG(DFATAL) << "recv";
+    return ERR_FAILED;
   }
-}
 
-void NetworkChangeNotifierLinux::StopWatching() {
-  DCHECK(CalledOnValidThread());
-  if (netlink_fd_ != kInvalidSocket) {
-    if (HANDLE_EINTR(close(netlink_fd_)) != 0)
-      PLOG(ERROR) << "Failed to close socket";
-    netlink_fd_ = kInvalidSocket;
-    netlink_watcher_.StopWatchingFileDescriptor();
-  }
+  return ERR_IO_PENDING;
 }
 
 }  // namespace net