Bug 1783930 [wpt PR 35407] - FSA: Make removeEntry() take an exclusiv…

…e lock, a=testonly

Automatic update from web-platform-tests
FSA: Make removeEntry() take an exclusive lock

removeEntry() currently takes a shared lock to allow for removal of a
file with an open writable. Taking an exclusive lock makes the behavior
consistent with move() and remove(), the other file-altering operations.

Discussed in whatwg/fs#39

Also updates the FSUnderlyingSink to reset its mojo connection when the
stream reaches an errored state. WritableStreams are no longer usable
after an error, so resetting the remote releases the corresponding file
lock held in the browser.

blink-dev PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/ddrh_bI1stY

Fixed: 1254078, 1380650
Change-Id: I5d471405d65f4d1920e7f0dfe9c783a4038e63e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3817911
Commit-Queue: Austin Sullivan <[email protected]>
Reviewed-by: Daseul Lee <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1074250}

--

wpt-commits: 703f1c81703c7014b0515518bc8467870ef8ec0e
wpt-pr: 35407

testing/web-platform/tests/fs/script-tests/FileSystemDirectoryHandle-removeEntry.js

@@ -10,6 +10,93 @@ directory_test(async (t, root) => {
   await promise_rejects_dom(t, 'NotFoundError', getFileContents(handle));
 }, 'removeEntry() to remove a file');
 
