Remove ref-counting from FilePathWatcher.

base/files/file_path_watcher_fsevents.cc

Index: base/files/file_path_watcher_fsevents.cc
diff --git a/base/files/file_path_watcher_fsevents.cc b/base/files/file_path_watcher_fsevents.cc
index 8e73b557aa291d7967ffb84d19183316cc619c1c..e9a87b0e0526a4e2b3e78f5dbff85ceba3513d2a 100644
--- a/base/files/file_path_watcher_fsevents.cc
+++ b/base/files/file_path_watcher_fsevents.cc
@@ -13,7 +13,6 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
-#include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 
@@ -69,10 +68,16 @@ FilePath ResolvePath(const FilePath& path) {
 
 FilePathWatcherFSEvents::FilePathWatcherFSEvents()
     : queue_(dispatch_queue_create(
-          base::StringPrintf(
-              "org.chromium.base.FilePathWatcher.%p", this).c_str(),
+          base::StringPrintf("org.chromium.base.FilePathWatcher.%p", this)
+              .c_str(),
           DISPATCH_QUEUE_SERIAL)),
-      fsevent_stream_(nullptr) {
+      fsevent_stream_(nullptr),
+      weak_factory_(this) {}
+
+FilePathWatcherFSEvents::~FilePathWatcherFSEvents() {
+  DCHECK(!task_runner() || task_runner()->RunsTasksOnCurrentThread());
+  DCHECK(callback_.is_null())
+      << "Cancel() must be called before FilePathWatcher is destroyed.";
 }
 
 bool FilePathWatcherFSEvents::Watch(const FilePath& path,
@@ -105,11 +110,15 @@ void FilePathWatcherFSEvents::Cancel() {
   set_cancelled();
   callback_.Reset();
 
-  // Switch to the dispatch queue to tear down the event stream. As the queue
-  // is owned by this object, and this method is called from the destructor,
-  // execute the block synchronously.
+  // Switch to the dispatch queue to tear down the event stream. As the queue is
+  // owned by |this|, and this method is called from the destructor, execute the
+  // block synchronously.
   dispatch_sync(queue_, ^{
-      CancelOnMessageLoopThread();
+    if (fsevent_stream_) {
+      DestroyEventStream();
+      target_.clear();
+      resolved_target_.clear();
+    }
   });
 }
 
@@ -140,31 +149,40 @@ void FilePathWatcherFSEvents::FSEventsCallback(
   // the directory to be watched gets created.
   if (root_changed) {
     // Resetting the event stream from within the callback fails (FSEvents spews
-    // bad file descriptor errors), so post a task to do the reset.
-    dispatch_async(watcher->queue_, ^{
-        watcher->UpdateEventStream(root_change_at);
-    });
+    // bad file descriptor errors), so do the reset asynchronously.
+    //
+    // We can't dispatch_async a call to UpdateEventStream() directly because
+    // there would be no guarantee that |watcher| still exists when it runs.
+    //
+    // Instead, bounce on task_runner() and use a WeakPtr to verify that
+    // |watcher| still exists. If it does, dispatch_async a call to
+    // UpdateEventStream(). Because the destructor of |watcher| runs on
+    // task_runner() and calls dispatch_sync, it is guaranteed that |watcher|
+    // still exists when UpdateEventStream() runs.
+    watcher->task_runner()->PostTask(
+        FROM_HERE, Bind(
+                       [](WeakPtr<FilePathWatcherFSEvents> weak_watcher,
+                          FSEventStreamEventId root_change_at) {
+                         if (!weak_watcher)
+                           return;
+                         FilePathWatcherFSEvents* watcher = weak_watcher.get();
+                         dispatch_async(watcher->queue_, ^{
+                           watcher->UpdateEventStream(root_change_at);
+                         });
+                       },
+                       watcher->weak_factory_.GetWeakPtr(), root_change_at));
   }
 
   watcher->OnFilePathsChanged(paths);
 }
 
-FilePathWatcherFSEvents::~FilePathWatcherFSEvents() {
-  // This method may be called on either the libdispatch or task_runner()
-  // thread. Checking callback_ on the libdispatch thread here is safe because
-  // it is executing in a task posted by Cancel() which first reset callback_.
-  // PostTask forms a sufficient memory barrier to ensure that the value is
-  // consistent on the target thread.
-  DCHECK(callback_.is_null())
-      << "Cancel() must be called before FilePathWatcher is destroyed.";
-}
-
 void FilePathWatcherFSEvents::OnFilePathsChanged(
     const std::vector<FilePath>& paths) {
   DCHECK(!resolved_target_.empty());
   task_runner()->PostTask(
-      FROM_HERE, Bind(&FilePathWatcherFSEvents::DispatchEvents, this, paths,
-                      target_, resolved_target_));
+      FROM_HERE,
+      Bind(&FilePathWatcherFSEvents::DispatchEvents, weak_factory_.GetWeakPtr(),
+           paths, target_, resolved_target_));
 }
 
 void FilePathWatcherFSEvents::DispatchEvents(const std::vector<FilePath>& paths,
@@ -185,18 +203,6 @@ void FilePathWatcherFSEvents::DispatchEvents(const std::vector<FilePath>& paths,
   }
 }
 
-void FilePathWatcherFSEvents::CancelOnMessageLoopThread() {
-  // For all other implementations, the "message loop thread" is the IO thread,
-  // as returned by task_runner(). This implementation, however, needs to
-  // cancel pending work on the Dispatch Queue thread.
-
-  if (fsevent_stream_) {
-    DestroyEventStream();
-    target_.clear();
-    resolved_target_.clear();
-  }
-}
-
 void FilePathWatcherFSEvents::UpdateEventStream(
     FSEventStreamEventId start_event) {
   // It can happen that the watcher gets canceled while tasks that call this
@@ -232,8 +238,9 @@ void FilePathWatcherFSEvents::UpdateEventStream(
   FSEventStreamSetDispatchQueue(fsevent_stream_, queue_);
 
   if (!FSEventStreamStart(fsevent_stream_)) {
-    task_runner()->PostTask(
-        FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_));
+    task_runner()->PostTask(FROM_HERE,
+                            Bind(&FilePathWatcherFSEvents::ReportError,
+                                 weak_factory_.GetWeakPtr(), target_));
   }
 }
 
@@ -242,8 +249,9 @@ bool FilePathWatcherFSEvents::ResolveTargetPath() {
   bool changed = resolved != resolved_target_;
   resolved_target_ = resolved;
   if (resolved_target_.empty()) {
-    task_runner()->PostTask(
-        FROM_HERE, Bind(&FilePathWatcherFSEvents::ReportError, this, target_));
+    task_runner()->PostTask(FROM_HERE,
+                            Bind(&FilePathWatcherFSEvents::ReportError,
+                                 weak_factory_.GetWeakPtr(), target_));
   }
   return changed;
 }