Bind: Reimplement remaining FileSystemFileUtilProxy methods using PostTaskAndReply

BUG=none
TEST=test_shell_tests:FileSystem*

Review URL: http://codereview.chromium.org/8425006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109198 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 110 insertions, 184 deletions

diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h
index 83a04c8..f96e532 100644
--- a/webkit/fileapi/file_system_file_util.h
+++ b/webkit/fileapi/file_system_file_util.h
@@ -20,8 +20,6 @@ using base::PlatformFileError;
 class FileSystemOperationContext;
 
 // A large part of this implementation is taken from base::FileUtilProxy.
-// TODO(dmikurube, kinuko): Clean up base::FileUtilProxy to factor out common
-// routines. It includes dropping FileAPI-specific routines from FileUtilProxy.
 //
 // The default implementations of the virtual methods below (*1) assume the
 // given paths are native ones for the host platform.  The subclasses that
diff --git a/webkit/fileapi/file_system_file_util_proxy.cc b/webkit/fileapi/file_system_file_util_proxy.cc
index 9569d41..4e2158f 100644
--- a/webkit/fileapi/file_system_file_util_proxy.cc
+++ b/webkit/fileapi/file_system_file_util_proxy.cc
@@ -6,200 +6,116 @@
 
 #include "base/bind.h"
 #include "base/message_loop_proxy.h"