+directory_test(async (t, root) => {
+  const handle =
+      await createFileWithContents(t, 'file-to-remove', '12345', root);
+  await root.removeEntry('file-to-remove');
+
+  await promise_rejects_dom(
+      t, 'NotFoundError', root.removeEntry('file-to-remove'));
+}, 'removeEntry() on an already removed file should fail');
+
+directory_test(async (t, root) => {
+  const dir = await root.getDirectoryHandle('dir-to-remove', {create: true});
+  await createFileWithContents(t, 'file-to-keep', 'abc', root);
+  await root.removeEntry('dir-to-remove');
+
+  assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
+}, 'removeEntry() to remove an empty directory');
+
+directory_test(async (t, root) => {
+  const dir = await createDirectory(t, 'dir-to-remove', root);
+  await createFileWithContents(t, 'file-in-dir', 'abc', dir);
+
+  await promise_rejects_dom(
+      t, 'InvalidModificationError', root.removeEntry('dir-to-remove'));
+  assert_array_equals(
+      await getSortedDirectoryEntries(root), ['dir-to-remove/']);
+  assert_array_equals(await getSortedDirectoryEntries(dir), ['file-in-dir']);
+}, 'removeEntry() on a non-empty directory should fail');
+
+directory_test(async (t, root) => {
+  // root
+  // ├──file-to-keep
+  // ├──dir-to-remove
+  //    ├── file0
+  //    ├── dir1-in-dir
+  //    │   └── file1
+  //    └── dir2
+  const dir = await root.getDirectoryHandle('dir-to-remove', {create: true});
+  await createFileWithContents(t, 'file-to-keep', 'abc', root);
+  await createEmptyFile(t, 'file0', dir);
+  const dir1_in_dir = await createDirectory(t, 'dir1-in-dir', dir);
+  await createEmptyFile(t, 'file1', dir1_in_dir);
+  await createDirectory(t, 'dir2-in-dir', dir);
+
+  await root.removeEntry('dir-to-remove', {recursive: true});
+  assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
+}, 'removeEntry() on a directory recursively should delete all sub-items');
+
+directory_test(async (t, root) => {
+  const dir = await createDirectory(t, 'dir', root);
+  await promise_rejects_js(t, TypeError, dir.removeEntry(''));
+}, 'removeEntry() with empty name should fail');
+
+directory_test(async (t, root) => {
+  const dir = await createDirectory(t, 'dir', root);
+  await promise_rejects_js(t, TypeError, dir.removeEntry(kCurrentDirectory));
+}, `removeEntry() with "${kCurrentDirectory}" name should fail`);
+
+directory_test(async (t, root) => {
+  const dir = await createDirectory(t, 'dir', root);
+  await promise_rejects_js(t, TypeError, dir.removeEntry(kParentDirectory));
+}, `removeEntry() with "${kParentDirectory}" name should fail`);
+
+directory_test(async (t, root) => {
+  const dir_name = 'dir-name';
+  const dir = await createDirectory(t, dir_name, root);
+
+  const file_name = 'file-name';
+  await createEmptyFile(t, file_name, dir);
+
+  for (let i = 0; i < kPathSeparators.length; ++i) {
+    const path_with_separator = `${dir_name}${kPathSeparators[i]}${file_name}`;
+    await promise_rejects_js(
+        t, TypeError, root.removeEntry(path_with_separator),
+        `removeEntry() must reject names containing "${kPathSeparators[i]}"`);
+  }
+}, 'removeEntry() with a path separator should fail.');
+
+directory_test(async (t, root) => {
+  const handle =
+      await createFileWithContents(t, 'file-to-remove', '12345', root);
+  await createFileWithContents(t, 'file-to-keep', 'abc', root);
+  await root.removeEntry('file-to-remove');
+
+  assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
+  await promise_rejects_dom(t, 'NotFoundError', getFileContents(handle));
+}, 'removeEntry() to remove a file');
+
 directory_test(async (t, root) => {
   const handle =
       await createFileWithContents(t, 'file-to-remove', '12345', root);
@@ -94,16 +181,34 @@ directory_test(async (t, root) => {
 
   const writable = await handle.createWritable();
   await promise_rejects_dom(
-    t, 'InvalidModificationError', root.removeEntry('file-to-remove'));
+      t, 'NoModificationAllowedError', root.removeEntry('file-to-remove'));
 
   await writable.close();
   await root.removeEntry('file-to-remove');
 
-  assert_array_equals(
-      await getSortedDirectoryEntries(root),
-      ['file-to-keep']);
+  assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
 }, 'removeEntry() while the file has an open writable fails');
 
+directory_test(async (t, root) => {
+  const dir_name = 'dir-name';
+  const dir = await createDirectory(t, dir_name, root);
+
+  const handle =
+      await createFileWithContents(t, 'file-to-remove', '12345', dir);
+  await createFileWithContents(t, 'file-to-keep', 'abc', dir);
+
+  const writable = await cleanup_writable(t, await handle.createWritable());
+  await promise_rejects_dom(
+      t, 'NoModificationAllowedError', root.removeEntry(dir_name));
+
+  await writable.close();
+  assert_array_equals(
+      await getSortedDirectoryEntries(dir), ['file-to-keep', 'file-to-remove']);
+
+  await dir.removeEntry('file-to-remove');
+  assert_array_equals(await getSortedDirectoryEntries(dir), ['file-to-keep']);
+}, 'removeEntry() of a directory while a containing file has an open writable fails');
+
 directory_test(async (t, root) => {
   const handle =
       await createFileWithContents(t, 'file-to-remove', '12345', root);

testing/web-platform/tests/fs/script-tests/FileSystemWritableFileStream-write.js

@@ -194,17 +194,6 @@ directory_test(async (t, root) => {
   assert_equals(await getFileSize(handle), 3);
 }, 'write() with a valid typed array buffer');
 
-directory_test(async (t, root) => {
-  const dir = await createDirectory(t, 'parent_dir', root);
-  const file_name = 'close_fails_when_dir_removed.txt';
-  const handle = await createEmptyFile(t, file_name, dir);
-  const stream = await cleanup_writable(t, await handle.createWritable());
-  await stream.write('foo');
-
-  await root.removeEntry('parent_dir', {recursive: true});
-  await promise_rejects_dom(t, 'NotFoundError', stream.close());
-}, 'atomic writes: close() fails when parent directory is removed');
-
 directory_test(async (t, root) => {
   const handle = await createEmptyFile(t, 'atomic_writes.txt', root);
   const stream = await cleanup_writable(t, await handle.createWritable());
@@ -276,22 +265,6 @@ directory_test(async (t, root) => {
   assert_equals(success_count, 1);
 }, 'atomic writes: only one close() operation may succeed');
 
-directory_test(async (t, root) => {
-  const dir = await createDirectory(t, 'parent_dir', root);
-  const file_name = 'atomic_writable_file_stream_persists_removed.txt';
-  const handle = await createFileWithContents(t, file_name, 'foo', dir);
-
-  const stream = await cleanup_writable(t, await handle.createWritable());
-  await stream.write('bar');
-
-  await dir.removeEntry(file_name);
-  await promise_rejects_dom(t, 'NotFoundError', getFileContents(handle));
-
-  await stream.close();
-  assert_equals(await getFileContents(handle), 'bar');
-  assert_equals(await getFileSize(handle), 3);
-}, 'atomic writes: writable file stream persists file on close, even if file is removed');
-
 directory_test(async (t, root) => {
   const handle = await createEmptyFile(t, 'writer_written', root);
   const stream = await cleanup_writable(t, await handle.createWritable());
@@ -315,36 +288,34 @@ directory_test(async (t, root) => {
   const stream = await cleanup_writable(t, await handle.createWritable());
 
   await promise_rejects_dom(
-      t, "SyntaxError", stream.write({type: 'truncate'}), 'truncate without size');
-
+      t, 'SyntaxError', stream.write({type: 'truncate'}),
+      'truncate without size');
 }, 'WriteParams: truncate missing size param');
 
 directory_test(async (t, root) => {
   const handle = await createEmptyFile(t, 'content.txt', root);
   const stream = await cleanup_writable(t, await handle.createWritable());
 
   await promise_rejects_dom(
-      t, "SyntaxError", stream.write({type: 'write'}), 'write without data');
-
+      t, 'SyntaxError', stream.write({type: 'write'}), 'write without data');
 }, 'WriteParams: write missing data param');
 
 directory_test(async (t, root) => {
   const handle = await createEmptyFile(t, 'content.txt', root);
   const stream = await cleanup_writable(t, await handle.createWritable());
 
   await promise_rejects_js(
-      t, TypeError, stream.write({type: 'write', data: null}), 'write with null data');
-
+      t, TypeError, stream.write({type: 'write', data: null}),
+      'write with null data');
 }, 'WriteParams: write null data param');
 
 directory_test(async (t, root) => {
-  const handle = await createFileWithContents(
-      t, 'content.txt', 'seekable', root);
+  const handle =
+      await createFileWithContents(t, 'content.txt', 'seekable', root);
   const stream = await cleanup_writable(t, await handle.createWritable());
 
   await promise_rejects_dom(
-      t, "SyntaxError", stream.write({type: 'seek'}), 'seek without position');
-
+      t, 'SyntaxError', stream.write({type: 'seek'}), 'seek without position');
 }, 'WriteParams: seek missing position param');
 
 directory_test(async (t, root) => {

testing/web-platform/tests/fs/script-tests/FileSystemWritableFileStream.js

@@ -33,26 +33,6 @@ directory_test(async (t, root) => {
   await promise_rejects_dom(t, 'NotFoundError', cleanup_writable(t, handle.createWritable()));
 }, 'createWritable() fails when parent directory is removed');
 
-directory_test(async (t, root) => {
-  const dir = await createDirectory(t, 'parent_dir', root);
-  const file_name = 'write_fails_when_dir_removed.txt';
-  const handle = await createEmptyFile(t, file_name, dir);
-  const stream = await cleanup_writable(t, await handle.createWritable());
-
-  await root.removeEntry('parent_dir', {recursive: true});
-  await promise_rejects_dom(t, 'NotFoundError', stream.write('foo'));
-}, 'write() fails when parent directory is removed');
-
-directory_test(async (t, root) => {
-  const dir = await createDirectory(t, 'parent_dir', root);
-  const file_name = 'truncate_fails_when_dir_removed.txt';
-  const handle = await createEmptyFile(t, file_name, dir);
-  const stream = await cleanup_writable(t, await handle.createWritable());
-
-  await root.removeEntry('parent_dir', {recursive: true});
-  await promise_rejects_dom(t, 'NotFoundError', stream.truncate(0));
-}, 'truncate() fails when parent directory is removed');
-
 directory_test(async (t, root) => {
   const handle = await createFileWithContents(
       t, 'atomic_file_is_copied.txt', 'fooks', root);