Revert 121620 - Refactor FileSystemOperation to take callback for each method.

This patch is the first step for supporting cross-filesystem copy/move on
the Filesystem API implementation. To accomplish it, I'm planning to
crack FileSystemOperation::{Move,Copy} to a series of other FSO operations.
For it, per-method callback is more handy.

BUG=110121
TEST=*File*


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

TBR=kinaba@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9380040

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

3 files changed, 171 insertions, 163 deletions

diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc
index 6a81e34..4dfb1ac 100644
--- a/webkit/tools/test_shell/simple_file_system.cc
+++ b/webkit/tools/test_shell/simple_file_system.cc
@@ -4,7 +4,6 @@
 
 #include "webkit/tools/test_shell/simple_file_system.h"
 
-#include "base/bind.h"
 #include "base/file_path.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
@@ -19,6 +18,10 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebURL.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebVector.h"
+#include "webkit/fileapi/file_system_callback_dispatcher.h"
+#include "webkit/fileapi/file_system_context.h"
+#include "webkit/fileapi/file_system_operation_interface.h"
+#include "webkit/fileapi/file_system_types.h"
 #include "webkit/fileapi/mock_file_system_options.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/tools/test_shell/simple_file_writer.h"
@@ -37,9 +40,95 @@ using WebKit::WebString;
 using WebKit::WebURL;
 using WebKit::WebVector;
 
+using fileapi::FileSystemCallbackDispatcher;
 using fileapi::FileSystemContext;
 using fileapi::FileSystemOperationInterface;
 
