Add FileSystemHandle.remove method

[a-sully]

Migrated from WICG/file-system-access#283 (see #2)

Addresses WICG/file-system-access#214

Adds the ability for a file or directory to delete itself. See the proposal: https://github.com/whatwg/fs/blob/main/proposals/Remove.md

Currently, it is not possible to remove a file or directory given its handle. You must obtain the handle of the parent directory and call FileSystemDirectoryHandle.removeEntry().

This would enable the common use case where you obtain a file handle from showSaveFilePicker(), but then decide you don't want to save after all, and delete the file, or clearing an entire bucket file system.

• At least two implementers are interested (and none opposed):
  • Chrome
  • …
• Tests are written and can be reviewed and commented upon at:
  • https://github.com/web-platform-tests/wpt/blob/master/file-system-access/script-tests/FileSystemBaseHandle-remove.js
• Implementation bugs are filed:
  • Chrome: https://crbug.com/1114923
  • Firefox: …
  • Safari: …

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[annevk]

You'll need to make your membership of the googlers GitHub organization public to appease the IPR bot.

[a-sully]

Since this is a new method, should we try to do task queueing correctly from the get go?

I've attempted to do this in the latest patchset (thank you for finishing off whatwg/storage#155!), but I'm not sure whether we should be queueing everything or just the bits that may touch quota? (in this case, the actual removal of the entry). This could alternatively look like:

1. Let |result| be [=a new promise=].
1. Run these steps [=in parallel=]:
  1. Check permissions
  1. Grab lock (on a parallel queue)
  1. Queue a task on the storage task source to actually remove the entry
1. Return |result|.

...but based on your comment here (#3 (comment)) it seems like the promise must be resolved/rejected on some task source, so maybe it should be this?

1. Let |result| be [=a new promise=].
1. Queue a global task to run these steps:
  1. Check permissions
  1. Grab lock (on a parallel queue)
  1. Queue a task on the storage task source to actually remove the entry
1. Return |result|.

Please let me know if you have a preference :)

[a-sully]

1. Let |result| be [=a new promise=].
1. Run these steps [=in parallel=]:
  1. Check permissions
  1. Grab lock (on a parallel queue)
  1. Queue a task on the storage task source to actually remove the entry
1. Return |result|.

I realized when putting together #95 that (I think) the parallel queue can be hidden to just the lock acquiring/releasing algorithms so the "Grab lock" step may actually not have to change

[annevk]

This doesn't seem to have a good delineation between the main thread and the storage thread operations.

The method and the eventual task run in the main thread and can access JS.

What's missing here is an "in parallel" section that runs in some other thread and deals with locks and the file system.

Or am I misunderstanding something?

[a-sully]

This doesn't seem to have a good delineation between the main thread and the storage thread operations.

The method and the eventual task run in the main thread and can access JS.

What's missing here is an "in parallel" section that runs in some other thread and deals with locks and the file system.

Or am I misunderstanding something?

You're correct, and I've updated this in the latest patchset. It's still not perfect since file locking is still done in parallel on the main thread, but once #95 adds the new framework for file locking we can follow that pattern. I'm happy to do that in a follow-up

[a-sully]

Also, #73 (comment) is relevant here, too... we may need to specify what taking locks on a directory looks like (i.e. you can't take the lock if a containing file is locked)

[a-sully]

Hi all, I've updated this PR to make use of the various infrastructural changes that have merged recently (primarily #95 and #96). Please take a look at the updated PR. Thanks!

[annevk]

Nit: https://infra.spec.whatwg.org doesn't use markup for null.

[annevk]

It seems weird that this would happen on the main thread.

[annevk]

Do we really need nested file system queue steps?