[fsp] Add support for observing entries and notifying about changes.

chrome/browser/chromeos/file_system_provider/provided_file_system.cc

Index: chrome/browser/chromeos/file_system_provider/provided_file_system.cc
diff --git a/chrome/browser/chromeos/file_system_provider/provided_file_system.cc b/chrome/browser/chromeos/file_system_provider/provided_file_system.cc
index 1c8a6526aafa867c7e2e983628227b9095a8e562..4d6ab07f9f5d70c021f7cece8f4c8cba70f4158f 100644
--- a/chrome/browser/chromeos/file_system_provider/provided_file_system.cc
+++ b/chrome/browser/chromeos/file_system_provider/provided_file_system.cc
@@ -17,11 +17,13 @@
 #include "chrome/browser/chromeos/file_system_provider/operations/delete_entry.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/get_metadata.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/move_entry.h"
+#include "chrome/browser/chromeos/file_system_provider/operations/observe_directory.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/open_file.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/read_directory.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/read_file.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/truncate.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/unmount.h"
+#include "chrome/browser/chromeos/file_system_provider/operations/unobserve_entry.h"
 #include "chrome/browser/chromeos/file_system_provider/operations/write_file.h"
 #include "chrome/browser/chromeos/file_system_provider/request_manager.h"
 #include "chrome/browser/profiles/profile.h"
