Use FileDescriptorWatcher in FilePathWatcherKQueue.

base/files/file_path_watcher_kqueue.cc

Index: base/files/file_path_watcher_kqueue.cc
diff --git a/base/files/file_path_watcher_kqueue.cc b/base/files/file_path_watcher_kqueue.cc
index 4b81bf2b31f4731cdea471e886d5a664a955ef12..d4b1ed0a522fb90c7854b4c9801454fc1d4c4a99 100644
--- a/base/files/file_path_watcher_kqueue.cc
+++ b/base/files/file_path_watcher_kqueue.cc
@@ -36,7 +36,6 @@ void FilePathWatcherKQueue::ReleaseEvent(struct kevent& event) {
 }
 
 int FilePathWatcherKQueue::EventsForPath(FilePath path, EventVector* events) {
-  DCHECK(MessageLoopForIO::current());
   // Make sure that we are working with a clean slate.
   DCHECK(events->empty());
 
@@ -230,9 +229,68 @@ bool FilePathWatcherKQueue::UpdateWatches(bool* target_file_affected) {
   return true;
 }
 
-void FilePathWatcherKQueue::OnFileCanReadWithoutBlocking(int fd) {
-  DCHECK(MessageLoopForIO::current());
-  DCHECK_EQ(fd, kqueue_);
+void FilePathWatcherKQueue::WillDestroyCurrentMessageLoop() {
+  CancelOnMessageLoopThread();
+}
+
+bool FilePathWatcherKQueue::Watch(const FilePath& path,
+                                  bool recursive,
+                                  const FilePathWatcher::Callback& callback) {
+  DCHECK(target_.value().empty());  // Can only watch one path.
+  DCHECK(!callback.is_null());
+  DCHECK_EQ(kqueue_, -1);
+  // Recursive watch is not supported using kqueue.
+  DCHECK(!recursive);
+
+  callback_ = callback;
+  target_ = path;
+
+  MessageLoop::current()->AddDestructionObserver(this);
+  set_task_runner(ThreadTaskRunnerHandle::Get());
+
+  kqueue_ = kqueue();
+  if (kqueue_ == -1) {
+    DPLOG(ERROR) << "kqueue";
+    return false;
+  }
+
+  int last_entry = EventsForPath(target_, &events_);
+  DCHECK_NE(last_entry, 0);
+
+  EventVector responses(last_entry);
+
+  int count = HANDLE_EINTR(kevent(kqueue_, &events_[0], last_entry,
+                                  &responses[0], last_entry, NULL));
+  if (!AreKeventValuesValid(&responses[0], count)) {
+    // Calling Cancel() here to close any file descriptors that were opened.
+    // This would happen in the destructor anyways, but FilePathWatchers tend to
+    // be long lived, and if an error has occurred, there is no reason to waste
+    // the file descriptors.
+    Cancel();
+    return false;
+  }
+
+  kqueue_watch_controller_ = FileDescriptorWatcher::WatchReadable(
+      kqueue_,
+      Bind(&FilePathWatcherKQueue::OnKQueueReadable, Unretained(this)));

[dcheng, 2016/09/20 23:22:04]

|this| is refcounted, so maybe just use this (and the automatic refcounting)
directly?

[fdoray, 2016/09/26 14:32:02]

Done.

Note that this creates an ownership cycle (FilePathWatcherKQueue owns
FileDescriptorWatcher::Controller, which owns FilePathWatcherKQueue through the
callback). I think it's ok since the cycle is broken when
CancelOnMessageLoopThread() is called (line 368).

[dcheng, 2016/09/29 18:06:53]

I'm OK with it either way, I guess. It's just what the original code did.

Either way is slightly surprising (at first glance), so maybe throw a comment on
here whichever way you choose.

+  return true;
+}
+
+void FilePathWatcherKQueue::Cancel() {
+  if (!task_runner()) {
+    set_cancelled();
+    return;
+  }
+  if (!task_runner()->BelongsToCurrentThread()) {
+    task_runner()->PostTask(FROM_HERE,
+                            base::Bind(&FilePathWatcherKQueue::Cancel, this));
+    return;
+  }
+  CancelOnMessageLoopThread();
+}
+
+void FilePathWatcherKQueue::OnKQueueReadable() {
+  DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK(events_.size());
 
   // Request the file system update notifications that have occurred and return
@@ -303,86 +361,17 @@ void FilePathWatcherKQueue::OnFileCanReadWithoutBlocking(int fd) {
   }
 }
 
-void FilePathWatcherKQueue::OnFileCanWriteWithoutBlocking(int fd) {
-  NOTREACHED();
-}
-
-void FilePathWatcherKQueue::WillDestroyCurrentMessageLoop() {
-  CancelOnMessageLoopThread();
-}
-
-bool FilePathWatcherKQueue::Watch(const FilePath& path,
-                                  bool recursive,
-                                  const FilePathWatcher::Callback& callback) {
-  DCHECK(MessageLoopForIO::current());
-  DCHECK(target_.value().empty());  // Can only watch one path.
-  DCHECK(!callback.is_null());
-  DCHECK_EQ(kqueue_, -1);
-
-  if (recursive) {
-    // Recursive watch is not supported using kqueue.
-    NOTIMPLEMENTED();
-    return false;
-  }
-
-  callback_ = callback;
-  target_ = path;
-
-  MessageLoop::current()->AddDestructionObserver(this);
-  io_task_runner_ = ThreadTaskRunnerHandle::Get();
-
-  kqueue_ = kqueue();
-  if (kqueue_ == -1) {
-    DPLOG(ERROR) << "kqueue";
-    return false;
-  }
-
-  int last_entry = EventsForPath(target_, &events_);
-  DCHECK_NE(last_entry, 0);
-
-  EventVector responses(last_entry);
-
-  int count = HANDLE_EINTR(kevent(kqueue_, &events_[0], last_entry,
-                                  &responses[0], last_entry, NULL));
-  if (!AreKeventValuesValid(&responses[0], count)) {
-    // Calling Cancel() here to close any file descriptors that were opened.
-    // This would happen in the destructor anyways, but FilePathWatchers tend to
-    // be long lived, and if an error has occurred, there is no reason to waste
-    // the file descriptors.
-    Cancel();
-    return false;
-  }
-
-  return MessageLoopForIO::current()->WatchFileDescriptor(
-      kqueue_, true, MessageLoopForIO::WATCH_READ, &kqueue_watcher_, this);
-}
-
-void FilePathWatcherKQueue::Cancel() {
-  SingleThreadTaskRunner* task_runner = io_task_runner_.get();
-  if (!task_runner) {
-    set_cancelled();
-    return;
-  }
-  if (!task_runner->BelongsToCurrentThread()) {
-    task_runner->PostTask(FROM_HERE,
-                          base::Bind(&FilePathWatcherKQueue::Cancel, this));
-    return;
-  }
-  CancelOnMessageLoopThread();
-}
-
 void FilePathWatcherKQueue::CancelOnMessageLoopThread() {
-  DCHECK(MessageLoopForIO::current());
+  DCHECK(!task_runner() || task_runner()->BelongsToCurrentThread());
   if (!is_cancelled()) {
     set_cancelled();
-    kqueue_watcher_.StopWatchingFileDescriptor();
+    kqueue_watch_controller_.reset();
     if (IGNORE_EINTR(close(kqueue_)) != 0) {
       DPLOG(ERROR) << "close kqueue";
     }
     kqueue_ = -1;
     std::for_each(events_.begin(), events_.end(), ReleaseEvent);
     events_.clear();
-    io_task_runner_ = NULL;
     MessageLoop::current()->RemoveDestructionObserver(this);
     callback_.Reset();
   }