Turn DriveFS directory deletes into moves into a trash directory.

Change NativeFileUtil to allow moving a directory to an existing
directory.

Bug: 829696
Change-Id: I6e233fa2a9ad161d1457d06fc6b6eda3ab3955d7
Reviewed-on: https://chromium-review.googlesource.com/c/1278624Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Sergei Datsenko <dats@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600283}

chrome/browser/chromeos/drive/fileapi/drivefs_async_file_util.cc

@@ -12,14 +12,19 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
+#include "storage/browser/fileapi/file_system_context.h"
+#include "storage/browser/fileapi/file_system_operation.h"
 #include "storage/browser/fileapi/file_system_operation_context.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "storage/browser/fileapi/local_file_util.h"
+#include "storage/common/fileapi/file_system_util.h"
 
 namespace drive {
 namespace internal {
 namespace {
 
+constexpr char kTrashDirectoryName[] = ".Trash";
+
 class CopyOperation {
  public:
   CopyOperation(
@@ -144,5 +149,25 @@ void DriveFsAsyncFileUtil::CopyFileLocal(
               weak_factory_.GetWeakPtr()))));
 }
 
+void DriveFsAsyncFileUtil::DeleteDirectory(
+    std::unique_ptr<storage::FileSystemOperationContext> context,
+    const storage::FileSystemURL& url,
+    StatusCallback callback) {
+  DeleteRecursively(std::move(context), url, std::move(callback));
+}
+
+void DriveFsAsyncFileUtil::DeleteRecursively(
+    std::unique_ptr<storage::FileSystemOperationContext> context,
+    const storage::FileSystemURL& url,
+    StatusCallback callback) {
+  auto dest_url = context->file_system_context()->CreateCrackedFileSystemURL(
+      url.origin(), url.mount_type(),
+      base::FilePath(url.filesystem_id())
+          .Append(kTrashDirectoryName)
+          .Append(url.virtual_path().BaseName()));
+  MoveFileLocal(std::move(context), url, dest_url,
+                storage::FileSystemOperation::OPTION_NONE, std::move(callback));
+}
+
 }  // namespace internal
 }  // namespace drive

chrome/browser/chromeos/drive/fileapi/drivefs_async_file_util.h

@@ -31,6 +31,14 @@ class DriveFsAsyncFileUtil : public storage::AsyncFileUtilAdapter {
       CopyOrMoveOption option,
       CopyFileProgressCallback progress_callback,
       StatusCallback callback) override;
