At least in the Linux impl the use of AutoLock wrapping accesses to the watcher set does mean that touching the watcher's TaskRunner and WeakPtrFactory is safe, though at risk if getting blocked on the watcher thread should it become unresponsive.

If we make watcher addition and removal asynchronous then we can get rid of the AutoLock and avoid confusing the WeakPtrFactory binding checks.

Yes, in base/files/file_path_watcher_linux.cc the use of a lock makes everything safe:

FilePathWatcherImpl destruction:
A1. ~FilePathWatcher calls FilePathWatcherImpl::Cancel()https://cs.chromium.org/chromium/src/base/files/file_path_watcher.cc?rcl=ef34992183fca33bc015826198d22ac458da7360&l=17
A2. FilePathWatcherImpl::Cancel() acquires INotifyReader::lock_ to remove itself from INotifyReader::watchers_.
A3. FilePathWatcherImpl is destroyed.

FilePathWatcherImpl notification:

B1. INotifyReader::OnINotifyEvent().
B2. Acquires INotifyReader::lock_.
B3. Notifies FilePatchWatcherImpl in INotifyReader::watchers_. Note that because of A2, it is impossible to notify a deleted FilePathWatcherImpl.
B4. FilePathWatcherImpl::OnFilePathChanged() safely accesses its task_runner_ and weak_ptr_factory_ https://cs.chromium.org/chromium/src/base/files/file_path_watcher_linux.cc?l=327
B5. FilePathWatcherImpl::OnFilePathChangedOnOriginSequence is called on the sequence where the notification is supposed to be dispatched, only if FilePathWatcherImpl hasn't been destroyed (guaranteed by the WeakPtr).

If we make addition and removal of watchers asynchronous, we would have to be careful not to dispatch a notification after a FileDescriptorWatcher has been destroye.

Other implementations should be safe too:
- fsevents: Cancel is synchronous.
- fuschia: Not implemented.
- kqueue: Single-threaded implementation.
- win: Single-threaded implementation (multi-threaded stuff handled by base::win::ObjectWatcher).

Specific clean-ups we could make here:
- Migrate from a Thread with perpetual-select() Task posted to it, to a proper MessageLoopForIO + FileDescriptorWatcher on the inotify fd.
- Replace FilePathWatcherImpl in the Add/Remove APIs with a notifier base::Callback, bound to the WatcherImpl's WeakPtr.
- Replace use of AutoLock in Add/Remove APIs with PostTask()s to the Inotify thread.

These would allow the implementation to be simplified and remove pesky locking.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77afc0f964fb4c0532f043fee9bef39d1b8fb1ee

commit 77afc0f964fb4c0532f043fee9bef39d1b8fb1ee
Author: Wez <wez@chromium.org>
Date: Mon Sep 11 20:49:46 2017

Update FilePathWatcher not to call GetWeakPtr on multiple threads.

WeakPtrFactory::GetWeakPtr() isn't designed to be thread-safe in
general, and in future will DCHECK against code which does so.

Bug: 760814
Change-Id: I04005b8dc7c3e1e63cc923ae92abf4dbf105b516
Reviewed-on: https://chromium-review.googlesource.com/645006
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Francois Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501031}
[modify] https://crrev.com/77afc0f964fb4c0532f043fee9bef39d1b8fb1ee/base/files/file_path_watcher_linux.cc