Skip to content

Commit 4ff5349

Browse files
a-sullychromium-wpt-export-bot
a-sully
authored and
chromium-wpt-export-bot
committedNov 18, 2022
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 TODO: link blink-dev PSA Fixed: 1254078 Change-Id: I5d471405d65f4d1920e7f0dfe9c783a4038e63e3
1 parent 99458bf commit 4ff5349

3 files changed

+117
-61
lines changed
 

‎fs/script-tests/FileSystemDirectoryHandle-removeEntry.js

+109-4
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,123 @@
11
'use strict';
22

3+
directory_test(async (t, root) => {
4+
const handle =
5+
await createFileWithContents(t, 'file-to-remove', '12345', root);
6+
await createFileWithContents(t, 'file-to-keep', 'abc', root);
7+
await root.removeEntry('file-to-remove');
8+
9+
assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
10+
await promise_rejects_dom(t, 'NotFoundError', getFileContents(handle));
11+
}, 'removeEntry() to remove a file');
12+
13+
directory_test(async (t, root) => {
14+
const handle =
15+
await createFileWithContents(t, 'file-to-remove', '12345', root);
16+
await root.removeEntry('file-to-remove');
17+
18+
await promise_rejects_dom(
19+
t, 'NotFoundError', root.removeEntry('file-to-remove'));
20+
}, 'removeEntry() on an already removed file should fail');
21+
22+
directory_test(async (t, root) => {
23+
const dir = await root.getDirectoryHandle('dir-to-remove', {create: true});
24+
await createFileWithContents(t, 'file-to-keep', 'abc', root);
25+
await root.removeEntry('dir-to-remove');
26+
27+
assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
28+
}, 'removeEntry() to remove an empty directory');
29+
30+
directory_test(async (t, root) => {
31+
const dir = await createDirectory(t, 'dir-to-remove', root);
32+
await createFileWithContents(t, 'file-in-dir', 'abc', dir);
33+
34+
await promise_rejects_dom(
35+
t, 'InvalidModificationError', root.removeEntry('dir-to-remove'));
36+
assert_array_equals(
37+
await getSortedDirectoryEntries(root), ['dir-to-remove/']);
38+
assert_array_equals(await getSortedDirectoryEntries(dir), ['file-in-dir']);
39+
}, 'removeEntry() on a non-empty directory should fail');
40+
41+
directory_test(async (t, root) => {
42+
// root
43+
// ├──file-to-keep
44+
// ├──dir-to-remove
45+
// ├── file0
46+
// ├── dir1-in-dir
47+
// │   └── file1
48+
// └── dir2
49+
const dir = await root.getDirectoryHandle('dir-to-remove', {create: true});
50+
await createFileWithContents(t, 'file-to-keep', 'abc', root);
51+
await createEmptyFile(t, 'file0', dir);
52+
const dir1_in_dir = await createDirectory(t, 'dir1-in-dir', dir);
53+
await createEmptyFile(t, 'file1', dir1_in_dir);
54+
await createDirectory(t, 'dir2-in-dir', dir);
55+
56+
await root.removeEntry('dir-to-remove', {recursive: true});
57+
assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
58+
}, 'removeEntry() on a directory recursively should delete all sub-items');
59+
60+
directory_test(async (t, root) => {
61+
const dir = await createDirectory(t, 'dir', root);
62+
await promise_rejects_js(t, TypeError, dir.removeEntry(''));
63+
}, 'removeEntry() with empty name should fail');
64+
65+
directory_test(async (t, root) => {
66+
const dir = await createDirectory(t, 'dir', root);
67+
await promise_rejects_js(t, TypeError, dir.removeEntry(kCurrentDirectory));
68+
}, `removeEntry() with "${kCurrentDirectory}" name should fail`);
69+
70+
directory_test(async (t, root) => {
71+
const dir = await createDirectory(t, 'dir', root);
72+
await promise_rejects_js(t, TypeError, dir.removeEntry(kParentDirectory));
73+
}, `removeEntry() with "${kParentDirectory}" name should fail`);
74+
75+
directory_test(async (t, root) => {
76+
const dir_name = 'dir-name';
77+
const dir = await createDirectory(t, dir_name, root);
78+
79+
const file_name = 'file-name';
80+
await createEmptyFile(t, file_name, dir);
81+
82+
for (let i = 0; i < kPathSeparators.length; ++i) {
83+
const path_with_separator = `${dir_name}${kPathSeparators[i]}${file_name}`;
84+
await promise_rejects_js(
85+
t, TypeError, root.removeEntry(path_with_separator),
86+
`removeEntry() must reject names containing "${kPathSeparators[i]}"`);
87+
}
88+
}, 'removeEntry() with a path separator should fail.');
89+
390
directory_test(async (t, root) => {
491
const handle =
592
await createFileWithContents(t, 'file-to-remove', '12345', root);
693
await createFileWithContents(t, 'file-to-keep', 'abc', root);
794

895
const writable = await cleanup_writable(t, await handle.createWritable());
996
await promise_rejects_dom(
10-
t, 'InvalidModificationError', root.removeEntry('file-to-remove'));
97+
t, 'NoModificationAllowedError', root.removeEntry('file-to-remove'));
1198

1299
await writable.close();
13100
await root.removeEntry('file-to-remove');
14101

15-
assert_array_equals(
16-
await getSortedDirectoryEntries(root),
17-
['file-to-keep']);
102+
assert_array_equals(await getSortedDirectoryEntries(root), ['file-to-keep']);
18103
}, 'removeEntry() while the file has an open writable fails');
104+
105+
directory_test(async (t, root) => {
106+
const dir_name = 'dir-name';
107+
const dir = await createDirectory(t, dir_name, root);
108+
109+
const handle =
110+
await createFileWithContents(t, 'file-to-remove', '12345', dir);
111+
await createFileWithContents(t, 'file-to-keep', 'abc', dir);
112+
113+
const writable = await cleanup_writable(t, await handle.createWritable());
114+
await promise_rejects_dom(
115+
t, 'NoModificationAllowedError', root.removeEntry(dir_name));
116+
117+
await writable.close();
118+
assert_array_equals(
119+
await getSortedDirectoryEntries(dir), ['file-to-keep', 'file-to-remove']);
120+
121+
await dir.removeEntry('file-to-remove');
122+
assert_array_equals(await getSortedDirectoryEntries(dir), ['file-to-keep']);
123+
}, 'removeEntry() of a directory while a containing file has an open writable fails');