-#include "webkit/fileapi/file_system_context.h"
-#include "webkit/fileapi/file_system_file_util.h"
-#include "webkit/fileapi/file_system_operation_context.h"
 
 namespace {
 
-class MessageLoopRelay
-    : public base::RefCountedThreadSafe<MessageLoopRelay> {
- public:
-  // FileSystemOperationContext is passed by value from the IO thread to the
-  // File thread.
-  explicit MessageLoopRelay(const fileapi::FileSystemOperationContext& context)
-      : origin_message_loop_proxy_(
-            base::MessageLoopProxy::current()),
-        error_code_(base::PLATFORM_FILE_OK),
-        context_(context),
-        file_util_(NULL) {
-  }
-
-  bool Start(scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
-             const tracked_objects::Location& from_here) {
-    return message_loop_proxy->PostTask(
-        from_here, base::Bind(&MessageLoopRelay::ProcessOnTargetThread, this));
-  }
+using base::Bind;
+using base::Callback;
+using base::Owned;
+using base::PlatformFileError;
+using base::Unretained;
 
- protected:
-  friend class base::RefCountedThreadSafe<MessageLoopRelay>;
-  virtual ~MessageLoopRelay() {}
+typedef fileapi::FileSystemFileUtilProxy Proxy;
 
-  // Called to perform work on the FILE thread.
-  virtual void RunWork() = 0;
-
-  // Called to notify the callback on the origin thread.
-  virtual void RunCallback() = 0;
-
-  void set_error_code(base::PlatformFileError error_code) {
-    error_code_ = error_code;
-  }
-
-  base::PlatformFileError error_code() const {
-    return error_code_;
-  }
-
-  fileapi::FileSystemOperationContext* context() {
-    return &context_;
-  }
-
-  fileapi::FileSystemFileUtil* file_util() const {
-    // TODO(ericu): Support calls that have two different FSFU subclasses.
-    return context_.src_file_util();
-  }
-
- private:
-  void ProcessOnTargetThread() {
-    RunWork();
-    origin_message_loop_proxy_->PostTask(
-        FROM_HERE, base::Bind(&MessageLoopRelay::RunCallback, this));
-  }
-
-  scoped_refptr<base::MessageLoopProxy> origin_message_loop_proxy_;
-  base::PlatformFileError error_code_;
-  fileapi::FileSystemOperationContext context_;
-  fileapi::FileSystemFileUtil* file_util_;
-};
-
-class RelayEnsureFileExists : public MessageLoopRelay {
+class EnsureFileExistsHelper {
  public:
-  RelayEnsureFileExists(
-      const fileapi::FileSystemOperationContext& context,
-      scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
-      const FilePath& file_path,
-      const fileapi::FileSystemFileUtilProxy::EnsureFileExistsCallback&
-          callback)
-      : MessageLoopRelay(context),
-        message_loop_proxy_(message_loop_proxy),
-        file_path_(file_path),
-        callback_(callback),
-        created_(false) {
-    DCHECK_EQ(false, callback.is_null());
-  }
+  EnsureFileExistsHelper() : error_(base::PLATFORM_FILE_OK) {}
 
- protected:
-  virtual void RunWork() {
-    set_error_code(file_util()->EnsureFileExists(
-        context(), file_path_, &created_));
+  void RunWork(const Proxy::EnsureFileExistsTask& task) {
+    error_ = task.Run(&created_);
   }
 
-  virtual void RunCallback() {
-    callback_.Run(error_code(), created_);
+  void Reply(const Proxy::EnsureFileExistsCallback& callback) {
+    if (!callback.is_null()) {
+      callback.Run(error_, created_);
+    }
   }
 
  private:
-  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
-  FilePath file_path_;
-  fileapi::FileSystemFileUtilProxy::EnsureFileExistsCallback callback_;
+  base::PlatformFileError error_;
   bool created_;
+  DISALLOW_COPY_AND_ASSIGN(EnsureFileExistsHelper);
 };
 
-
-class RelayGetFileInfo : public MessageLoopRelay {
+class GetFileInfoHelper {
  public:
-  RelayGetFileInfo(
-      const fileapi::FileSystemOperationContext& context,
-      const FilePath& file_path,
-      const fileapi::FileSystemFileUtilProxy::GetFileInfoCallback& callback)
-      : MessageLoopRelay(context),
-        callback_(callback),
-        file_path_(file_path) {
-    DCHECK_EQ(false, callback.is_null());
-  }
+  GetFileInfoHelper() : error_(base::PLATFORM_FILE_OK) {}
 
- protected:
-  virtual void RunWork() {
-    set_error_code(file_util()->GetFileInfo(
-        context(), file_path_, &file_info_, &platform_path_));
+  void RunWork(const Proxy::GetFileInfoTask& task) {
+    error_ = task.Run(&file_info_, &platform_path_);
   }
 
-  virtual void RunCallback() {
-    callback_.Run(error_code(), file_info_, platform_path_);
+  void Reply(const Proxy::GetFileInfoCallback& callback) {
+    if (!callback.is_null()) {
+      callback.Run(error_, file_info_, platform_path_);
+    }
   }
 
  private:
-  fileapi::FileSystemFileUtilProxy::GetFileInfoCallback callback_;
-  FilePath file_path_;
+  base::PlatformFileError error_;
   base::PlatformFileInfo file_info_;
   FilePath platform_path_;
+  DISALLOW_COPY_AND_ASSIGN(GetFileInfoHelper);
 };
 
-class RelayReadDirectory : public MessageLoopRelay {
+class ReadDirectoryHelper {
  public:
-  RelayReadDirectory(
-      const fileapi::FileSystemOperationContext& context,
-      const FilePath& file_path,
-      const fileapi::FileSystemFileUtilProxy::ReadDirectoryCallback& callback)
-      : MessageLoopRelay(context),
-        callback_(callback), file_path_(file_path) {
-    DCHECK_EQ(false, callback.is_null());
-  }
+  ReadDirectoryHelper() : error_(base::PLATFORM_FILE_OK) {}
 
- protected:
-  virtual void RunWork() {
-    // TODO(kkanetkar): Implement directory read in multiple chunks.
-    set_error_code(file_util()->ReadDirectory(
-        context(), file_path_, &entries_));
+  void RunWork(const Proxy::ReadDirectoryTask& task) {
+    error_ = task.Run(&entries_);
   }
 
-  virtual void RunCallback() {
-    callback_.Run(error_code(), entries_);
+  void Reply(const Proxy::ReadDirectoryCallback& callback) {
+    if (!callback.is_null()) {
+      callback.Run(error_, entries_);
+    }
   }
 
  private:
-  fileapi::FileSystemFileUtilProxy::ReadDirectoryCallback callback_;
-  FilePath file_path_;
-  std::vector<base::FileUtilProxy::Entry> entries_;
+  base::PlatformFileError error_;
+  std::vector<Proxy::Entry> entries_;
+  DISALLOW_COPY_AND_ASSIGN(ReadDirectoryHelper);
 };
 
-bool Start(const tracked_objects::Location& from_here,
-           scoped_refptr<base::MessageLoopProxy> message_loop_proxy,
-           scoped_refptr<MessageLoopRelay> relay) {
-  return relay->Start(message_loop_proxy, from_here);
-}
-
 }  // namespace
 
 namespace fileapi {
 
 // static
-bool FileSystemFileUtilProxy::EnsureFileExists(
-    const FileSystemOperationContext& context,
+bool FileSystemFileUtilProxy::RelayEnsureFileExists(
     scoped_refptr<MessageLoopProxy> message_loop_proxy,
-    const FilePath& file_path,
+    const EnsureFileExistsTask& task,
     const EnsureFileExistsCallback& callback) {
-  return Start(FROM_HERE, message_loop_proxy, new RelayEnsureFileExists(
-      context, message_loop_proxy, file_path, callback));
+  EnsureFileExistsHelper* helper = new EnsureFileExistsHelper;
+  return message_loop_proxy->PostTaskAndReply(
+        FROM_HERE,
+        Bind(&EnsureFileExistsHelper::RunWork, Unretained(helper), task),
+        Bind(&EnsureFileExistsHelper::Reply, Owned(helper), callback));
 }
 
 // static
-bool FileSystemFileUtilProxy::GetFileInfo(
-    const FileSystemOperationContext& context,
+bool FileSystemFileUtilProxy::RelayGetFileInfo(
     scoped_refptr<MessageLoopProxy> message_loop_proxy,
-    const FilePath& file_path,
+    const GetFileInfoTask& task,
     const GetFileInfoCallback& callback) {
-  return Start(FROM_HERE, message_loop_proxy, new RelayGetFileInfo(context,
-               file_path, callback));
+  GetFileInfoHelper* helper = new GetFileInfoHelper;
+  return message_loop_proxy->PostTaskAndReply(
+        FROM_HERE,
+        Bind(&GetFileInfoHelper::RunWork, Unretained(helper), task),
+        Bind(&GetFileInfoHelper::Reply, Owned(helper), callback));
 }
 
 // static
-bool FileSystemFileUtilProxy::ReadDirectory(
-    const FileSystemOperationContext& context,
+bool FileSystemFileUtilProxy::RelayReadDirectory(
     scoped_refptr<MessageLoopProxy> message_loop_proxy,
-    const FilePath& file_path,
+    const ReadDirectoryTask& task,
     const ReadDirectoryCallback& callback) {
-  return Start(FROM_HERE, message_loop_proxy, new RelayReadDirectory(context,
-               file_path, callback));
+  ReadDirectoryHelper* helper = new ReadDirectoryHelper;
+  return message_loop_proxy->PostTaskAndReply(
+        FROM_HERE,
+        Bind(&ReadDirectoryHelper::RunWork, Unretained(helper), task),
+        Bind(&ReadDirectoryHelper::Reply, Owned(helper), callback));
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/file_system_file_util_proxy.h b/webkit/fileapi/file_system_file_util_proxy.h
index 48ac923..d8e52ad 100644
--- a/webkit/fileapi/file_system_file_util_proxy.h
+++ b/webkit/fileapi/file_system_file_util_proxy.h
@@ -29,8 +29,10 @@ using base::PlatformFile;
 using base::PlatformFileError;
 using base::PlatformFileInfo;
 
-// This class provides asynchronous access to common file routines for the
-// FileSystem API.
+// This class provides relay methods for supporting asynchronous access to
+// FileSystem API operations.  (Most of necessary relay methods are provided
+// by base::FileUtilProxy, but there are a few operations that are not
+// covered or are slightly different from the version of base::FileUtilProxy.
 class FileSystemFileUtilProxy {
  public:
   typedef base::FileUtilProxy::Entry Entry;
@@ -46,33 +48,31 @@ class FileSystemFileUtilProxy {
                               const std::vector<Entry>&
                               )> ReadDirectoryCallback;
 
-  // Ensures that the given |file_path| exist.  This creates a empty new file
-  // at |file_path| if the |file_path| does not exist.
-  // If a new file han not existed and is created at the |file_path|,
-  // |created| of the callback argument is set true and |error code|
-  // is set PLATFORM_FILE_OK.
-  // If the file already exists, |created| is set false and |error code|
-  // is set PLATFORM_FILE_OK.
-  // If the file hasn't existed but it couldn't be created for some other
-  // reasons, |created| is set false and |error code| indicates the error.
-  static bool EnsureFileExists(
-      const FileSystemOperationContext& context,
+  typedef base::Callback<PlatformFileError(bool* /* created */
+                                           )> EnsureFileExistsTask;
+  typedef base::Callback<PlatformFileError(PlatformFileInfo*,
+                                           FilePath*)> GetFileInfoTask;
+  typedef base::Callback<PlatformFileError(std::vector<Entry>*
+                                           )> ReadDirectoryTask;
+
+  // Calls EnsureFileExistsTask |task| on the given |message_loop_proxy|.
+  static bool RelayEnsureFileExists(
       scoped_refptr<MessageLoopProxy> message_loop_proxy,
-      const FilePath& file_path,
+      const EnsureFileExistsTask& task,
       const EnsureFileExistsCallback& callback);
 
-  // Retrieves the information about a file. It is invalid to pass NULL for the
-  // callback.
-  static bool GetFileInfo(
-      const FileSystemOperationContext& context,
+  // Calls GetFileInfoTask |task| on the given |message_loop_proxy|.
+  static bool RelayGetFileInfo(
       scoped_refptr<MessageLoopProxy> message_loop_proxy,
-      const FilePath& file_path,
+      const GetFileInfoTask& task,
       const GetFileInfoCallback& callback);
 
-  static bool ReadDirectory(const FileSystemOperationContext& context,
-                            scoped_refptr<MessageLoopProxy> message_loop_proxy,
-                            const FilePath& file_path,
-                            const ReadDirectoryCallback& callback);
+  // Calls ReadDirectoryTask |task| on the given |message_loop_proxy|.
+  // TODO: this should support returning entries in multiple chunks.
+  static bool RelayReadDirectory(
+      scoped_refptr<MessageLoopProxy> message_loop_proxy,
+      const ReadDirectoryTask& task,
+      const ReadDirectoryCallback& callback);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(FileSystemFileUtilProxy);
diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index f329cc1..5db7268 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -132,10 +132,11 @@ void FileSystemOperation::DelayedCreateFileForQuota(
       operation_context_.src_origin_url(),
       operation_context_.src_type()));
 
-  FileSystemFileUtilProxy::EnsureFileExists(
-      operation_context_,
+  FileSystemFileUtilProxy::RelayEnsureFileExists(
       proxy_,
-      src_virtual_path_,
+      base::Bind(&FileSystemFileUtil::EnsureFileExists,
+                 base::Unretained(operation_context_.src_file_util()),
+                 &operation_context_, src_virtual_path_),
       base::Bind(
           exclusive ? &FileSystemOperation::DidEnsureFileExistsExclusive
                     : &FileSystemOperation::DidEnsureFileExistsNonExclusive,
@@ -263,8 +264,11 @@ void FileSystemOperation::DirectoryExists(const GURL& path) {
     return;
   }
 
-  FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_, proxy_, src_virtual_path_,
+  FileSystemFileUtilProxy::RelayGetFileInfo(
+      proxy_,
+      base::Bind(&FileSystemFileUtil::GetFileInfo,
+                 base::Unretained(operation_context_.src_file_util()),
+                 &operation_context_, src_virtual_path_),
       base::Bind(&FileSystemOperation::DidDirectoryExists, base::Owned(this)));
 }
 
@@ -278,8 +282,11 @@ void FileSystemOperation::FileExists(const GURL& path) {
     return;
   }
 
-  FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_, proxy_, src_virtual_path_,
+  FileSystemFileUtilProxy::RelayGetFileInfo(
+      proxy_,
+      base::Bind(&FileSystemFileUtil::GetFileInfo,
+                 base::Unretained(operation_context_.src_file_util()),
+                 &operation_context_, src_virtual_path_),
       base::Bind(&FileSystemOperation::DidFileExists, base::Owned(this)));
 }
 
@@ -293,8 +300,11 @@ void FileSystemOperation::GetMetadata(const GURL& path) {
     return;
   }
 
-  FileSystemFileUtilProxy::GetFileInfo(
-      operation_context_, proxy_, src_virtual_path_,
+  FileSystemFileUtilProxy::RelayGetFileInfo(
+      proxy_,
+      base::Bind(&FileSystemFileUtil::GetFileInfo,
+                 base::Unretained(operation_context_.src_file_util()),
+                 &operation_context_, src_virtual_path_),
       base::Bind(&FileSystemOperation::DidGetMetadata, base::Owned(this)));
 }
 
@@ -308,8 +318,11 @@ void FileSystemOperation::ReadDirectory(const GURL& path) {
     return;
   }
 
-  FileSystemFileUtilProxy::ReadDirectory(
-      operation_context_, proxy_, src_virtual_path_,
+  FileSystemFileUtilProxy::RelayReadDirectory(
+      proxy_,
+      base::Bind(&FileSystemFileUtil::ReadDirectory,
+                 base::Unretained(operation_context_.src_file_util()),
+                 &operation_context_, src_virtual_path_),
       base::Bind(&FileSystemOperation::DidReadDirectory, base::Owned(this)));
 }
 
@@ -327,8 +340,7 @@ void FileSystemOperation::Remove(const GURL& path, bool recursive) {
       proxy_, FROM_HERE,
       base::Bind(&FileSystemFileUtil::Delete,
                  base::Unretained(operation_context_.src_file_util()),
-                 &operation_context_,
-                 src_virtual_path_, recursive),
+                 &operation_context_, src_virtual_path_, recursive),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this)));
 }
@@ -375,8 +387,7 @@ void FileSystemOperation::DelayedWriteForQuota(quota::QuotaStatusCode status,
       proxy_,
       base::Bind(&FileSystemFileUtil::CreateOrOpen,
                  base::Unretained(operation_context_.src_file_util()),
-                 &operation_context_,
-                 src_virtual_path_, file_flags),
+                 &operation_context_, src_virtual_path_, file_flags),
       base::Bind(&FileSystemFileUtil::Close,
                  base::Unretained(operation_context_.src_file_util()),
                  &operation_context_),
@@ -413,8 +424,7 @@ void FileSystemOperation::DelayedTruncateForQuota(int64 length,
       proxy_, FROM_HERE,
       base::Bind(&FileSystemFileUtil::Truncate,
                  base::Unretained(operation_context_.src_file_util()),
-                 &operation_context_,
-                 src_virtual_path_, length),
+                 &operation_context_, src_virtual_path_, length),
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this)));
 }
@@ -575,6 +585,8 @@ void FileSystemOperation::GetUsageAndQuotaThenCallback(
 void FileSystemOperation::DidGetRootPath(
     bool success,
     const FilePath& path, const std::string& name) {
+  if (!dispatcher_.get())
+    return;
   DCHECK(success || path.empty());
   GURL result;
   if (!dispatcher_.get())