+  void DeleteDirectory(
+      std::unique_ptr<storage::FileSystemOperationContext> context,
+      const storage::FileSystemURL& url,
+      StatusCallback callback) override;
+  void DeleteRecursively(
+      std::unique_ptr<storage::FileSystemOperationContext> context,
+      const storage::FileSystemURL& url,
+      StatusCallback callback) override;
 
  private:
   Profile* const profile_;

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -911,6 +911,7 @@ class DriveFsTestVolume : public DriveTestVolume {
   CreateDriveFsConnectionDelegate() override {
     CHECK(base::CreateDirectory(GetMyDrivePath()));
     CHECK(base::CreateDirectory(GetTeamDriveGrandRoot()));
+    CHECK(base::CreateDirectory(mount_path().Append(".Trash")));
 
     if (!fake_drivefs_helper_) {
       fake_drivefs_helper_ =

storage/browser/fileapi/native_file_util.cc

@@ -10,6 +10,7 @@
 
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
+#include "build/build_config.h"
 #include "storage/browser/fileapi/file_system_operation_context.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "storage/common/fileapi/file_system_mount_option.h"
@@ -262,8 +263,17 @@ base::File::Error NativeFileUtil::CopyOrMoveFile(
   if (error != base::File::FILE_OK &&
       error != base::File::FILE_ERROR_NOT_FOUND)
     return error;
-  if (error == base::File::FILE_OK && (info.is_directory || src_is_directory))
-    return base::File::FILE_ERROR_INVALID_OPERATION;
+  if (error == base::File::FILE_OK) {
+    if (info.is_directory != src_is_directory)
+      return base::File::FILE_ERROR_INVALID_OPERATION;
+#if defined(OS_WIN)
+    // Overwriting an empty directory with another directory isn't supported
+    // natively on Windows, so treat this an unsupported. A higher layer is
+    // responsible for handling it.
+    if (info.is_directory)
+      return base::File::FILE_ERROR_NOT_A_FILE;
+#endif
+  }
   if (error == base::File::FILE_ERROR_NOT_FOUND) {
     error = NativeFileUtil::GetFileInfo(dest_path.DirName(), &info);
     if (error != base::File::FILE_OK)

storage/browser/fileapi/native_file_util_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
+#include "build/build_config.h"
 #include "storage/browser/fileapi/native_file_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -335,10 +336,12 @@ TEST_F(NativeFileUtilTest, MoveFile) {
             NativeFileUtil::CopyOrMoveFile(
                 dir, to_file, FileSystemOperation::OPTION_NONE, move));
 
+#if defined(OS_WIN)
   // Source is a directory, destination is a directory.
-  EXPECT_EQ(base::File::FILE_ERROR_INVALID_OPERATION,
+  EXPECT_EQ(base::File::FILE_ERROR_NOT_A_FILE,
             NativeFileUtil::CopyOrMoveFile(
                 dir, dir2, FileSystemOperation::OPTION_NONE, move));
+#endif
 
   ASSERT_EQ(base::File::FILE_OK,
             NativeFileUtil::EnsureFileExists(from_file, &created));
@@ -372,8 +375,37 @@ TEST_F(NativeFileUtilTest, MoveFile_Directory) {
   const NativeFileUtil::CopyOrMoveMode move = NativeFileUtil::MOVE;
   bool created = false;
   ASSERT_EQ(base::File::FILE_OK,
-            NativeFileUtil::EnsureFileExists(
-                from_directory.AppendASCII("fromfile"), &created));
+            NativeFileUtil::EnsureFileExists(from_file, &created));
+  ASSERT_TRUE(created);
+
+  ASSERT_EQ(base::File::FILE_OK, NativeFileUtil::Truncate(from_file, 1020));
+
+  EXPECT_TRUE(FileExists(from_file));
+  EXPECT_EQ(1020, GetSize(from_file));
+
+  ASSERT_EQ(base::File::FILE_OK, NativeFileUtil::CopyOrMoveFile(
+                                     from_directory, to_directory,
+                                     FileSystemOperation::OPTION_NONE, move));
+
+  EXPECT_FALSE(base::DirectoryExists(from_directory));
+  EXPECT_FALSE(FileExists(from_file));
+  EXPECT_TRUE(base::DirectoryExists(to_directory));
+  EXPECT_TRUE(FileExists(to_file));
+  EXPECT_EQ(1020, GetSize(to_file));
+}
+
+#if !defined(OS_WIN)
+TEST_F(NativeFileUtilTest, MoveFile_OverwriteEmptyDirectory) {
+  base::FilePath from_directory = Path("fromdirectory");
+  base::FilePath to_directory = Path("todirectory");
+  base::FilePath from_file = from_directory.AppendASCII("fromfile");
+  base::FilePath to_file = to_directory.AppendASCII("fromfile");
+  ASSERT_TRUE(base::CreateDirectory(from_directory));
+  ASSERT_TRUE(base::CreateDirectory(to_directory));
+  const NativeFileUtil::CopyOrMoveMode move = NativeFileUtil::MOVE;
+  bool created = false;
+  ASSERT_EQ(base::File::FILE_OK,
+            NativeFileUtil::EnsureFileExists(from_file, &created));
   ASSERT_TRUE(created);
 
   ASSERT_EQ(base::File::FILE_OK, NativeFileUtil::Truncate(from_file, 1020));
@@ -391,6 +423,7 @@ TEST_F(NativeFileUtilTest, MoveFile_Directory) {
   EXPECT_TRUE(FileExists(to_file));
   EXPECT_EQ(1020, GetSize(to_file));
 }
+#endif
 
 TEST_F(NativeFileUtilTest, PreserveLastModified) {
   base::FilePath from_file = Path("fromfile");