[fsp] Pass more detailed errors to the providing extension.

chrome/browser/chromeos/file_system_provider/provided_file_system.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/file_system_provider/provided_file_system.h"
 
 #include <vector>
 
 #include "base/debug/trace_event.h"
 #include "base/files/file.h"
@@ skipping 483 matching lines @@
 void ProvidedFileSystem::AddObserver(ProvidedFileSystemObserver* observer) {
   DCHECK(observer);
   observers_.AddObserver(observer);
 }
 
 void ProvidedFileSystem::RemoveObserver(ProvidedFileSystemObserver* observer) {
   DCHECK(observer);
   observers_.RemoveObserver(observer);
 }
 
-bool ProvidedFileSystem::Notify(
+void ProvidedFileSystem::Notify(
     const base::FilePath& entry_path,
     bool recursive,
     storage::WatcherManager::ChangeType change_type,
     scoped_ptr<ProvidedFileSystemObserver::Changes> changes,
-    const std::string& tag) {
+    const std::string& tag,
+    const storage::AsyncFileUtil::StatusCallback& callback) {
   const WatcherKey key(entry_path, recursive);
   const auto& watcher_it = watchers_.find(key);
-  if (watcher_it == watchers_.end())
-    return false;
+  if (watcher_it == watchers_.end()) {
+    callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return;
+  }
 
   // The tag must be provided if and only if it's explicitly supported.
-  if (file_system_info_.supports_notify_tag() == tag.empty())
-    return false;
+  if (file_system_info_.supports_notify_tag() == tag.empty()) {
+    callback.Run(base::File::FILE_ERROR_INVALID_OPERATION);
+    return;
+  }
+
+  // It's illegal to provide a tag which is not unique.
+  if (!tag.empty() && tag == watcher_it->second.last_tag) {
+    callback.Run(base::File::FILE_ERROR_INVALID_OPERATION);
+    return;
+  }
 
   // The object is owned by AutoUpdated, so the reference is valid as long as
   // callbacks created with AutoUpdater::CreateCallback().
   const ProvidedFileSystemObserver::Changes& changes_ref = *changes.get();
 
-  scoped_refptr<AutoUpdater> auto_updater(
-      new AutoUpdater(base::Bind(&ProvidedFileSystem::OnNotifyCompleted,
-                                 weak_ptr_factory_.GetWeakPtr(),
-                                 entry_path,
-                                 recursive,
-                                 change_type,
-                                 base::Passed(&changes),
-                                 watcher_it->second.last_tag,
-                                 tag)));
+  scoped_refptr<AutoUpdater> auto_updater(new AutoUpdater(
+      base::Bind(&ProvidedFileSystem::OnNotifyCompleted,
+                 weak_ptr_factory_.GetWeakPtr(), entry_path, recursive,
+                 change_type, base::Passed(&changes), tag, callback)));
 
   // Call all notification callbacks (if any).
   for (const auto& subscriber_it : watcher_it->second.subscribers) {
     const storage::WatcherManager::NotificationCallback& notification_callback =
         subscriber_it.second.notification_callback;
     if (!notification_callback.is_null())
       notification_callback.Run(change_type);
   }
 
   // Notify all observers.
   FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
                     observers_,
                     OnWatcherChanged(file_system_info_,
                                      watcher_it->second,
                                      change_type,
                                      changes_ref,
                                      auto_updater->CreateCallback()));
-
-  return true;
 }
 
 base::WeakPtr<ProvidedFileSystemInterface> ProvidedFileSystem::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 void ProvidedFileSystem::Abort(
     int operation_request_id,
     const storage::AsyncFileUtil::StatusCallback& callback) {
   request_manager_->RejectRequest(operation_request_id,
@@ skipping 39 matching lines @@
                     OnWatcherListChanged(file_system_info_, watchers_));
 
   callback.Run(base::File::FILE_OK);
 }
 
 void ProvidedFileSystem::OnNotifyCompleted(
     const base::FilePath& entry_path,
     bool recursive,
     storage::WatcherManager::ChangeType change_type,
     scoped_ptr<ProvidedFileSystemObserver::Changes> /* changes */,
-    const std::string& last_tag,
-    const std::string& tag) {
+    const std::string& tag,
+    const storage::AsyncFileUtil::StatusCallback& callback) {
   const WatcherKey key(entry_path, recursive);
   const Watchers::iterator it = watchers_.find(key);
   // Check if the entry is still watched.
-  if (it == watchers_.end())
+  if (it == watchers_.end()) {
+    callback.Run(base::File::FILE_ERROR_NOT_FOUND);
     return;
+  }
 
-  // Another following notification finished earlier.
-  if (it->second.last_tag != last_tag)
-    return;
-
-  // It's illegal to provide a tag which is not unique. As for now only an error
-  // message is printed, but we may want to pass the error to the providing
-  // extension. TODO(mtomasz): Consider it.
-  if (!tag.empty() && tag == it->second.last_tag)
-    LOG(ERROR) << "Tag specified, but same as the previous one.";
+  // TODO(mtomasz): Add an async queue around notify and other watcher related
+  // methods so there is no rate.

[fukino, 2014/11/07 10:26:53]

nit: rate -> race?

[mtomasz, 2014/11/07 11:20:22]

Done.

 
   it->second.last_tag = tag;
 
   FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
                     observers_,
                     OnWatcherTagUpdated(file_system_info_, it->second));
 
   // If the watched entry is deleted, then remove the watcher.
   if (change_type == storage::WatcherManager::DELETED) {
     // Make a copy, since the |it| iterator will get invalidated on the last
     // subscriber.
     Subscribers subscribers = it->second.subscribers;
     for (const auto& subscriber_it : subscribers) {
       RemoveWatcher(subscriber_it.second.origin,
                     entry_path,
                     recursive,
                     base::Bind(&EmptyStatusCallback));
     }
   }
+
+  callback.Run(base::File::FILE_OK);
 }
 
 }  // namespace file_system_provider
 }  // namespace chromeos