[FS] Use MoveFileLocal instead of Move to rename a file when write completes.

This ensures we always do a rename instead of a copy+delete. Also losens
the checks in MoveFileLocal to actually allow this, FileSystemURL::IsSameFileSystem
is stricter than necessary, as long as two FileSystemURLs share the
same origin and final file system type a local move should be fine, regardless
of what filesystem_id either URL has.

Bug: 1120291
Change-Id: Ic06bbbb9a7b03c856272e1fb1e50dd026606e448
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2369352Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800715}

content/browser/native_file_system/native_file_system_file_writer_impl.cc

@@ -558,8 +558,8 @@ void NativeFileSystemFileWriterImpl::DidPassAfterWriteCheck(
         weak_factory_.GetWeakPtr(), std::move(callback));
   }
   DoFileSystemOperation(
-      FROM_HERE, &FileSystemOperationRunner::Move, std::move(result_callback),
-      swap_url(), url(),
+      FROM_HERE, &FileSystemOperationRunner::MoveFileLocal,
+      std::move(result_callback), swap_url(), url(),
       storage::FileSystemOperation::OPTION_PRESERVE_LAST_MODIFIED);
 }
 

content/browser/native_file_system/native_file_system_file_writer_impl_unittest.cc

@@ -749,7 +749,7 @@ TEST_F(NativeFileSystemFileWriterAfterWriteChecksTest,
   base::RunLoop move_loop;
   test_file_system_backend_->SetOperationCreatedCallback(
       base::BindLambdaForTesting([&](const storage::FileSystemURL& url) {
-        EXPECT_EQ(url, test_file_url_);
+        EXPECT_EQ(url, test_swap_url_);
         move_loop.Quit();
       }));
   move_loop.Run();

storage/browser/file_system/file_system_operation_impl.cc

@@ -330,7 +330,12 @@ void FileSystemOperationImpl::CopyFileLocal(
     const CopyFileProgressCallback& progress_callback,
     StatusCallback callback) {
   DCHECK(SetPendingOperationType(kOperationCopy));
-  DCHECK(src_url.IsInSameFileSystem(dest_url));
+  // Don't just DCHECK src_url.IsInSameFileSystem(dest_url). We don't care if
+  // the two URLs are mounted in two different isolated file systems. As long
+  // as their origin and type are the same, they are part of the same file
+  // system, and local operations are allowed.
+  DCHECK_EQ(src_url.origin(), dest_url.origin());
+  DCHECK_EQ(src_url.type(), dest_url.type());
 
   auto repeatable_callback =
       base::AdaptCallbackForRepeating(std::move(callback));
@@ -347,7 +352,12 @@ void FileSystemOperationImpl::MoveFileLocal(const FileSystemURL& src_url,
                                             CopyOrMoveOption option,
                                             StatusCallback callback) {
   DCHECK(SetPendingOperationType(kOperationMove));
-  DCHECK(src_url.IsInSameFileSystem(dest_url));
+  // Don't just DCHECK src_url.IsInSameFileSystem(dest_url). We don't care if
+  // the two URLs are mounted in two different isolated file systems. As long
+  // as their origin and type are the same, they are part of the same file
+  // system, and local operations are allowed.
+  DCHECK_EQ(src_url.origin(), dest_url.origin());
+  DCHECK_EQ(src_url.type(), dest_url.type());
 
   auto repeatable_callback =
       base::AdaptCallbackForRepeating(std::move(callback));