Reland "Turn DriveFS directory deletes into moves into a trash directory."

This is a reland of 9ecd235870 that:
- Updates FakeDriveFs to create a .Trash directory.
- Move files directly instead of delegating to MoveFileLocal.
- Leave (non-recursive) DeleteDirectory unchanged.

Original change's description:
> 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/1278624
> Reviewed-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}

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

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

@@ -6,20 +6,26 @@
 
 #include <utility>
 
+#include "base/files/file_util.h"
 #include "base/task/post_task.h"
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #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(
@@ -117,6 +123,67 @@ class CopyOperation {
   DISALLOW_COPY_AND_ASSIGN(CopyOperation);
 };
 
+// Recursively deletes a folder by moving it into the .Trash folder within the
+// DriveFS mount point.
+class DeleteOperation {
+ public:
+  DeleteOperation(Profile* profile,
+                  const base::FilePath& path,
+                  storage::AsyncFileUtil::StatusCallback callback,
+                  scoped_refptr<base::SequencedTaskRunner> origin_task_runner,
+                  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
+      : profile_(profile),
+        path_(path),
+        callback_(std::move(callback)),
+        origin_task_runner_(std::move(origin_task_runner)),
+        blocking_task_runner_(std::move(blocking_task_runner)) {
+    DCHECK(origin_task_runner_->RunsTasksInCurrentSequence());
+  }
+
+  void Start() {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+    auto* drive_integration_service =
+        drive::util::GetIntegrationServiceByProfile(profile_);
+    base::FilePath relative_path;
+    if (!drive_integration_service ||
+        !drive_integration_service->GetMountPointPath().IsParent(path_)) {
+      origin_task_runner_->PostTask(
+          FROM_HERE,
+          base::BindOnce(std::move(callback_), base::File::FILE_ERROR_FAILED));
+      origin_task_runner_->DeleteSoon(FROM_HERE, this);
+      return;
+    }
+
+    path_in_trash_ = drive_integration_service->GetMountPointPath()
+                         .Append(kTrashDirectoryName)
+                         .Append(path_.BaseName());
+
+    blocking_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&DeleteOperation::MoveToTrash, base::Unretained(this)));
+  }
+
+  void MoveToTrash() {
+    base::File::Error error = base::Move(path_, path_in_trash_)
+                                  ? base::File::FILE_OK
+                                  : base::File::FILE_ERROR_FAILED;
+    origin_task_runner_->PostTask(FROM_HERE,
+                                  base::BindOnce(std::move(callback_), error));
+    origin_task_runner_->DeleteSoon(FROM_HERE, this);
+  }
+
+  Profile* const profile_;
+  const base::FilePath path_;
+  storage::AsyncFileUtil::StatusCallback callback_;
+  scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
+  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
+
+  base::FilePath path_in_trash_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeleteOperation);
+};
+
 }  // namespace
 
 DriveFsAsyncFileUtil::DriveFsAsyncFileUtil(Profile* profile)
@@ -144,5 +211,18 @@ void DriveFsAsyncFileUtil::CopyFileLocal(
               weak_factory_.GetWeakPtr()))));
 }
 
+void DriveFsAsyncFileUtil::DeleteRecursively(
+    std::unique_ptr<storage::FileSystemOperationContext> context,
+    const storage::FileSystemURL& url,
+    StatusCallback callback) {
+  base::PostTaskWithTraits(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&DeleteOperation::Start,
+                     base::Unretained(new DeleteOperation(
+                         profile_, url.path(), std::move(callback),
+                         base::SequencedTaskRunnerHandle::Get(),
+                         context->task_runner()))));
+}
+
 }  // namespace internal
 }  // namespace drive

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

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

chromeos/components/drivefs/fake_drivefs.cc

@@ -287,6 +287,7 @@ void FakeDriveFs::SetMetadata(const base::FilePath& path,
 void FakeDriveFs::Init(drivefs::mojom::DriveFsConfigurationPtr config,
                        drivefs::mojom::DriveFsRequest drive_fs_request,
                        drivefs::mojom::DriveFsDelegatePtr delegate) {
+  CHECK(base::CreateDirectory(mount_path_.Append(".Trash")));
   mojo::FuseInterface(std::move(pending_delegate_request_),
                       delegate.PassInterface());
   if (binding_.is_bound())

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,7 @@ 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));
@@ -392,6 +394,37 @@ TEST_F(NativeFileUtilTest, MoveFile_Directory) {
   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));
+
+  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));
+}
+#endif
+
 TEST_F(NativeFileUtilTest, PreserveLastModified) {
   base::FilePath from_file = Path("fromfile");
   base::FilePath to_file1 = Path("tofile1");