‎fs/script-tests/FileSystemWritableFileStream-write.js

+8-37
Original file line numberDiff line numberDiff line change
@@ -194,17 +194,6 @@ directory_test(async (t, root) => {
194194
assert_equals(await getFileSize(handle), 3);
195195
}, 'write() with a valid typed array buffer');
196196

197-
directory_test(async (t, root) => {
198-
const dir = await createDirectory(t, 'parent_dir', root);
199-
const file_name = 'close_fails_when_dir_removed.txt';
200-
const handle = await createEmptyFile(t, file_name, dir);
201-
const stream = await handle.createWritable();
202-
await stream.write('foo');
203-
204-
await root.removeEntry('parent_dir', {recursive: true});
205-
await promise_rejects_dom(t, 'NotFoundError', stream.close());
206-
}, 'atomic writes: close() fails when parent directory is removed');
207-
208197
directory_test(async (t, root) => {
209198
const handle = await createEmptyFile(t, 'atomic_writes.txt', root);
210199
const stream = await handle.createWritable();
@@ -276,22 +265,6 @@ directory_test(async (t, root) => {
276265
assert_equals(success_count, 1);
277266
}, 'atomic writes: only one close() operation may succeed');
278267

279-
directory_test(async (t, root) => {
280-
const dir = await createDirectory(t, 'parent_dir', root);
281-
const file_name = 'atomic_writable_file_stream_persists_removed.txt';
282-
const handle = await createFileWithContents(t, file_name, 'foo', dir);
283-
284-
const stream = await handle.createWritable();
285-
await stream.write('bar');
286-
287-
await dir.removeEntry(file_name);
288-
await promise_rejects_dom(t, 'NotFoundError', getFileContents(handle));
289-
290-
await stream.close();
291-
assert_equals(await getFileContents(handle), 'bar');
292-
assert_equals(await getFileSize(handle), 3);
293-
}, 'atomic writes: writable file stream persists file on close, even if file is removed');
294-
295268
directory_test(async (t, root) => {
296269
const handle = await createEmptyFile(t, 'writer_written', root);
297270
const stream = await handle.createWritable();
@@ -315,36 +288,34 @@ directory_test(async (t, root) => {
315288
const stream = await handle.createWritable();
316289

317290
await promise_rejects_dom(
318-
t, "SyntaxError", stream.write({type: 'truncate'}), 'truncate without size');
319-
291+
t, 'SyntaxError', stream.write({type: 'truncate'}),
292+
'truncate without size');
320293
}, 'WriteParams: truncate missing size param');
321294

322295
directory_test(async (t, root) => {
323296
const handle = await createEmptyFile(t, 'content.txt', root);
324297
const stream = await handle.createWritable();
325298

326299
await promise_rejects_dom(
327-
t, "SyntaxError", stream.write({type: 'write'}), 'write without data');
328-
300+
t, 'SyntaxError', stream.write({type: 'write'}), 'write without data');
329301
}, 'WriteParams: write missing data param');
330302

331303
directory_test(async (t, root) => {
332304
const handle = await createEmptyFile(t, 'content.txt', root);
333305
const stream = await handle.createWritable();
334306

335307
await promise_rejects_js(
336-
t, TypeError, stream.write({type: 'write', data: null}), 'write with null data');
337-
308+
t, TypeError, stream.write({type: 'write', data: null}),
309+
'write with null data');
338310
}, 'WriteParams: write null data param');
339311

340312
directory_test(async (t, root) => {
341-
const handle = await createFileWithContents(
342-
t, 'content.txt', 'seekable', root);
313+
const handle =
314+
await createFileWithContents(t, 'content.txt', 'seekable', root);
343315
const stream = await handle.createWritable();
344316

345317
await promise_rejects_dom(
346-
t, "SyntaxError", stream.write({type: 'seek'}), 'seek without position');
347-
318+
t, 'SyntaxError', stream.write({type: 'seek'}), 'seek without position');
348319
}, 'WriteParams: seek missing position param');
349320

