Implement copy and move operations within the same remote file system.

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
===================================================================
--- chrome/browser/chromeos/gdata/gdata_file_system.cc	(revision 126063)
+++ chrome/browser/chromeos/gdata/gdata_file_system.cc	(working copy)
@@ -14,16 +14,15 @@
 #include "base/platform_file.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_dependency_manager.h"
 #include "chrome/browser/chromeos/gdata/gdata.h"
 #include "chrome/browser/chromeos/gdata/gdata_download_observer.h"
 #include "chrome/browser/chromeos/gdata/gdata_parser.h"
-#include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_paths_internal.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_dependency_manager.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_paths_internal.h"
 #include "content/public/browser/browser_thread.h"
 #include "webkit/fileapi/file_system_file_util_proxy.h"
 #include "webkit/fileapi/file_system_types.h"
@@ -535,7 +534,17 @@
 GDataFileSystem::CreateDirectoryParams::~CreateDirectoryParams() {
 }
 
+// GDataFileSystem::CopyMoveFileParams struct implementation.
 
+GDataFileSystem::CopyMoveFileParams::CopyMoveFileParams()
+  : is_directory(false),
+    is_root_directory(false),
+    is_hosted_document(false) {
+}
+
+GDataFileSystem::CopyMoveFileParams::~CopyMoveFileParams() {
+}
+
 // GDataFileSystem class implementatsion.
 
 GDataFileSystem::GDataFileSystem(Profile* profile,
@@ -608,6 +617,239 @@
   ContinueDirectoryRefresh(params, feed_list.Pass());
 }
 
+bool GDataFileSystem::GetCopyMoveFileParams(const FilePath& path,
+                                            CopyMoveFileParams* params) {
+  DCHECK(params);
+
+  scoped_refptr<ReadOnlyFindFileDelegate> delegate(
+      new ReadOnlyFindFileDelegate());
+
+  base::AutoLock lock(lock_);
+
+  UnsafeFindFileByPath(path, delegate);
+  GDataFileBase* file = delegate->file();
+  if (!file)
+    return false;

[zel, 2012/03/12 20:40:40]

there is a helper method GetGDataFileInfoFromPath() that give you everything
from 624-632. We just need to make it thread safe, I can show you what's wrong
with it in other comments.

[Ben Chan, 2012/03/13 02:16:36]

Done.

+
+  params->resource_id = file->resource();
+  params->self_url = file->self_url();
+  params->content_url = file->content_url();
+  params->is_directory = file->AsGDataDirectory();
+  params->is_root_directory = file->AsGDataRootDirectory();
+
+  GDataFile* gdata_file = file->AsGDataFile();
+  if (gdata_file) {
+    params->is_hosted_document = gdata_file->is_hosted_document();
+    params->document_extension = gdata_file->document_extension();
+  }
+
+  return true;
+}
+
+void GDataFileSystem::Copy(const FilePath& src_file_path,
+                           const FilePath& dest_file_path,
+                           const FileOperationCallback& callback) {
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath dest_parent_path = dest_file_path.DirName();
+  CopyMoveFileParams src_file_params, dest_parent_params;
+  if (!GetCopyMoveFileParams(src_file_path, &src_file_params) ||
+      !GetCopyMoveFileParams(dest_parent_path, &dest_parent_params)) {
+    error = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  } else {
+    // TODO(benchan): Implement copy for regular files and directories.

[satorux1, 2012/03/12 17:48:24]

Please document this in the header file too. :) Please also file bugs and add
URLs to them.

[Ben Chan, 2012/03/13 00:29:21]

Clarified in the header. I'll keep the bug open until this is fixed.  To copy
regular files, it seems like we need to first download the file and then upload
it, which requires wiring the download/upload pipeline.

+    // In the interim, We handle regular file and directory copy in the
+    // file manager.

[satorux1, 2012/03/12 17:48:24]

How does this work?

[Ben Chan, 2012/03/13 00:29:21]

The file manager recursively traverses the directory being copied and invokes
FileEntry.copyTo on each file encountered. That way, the file manager can show
progress of the copy operations. We will probably use the same approach for
Gdata file system, regardless of whether or not directory copy being implemented
in GDataFileSystem::Copy().

[satorux1, 2012/03/13 01:12:17]

thanks.

+    if (!src_file_params.is_hosted_document) {
+      error = base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
+    } else if (!dest_parent_params.is_directory) {
+      error = base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+    }
+  }
+
+  if (error != base::PLATFORM_FILE_OK) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, error));
+    }
+    return;
+  }
+
+  FilePathUpdateCallback add_file_to_directory_callback =
+      base::Bind(&GDataFileSystem::AddFileToDirectory,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 dest_parent_path,
+                 callback);

