Skip to content

Commit c035ad1

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

File tree

3 files changed

+117
-61
lines changed

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)