350321
directory_test(async (t, root) => {

‎fs/script-tests/FileSystemWritableFileStream.js

-20
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,6 @@ directory_test(async (t, root) => {
3333
await promise_rejects_dom(t, 'NotFoundError', handle.createWritable());
3434
}, 'createWritable() fails when parent directory is removed');
3535

36-
directory_test(async (t, root) => {
37-
const dir = await createDirectory(t, 'parent_dir', root);
38-
const file_name = 'write_fails_when_dir_removed.txt';
39-
const handle = await createEmptyFile(t, file_name, dir);
40-
const stream = await handle.createWritable();
41-
42-
await root.removeEntry('parent_dir', {recursive: true});
43-
await promise_rejects_dom(t, 'NotFoundError', stream.write('foo'));
44-
}, 'write() fails when parent directory is removed');
45-
46-
directory_test(async (t, root) => {
47-
const dir = await createDirectory(t, 'parent_dir', root);
48-
const file_name = 'truncate_fails_when_dir_removed.txt';
49-
const handle = await createEmptyFile(t, file_name, dir);
50-
const stream = await handle.createWritable();
51-
52-
await root.removeEntry('parent_dir', {recursive: true});
53-
await promise_rejects_dom(t, 'NotFoundError', stream.truncate(0));
54-
}, 'truncate() fails when parent directory is removed');
55-
5636
directory_test(async (t, root) => {
5737
const handle = await createFileWithContents(
5838
t, 'atomic_file_is_copied.txt', 'fooks', root);

0 commit comments

Comments
 (0)
Please sign in to comment.