[satorux1, 2012/03/12 17:48:24]

Shouldn't we pass MessageLoopProxy::current() so |callback| is run on the origin
thread?

[Ben Chan, 2012/03/13 00:29:21]

IIUC, the DocumentsService layer handles the transition between the origin
thread and the UI thread. Rename(), AddResourceToDirectory(), and
RemoveResourceFromDirectory() run on the same thread as Copy() and Move(), and
they use MessageLoop::current(). Do we still need to pass MessageLoop::current()
as an argument to the callback?

[satorux1, 2012/03/13 01:12:17]

Oh you are right. forget about my comments.

+
+  documents_service_->CopyDocument(
+      src_file_params.self_url,
+      // Drop the document extension, which should not be in the document title.
+      dest_file_path.BaseName().RemoveExtension().value(),
+      base::Bind(&GDataFileSystem::OnCopyDocumentCompleted,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 add_file_to_directory_callback));
+}
+
+void GDataFileSystem::Rename(const FilePath& file_path,
+                             const FilePath::StringType& new_name,
+                             const FilePathUpdateCallback& callback) {
+  // It is a no-op if the file is renamed to the same name.
+  if (file_path.BaseName().value() == new_name) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(
+          FROM_HERE, base::Bind(callback, base::PLATFORM_FILE_OK, file_path));
+    }
+    return;
+  }
+
+  CopyMoveFileParams file_params;
+  if (!GetCopyMoveFileParams(file_path, &file_params)) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          base::Bind(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND, file_path));
+    }
+    return;
+  }
+
+  // Drop the .g<something> extension from |new_name| if the file being
+  // renamed is a hosted document and |new_name| has the same .g<something>
+  // extension as the file.
+  std::string file_name = new_name;

[satorux1, 2012/03/12 17:48:24]

std::string -> FilePath::StringType

[Ben Chan, 2012/03/13 00:29:21]

Done.  Good catch!

+  if (file_params.is_hosted_document) {
+    FilePath new_file(file_name);
+    if (new_file.Extension() == file_params.document_extension) {
+      file_name = new_file.RemoveExtension().value();
+    }
+  }
+
+  documents_service_->RenameResource(
+      file_params.self_url,
+      file_name,
+      base::Bind(&GDataFileSystem::OnRenameResourceCompleted,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 file_path,
+                 file_name,
+                 callback));
+}
+
+void GDataFileSystem::Move(const FilePath& src_file_path,
+                           const FilePath& dest_file_path,
+                           const FileOperationCallback& callback) {
+  base::PlatformFileError error = base::PLATFORM_FILE_OK;
+  FilePath dest_parent_path = dest_file_path.DirName();
+  CopyMoveFileParams src_file_params, dest_parent_params;
+  if (!GetCopyMoveFileParams(src_file_path, &src_file_params) ||
+      !GetCopyMoveFileParams(dest_parent_path, &dest_parent_params)) {
+    error = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  } else {
+    if (!dest_parent_params.is_directory) {
+      error = base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+    }
+  }
+
+  if (error != base::PLATFORM_FILE_OK) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, error));
+    }
+    return;
+  }
+
+  if (src_file_path.DirName() == dest_parent_path) {

[satorux1, 2012/03/12 17:48:24]

Having a comment would make it a bit easier to read:

// If the file is moved to the same directory, just rename it.

[Ben Chan, 2012/03/13 00:29:21]

Done.

+    FilePathUpdateCallback final_file_path_update_callback =
+        base::Bind(&GDataFileSystem::OnFilePathUpdated,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   callback);

[satorux1, 2012/03/12 17:48:24]

Shouldn't we pass MessageLoopProxy::current() so the |callback| is run on the
origin thread?

[Ben Chan, 2012/03/13 00:29:21]

Rename(), AddResourceToDirectory(), and RemoveResourceFromDirectory() currently
run on the same thread as Copy() and Move(), and they all use
MessageLoop::current(). Should MessageLoop::current() be passed as an argument
instead?

+
+    Rename(src_file_path, dest_file_path.BaseName().value(),
+           final_file_path_update_callback);
+    return;
+  }
+
+  FilePathUpdateCallback add_file_to_directory_callback =

