Call WatcherManager functions on the IO thread.

chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc

Index: chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc
diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc b/chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc
index 837d152adcf8fa61e72950f2a59e74ba3c8a6cb6..0d8faefff6cbeb334b0c44993563a366431e5b3e 100644
--- a/chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc
+++ b/chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/file_system_provider/fileapi/watcher_manager.h"
 
+#include "base/files/file.h"
 #include "chrome/browser/chromeos/file_system_provider/mount_path_util.h"
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system_interface.h"
 #include "content/public/browser/browser_thread.h"
@@ -14,16 +15,30 @@ using content::BrowserThread;
 namespace chromeos {
 namespace file_system_provider {
 
-WatcherManager::WatcherManager() {
+namespace {
+
+using StatusCallback = storage::WatcherManager::StatusCallback;
+using NotificationCallback = storage::WatcherManager::NotificationCallback;
+using ChangeType = storage::WatcherManager::ChangeType;
+
+void CallStatusCallbackOnIOThread(const StatusCallback& callback,
+                                  base::File::Error error) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(callback, error));
 }
-WatcherManager::~WatcherManager() {
+
+void CallNotificationCallbackOnIOThread(const NotificationCallback& callback,
+                                        ChangeType type) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(callback, type));
 }
 
-void WatcherManager::AddWatcher(
-    const storage::FileSystemURL& url,
-    bool recursive,
-    const StatusCallback& callback,
-    const NotificationCallback& notification_callback) {
+void AddWatcherOnUIThread(const storage::FileSystemURL& url,
+                          bool recursive,
+                          const StatusCallback& callback,
+                          const NotificationCallback& notification_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   util::FileSystemURLParser parser(url);
@@ -40,9 +55,9 @@ void WatcherManager::AddWatcher(
                                    notification_callback);
 }
 
-void WatcherManager::RemoveWatcher(const storage::FileSystemURL& url,
-                                   bool recursive,
-                                   const StatusCallback& callback) {
+void RemoveWatcherOnUIThread(const storage::FileSystemURL& url,
+                             bool recursive,
+                             const StatusCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   util::FileSystemURLParser parser(url);
@@ -55,5 +70,34 @@ void WatcherManager::RemoveWatcher(const storage::FileSystemURL& url,
       url.origin(), parser.file_path(), recursive, callback);
 }
 
+}  // namespace
+
+WatcherManager::WatcherManager() {}
+WatcherManager::~WatcherManager() {}
+
+void WatcherManager::AddWatcher(

[mtomasz, 2017/02/23 05:10:05]

Can we add a comment in watcher_manager.h that all methods have to be called on
IO thead and that callbacks are always called on IO thread?

[Shuhei Takahashi, 2017/02/23 06:16:03]

Sure, added more comments to watcher_manager.h (in storage/browser/fileapi/).

+    const storage::FileSystemURL& url,
+    bool recursive,
+    const StatusCallback& callback,
+    const NotificationCallback& notification_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&AddWatcherOnUIThread, url, recursive,
+                 base::Bind(&CallStatusCallbackOnIOThread, callback),

[mtomasz, 2017/02/23 05:10:05]

(We're getting a lot of jumping between threads, but I think it's unavoidable,
as FSP API is primarily done on UI thread, and File API on IO.)

+                 base::Bind(&CallNotificationCallbackOnIOThread,
+                            notification_callback)));
+}
+
+void WatcherManager::RemoveWatcher(const storage::FileSystemURL& url,
+                                   bool recursive,
+                                   const StatusCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&RemoveWatcherOnUIThread, url, recursive,
+                 base::Bind(&CallStatusCallbackOnIOThread, callback)));
+}
+
 }  // namespace file_system_provider
 }  // namespace chromeos