@@ -36,12 +38,39 @@ namespace chromeos {
 namespace file_system_provider {
 namespace {
 
-// Dicards the result of Abort() when called from the destructor.
+// Discards the result of Abort() when called from the destructor.
 void EmptyStatusCallback(base::File::Error /* result */) {
 }
 
 }  // namespace
 
+AutoUpdater::AutoUpdater(const base::Closure& update_callback)
+    : update_callback_(update_callback),
+      created_callbacks_(0),
+      pending_callbacks_(0) {
+}
+
+base::Closure AutoUpdater::CreateCallback() {
+  ++created_callbacks_;
+  ++pending_callbacks_;
+  return base::Bind(&AutoUpdater::OnPendingCallback, this);
+}
+
+void AutoUpdater::OnPendingCallback() {
+  DCHECK_LT(0, pending_callbacks_);
+  if (--pending_callbacks_ == 0)
+    update_callback_.Run();
+}
+
+AutoUpdater::~AutoUpdater() {
+  // If no callbacks are created, then we need to invoke updating in the
+  // destructor.
+  if (!created_callbacks_)

[hirono, 2014/10/03 07:19:13]

After update_callback_ is called in OnPendingCallback, the destructor still
calls update_callback_ because pending_callback_ is 0. Is it OK?

[mtomasz, 2014/10/03 08:35:33]

In destructor we check created_callbacks_, not pending_callbacks_. So in
destructor we will call update_callback_ only if there were no created
callbacks.

+    update_callback_.Run();
+  else if (pending_callbacks_)
+    LOG(ERROR) << "Not all callbacks called. This may happen on shutdown.";
+}
+
 ProvidedFileSystem::ProvidedFileSystem(
     Profile* profile,
     const ProvidedFileSystemInfo& file_system_info)
@@ -50,20 +79,31 @@ ProvidedFileSystem::ProvidedFileSystem(
       file_system_info_(file_system_info),
       notification_manager_(
           new NotificationManager(profile_, file_system_info_)),
-      request_manager_(notification_manager_.get()),
+      request_manager_(new RequestManager(notification_manager_.get())),
       weak_ptr_factory_(this) {
 }
 
 ProvidedFileSystem::~ProvidedFileSystem() {
-  const std::vector<int> request_ids = request_manager_.GetActiveRequestIds();
+  const std::vector<int> request_ids = request_manager_->GetActiveRequestIds();
   for (size_t i = 0; i < request_ids.size(); ++i) {
     Abort(request_ids[i], base::Bind(&EmptyStatusCallback));
   }
 }
 
+void ProvidedFileSystem::SetEventRouterForTesting(
+    extensions::EventRouter* event_router) {
+  event_router_ = event_router;
+}
+
+void ProvidedFileSystem::SetNotificationManagerForTesting(
+    scoped_ptr<NotificationManagerInterface> notification_manager) {
+  notification_manager_ = notification_manager.Pass();
+  request_manager_.reset(new RequestManager(notification_manager_.get()));
+}
+
 ProvidedFileSystem::AbortCallback ProvidedFileSystem::RequestUnmount(
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       REQUEST_UNMOUNT,
       scoped_ptr<RequestManager::HandlerInterface>(
           new operations::Unmount(event_router_, file_system_info_, callback)));
@@ -80,7 +120,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::GetMetadata(
     const base::FilePath& entry_path,
     MetadataFieldMask fields,
     const GetMetadataCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       GET_METADATA,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::GetMetadata(
           event_router_, file_system_info_, entry_path, fields, callback)));
@@ -97,7 +137,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::GetMetadata(
 ProvidedFileSystem::AbortCallback ProvidedFileSystem::ReadDirectory(
     const base::FilePath& directory_path,
     const storage::AsyncFileUtil::ReadDirectoryCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       READ_DIRECTORY,
       scoped_ptr<RequestManager::HandlerInterface>(
           new operations::ReadDirectory(
@@ -121,7 +161,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::ReadFile(
     const ReadChunkReceivedCallback& callback) {
   TRACE_EVENT1(
       "file_system_provider", "ProvidedFileSystem::ReadFile", "length", length);
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       READ_FILE,
       make_scoped_ptr<RequestManager::HandlerInterface>(
           new operations::ReadFile(event_router_,
@@ -146,7 +186,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::OpenFile(
     const base::FilePath& file_path,
     OpenFileMode mode,
     const OpenFileCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       OPEN_FILE,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::OpenFile(
           event_router_, file_system_info_, file_path, mode, callback)));
@@ -162,7 +202,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::OpenFile(
 ProvidedFileSystem::AbortCallback ProvidedFileSystem::CloseFile(
     int file_handle,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       CLOSE_FILE,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::CloseFile(
           event_router_, file_system_info_, file_handle, callback)));
@@ -179,7 +219,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::CreateDirectory(
     const base::FilePath& directory_path,
     bool recursive,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       CREATE_DIRECTORY,
       scoped_ptr<RequestManager::HandlerInterface>(
           new operations::CreateDirectory(event_router_,
@@ -200,7 +240,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::DeleteEntry(
     const base::FilePath& entry_path,
     bool recursive,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       DELETE_ENTRY,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::DeleteEntry(
           event_router_, file_system_info_, entry_path, recursive, callback)));
@@ -216,7 +256,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::DeleteEntry(
 ProvidedFileSystem::AbortCallback ProvidedFileSystem::CreateFile(
     const base::FilePath& file_path,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       CREATE_FILE,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::CreateFile(
           event_router_, file_system_info_, file_path, callback)));
@@ -233,7 +273,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::CopyEntry(
     const base::FilePath& source_path,
     const base::FilePath& target_path,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       COPY_ENTRY,
       scoped_ptr<RequestManager::HandlerInterface>(
           new operations::CopyEntry(event_router_,
@@ -260,7 +300,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::WriteFile(
                "ProvidedFileSystem::WriteFile",
                "length",
                length);
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       WRITE_FILE,
       make_scoped_ptr<RequestManager::HandlerInterface>(
           new operations::WriteFile(event_router_,
@@ -283,7 +323,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::MoveEntry(
     const base::FilePath& source_path,
     const base::FilePath& target_path,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       MOVE_ENTRY,
       scoped_ptr<RequestManager::HandlerInterface>(
           new operations::MoveEntry(event_router_,
@@ -304,7 +344,7 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::Truncate(
     const base::FilePath& file_path,
     int64 length,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  const int request_id = request_manager_.CreateRequest(
+  const int request_id = request_manager_->CreateRequest(
       TRUNCATE,
       scoped_ptr<RequestManager::HandlerInterface>(new operations::Truncate(
           event_router_, file_system_info_, file_path, length, callback)));
@@ -317,12 +357,127 @@ ProvidedFileSystem::AbortCallback ProvidedFileSystem::Truncate(
       &ProvidedFileSystem::Abort, weak_ptr_factory_.GetWeakPtr(), request_id);
 }
 
+ProvidedFileSystem::AbortCallback ProvidedFileSystem::ObserveDirectory(
+    const base::FilePath& directory_path,
+    bool recursive,
+    const storage::AsyncFileUtil::StatusCallback& callback) {
+  const ObservedEntries::const_iterator it =
+      observed_entries_.find(directory_path);
+  if (it != observed_entries_.end()) {
+    if (!recursive || it->second.recursive) {
+      callback.Run(base::File::FILE_ERROR_EXISTS);
+      return AbortCallback();
+    }
+  }
+
+  const int request_id = request_manager_->CreateRequest(
+      OBSERVE_DIRECTORY,
+      scoped_ptr<RequestManager::HandlerInterface>(
+          new operations::ObserveDirectory(
+              event_router_,
+              file_system_info_,
+              directory_path,
+              recursive,
+              base::Bind(&ProvidedFileSystem::OnObserveDirectoryCompleted,
+                         weak_ptr_factory_.GetWeakPtr(),
+                         directory_path,
+                         recursive,
+                         callback))));
+  if (!request_id) {
+    callback.Run(base::File::FILE_ERROR_SECURITY);
+    return AbortCallback();
+  }
+
+  return base::Bind(
+      &ProvidedFileSystem::Abort, weak_ptr_factory_.GetWeakPtr(), request_id);
+}
+
+ProvidedFileSystem::AbortCallback ProvidedFileSystem::UnobserveEntry(

[hirono, 2014/10/03 07:19:13]

Should the operation be abortable?

[mtomasz, 2014/10/03 08:35:33]

Good point. It shouldn't. Done.

+    const base::FilePath& entry_path,
+    const storage::AsyncFileUtil::StatusCallback& callback) {
+  const ObservedEntries::const_iterator it = observed_entries_.find(entry_path);
+  if (it == observed_entries_.end()) {
+    callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return AbortCallback();
+  }
+
+  // Delete the watcher in advance since the list of observed entries is owned
+  // by the C++ layer, not by the extension.
+  observed_entries_.erase(it);
+
+  FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
+                    observers_,
+                    OnObservedEntryListChanged(file_system_info_));
+
+  // TODO(mtomasz): Consider returning always an OK error code, since for the
+  // callers it's important that the entry is not watched anymore. The watcher
+  // is removed even if the extension returns an error.
+  const int request_id = request_manager_->CreateRequest(
+      UNOBSERVE_ENTRY,
+      scoped_ptr<RequestManager::HandlerInterface>(
+          new operations::UnobserveEntry(
+              event_router_, file_system_info_, entry_path, callback)));
+  if (!request_id) {
+    callback.Run(base::File::FILE_ERROR_SECURITY);
+    return AbortCallback();
+  }
+
+  return base::Bind(
+      &ProvidedFileSystem::Abort, weak_ptr_factory_.GetWeakPtr(), request_id);
+}
+
 const ProvidedFileSystemInfo& ProvidedFileSystem::GetFileSystemInfo() const {
   return file_system_info_;
 }
 
 RequestManager* ProvidedFileSystem::GetRequestManager() {
-  return &request_manager_;
+  return request_manager_.get();
+}
+
+ProvidedFileSystem::ObservedEntries* ProvidedFileSystem::GetObservedEntries() {
+  return &observed_entries_;
+}
+
+void ProvidedFileSystem::AddObserver(ProvidedFileSystemObserver* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void ProvidedFileSystem::RemoveObserver(ProvidedFileSystemObserver* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
+bool ProvidedFileSystem::Notify(
+    const base::FilePath& observed_path,
+    ProvidedFileSystemObserver::ChangeType change_type,
+    const ProvidedFileSystemObserver::ChildChanges& child_changes,
+    const std::string& tag) {
+  const ObservedEntries::iterator it = observed_entries_.find(observed_path);
+  if (it == observed_entries_.end())
+    return false;
+
+  // The tag must be provided if and only if it's explicitly supported.
+  if (file_system_info_.supports_notify_tag() == tag.empty())
+    return false;
+
+  scoped_refptr<AutoUpdater> auto_updater(
+      new AutoUpdater(base::Bind(&ProvidedFileSystem::OnNotifyCompleted,
+                                 weak_ptr_factory_.GetWeakPtr(),
+                                 observed_path,
+                                 change_type,
+                                 it->second.last_tag,
+                                 tag)));
+
+  FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
+                    observers_,
+                    OnObservedEntryChanged(file_system_info_,
+                                           observed_path,
+                                           change_type,
+                                           child_changes,
+                                           auto_updater->CreateCallback()));
+
+  return true;
 }
 
 base::WeakPtr<ProvidedFileSystemInterface> ProvidedFileSystem::GetWeakPtr() {
@@ -332,10 +487,10 @@ base::WeakPtr<ProvidedFileSystemInterface> ProvidedFileSystem::GetWeakPtr() {
 void ProvidedFileSystem::Abort(
     int operation_request_id,
     const storage::AsyncFileUtil::StatusCallback& callback) {
-  request_manager_.RejectRequest(operation_request_id,
-                                 make_scoped_ptr(new RequestValue()),
-                                 base::File::FILE_ERROR_ABORT);
-  if (!request_manager_.CreateRequest(
+  request_manager_->RejectRequest(operation_request_id,
+                                  make_scoped_ptr(new RequestValue()),
+                                  base::File::FILE_ERROR_ABORT);
+  if (!request_manager_->CreateRequest(
           ABORT,
           scoped_ptr<RequestManager::HandlerInterface>(
               new operations::Abort(event_router_,
@@ -346,5 +501,57 @@ void ProvidedFileSystem::Abort(
   }
 }
 
+void ProvidedFileSystem::OnObserveDirectoryCompleted(
+    const base::FilePath& directory_path,
+    bool recursive,
+    const storage::AsyncFileUtil::StatusCallback& callback,
+    base::File::Error result) {
+  if (result != base::File::FILE_OK) {
+    callback.Run(result);
+    return;
+  }
+
+  observed_entries_[directory_path].entry_path = directory_path;
+  observed_entries_[directory_path].recursive |= recursive;
+
+  FOR_EACH_OBSERVER(ProvidedFileSystemObserver,
+                    observers_,
+                    OnObservedEntryListChanged(file_system_info_));
+
+  callback.Run(result);
+}
+
+void ProvidedFileSystem::OnNotifyCompleted(
+    const base::FilePath& observed_path,
+    ProvidedFileSystemObserver::ChangeType change_type,
+    const std::string& last_tag,
+    const std::string& tag) {
+  const ObservedEntries::iterator it = observed_entries_.find(observed_path);
+  // Check if the entry is still observed.
+  if (it == observed_entries_.end())
+    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.";
+
+  it->second.last_tag = tag;
+
+  FOR_EACH_OBSERVER(
+      ProvidedFileSystemObserver,
+      observers_,
+      OnObservedEntryTagUpdated(file_system_info_, observed_path));
+
+  // If the observed entry is deleted, then unobserve it.
+  if (change_type == ProvidedFileSystemObserver::DELETED)
+    UnobserveEntry(observed_path, base::Bind(&EmptyStatusCallback));
+}
+
 }  // namespace file_system_provider
 }  // namespace chromeos