[satorux1, 2012/03/12 17:48:24]

This logic is a bit hard to follow. If I follow right, the code behaves like:

1) Rename the file to basename(dest_file_path) in the current directory
2) Remove it from the current directory
3) Add it to the target directory

Is this correct? If so, the step 1 looks unnecessary. Removing it first looks
scary. maybe we should add it first? Maybe I'm just not following right.

Please also add some comment about the behavior here to make it easier to
follow.

[Ben Chan, 2012/03/13 00:29:21]

Done. Added more details in comments.

+      base::Bind(&GDataFileSystem::AddFileToDirectory,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 dest_file_path.DirName(),
+                 callback);

[satorux1, 2012/03/12 17:48:24]

 MessageLoopProxy::current() ?

+
+  FilePathUpdateCallback remove_file_from_directory_callback =
+      base::Bind(&GDataFileSystem::RemoveFileFromDirectory,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 src_file_path.DirName(),
+                 add_file_to_directory_callback);
+
+  Rename(src_file_path, dest_file_path.BaseName().value(),
+         remove_file_from_directory_callback);
+}
+
+void GDataFileSystem::AddFileToDirectory(const FilePath& dir_path,
+                                         const FileOperationCallback& callback,
+                                         base::PlatformFileError error,
+                                         const FilePath& file_path) {
+  CopyMoveFileParams file_params, dir_params;
+  if (error == base::PLATFORM_FILE_OK) {
+    if (!GetCopyMoveFileParams(file_path, &file_params) ||
+        !GetCopyMoveFileParams(dir_path, &dir_params)) {
+      error = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+    } else {
+      if (!dir_params.is_directory) {

[zel, 2012/03/12 20:40:40]

nit: remove {}

[Ben Chan, 2012/03/13 00:29:21]

Done.

+        error = base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+      }
+    }
+  }
+
+  // Returns if there is an error or |dir_path| is the root directory.
+  if (error != base::PLATFORM_FILE_OK || dir_params.is_root_directory) {
+    if (!callback.is_null()) {

[zel, 2012/03/12 20:40:40]

nit: remove {}

[Ben Chan, 2012/03/13 00:29:21]

Done.

+      MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, error));
+    }
+    return;
+  }
+
+  documents_service_->AddResourceToDirectory(
+      dir_params.content_url,
+      file_params.self_url,
+      base::Bind(&GDataFileSystem::OnAddFileToDirectoryCompleted,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 callback,
+                 file_path,
+                 dir_path));
+}
+
+void GDataFileSystem::RemoveFileFromDirectory(
+    const FilePath& dir_path,
+    const FilePathUpdateCallback& callback,
+    base::PlatformFileError error,
+    const FilePath& file_path) {
+  CopyMoveFileParams file_params, dir_params;
+  if (error == base::PLATFORM_FILE_OK) {
+    if (!GetCopyMoveFileParams(file_path, &file_params) ||
+        !GetCopyMoveFileParams(dir_path, &dir_params)) {
+      error = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+    } else {
+      if (!dir_params.is_directory) {
+        error = base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+      }
+    }
+  }
+
+  // Returns if there is an error or |dir_path| is the root directory.
+  if (error != base::PLATFORM_FILE_OK || dir_params.is_root_directory) {
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          base::Bind(callback, error, file_path));
+    }
+    return;
+  }
+
+  documents_service_->RemoveResourceFromDirectory(
+      dir_params.content_url,
+      file_params.self_url,
+      file_params.resource_id,
+      base::Bind(&GDataFileSystem::OnRemoveFileFromDirectoryCompleted,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 callback,
+                 file_path,
+                 dir_path));
+}
+
 void GDataFileSystem::Remove(const FilePath& file_path,
     bool is_recursive,
     const FileOperationCallback& callback) {
@@ -973,6 +1215,110 @@
                  params.delegate);
 }
 
