Simplifies CopyOrMoveOperationDelegate implementation.

RecursiveOperationDelegate supports PostProcessDirectory(), so
CopyOrMoveOperationDelegate implementation can be simplified with the method.
This is the preparation for preserving the last-modified-time.

BUG=282107
TEST=Ran content_unittests and unit_tests

Review URL: https://chromiumcodereview.appspot.com/23483055

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

5 files changed, 116 insertions, 139 deletions

diff --git a/webkit/browser/fileapi/copy_or_move_operation_delegate.cc b/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
index 9078773..43470ca 100644
--- a/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
+++ b/webkit/browser/fileapi/copy_or_move_operation_delegate.cc
@@ -325,15 +325,11 @@ void CopyOrMoveOperationDelegate::RunRecursively() {
     return;
   }
 
-  if (!progress_callback_.is_null()) {
-    progress_callback_.Run(
-        FileSystemOperation::BEGIN_COPY_ENTRY, src_root_, 0);
-  }
-
-  // First try to copy/move it as a file.
-  CopyOrMoveFile(src_root_, dest_root_,
-                 base::Bind(&CopyOrMoveOperationDelegate::DidTryCopyOrMoveFile,
-                            weak_factory_.GetWeakPtr()));
+  // Start to process the source directory recursively.
+  // TODO(kinuko): This could be too expensive for same_file_system_==true
+  // and operation==MOVE case, probably we can just rename the root directory.
+  // http://crbug.com/172187
+  StartRecursiveOperation(src_root_, callback_);
 }
 
 void CopyOrMoveOperationDelegate::ProcessFile(
@@ -342,60 +338,94 @@ void CopyOrMoveOperationDelegate::ProcessFile(
   if (!progress_callback_.is_null())
     progress_callback_.Run(FileSystemOperation::BEGIN_COPY_ENTRY, src_url, 0);
 
-  CopyOrMoveFile(src_url, CreateDestURL(src_url),
-                 base::Bind(&CopyOrMoveOperationDelegate::DidCopyEntry,
-                            weak_factory_.GetWeakPtr(), src_url, callback));
+  FileSystemURL dest_url = CreateDestURL(src_url);
+  CopyOrMoveImpl* impl = NULL;
+  if (same_file_system_) {
+    impl = new CopyOrMoveOnSameFileSystemImpl(
+        operation_runner(), operation_type_, src_url, dest_url,
+        base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
+                   weak_factory_.GetWeakPtr(), src_url));
+  } else {
+    // Cross filesystem case.
+    // TODO(hidehiko): Support stream based copy. crbug.com/279287.
+    base::PlatformFileError error = base::PLATFORM_FILE_ERROR_FAILED;
+    CopyOrMoveFileValidatorFactory* validator_factory =
+        file_system_context()->GetCopyOrMoveFileValidatorFactory(
+            dest_root_.type(), &error);
+    if (error != base::PLATFORM_FILE_OK) {
+      callback.Run(error);
+      return;
+    }
+
+    impl = new SnapshotCopyOrMoveImpl(
+        operation_runner(), operation_type_, src_url, dest_url,
+        validator_factory,
+        base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
+                   weak_factory_.GetWeakPtr(), src_url));
+  }
+
+  // Register the running task.
+  running_copy_set_.insert(impl);
+  impl->Run(base::Bind(&CopyOrMoveOperationDelegate::DidCopyOrMoveFile,
+                       weak_factory_.GetWeakPtr(), src_url, callback, impl));
 }
 
 void CopyOrMoveOperationDelegate::ProcessDirectory(
     const FileSystemURL& src_url,
     const StatusCallback& callback) {
-  FileSystemURL dest_url = CreateDestURL(src_url);
-
-  if (!progress_callback_.is_null() && src_url != src_root_) {
+  if (src_url == src_root_) {
+    // The src_root_ looks to be a directory.
+    // Try removing the dest_root_ to see if it exists and/or it is an
+    // empty directory.
     // We do not invoke |progress_callback_| for source root, because it is
-    // already called in RunRecursively().
-    progress_callback_.Run(FileSystemOperation::BEGIN_COPY_ENTRY, src_url, 0);
+    // already called in ProcessFile().
+    operation_runner()->RemoveDirectory(
+        dest_root_,
+        base::Bind(&CopyOrMoveOperationDelegate::DidTryRemoveDestRoot,
+                   weak_factory_.GetWeakPtr(), callback));
+    return;
   }
 
-  // If operation_type == Move we may need to record directories and
-  // restore directory timestamps in the end, though it may have
-  // negative performance impact.
-  // See http://crbug.com/171284 for more details.
-  operation_runner()->CreateDirectory(
-      dest_url, false /* exclusive */, false /* recursive */,
-      base::Bind(&CopyOrMoveOperationDelegate::DidCopyEntry,
-                 weak_factory_.GetWeakPtr(), src_url, callback));
+  if (!progress_callback_.is_null())
+    progress_callback_.Run(FileSystemOperation::BEGIN_COPY_ENTRY, src_url, 0);
+
+  ProcessDirectoryInternal(src_url, CreateDestURL(src_url), callback);
 }
 
 void CopyOrMoveOperationDelegate::PostProcessDirectory(
     const FileSystemURL& src_url,
     const StatusCallback& callback) {
-  callback.Run(base::PLATFORM_FILE_OK);
+  if (operation_type_ == OPERATION_COPY) {
+    callback.Run(base::PLATFORM_FILE_OK);
+    return;
+  }
+
+  DCHECK_EQ(OPERATION_MOVE, operation_type_);
+
+  // All files and subdirectories in the directory should be moved here,
+  // so remove the source directory for finalizing move operation.
+  operation_runner()->Remove(
+      src_url, false /* recursive */,
+      base::Bind(&CopyOrMoveOperationDelegate::DidRemoveSourceForMove,
+                 weak_factory_.GetWeakPtr(), callback));
 }
 
-void CopyOrMoveOperationDelegate::DidTryCopyOrMoveFile(
+void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
+    const FileSystemURL& src_url,
+    const StatusCallback& callback,
+    CopyOrMoveImpl* impl,
     base::PlatformFileError error) {
-  if (error != base::PLATFORM_FILE_ERROR_NOT_A_FILE) {
-    if (error == base::PLATFORM_FILE_OK && !progress_callback_.is_null()) {
-      progress_callback_.Run(
-          FileSystemOperation::END_COPY_ENTRY, src_root_, 0);
-    }
+  running_copy_set_.erase(impl);
+  delete impl;
 
-    callback_.Run(error);
-    return;
-  }
+  if (!progress_callback_.is_null() && error == base::PLATFORM_FILE_OK)
+    progress_callback_.Run(FileSystemOperation::END_COPY_ENTRY, src_url, 0);
 
-  // The src_root_ looks to be a directory.
-  // Try removing the dest_root_ to see if it exists and/or it is an
-  // empty directory.
-  operation_runner()->RemoveDirectory(
-      dest_root_,
-      base::Bind(&CopyOrMoveOperationDelegate::DidTryRemoveDestRoot,
-                 weak_factory_.GetWeakPtr()));
+  callback.Run(error);
 }
 
 void CopyOrMoveOperationDelegate::DidTryRemoveDestRoot(
+    const StatusCallback& callback,
     base::PlatformFileError error) {
   if (error == base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY) {
     callback_.Run(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
@@ -407,79 +437,24 @@ void CopyOrMoveOperationDelegate::DidTryRemoveDestRoot(
     return;
   }
 
-  // Start to process the source directory recursively.
-  // TODO(kinuko): This could be too expensive for same_file_system_==true
-  // and operation==MOVE case, probably we can just rename the root directory.
-  // http://crbug.com/172187
-  StartRecursiveOperation(
-      src_root_,
-      base::Bind(&CopyOrMoveOperationDelegate::DidFinishRecursiveCopyDir,
-                 weak_factory_.GetWeakPtr(), src_root_, callback_));
+  ProcessDirectoryInternal(src_root_, dest_root_, callback);
 }
 
-void CopyOrMoveOperationDelegate::DidFinishRecursiveCopyDir(
-    const FileSystemURL& src,
-    const StatusCallback& callback,
-    base::PlatformFileError error) {
-  if (error != base::PLATFORM_FILE_OK ||
-      operation_type_ == OPERATION_COPY) {
-    callback.Run(error);
-    return;
-  }
-
-  DCHECK_EQ(OPERATION_MOVE, operation_type_);
-
-  // Remove the source for finalizing move operation.
-  operation_runner()->Remove(
-      src, true /* recursive */,
-      base::Bind(&CopyOrMoveOperationDelegate::DidRemoveSourceForMove,
-                 weak_factory_.GetWeakPtr(), callback));
-}
-
-void CopyOrMoveOperationDelegate::DidRemoveSourceForMove(
-    const StatusCallback& callback,
-    base::PlatformFileError error) {
-  if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
-    error = base::PLATFORM_FILE_OK;
-  callback.Run(error);
-}
-
-void CopyOrMoveOperationDelegate::CopyOrMoveFile(
+void CopyOrMoveOperationDelegate::ProcessDirectoryInternal(
     const FileSystemURL& src_url,
     const FileSystemURL& dest_url,
     const StatusCallback& callback) {
-  CopyOrMoveImpl* impl = NULL;
-  if (same_file_system_) {
-    impl = new CopyOrMoveOnSameFileSystemImpl(
-        operation_runner(), operation_type_, src_url, dest_url,
-        base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
-                   weak_factory_.GetWeakPtr(), src_url));
-  } else {
-    // Cross filesystem case.
-    // TODO(hidehiko): Support stream based copy. crbug.com/279287.
-    base::PlatformFileError error = base::PLATFORM_FILE_ERROR_FAILED;
-    CopyOrMoveFileValidatorFactory* validator_factory =
-        file_system_context()->GetCopyOrMoveFileValidatorFactory(
-            dest_root_.type(), &error);
-    if (error != base::PLATFORM_FILE_OK) {
-      callback.Run(error);
-      return;
-    }
-
-    impl = new SnapshotCopyOrMoveImpl(
-        operation_runner(), operation_type_, src_url, dest_url,
-        validator_factory,
-        base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
-                   weak_factory_.GetWeakPtr(), src_url));
-  }
-
-  // Register the running task.
-  running_copy_set_.insert(impl);
-  impl->Run(base::Bind(&CopyOrMoveOperationDelegate::DidCopyOrMoveFile,
-                       weak_factory_.GetWeakPtr(), impl, callback));
+  // If operation_type == Move we may need to record directories and
+  // restore directory timestamps in the end, though it may have
+  // negative performance impact.
+  // See http://crbug.com/171284 for more details.
+  operation_runner()->CreateDirectory(
+      dest_url, false /* exclusive */, false /* recursive */,
+      base::Bind(&CopyOrMoveOperationDelegate::DidCreateDirectory,
+                 weak_factory_.GetWeakPtr(), src_url, callback));
 }
 
-void CopyOrMoveOperationDelegate::DidCopyEntry(
+void CopyOrMoveOperationDelegate::DidCreateDirectory(
     const FileSystemURL& src_url,
     const StatusCallback& callback,
     base::PlatformFileError error) {
@@ -489,12 +464,11 @@ void CopyOrMoveOperationDelegate::DidCopyEntry(
   callback.Run(error);
 }
 
-void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
-    CopyOrMoveImpl* impl,
+void CopyOrMoveOperationDelegate::DidRemoveSourceForMove(
     const StatusCallback& callback,
     base::PlatformFileError error) {
-  running_copy_set_.erase(impl);
-  delete impl;
+  if (error == base::PLATFORM_FILE_ERROR_NOT_FOUND)
+    error = base::PLATFORM_FILE_OK;
   callback.Run(error);
 }
 
diff --git a/webkit/browser/fileapi/copy_or_move_operation_delegate.h b/webkit/browser/fileapi/copy_or_move_operation_delegate.h
index b452c22..7b4fb7e 100644
--- a/webkit/browser/fileapi/copy_or_move_operation_delegate.h
+++ b/webkit/browser/fileapi/copy_or_move_operation_delegate.h
@@ -51,29 +51,22 @@ class CopyOrMoveOperationDelegate
                                     const StatusCallback& callback) OVERRIDE;
 
  private:
-  void DidTryCopyOrMoveFile(base::PlatformFileError error);
-  void DidTryRemoveDestRoot(base::PlatformFileError error);
-  void DidFinishRecursiveCopyDir(const FileSystemURL& src,
-                                 const StatusCallback& callback,
-                                 base::PlatformFileError error);
+  void DidCopyOrMoveFile(const FileSystemURL& src_url,
+                         const StatusCallback& callback,
+                         CopyOrMoveImpl* impl,
+                         base::PlatformFileError error);
+  void DidTryRemoveDestRoot(const StatusCallback& callback,
+                            base::PlatformFileError error);
+  void ProcessDirectoryInternal(const FileSystemURL& src_url,
+                                const FileSystemURL& dest_url,
+                                const StatusCallback& callback);
+  void DidCreateDirectory(const FileSystemURL& src_url,
+                          const StatusCallback& callback,
+                          base::PlatformFileError error);
   void DidRemoveSourceForMove(const StatusCallback& callback,
                               base::PlatformFileError error);
 
-  void DidCopyEntry(const FileSystemURL& src_url,
-                    const StatusCallback& callback,
-                    base::PlatformFileError error);
   void OnCopyFileProgress(const FileSystemURL& src_url, int64 size);
-
-  // Starts Copy (or Move based on |operation_type_|) from |src_url| to
-  // |dest_url|. Upon completion |callback| is invoked.
-  // This can be run for multiple files in parallel.
-  void CopyOrMoveFile(const FileSystemURL& src_url,
-                      const FileSystemURL& dest_url,
-                      const StatusCallback& callback);
-  void DidCopyOrMoveFile(CopyOrMoveImpl* impl,
-                         const StatusCallback& callback,
-                         base::PlatformFileError error);
-
   FileSystemURL CreateDestURL(const FileSystemURL& src_url) const;
 
   FileSystemURL src_root_;
diff --git a/webkit/browser/fileapi/recursive_operation_delegate.cc b/webkit/browser/fileapi/recursive_operation_delegate.cc
index e2bcdc4..81f26ff 100644
--- a/webkit/browser/fileapi/recursive_operation_delegate.cc
+++ b/webkit/browser/fileapi/recursive_operation_delegate.cc
@@ -56,12 +56,7 @@ void RecursiveOperationDelegate::DidTryProcessFile(
   DCHECK_EQ(1, inflight_operations_);
 
   --inflight_operations_;
-  // If the operation for a file is not permitted, the operation may fail
-  // with SECURITY, even if the path points a directory and the operation is
-  // permitted for a directory.
-  if (canceled_ ||
-      (error != base::PLATFORM_FILE_ERROR_NOT_A_FILE &&
-       error != base::PLATFORM_FILE_ERROR_SECURITY)) {
+  if (canceled_ || error != base::PLATFORM_FILE_ERROR_NOT_A_FILE) {
     Done(error);
     return;
   }
diff --git a/webkit/browser/fileapi/remove_operation_delegate.cc b/webkit/browser/fileapi/remove_operation_delegate.cc
index b053960..4819f01 100644
--- a/webkit/browser/fileapi/remove_operation_delegate.cc
+++ b/webkit/browser/fileapi/remove_operation_delegate.cc
@@ -51,11 +51,24 @@ void RemoveOperationDelegate::PostProcessDirectory(
 
 void RemoveOperationDelegate::DidTryRemoveFile(
     base::PlatformFileError error) {
-  if (error != base::PLATFORM_FILE_ERROR_NOT_A_FILE) {
+  if (error != base::PLATFORM_FILE_ERROR_NOT_A_FILE &&
+      error != base::PLATFORM_FILE_ERROR_SECURITY) {
     callback_.Run(error);
     return;
   }
-  operation_runner()->RemoveDirectory(url_, callback_);
+  operation_runner()->RemoveDirectory(
+      url_,
+      base::Bind(&RemoveOperationDelegate::DidTryRemoveDirectory,
+                 weak_factory_.GetWeakPtr(), error));
+}
+
+void RemoveOperationDelegate::DidTryRemoveDirectory(
+    base::PlatformFileError remove_file_error,
+    base::PlatformFileError remove_directory_error) {
+  callback_.Run(
+      remove_directory_error == base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY ?
+      remove_file_error :
+      remove_directory_error);
 }
 
 void RemoveOperationDelegate::DidRemoveFile(const StatusCallback& callback,
diff --git a/webkit/browser/fileapi/remove_operation_delegate.h b/webkit/browser/fileapi/remove_operation_delegate.h
index bee2446..22d25cb 100644
--- a/webkit/browser/fileapi/remove_operation_delegate.h
+++ b/webkit/browser/fileapi/remove_operation_delegate.h
@@ -30,6 +30,8 @@ class RemoveOperationDelegate : public RecursiveOperationDelegate {
 
  private:
   void DidTryRemoveFile(base::PlatformFileError error);
+  void DidTryRemoveDirectory(base::PlatformFileError remove_file_error,
+                             base::PlatformFileError remove_directory_error);
   void DidRemoveFile(const StatusCallback& callback,
                      base::PlatformFileError error);