Add FileSystemHandle::move() and FileSystemHandle::rename() methods

Currently, it is not possible to move or rename a file or directory without creating a new file/directory, copying over data (recursively, in the case of a directory), and removing the original. This CL allows for the atomic moving of a file or directory on the local file system without needing to duplicate data. Moves to non-local file systems will are not guaranteed to be atomic and will involve duplicating data. PR: WICG/file-system-access#317 Bug: 1140805 Change-Id: I774ed1d9616249b6ecc80783db48a7bfee915aab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2984739 Commit-Queue: Austin Sullivan <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Reviewed-by: Victor Costan <[email protected]> Reviewed-by: Marijn Kruisselbrink <[email protected]> Cr-Commit-Position: refs/heads/main@{#919810} NOKEYCHECK=True GitOrigin-RevId: 89be8a84bf5c962fa363bd093f4f125ac923838c
mjfroman · Sep 9, 2021 · 69608c4 · 69608c4
1 parent ac7285c
commit 69608c4
Show file tree

Hide file tree

Showing 16 changed files with 515 additions and 0 deletions.
diff --git a/blink/public/mojom/file_system_access/file_system_access_directory_handle.mojom b/blink/public/mojom/file_system_access/file_system_access_directory_handle.mojom
@@ -66,6 +66,20 @@ interface FileSystemAccessDirectoryHandle {
   // Returns all the direct children of this directory.
   GetEntries(pending_remote<FileSystemAccessDirectoryEntriesListener> listener);
 
+  // Renames the directory represented by this handle to `new_entry_name`, which
+  // cannot be empty. This is guaranteed to be atomic.
+  // Returns an error if the directory does not exist.
+  Rename(string new_entry_name) => (FileSystemAccessError result);
+
+  // Same as above, but `destination_directory` is resolved into a directory to
+  // be used as the destination directory for the move operation. This is only
+  // guaranteed to be atomic if the directory is moved within the same file
+  // system. Moving across file systems may result in partially written data if
+  // the move fails. Returns an error if the directory does not exist or the
+  // move operation fails.
+  Move(pending_remote<FileSystemAccessTransferToken> destination_directory,
+                      string new_entry_name) => (FileSystemAccessError result);
+
   // Deletes the directory represented by this handle.
   // To delete recursively, set `recurse` to true.
   // Returns an error if the directory does not exist or is not empty and

diff --git a/blink/public/mojom/file_system_access/file_system_access_file_handle.mojom b/blink/public/mojom/file_system_access/file_system_access_file_handle.mojom
@@ -66,6 +66,19 @@ interface FileSystemAccessFileHandle {
   CreateFileWriter(bool keep_existing_data, bool auto_close) => (
       FileSystemAccessError result, pending_remote<FileSystemAccessFileWriter>? writer);
 
+  // Renames the file represented by this handle to `new_entry_name`, which
+  // cannot be empty. This is guaranteed to be atomic.
+  // Returns an error if the file does not exist.
+  Rename(string new_entry_name) => (FileSystemAccessError result);
+
+  // Same as above, but `destination_directory` is resolved into a directory to
+  // be used as the destination directory for the move operation. This is only
+  // guaranteed to be atomic if the file is moved within the same file system.
+  // Moving across file systems may result in partially written data if the move
+  // fails. Returns an error if the file does not exist or the operation fails.
+  Move(pending_remote<FileSystemAccessTransferToken> destination_directory,
+       string new_entry_name) => (FileSystemAccessError result);
+
   // Deletes the file represented by this handle.
   // Returns an error if the file does not exist.
   Remove() => (FileSystemAccessError result);

diff --git a/blink/renderer/modules/file_system_access/file_system_directory_handle.cc b/blink/renderer/modules/file_system_access/file_system_directory_handle.cc
@@ -25,6 +25,7 @@
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
@@ -241,6 +242,33 @@ void FileSystemDirectoryHandle::RequestPermissionImpl(
   mojo_ptr_->RequestPermission(writable, std::move(callback));
 }
 
+void FileSystemDirectoryHandle::RenameImpl(
+    const String& new_entry_name,
+    base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {
+  if (!mojo_ptr_.is_bound()) {
+    std::move(callback).Run(mojom::blink::FileSystemAccessError::New(
+        mojom::blink::FileSystemAccessStatus::kInvalidState,
+        base::File::Error::FILE_ERROR_FAILED, "Context Destroyed"));
+    return;
+  }
+
+  mojo_ptr_->Rename(new_entry_name, std::move(callback));
+}
+
+void FileSystemDirectoryHandle::MoveImpl(
+    mojo::PendingRemote<mojom::blink::FileSystemAccessTransferToken> dest,
+    const String& new_entry_name,
+    base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {
+  if (!mojo_ptr_.is_bound()) {
+    std::move(callback).Run(mojom::blink::FileSystemAccessError::New(
+        mojom::blink::FileSystemAccessStatus::kInvalidState,
+        base::File::Error::FILE_ERROR_FAILED, "Context Destroyed"));
+    return;
+  }
+
+  mojo_ptr_->Move(std::move(dest), new_entry_name, std::move(callback));
+}
+
 void FileSystemDirectoryHandle::RemoveImpl(
     const FileSystemRemoveOptions* options,
     base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {

diff --git a/blink/renderer/modules/file_system_access/file_system_directory_handle.h b/blink/renderer/modules/file_system_access/file_system_directory_handle.h
@@ -62,6 +62,15 @@ class FileSystemDirectoryHandle final : public FileSystemHandle {
       bool writable,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr,
                               mojom::blink::PermissionStatus)>) override;
+  void RenameImpl(
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)
+      override;
+  void MoveImpl(
+      mojo::PendingRemote<mojom::blink::FileSystemAccessTransferToken> dest,
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)
+      override;
   void RemoveImpl(
       const FileSystemRemoveOptions* options,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)

diff --git a/blink/renderer/modules/file_system_access/file_system_file_handle.cc b/blink/renderer/modules/file_system_access/file_system_file_handle.cc
@@ -21,6 +21,7 @@
 #include "third_party/blink/renderer/platform/file_metadata.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
@@ -193,6 +194,33 @@ void FileSystemFileHandle::RequestPermissionImpl(
   mojo_ptr_->RequestPermission(writable, std::move(callback));
 }
 
+void FileSystemFileHandle::RenameImpl(
+    const String& new_entry_name,
+    base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {
+  if (!mojo_ptr_.is_bound()) {
+    std::move(callback).Run(mojom::blink::FileSystemAccessError::New(
+        mojom::blink::FileSystemAccessStatus::kInvalidState,
+        base::File::Error::FILE_ERROR_FAILED, "Context Destroyed"));
+    return;
+  }
+
+  mojo_ptr_->Rename(new_entry_name, std::move(callback));
+}
+
+void FileSystemFileHandle::MoveImpl(
+    mojo::PendingRemote<mojom::blink::FileSystemAccessTransferToken> dest,
+    const String& new_entry_name,
+    base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {
+  if (!mojo_ptr_.is_bound()) {
+    std::move(callback).Run(mojom::blink::FileSystemAccessError::New(
+        mojom::blink::FileSystemAccessStatus::kInvalidState,
+        base::File::Error::FILE_ERROR_FAILED, "Context Destroyed"));
+    return;
+  }
+
+  mojo_ptr_->Move(std::move(dest), new_entry_name, std::move(callback));
+}
+
 void FileSystemFileHandle::RemoveImpl(
     const FileSystemRemoveOptions* options,
     base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)> callback) {

diff --git a/blink/renderer/modules/file_system_access/file_system_file_handle.h b/blink/renderer/modules/file_system_access/file_system_file_handle.h
@@ -50,6 +50,15 @@ class FileSystemFileHandle final : public FileSystemHandle {
       bool writable,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr,
                               mojom::blink::PermissionStatus)>) override;
+  void RenameImpl(
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)
+      override;
+  void MoveImpl(
+      mojo::PendingRemote<mojom::blink::FileSystemAccessTransferToken> dest,
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)
+      override;
   void RemoveImpl(
       const FileSystemRemoveOptions* options,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>)

diff --git a/blink/renderer/modules/file_system_access/file_system_handle.cc b/blink/renderer/modules/file_system_access/file_system_handle.cc
@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/modules/file_system_access/file_system_handle.h"
 
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/blink/public/mojom/file_system_access/file_system_access_error.mojom-blink.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
@@ -91,6 +92,67 @@ ScriptPromise FileSystemHandle::requestPermission(
   return result;
 }
 
+ScriptPromise FileSystemHandle::rename(ScriptState* script_state,
+                                       const String& new_entry_name) {
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  ScriptPromise result = resolver->Promise();
+
+  RenameImpl(
+      new_entry_name,
+      WTF::Bind(
+          [](FileSystemHandle* handle, const String& new_name,
+             ScriptPromiseResolver* resolver, FileSystemAccessErrorPtr result) {
+            if (result->status == mojom::blink::FileSystemAccessStatus::kOk) {
+              handle->name_ = new_name;
+            }
+            file_system_access_error::ResolveOrReject(resolver, *result);
+          },
+          WrapPersistent(this), new_entry_name, WrapPersistent(resolver)));
+
+  return result;
+}
+
+ScriptPromise FileSystemHandle::move(
+    ScriptState* script_state,
+    FileSystemDirectoryHandle* destination_directory) {
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  ScriptPromise result = resolver->Promise();
+
+  MoveImpl(
+      destination_directory->Transfer(), name_,
+      WTF::Bind(
+          [](ScriptPromiseResolver* resolver, FileSystemAccessErrorPtr result) {
+            file_system_access_error::ResolveOrReject(resolver, *result);
+          },
+          WrapPersistent(resolver)));
+
+  return result;
+}
+
+ScriptPromise FileSystemHandle::move(
+    ScriptState* script_state,
+    FileSystemDirectoryHandle* destination_directory,
+    const String& new_entry_name) {
+  auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  ScriptPromise result = resolver->Promise();
+
+  String dest_name = new_entry_name.IsEmpty() ? name_ : new_entry_name;
+
+  MoveImpl(
+      destination_directory->Transfer(), dest_name,
+      WTF::Bind(
+          [](FileSystemHandle* handle, const String& new_name,
+             ScriptPromiseResolver* resolver, FileSystemAccessErrorPtr result) {
+            if (result->status == mojom::blink::FileSystemAccessStatus::kOk) {
+              handle->name_ = new_name;
+            }
+            file_system_access_error::ResolveOrReject(resolver, *result);
+          },
+          WrapPersistent(this), dest_name, WrapPersistent(resolver)));
+
+  return result;
+}
+
 ScriptPromise FileSystemHandle::remove(ScriptState* script_state,
                                        const FileSystemRemoveOptions* options) {
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);

diff --git a/blink/renderer/modules/file_system_access/file_system_handle.h b/blink/renderer/modules/file_system_access/file_system_handle.h
@@ -21,6 +21,7 @@ namespace blink {
 class ExecutionContext;
 class FileSystemHandlePermissionDescriptor;
 class FileSystemRemoveOptions;
+class FileSystemDirectoryHandle;
 
 class FileSystemHandle : public ScriptWrappable, public ExecutionContextClient {
   DEFINE_WRAPPERTYPEINFO();
@@ -41,6 +42,12 @@ class FileSystemHandle : public ScriptWrappable, public ExecutionContextClient {
   ScriptPromise requestPermission(ScriptState*,
                                   const FileSystemHandlePermissionDescriptor*);
 
+  ScriptPromise rename(ScriptState*, const String& new_entry_name);
+  ScriptPromise move(ScriptState*,
+                     FileSystemDirectoryHandle* destination_directory);
+  ScriptPromise move(ScriptState*,
+                     FileSystemDirectoryHandle* destination_directory,
+                     const String& new_entry_name);
   ScriptPromise remove(ScriptState*, const FileSystemRemoveOptions* options);
 
   ScriptPromise isSameEntry(ScriptState*, FileSystemHandle* other);
@@ -60,6 +67,13 @@ class FileSystemHandle : public ScriptWrappable, public ExecutionContextClient {
       bool writable,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr,
                               mojom::blink::PermissionStatus)>) = 0;
+  virtual void RenameImpl(
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>) = 0;
+  virtual void MoveImpl(
+      mojo::PendingRemote<mojom::blink::FileSystemAccessTransferToken> dest,
+      const String& new_entry_name,
+      base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>) = 0;
   virtual void RemoveImpl(
       const FileSystemRemoveOptions* options,
       base::OnceCallback<void(mojom::blink::FileSystemAccessErrorPtr)>) = 0;

diff --git a/blink/renderer/modules/file_system_access/file_system_handle.idl b/blink/renderer/modules/file_system_access/file_system_handle.idl
@@ -27,6 +27,19 @@ enum FileSystemHandleKind {
     [CallWith=ScriptState] Promise<PermissionState> requestPermission(
         optional FileSystemHandlePermissionDescriptor descriptor = {});
 
+    [
+        CallWith=ScriptState,
+        RuntimeEnabled=FileSystemAccessAPIExperimental
+    ]  Promise<void> rename(USVString new_entry_name);
+    [
+        CallWith=ScriptState,
+        RuntimeEnabled=FileSystemAccessAPIExperimental
+    ]  Promise<void> move(FileSystemDirectoryHandle destination_directory);
+    [
+        CallWith=ScriptState,
+        RuntimeEnabled=FileSystemAccessAPIExperimental
+    ]  Promise<void> move(FileSystemDirectoryHandle destination_directory,
+                          USVString new_entry_name);
     [
         CallWith=ScriptState,
         RuntimeEnabled=FileSystemAccessAPIExperimental

diff --git a/...b_tests/external/wpt/file-system-access/local_FileSystemBaseHandle-move-manual.https.html b/...b_tests/external/wpt/file-system-access/local_FileSystemBaseHandle-move-manual.https.html
@@ -0,0 +1,10 @@
+<!doctype html>
+<meta charset=utf-8>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="resources/test-helpers.js"></script>
+<script src="resources/local-fs-test-helpers.js"></script>
+<script src="script-tests/FileSystemBaseHandle-move.js"></script>
diff --git a/...eb_tests/external/wpt/file-system-access/sandboxed_FileSystemBaseHandle-move.https.any.js b/...eb_tests/external/wpt/file-system-access/sandboxed_FileSystemBaseHandle-move.https.any.js
@@ -0,0 +1,3 @@
+// META: script=resources/test-helpers.js
+// META: script=resources/sandboxed-fs-test-helpers.js
+// META: script=script-tests/FileSystemBaseHandle-move.js