+void GDataFileSystem::OnFilePathUpdated(const FileOperationCallback& callback,
+                                        base::PlatformFileError error,
+                                        const FilePath& file_path) {
+  if (!callback.is_null())
+    callback.Run(error);
+}
+
+void GDataFileSystem::OnRenameResourceCompleted(
+    const FilePath& file_path,
+    const FilePath::StringType& new_name,
+    const FilePathUpdateCallback& callback,
+    GDataErrorCode status,
+    const GURL& document_url) {
+  FilePath updated_file_path;
+  base::PlatformFileError error = GDataToPlatformError(status);
+  if (error == base::PLATFORM_FILE_OK)
+    error = RenameFileOnFilesystem(file_path, new_name, &updated_file_path);
+
+  if (!callback.is_null())
+    callback.Run(error, updated_file_path);
+}
+
+void GDataFileSystem::OnCopyDocumentCompleted(
+    const FilePathUpdateCallback& callback,
+    GDataErrorCode status,
+    scoped_ptr<base::Value> data) {
+  base::PlatformFileError error = GDataToPlatformError(status);
+  if (error != base::PLATFORM_FILE_OK) {
+    if (!callback.is_null())
+      callback.Run(error, FilePath());
+
+    return;
+  }
+
+  base::DictionaryValue* dict_value = NULL;
+  base::Value* entry_value = NULL;
+  if (data.get() && data->GetAsDictionary(&dict_value) && dict_value)
+    dict_value->Get("entry", &entry_value);
+
+  if (!entry_value) {
+    if (!callback.is_null()) {
+      callback.Run(base::PLATFORM_FILE_ERROR_FAILED, FilePath());
+    }
+    return;
+  }
+
+  scoped_ptr<DocumentEntry> entry(DocumentEntry::CreateFrom(entry_value));
+  if (!entry.get()) {
+    if (!callback.is_null()) {
+      callback.Run(base::PLATFORM_FILE_ERROR_FAILED, FilePath());
+    }
+    return;
+  }
+
+  FilePath file_path;
+  {
+    base::AutoLock lock(lock_);
+    GDataFileBase* file =
+        GDataFileBase::FromDocumentEntry(root_.get(), entry.get());
+    if (!file) {
+      if (!callback.is_null()) {
+        callback.Run(base::PLATFORM_FILE_ERROR_FAILED, FilePath());
+      }
+      return;
+    }
+    root_->AddFile(file);
+    file_path = file->GetFilePath();
+  }
+
+  if (!callback.is_null()) {
+    callback.Run(error, file_path);
+  }
+}
+
+void GDataFileSystem::OnAddFileToDirectoryCompleted(
+    const FileOperationCallback& callback,
+    const FilePath& file_path,
+    const FilePath& dir_path,
+    GDataErrorCode status,
+    const GURL& document_url) {
+  base::PlatformFileError error = GDataToPlatformError(status);
+  if (error == base::PLATFORM_FILE_OK)
+    error = AddFileToDirectoryOnFilesystem(file_path, dir_path);
+
+  if (!callback.is_null())
+    callback.Run(error);
+}
+
+void GDataFileSystem::OnRemoveFileFromDirectoryCompleted(
+    const FilePathUpdateCallback& callback,
+    const FilePath& file_path,
+    const FilePath& dir_path,
+    GDataErrorCode status,
+    const GURL& document_url) {
+  FilePath updated_file_path = file_path;
+  base::PlatformFileError error = GDataToPlatformError(status);
+  if (error == base::PLATFORM_FILE_OK)
+    error = RemoveFileFromDirectoryOnFilesystem(file_path, dir_path,
+                                                &updated_file_path);
+
+  if (!callback.is_null())
+    callback.Run(error, updated_file_path);
+}
+
 void GDataFileSystem::SaveRootFeeds(scoped_ptr<base::ListValue> feed_vector) {
   BrowserThread::PostBlockingPoolTask(FROM_HERE,
       base::Bind(&GDataFileSystem::SaveRootFeedsOnIOThreadPool,
@@ -1037,6 +1383,98 @@
   }
 }
 
+base::PlatformFileError GDataFileSystem::RenameFileOnFilesystem(
+    const FilePath& file_path,
+    const FilePath::StringType& new_name,
+    FilePath* updated_file_path) {
+  DCHECK(updated_file_path);
+
+  base::AutoLock lock(lock_);
+
+  scoped_refptr<ReadOnlyFindFileDelegate> find_file_delegate(
+      new ReadOnlyFindFileDelegate());
+  UnsafeFindFileByPath(file_path, find_file_delegate);
+
+  GDataFileBase* file = find_file_delegate->file();
+  if (!file) {
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+
+  DCHECK(file->parent());
+  file->set_original_file_name(new_name);
+  // After changing the original file name, removing the file from its
+  // parent directory and then adding it back to go through the file name
+  // de-duplication.
+  file->parent()->MoveFile(file, file->parent());
+
+  *updated_file_path = file->GetFilePath();
+  return base::PLATFORM_FILE_OK;
+}
+
+base::PlatformFileError GDataFileSystem::AddFileToDirectoryOnFilesystem(
+    const FilePath& file_path, const FilePath& dir_path) {
+  base::AutoLock lock(lock_);
+
+  scoped_refptr<ReadOnlyFindFileDelegate> find_file_delegate(
+      new ReadOnlyFindFileDelegate());
+  UnsafeFindFileByPath(file_path, find_file_delegate);
+
+  GDataFileBase* file = find_file_delegate->file();
+  if (!file) {
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+
+  scoped_refptr<ReadOnlyFindFileDelegate> find_dir_delegate(
+      new ReadOnlyFindFileDelegate());
+  UnsafeFindFileByPath(dir_path, find_dir_delegate);
+  GDataFileBase* dir = find_dir_delegate->file();
+  if (!dir) {
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+  if (!dir->AsGDataDirectory()) {
+    return base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+  }
+
+  DCHECK_EQ(root_.get(), file->parent());
+  file->parent()->MoveFile(file, dir->AsGDataDirectory());
+
+  return base::PLATFORM_FILE_OK;
+}
+
+base::PlatformFileError GDataFileSystem::RemoveFileFromDirectoryOnFilesystem(
+    const FilePath& file_path, const FilePath& dir_path,
+    FilePath* updated_file_path) {
+  DCHECK(updated_file_path);
+
+  base::AutoLock lock(lock_);
+
+  scoped_refptr<ReadOnlyFindFileDelegate> find_file_delegate(
+      new ReadOnlyFindFileDelegate());
+  UnsafeFindFileByPath(file_path, find_file_delegate);
+
+  GDataFileBase* file = find_file_delegate->file();
+  if (!file) {
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+
+  scoped_refptr<ReadOnlyFindFileDelegate> find_dir_delegate(
+      new ReadOnlyFindFileDelegate());
+  UnsafeFindFileByPath(dir_path, find_dir_delegate);
+  GDataFileBase* dir = find_dir_delegate->file();
+  if (!dir) {
+    return base::PLATFORM_FILE_ERROR_NOT_FOUND;
+  }
+  if (!dir->AsGDataDirectory()) {
+    return base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY;
+  }
+
+  if (!dir->AsGDataDirectory()->MoveFile(file, root_.get()))
+    return base::PLATFORM_FILE_ERROR_FAILED;
+
+  *updated_file_path = file->GetFilePath();
+  return base::PLATFORM_FILE_OK;
+}
+
 base::PlatformFileError GDataFileSystem::RemoveFileFromFileSystem(
     const FilePath& file_path) {
   // We need to lock here as well (despite FindFileByPath lock) since directory