Cleanup: Deprecate FileSystemCallbackDispatcher

content/common/fileapi/file_system_dispatcher.cc

Index: content/common/fileapi/file_system_dispatcher.cc
diff --git a/content/common/fileapi/file_system_dispatcher.cc b/content/common/fileapi/file_system_dispatcher.cc
index 425b00c71b411f583b1127e856ca759e2474c5a9..f2b6ce48068a06e699909737e6e7f132a26e536e 100644
--- a/content/common/fileapi/file_system_dispatcher.cc
+++ b/content/common/fileapi/file_system_dispatcher.cc
@@ -4,6 +4,7 @@
 
 #include "content/common/fileapi/file_system_dispatcher.h"
 
+#include "base/callback.h"
 #include "base/file_util.h"
 #include "base/process.h"
 #include "content/common/child_thread.h"
@@ -11,15 +12,135 @@
 
 namespace content {
 
+class FileSystemDispatcher::CallbackDispatcher {
+ public:
+  typedef CallbackDispatcher self;
+  typedef FileSystemDispatcher::StatusCallback StatusCallback;
+  typedef FileSystemDispatcher::MetadataCallback MetadataCallback;
+  typedef FileSystemDispatcher::ReadDirectoryCallback ReadDirectoryCallback;
+  typedef FileSystemDispatcher::OpenFileSystemCallback OpenFileSystemCallback;
+  typedef FileSystemDispatcher::WriteCallback WriteCallback;
+  typedef FileSystemDispatcher::OpenFileCallback OpenFileCallback;
+
+  static CallbackDispatcher* Create(const StatusCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->status_callback_ = callback;
+    dispatcher->onerror_callback_ = callback;
+    return dispatcher;
+  }
+  static CallbackDispatcher* Create(const MetadataCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->metadata_callback_ = callback;
+    dispatcher->onerror_callback_ =
+        base::Bind(&self::DidReadMetadata, base::Unretained(dispatcher),
+                   base::PlatformFileInfo(), base::FilePath());
+    return dispatcher;
+  }
+  static CallbackDispatcher* Create(const ReadDirectoryCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->directory_callback_ = callback;
+    dispatcher->onerror_callback_ =
+        base::Bind(&self::DidReadDirectory, base::Unretained(dispatcher),
+                   std::vector<base::FileUtilProxy::Entry>(), false);
+    return dispatcher;
+  }
+  static CallbackDispatcher* Create(const OpenFileSystemCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->filesystem_callback_ = callback;
+    dispatcher->onerror_callback_ =

[michaeln, 2013/05/14 22:18:58]

I see, here's the code path by which error value seen on DidSomething may be
!OK. A late bound closure.

[kinuko, 2013/05/15 13:58:03]

I removed these lazy bound closures as this may confuse readers.

+        base::Bind(&self::DidOpenFileSystem, base::Unretained(dispatcher),
+                   std::string(), GURL());
+    return dispatcher;
+  }
+  static CallbackDispatcher* Create(const WriteCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->write_callback_ = callback;
+    dispatcher->onerror_callback_ =
+        base::Bind(&self::DidWrite, base::Unretained(dispatcher), 0, true);
+    return dispatcher;
+  }
+  static CallbackDispatcher* Create(const OpenFileCallback& callback) {
+    CallbackDispatcher* dispatcher = new CallbackDispatcher;
+    dispatcher->open_callback_ = callback;
+    dispatcher->onerror_callback_ =
+        base::Bind(&self::DidOpenFile, base::Unretained(dispatcher),
+                   base::kInvalidPlatformFileValue, -1,
+                   quota::kQuotaLimitTypeUnknown);
+    return dispatcher;
+  }
+
+  ~CallbackDispatcher() {}
+
+  void DidSucceed() {
+    status_callback_.Run(base::PLATFORM_FILE_OK);
+  }
+
+  void DidFail(base::PlatformFileError error_code) {
+    onerror_callback_.Run(error_code);
+  }
+
+  void DidReadMetadata(
+      const base::PlatformFileInfo& file_info,
+      const base::FilePath& platform_path,
+      base::PlatformFileError error) {
+    metadata_callback_.Run(error, file_info, platform_path);
+  }
+
+  void DidCreateSnapshotFile(
+      const base::PlatformFileInfo& file_info,
+      const base::FilePath& platform_path,
+      base::PlatformFileError error) {
+    metadata_callback_.Run(error, file_info, platform_path);
+  }
+
+  void DidReadDirectory(
+      const std::vector<base::FileUtilProxy::Entry>& entries,
+      bool has_more,
+      base::PlatformFileError error) {
+    directory_callback_.Run(error, entries, has_more);
+  }
+
+  void DidOpenFileSystem(const std::string& name,
+                         const GURL& root,
+                         base::PlatformFileError error) {
+    filesystem_callback_.Run(error, name, root);
+  }
+
+  void DidWrite(int64 bytes, bool complete, base::PlatformFileError error) {
+    write_callback_.Run(error, bytes, complete);
+  }
+
+  void DidOpenFile(base::PlatformFile file,
+                   int file_open_id,
+                   quota::QuotaLimitType quota_policy,
+                   base::PlatformFileError error) {
+    open_callback_.Run(error, file, file_open_id, quota_policy);
+  }
+
+ private:
+  CallbackDispatcher() {}
+
+  StatusCallback status_callback_;
+  MetadataCallback metadata_callback_;
+  ReadDirectoryCallback directory_callback_;
+  OpenFileSystemCallback filesystem_callback_;
+  WriteCallback write_callback_;
+  OpenFileCallback open_callback_;
+
+  StatusCallback onerror_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallbackDispatcher);
+};
+
 FileSystemDispatcher::FileSystemDispatcher() {
 }
 
 FileSystemDispatcher::~FileSystemDispatcher() {
   // Make sure we fire all the remaining callbacks.
-  for (IDMap<fileapi::FileSystemCallbackDispatcher, IDMapOwnPointer>::iterator
+  for (IDMap<CallbackDispatcher, IDMapOwnPointer>::iterator
            iter(&dispatchers_); !iter.IsAtEnd(); iter.Advance()) {
     int request_id = iter.GetCurrentKey();
-    fileapi::FileSystemCallbackDispatcher* dispatcher = iter.GetCurrentValue();
+    CallbackDispatcher* dispatcher = iter.GetCurrentValue();
     DCHECK(dispatcher);
     dispatcher->DidFail(base::PLATFORM_FILE_ERROR_ABORT);
     dispatchers_.Remove(request_id);
@@ -46,8 +167,8 @@ bool FileSystemDispatcher::OnMessageReceived(const IPC::Message& msg) {
 bool FileSystemDispatcher::OpenFileSystem(
     const GURL& origin_url, fileapi::FileSystemType type,
     long long size, bool create,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const OpenFileSystemCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_Open(
           request_id, origin_url, type, size, create))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -60,8 +181,8 @@ bool FileSystemDispatcher::OpenFileSystem(
 bool FileSystemDispatcher::DeleteFileSystem(
     const GURL& origin_url,
     fileapi::FileSystemType type,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_DeleteFileSystem(
           request_id, origin_url, type))) {
     dispatchers_.Remove(request_id);
@@ -73,8 +194,8 @@ bool FileSystemDispatcher::DeleteFileSystem(
 bool FileSystemDispatcher::Move(
     const GURL& src_path,
     const GURL& dest_path,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_Move(
           request_id, src_path, dest_path))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -87,8 +208,8 @@ bool FileSystemDispatcher::Move(
 bool FileSystemDispatcher::Copy(
     const GURL& src_path,
     const GURL& dest_path,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_Copy(
           request_id, src_path, dest_path))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -101,8 +222,8 @@ bool FileSystemDispatcher::Copy(
 bool FileSystemDispatcher::Remove(
     const GURL& path,
     bool recursive,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemMsg_Remove(request_id, path, recursive))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -114,8 +235,8 @@ bool FileSystemDispatcher::Remove(
 
 bool FileSystemDispatcher::ReadMetadata(
     const GURL& path,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const MetadataCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_ReadMetadata(request_id, path))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -130,8 +251,8 @@ bool FileSystemDispatcher::Create(
     bool exclusive,
     bool is_directory,
     bool recursive,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_Create(
           request_id, path, exclusive, is_directory, recursive))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -144,8 +265,8 @@ bool FileSystemDispatcher::Create(
 bool FileSystemDispatcher::Exists(
     const GURL& path,
     bool is_directory,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_Exists(request_id, path, is_directory))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -157,8 +278,8 @@ bool FileSystemDispatcher::Exists(
 
 bool FileSystemDispatcher::ReadDirectory(
     const GURL& path,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const ReadDirectoryCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_ReadDirectory(request_id, path))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -172,8 +293,8 @@ bool FileSystemDispatcher::Truncate(
     const GURL& path,
     int64 offset,
     int* request_id_out,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_Truncate(request_id, path, offset))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -190,8 +311,8 @@ bool FileSystemDispatcher::Write(
     const GURL& blob_url,
     int64 offset,
     int* request_id_out,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const WriteCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_Write(request_id, path, blob_url, offset))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -205,8 +326,8 @@ bool FileSystemDispatcher::Write(
 
 bool FileSystemDispatcher::Cancel(
     int request_id_to_cancel,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(new FileSystemHostMsg_CancelWrite(
           request_id, request_id_to_cancel))) {
     dispatchers_.Remove(request_id);  // destroys |dispatcher|
@@ -220,8 +341,8 @@ bool FileSystemDispatcher::TouchFile(
     const GURL& path,
     const base::Time& last_access_time,
     const base::Time& last_modified_time,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const StatusCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_TouchFile(
               request_id, path, last_access_time, last_modified_time))) {
@@ -235,8 +356,8 @@ bool FileSystemDispatcher::TouchFile(
 bool FileSystemDispatcher::OpenFile(
     const GURL& file_path,
     int file_flags,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const OpenFileCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_OpenFile(
               request_id, file_path, file_flags))) {
@@ -254,8 +375,8 @@ bool FileSystemDispatcher::NotifyCloseFile(int file_open_id) {
 
 bool FileSystemDispatcher::CreateSnapshotFile(
     const GURL& file_path,
-    fileapi::FileSystemCallbackDispatcher* dispatcher) {
-  int request_id = dispatchers_.Add(dispatcher);
+    const CreateSnapshotFileCallback& callback) {
+  int request_id = dispatchers_.Add(CallbackDispatcher::Create(callback));
   if (!ChildThread::current()->Send(
           new FileSystemHostMsg_CreateSnapshotFile(
               request_id, file_path))) {
@@ -269,16 +390,14 @@ void FileSystemDispatcher::OnDidOpenFileSystem(int request_id,
                                                const std::string& name,
                                                const GURL& root) {
   DCHECK(root.is_valid());
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidOpenFileSystem(name, root);
+  dispatcher->DidOpenFileSystem(name, root, base::PLATFORM_FILE_OK);
   dispatchers_.Remove(request_id);
 }
 
 void FileSystemDispatcher::OnDidSucceed(int request_id) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
   dispatcher->DidSucceed();
   dispatchers_.Remove(request_id);
@@ -287,20 +406,19 @@ void FileSystemDispatcher::OnDidSucceed(int request_id) {
 void FileSystemDispatcher::OnDidReadMetadata(
     int request_id, const base::PlatformFileInfo& file_info,
     const base::FilePath& platform_path) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidReadMetadata(file_info, platform_path);
+  dispatcher->DidReadMetadata(file_info, platform_path, base::PLATFORM_FILE_OK);

[michaeln, 2013/05/14 22:18:58]

Are there any code paths where the error parameter is not OK for this
DidDoSomething call (and the others)? It looks like errors come back from the
browser process in the form of OnDidFail ipcs.

[kinuko, 2013/05/15 13:58:03]

If so it must be a coding error, but anyway I dropped these weird error
handlings but instead changed some methods. Hope it clarifies the code.

   dispatchers_.Remove(request_id);
 }
 
 void FileSystemDispatcher::OnDidCreateSnapshotFile(
     int request_id, const base::PlatformFileInfo& file_info,
     const base::FilePath& platform_path) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidCreateSnapshotFile(file_info, platform_path);
+  dispatcher->DidCreateSnapshotFile(
+      file_info, platform_path, base::PLATFORM_FILE_OK);
   dispatchers_.Remove(request_id);
   ChildThread::current()->Send(
       new FileSystemHostMsg_DidReceiveSnapshotFile(request_id));
@@ -310,17 +428,15 @@ void FileSystemDispatcher::OnDidReadDirectory(
     int request_id,
     const std::vector<base::FileUtilProxy::Entry>& entries,
     bool has_more) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidReadDirectory(entries, has_more);
+  dispatcher->DidReadDirectory(entries, has_more, base::PLATFORM_FILE_OK);
   dispatchers_.Remove(request_id);
 }
 
 void FileSystemDispatcher::OnDidFail(
     int request_id, base::PlatformFileError error_code) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
   dispatcher->DidFail(error_code);
   dispatchers_.Remove(request_id);
@@ -328,10 +444,9 @@ void FileSystemDispatcher::OnDidFail(
 
 void FileSystemDispatcher::OnDidWrite(
     int request_id, int64 bytes, bool complete) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidWrite(bytes, complete);
+  dispatcher->DidWrite(bytes, complete, base::PLATFORM_FILE_OK);
   if (complete)
     dispatchers_.Remove(request_id);
 }
@@ -341,12 +456,12 @@ void FileSystemDispatcher::OnDidOpenFile(
     IPC::PlatformFileForTransit file,
     int file_open_id,
     quota::QuotaLimitType quota_policy) {
-  fileapi::FileSystemCallbackDispatcher* dispatcher =
-      dispatchers_.Lookup(request_id);
+  CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
   dispatcher->DidOpenFile(IPC::PlatformFileForTransitToPlatformFile(file),
                           file_open_id,
-                          quota_policy);
+                          quota_policy,
+                          base::PLATFORM_FILE_OK);
   dispatchers_.Remove(request_id);
 }