Remove destruction observer from base::FilePathWatcherKQueue.

base/files/file_path_watcher.h

Index: base/files/file_path_watcher.h
diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index 267c03ae00b7bb8600ade73b74236d3e68e12562..0ee56af94016271d3737d94137154c86bca20593 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -90,9 +90,12 @@ class BASE_EXPORT FilePathWatcher {
   static bool RecursiveWatchAvailable();
 
   // Invokes |callback| whenever updates to |path| are detected. This should be
-  // called at most once, and from a MessageLoop of TYPE_IO. Set |recursive| to
-  // true, to watch |path| and its children. The callback will be invoked on
-  // the same loop. Returns true on success.
+  // called at most once. Set |recursive| to true, to watch |path| and its
+  // children. The callback will be invoked on the same thread. Returns true on
+  // success.
+  //
+  // On Mac, this must be called from a thread that supports
+  // FileDescriptorWatcher when |recursive| is false.

[dcheng, 2016/11/23 02:14:25]

I don't understand this comment: is there more background to this that I can
read somewhere?

[fdoray, 2016/11/23 12:58:37]

file_path_watcher_mac.cc:31 -> FilePathWatcherKQueue is used when |recursive| is
false
file_path_watcher_kqueue.cc:276 -> FilePathWatcherKQueue uses
FileDescriptorWatcher, which means that it can only be used from a thread that
instantiated a FileDescriptorWatcher (see file_descriptor_watcher_posix.h:85)

[dcheng, 2016/11/23 14:16:11]

I'm wondering more about the Mac-specific bits. It seems weird to expose this
(implementation-specific) difference in the API, and I guess I'm wondering if
there's a way to not require that difference.

[fdoray, 2016/11/23 16:21:41]

Three potential solutions:

1. Document that this must always be called from a thread that supports
FileDescriptorWatcher (even though it's only true for Mac/non-recursive).
2. Use FilePathWatcherFSEvents instead of FilePathWatcherKQueue when we detect
that the thread doesn't support FileDescriptorWatcher.
3. Delete FilePathWatcherKQueue and use FilePathWatcherFSEvents all the time. It
seems like the reason why we have FilePathWatcherKQueue is because FSEvents is
flaky on Mac OS X 10.6.7
https://codereview.chromium.org/6731064/diff/22001/content/common/file_path_w...
We only support OS X 10.9+. I verified that FSEvents is not flaky on OS X 10.11
using http://code.google.com/p/chromium/issues/detail?id=54822#c31. I don't have
Macs running other versions of OS X.

I like solution #3.

[Mark Mentovai, 2016/11/29 19:00:22]

FSEvents are more heavyweight than a kqueue()-based watcher. They’ll also break
down sooner. I would generally prefer to use the kqueue()-based watcher where we
are able.

[fdoray, 2016/11/29 19:09:47]

Then I'll do #1 (document that FilePathWatcher::Watch() may only be called from
a thread that supports FileDescriptorWatcher).

   //
   // Recursive watch is not supported on all platforms and file systems.
   // Watch() will return false in the case of failure.