+namespace {
+
+class SimpleFileSystemCallbackDispatcher
+    : public FileSystemCallbackDispatcher {
+ public:
+  // An instance of this class must be created by Create()
+  // (so that we do not leak ownerships).
+  static scoped_ptr<FileSystemCallbackDispatcher> Create(
+      const WeakPtr<SimpleFileSystem>& file_system,
+      WebFileSystemCallbacks* callbacks) {
+    return scoped_ptr<FileSystemCallbackDispatcher>(
+        new SimpleFileSystemCallbackDispatcher(file_system, callbacks));
+  }
+
+  ~SimpleFileSystemCallbackDispatcher() {
+  }
+
+  virtual void DidSucceed() {
+    DCHECK(file_system_);
+    callbacks_->didSucceed();
+  }
+
+  virtual void DidReadMetadata(const base::PlatformFileInfo& info,
+      const FilePath& platform_path) {
+    DCHECK(file_system_);
+    WebFileInfo web_file_info;
+    web_file_info.length = info.size;
+    web_file_info.modificationTime = info.last_modified.ToDoubleT();
+    web_file_info.type = info.is_directory ?
+        WebFileInfo::TypeDirectory : WebFileInfo::TypeFile;
+    web_file_info.platformPath =
+        webkit_glue::FilePathToWebString(platform_path);
+    callbacks_->didReadMetadata(web_file_info);
+  }
+
+  virtual void DidReadDirectory(
+      const std::vector<base::FileUtilProxy::Entry>& entries,
+      bool has_more) {
+    DCHECK(file_system_);
+    std::vector<WebFileSystemEntry> web_entries_vector;
+    for (std::vector<base::FileUtilProxy::Entry>::const_iterator it =
+            entries.begin(); it != entries.end(); ++it) {
+      WebFileSystemEntry entry;
+      entry.name = webkit_glue::FilePathStringToWebString(it->name);
+      entry.isDirectory = it->is_directory;
+      web_entries_vector.push_back(entry);
+    }
+    WebVector<WebKit::WebFileSystemEntry> web_entries =
+        web_entries_vector;
+    callbacks_->didReadDirectory(web_entries, has_more);
+  }
+
+  virtual void DidOpenFileSystem(
+      const std::string& name, const GURL& root) {
+    DCHECK(file_system_);
+    if (!root.is_valid())
+      callbacks_->didFail(WebKit::WebFileErrorSecurity);
+    else
+      callbacks_->didOpenFileSystem(WebString::fromUTF8(name), root);
+  }
+
+  virtual void DidFail(base::PlatformFileError error_code) {
+    DCHECK(file_system_);
+    callbacks_->didFail(
+        webkit_glue::PlatformFileErrorToWebFileError(error_code));
+  }
+
+  virtual void DidWrite(int64, bool) {
+    NOTREACHED();
+  }
+
+ private:
+  SimpleFileSystemCallbackDispatcher(
+      const WeakPtr<SimpleFileSystem>& file_system,
+      WebFileSystemCallbacks* callbacks)
+      : file_system_(file_system),
+        callbacks_(callbacks) {
+  }
+
+  WeakPtr<SimpleFileSystem> file_system_;
+  WebFileSystemCallbacks* callbacks_;
+};
+
+}  // namespace
+
 SimpleFileSystem::SimpleFileSystem() {
   if (file_system_dir_.CreateUniqueTempDir()) {
     file_system_context_ = new FileSystemContext(
@@ -83,64 +172,60 @@ void SimpleFileSystem::OpenFileSystem(
 
   GURL origin_url(frame->document().securityOrigin().toString());
   file_system_context_->OpenFileSystem(
-      origin_url, type, create, OpenFileSystemHandler(callbacks));
+      origin_url, type, create,
+      SimpleFileSystemCallbackDispatcher::Create(AsWeakPtr(), callbacks));
 }
 
 void SimpleFileSystem::move(
     const WebURL& src_path,
     const WebURL& dest_path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(src_path)->Move(GURL(src_path), GURL(dest_path),
-                                  FinishHandler(callbacks));
+  GetNewOperation(src_path, callbacks)->Move(GURL(src_path), GURL(dest_path));
 }
 
 void SimpleFileSystem::copy(
     const WebURL& src_path, const WebURL& dest_path,
     WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(src_path)->Copy(GURL(src_path), GURL(dest_path),
-                                  FinishHandler(callbacks));
+  GetNewOperation(src_path, callbacks)->Copy(GURL(src_path), GURL(dest_path));
 }
 
 void SimpleFileSystem::remove(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->Remove(path, false /* recursive */,
-                                FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->Remove(path, false /* recursive */);
 }
 
 void SimpleFileSystem::removeRecursively(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->Remove(path, true /* recursive */,
-                                FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->Remove(path, true /* recursive */);
 }
 
 void SimpleFileSystem::readMetadata(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->GetMetadata(path, GetMetadataHandler(callbacks));
+  GetNewOperation(path, callbacks)->GetMetadata(path);
 }
 
 void SimpleFileSystem::createFile(
     const WebURL& path, bool exclusive, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->CreateFile(path, exclusive, FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->CreateFile(path, exclusive);
 }
 
 void SimpleFileSystem::createDirectory(
     const WebURL& path, bool exclusive, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->CreateDirectory(path, exclusive, false,
-                                         FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->CreateDirectory(path, exclusive, false);
 }
 
 void SimpleFileSystem::fileExists(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->FileExists(path, FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->FileExists(path);
 }
 
 void SimpleFileSystem::directoryExists(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->DirectoryExists(path, FinishHandler(callbacks));
+  GetNewOperation(path, callbacks)->DirectoryExists(path);
 }
 
 void SimpleFileSystem::readDirectory(
     const WebURL& path, WebFileSystemCallbacks* callbacks) {
-  GetNewOperation(path)->ReadDirectory(path, ReadDirectoryHandler(callbacks));
+  GetNewOperation(path, callbacks)->ReadDirectory(path);
 }
 
 WebFileWriter* SimpleFileSystem::createFileWriter(
@@ -149,93 +234,9 @@ WebFileWriter* SimpleFileSystem::createFileWriter(
 }
 
 FileSystemOperationInterface* SimpleFileSystem::GetNewOperation(
-    const WebURL& url) {
+    const WebURL& url, WebFileSystemCallbacks* callbacks) {
   return file_system_context_->CreateFileSystemOperation(
       GURL(url),
+      SimpleFileSystemCallbackDispatcher::Create(AsWeakPtr(), callbacks),
       base::MessageLoopProxy::current());
 }
-
-FileSystemOperationInterface::StatusCallback
-SimpleFileSystem::FinishHandler(WebFileSystemCallbacks* callbacks) {
-  return base::Bind(&SimpleFileSystem::DidFinish,
-                    AsWeakPtr(), base::Unretained(callbacks));
-}
-
-FileSystemOperationInterface::ReadDirectoryCallback
-SimpleFileSystem::ReadDirectoryHandler(WebFileSystemCallbacks* callbacks) {
-  return base::Bind(&SimpleFileSystem::DidReadDirectory,
-                    AsWeakPtr(), base::Unretained(callbacks));
-}
-
-FileSystemOperationInterface::GetMetadataCallback
-SimpleFileSystem::GetMetadataHandler(WebFileSystemCallbacks* callbacks) {
-  return base::Bind(&SimpleFileSystem::DidGetMetadata,
-                    AsWeakPtr(), base::Unretained(callbacks));
-}
-
-FileSystemContext::OpenFileSystemCallback
-SimpleFileSystem::OpenFileSystemHandler(WebFileSystemCallbacks* callbacks) {
-  return base::Bind(&SimpleFileSystem::DidOpenFileSystem,
-                    AsWeakPtr(), base::Unretained(callbacks));
-}
-
-void SimpleFileSystem::DidFinish(WebFileSystemCallbacks* callbacks,
-                                 base::PlatformFileError result) {
-  if (result == base::PLATFORM_FILE_OK)
-    callbacks->didSucceed();
-  else
-    callbacks->didFail(webkit_glue::PlatformFileErrorToWebFileError(result));
-}
-
-void SimpleFileSystem::DidGetMetadata(WebFileSystemCallbacks* callbacks,
-                                      base::PlatformFileError result,
-                                      const base::PlatformFileInfo& info,
-                                      const FilePath& platform_path) {
-  if (result == base::PLATFORM_FILE_OK) {
-    WebFileInfo web_file_info;
-    web_file_info.length = info.size;
-    web_file_info.modificationTime = info.last_modified.ToDoubleT();
-    web_file_info.type = info.is_directory ?
-        WebFileInfo::TypeDirectory : WebFileInfo::TypeFile;
-    web_file_info.platformPath =
-        webkit_glue::FilePathToWebString(platform_path);
-    callbacks->didReadMetadata(web_file_info);
-  } else {
-    callbacks->didFail(webkit_glue::PlatformFileErrorToWebFileError(result));
-  }
-}
-
-void SimpleFileSystem::DidReadDirectory(
-    WebFileSystemCallbacks* callbacks,
-    base::PlatformFileError result,
-    const std::vector<base::FileUtilProxy::Entry>& entries,
-    bool has_more) {
-  if (result == base::PLATFORM_FILE_OK) {
-    std::vector<WebFileSystemEntry> web_entries_vector;
-    for (std::vector<base::FileUtilProxy::Entry>::const_iterator it =
-            entries.begin(); it != entries.end(); ++it) {
-      WebFileSystemEntry entry;
-      entry.name = webkit_glue::FilePathStringToWebString(it->name);
-      entry.isDirectory = it->is_directory;
-      web_entries_vector.push_back(entry);
-    }
-    WebVector<WebKit::WebFileSystemEntry> web_entries = web_entries_vector;
-    callbacks->didReadDirectory(web_entries, has_more);
-  } else {
-    callbacks->didFail(webkit_glue::PlatformFileErrorToWebFileError(result));
-  }
-}
-
-void SimpleFileSystem::DidOpenFileSystem(
-    WebFileSystemCallbacks* callbacks,
-    base::PlatformFileError result,
-    const std::string& name, const GURL& root) {
-  if (result == base::PLATFORM_FILE_OK) {
-    if (!root.is_valid())
-      callbacks->didFail(WebKit::WebFileErrorSecurity);
-    else
-      callbacks->didOpenFileSystem(WebString::fromUTF8(name), root);
-  } else {
-    callbacks->didFail(webkit_glue::PlatformFileErrorToWebFileError(result));
-  }
-}
diff --git a/webkit/tools/test_shell/simple_file_system.h b/webkit/tools/test_shell/simple_file_system.h
index 81a61ab..82c2e01 100644
--- a/webkit/tools/test_shell/simple_file_system.h
+++ b/webkit/tools/test_shell/simple_file_system.h
@@ -10,8 +10,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebFileSystem.h"
-#include "webkit/fileapi/file_system_context.h"
-#include "webkit/fileapi/file_system_operation_interface.h"
 #include "webkit/fileapi/file_system_types.h"
 #include <vector>
 
@@ -23,6 +21,7 @@ class WebURL;
 
 namespace fileapi {
 class FileSystemContext;
+class FileSystemOperationInterface;
 }
 
 class SimpleFileSystem
@@ -83,31 +82,7 @@ class SimpleFileSystem
  private:
   // Helpers.
   fileapi::FileSystemOperationInterface* GetNewOperation(
-      const WebKit::WebURL& path);
-
-  // Callback Handlers
-  fileapi::FileSystemOperationInterface::StatusCallback FinishHandler(
-      WebKit::WebFileSystemCallbacks* callbacks);
-  fileapi::FileSystemOperationInterface::GetMetadataCallback GetMetadataHandler(
-      WebKit::WebFileSystemCallbacks* callbacks);
-  fileapi::FileSystemOperationInterface::ReadDirectoryCallback
-      ReadDirectoryHandler(WebKit::WebFileSystemCallbacks* callbacks);
-  fileapi::FileSystemContext::OpenFileSystemCallback OpenFileSystemHandler(
-      WebKit::WebFileSystemCallbacks* callbacks);
-  void DidFinish(WebKit::WebFileSystemCallbacks* callbacks,
-                 base::PlatformFileError result);
-  void DidGetMetadata(WebKit::WebFileSystemCallbacks* callbacks,
-                      base::PlatformFileError result,
-                      const base::PlatformFileInfo& info,
-                      const FilePath& platform_path);
-  void DidReadDirectory(
-      WebKit::WebFileSystemCallbacks* callbacks,
-      base::PlatformFileError result,
-      const std::vector<base::FileUtilProxy::Entry>& entries,
-      bool has_more);
-  void DidOpenFileSystem(WebKit::WebFileSystemCallbacks* callbacks,
-                         base::PlatformFileError result,
-                         const std::string& name, const GURL& root);
+      const WebKit::WebURL& path, WebKit::WebFileSystemCallbacks* callbacks);
 
   // A temporary directory for FileSystem API.
   ScopedTempDir file_system_dir_;
diff --git a/webkit/tools/test_shell/simple_file_writer.cc b/webkit/tools/test_shell/simple_file_writer.cc
index 251d8e9..cdd36b1 100644
--- a/webkit/tools/test_shell/simple_file_writer.cc
+++ b/webkit/tools/test_shell/simple_file_writer.cc
@@ -8,11 +8,13 @@
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "net/url_request/url_request_context.h"
+#include "webkit/fileapi/file_system_callback_dispatcher.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_operation_interface.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/tools/test_shell/simple_resource_loader_bridge.h"
 
+using fileapi::FileSystemCallbackDispatcher;
 using fileapi::FileSystemContext;
 using fileapi::FileSystemOperationInterface;
 using fileapi::WebFileWriterBase;
@@ -51,8 +53,7 @@ class SimpleFileWriter::IOThreadProxy
     }
     DCHECK(!operation_);
     operation_ = GetNewOperation(path);
-    operation_->Truncate(path, offset,
-                         base::Bind(&IOThreadProxy::DidFinish, this));
+    operation_->Truncate(path, offset);
   }
 
   void Write(const GURL& path, const GURL& blob_url, int64 offset) {
@@ -65,8 +66,7 @@ class SimpleFileWriter::IOThreadProxy
     DCHECK(request_context_);
     DCHECK(!operation_);
     operation_ = GetNewOperation(path);
-    operation_->Write(request_context_, path, blob_url, offset,
-                      base::Bind(&IOThreadProxy::DidWrite, this));
+    operation_->Write(request_context_, path, blob_url, offset);
   }
 
   void Cancel() {
@@ -77,45 +77,96 @@ class SimpleFileWriter::IOThreadProxy
       return;
     }
     if (!operation_) {
-      DidFailOnMainThread(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
+      DidFail(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
       return;
     }
-    operation_->Cancel(base::Bind(&IOThreadProxy::DidFinish, this));
+    operation_->Cancel(CallbackDispatcher::Create(this));
   }
 
  private:
+  // Inner class to receive callbacks from FileSystemOperation.
+  class CallbackDispatcher : public FileSystemCallbackDispatcher {
+   public:
+    // An instance of this class must be created by Create()
+    // (so that we do not leak ownerships).
+    static scoped_ptr<FileSystemCallbackDispatcher> Create(
+        IOThreadProxy* proxy) {
+      return scoped_ptr<FileSystemCallbackDispatcher>(
+          new CallbackDispatcher(proxy));
+    }
+
+    ~CallbackDispatcher() {
+      proxy_->ClearOperation();
+    }
+
+    virtual void DidSucceed() {
+      proxy_->DidSucceed();
+    }
+
+    virtual void DidFail(base::PlatformFileError error_code) {
+      proxy_->DidFail(error_code);
+    }
+
+    virtual void DidWrite(int64 bytes, bool complete) {
+      proxy_->DidWrite(bytes, complete);
+    }
+
+    virtual void DidReadMetadata(
+        const base::PlatformFileInfo&,
+        const FilePath&) {
+      NOTREACHED();
+    }
+
+    virtual void DidReadDirectory(
+        const std::vector<base::FileUtilProxy::Entry>& entries,
+        bool has_more) {
+      NOTREACHED();
+    }
+
+    virtual void DidOpenFileSystem(
+        const std::string& name,
+        const GURL& root) {
+      NOTREACHED();
+    }
+
+   private:
+    explicit CallbackDispatcher(IOThreadProxy* proxy) : proxy_(proxy) {}
+    scoped_refptr<IOThreadProxy> proxy_;
+  };
+
   FileSystemOperationInterface* GetNewOperation(const GURL& path) {
-    return file_system_context_->CreateFileSystemOperation(path, io_thread_);
+    // The FileSystemOperation takes ownership of the CallbackDispatcher.
+    return file_system_context_->CreateFileSystemOperation(
+        path, CallbackDispatcher::Create(this), io_thread_);
   }
 
-  void DidSucceedOnMainThread() {
+  void DidSucceed() {
     if (!main_thread_->BelongsToCurrentThread()) {
       main_thread_->PostTask(
           FROM_HERE,
-          base::Bind(&IOThreadProxy::DidSucceedOnMainThread, this));
+          base::Bind(&IOThreadProxy::DidSucceed, this));
       return;
     }
     if (simple_writer_)
       simple_writer_->DidSucceed();
   }
 
-  void DidFailOnMainThread(base::PlatformFileError error_code) {
+  void DidFail(base::PlatformFileError error_code) {
     if (!main_thread_->BelongsToCurrentThread()) {
       main_thread_->PostTask(
           FROM_HERE,
-          base::Bind(&IOThreadProxy::DidFailOnMainThread, this, error_code));
+          base::Bind(&IOThreadProxy::DidFail, this, error_code));
       return;
     }
     if (simple_writer_)
       simple_writer_->DidFail(error_code);
   }
 
-  void DidWriteOnMainThread(int64 bytes, bool complete) {
+  void DidWrite(int64 bytes, bool complete) {
     if (!main_thread_->BelongsToCurrentThread()) {
       main_thread_->PostTask(
           FROM_HERE,
-          base::Bind(&IOThreadProxy::DidWriteOnMainThread,
-                     this, bytes, complete));
+          base::Bind(&IOThreadProxy::DidWrite, this, bytes, complete));
       return;
     }
     if (simple_writer_)
@@ -127,25 +178,6 @@ class SimpleFileWriter::IOThreadProxy
     operation_ = NULL;
   }
 
-  void DidFinish(base::PlatformFileError result) {
-    if (result == base::PLATFORM_FILE_OK)
-      DidSucceedOnMainThread();
-    else
-      DidFailOnMainThread(result);
-    ClearOperation();
-  }
-
-  void DidWrite(base::PlatformFileError result, int64 bytes, bool complete) {
-    if (result == base::PLATFORM_FILE_OK) {
-      DidWriteOnMainThread(bytes, complete);
-      if (complete)
-        ClearOperation();
-    } else {
-      DidFailOnMainThread(result);
-      ClearOperation();
-    }
-  }
-
   scoped_refptr<base::MessageLoopProxy> io_thread_;
   scoped_refptr<base::MessageLoopProxy> main_thread_;