Good catch. +1 for (a) :)

+1 for a), if it's feasible for the callers.

Just FYI: As for a), any change is needed for the lifetime management of WatcherManager?
Will it be created and destructed on IO thread (IIUC, WeakPtrFactory requires it)?
I haven't thought about it deeper, but as chatted offline, FSC is destructed on ProfileImpl destruction after main loop stop on the main thread, so I just wonder it's lifetime management.

Thanks for your comments! Then I will go forward with (a).

> Will it be created and destructed on IO thread (IIUC, WeakPtrFactory requires it)?

FileSystemBackendDelegate and WatcherManager are created on the UI thread [1], but always destructed on the IO thread [2]. It is a sufficient guarantee for us to use WeakPtrFactory because it is bound to a thread on the first dereference of a weak pointer, not on the call of WeakPtrFactory constructor [3].

[1] https://cs.chromium.org/chromium/src/content/browser/fileapi/browser_file_system_helper.cc?type=cs&q=new.*FileSystemContext+-file:test&l=78
[2] https://cs.chromium.org/chromium/src/storage/browser/fileapi/file_system_context.h?type=cs&l=410
[3] https://cs.chromium.org/chromium/src/base/memory/weak_ptr.h?type=cs&l=55

#4, great to know and thank you very much for the references. I was misunderstanding about WeakPtrFactory spec.

Fully +1 for (a), now!

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

commit bf45258a17f7f528b652a6b033c606787f8f9a0d
Author: nya <nya@chromium.org>
Date: Fri Feb 24 05:58:21 2017

Call WatcherManager functions on the IO thread.

Currently it is very difficult to correctly implement WatcherManager
if it needs async operations due to following two facts:

1. WatcherManager functions do not receive
   storage::FileSystemOperationContext, while storage::AsyncFileUtil
   methods do.
2. WatcherManager functions are called on the UI thread, while other
   methods and destructors of its siblings (like
   storage::FileSystemBackend and storage::AsyncFileUtil) are all called
   on the IO thread.

Due to (1), we can't assume WatcherManager and its siblings are alive
after we post tasks to other threads in WatcherManager functions.
Typically we use weak pointers to avoid this kind of race conditions,
but due to (2) it is not possible because WatcherManager is destructed
on the IO thread.

This patch will fix the issue by requiring WatcherManager functions
to be called on the IO thread.

What this patch does are:
- Update private_api_file_system.{cc,h}, the only caller of
  WatcherManager today, to call WatcherManager functions on the IO
  thread.
- Update WatcherManager implementation for File System Providers
  to post tasks to the UI thread.
- Update DCHECK / comments accordingly.

Note that MTPWatcherManager actually expects its functions to be
called on the IO thread and is broken today, so no change is needed.

BUG= chromium:692586 
TEST=trybot

Review-Url: https://codereview.chromium.org/2712613002
Cr-Commit-Position: refs/heads/master@{#452768}

[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_backend_delegate.cc
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.h
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/file_system_provider/fileapi/backend_delegate.cc
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/chrome/browser/chromeos/fileapi/mtp_watcher_manager.cc
[modify] https://crrev.com/bf45258a17f7f528b652a6b033c606787f8f9a0d/storage/browser/fileapi/watcher_manager.h