fs icon indicating copy to clipboard operation
fs copied to clipboard

Add FileSystemHandle.remove method

Open a-sully opened this issue 3 years ago • 6 comments

Migrated from https://github.com/WICG/file-system-access/pull/283 (see #2)

Addresses https://github.com/WICG/file-system-access/issues/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
  • [x] 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
  • [x] Implementation bugs are filed:
    • Chrome: https://crbug.com/1114923
    • Firefox: …
    • Safari: …

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


Preview | Diff

a-sully avatar Mar 07 '22 22:03 a-sully

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

Do we need the new task source from #3 to be created for this, or is there some other task source we should queue to in the meantime?

a-sully avatar Jan 09 '23 06:01 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 https://github.com/whatwg/storage/pull/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 (https://github.com/whatwg/fs/issues/3#issue-1070369311) 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 avatar Jan 18 '23 00:01 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

a-sully avatar Jan 18 '23 01:01 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 avatar Feb 02 '23 18:02 a-sully

Also, https://github.com/whatwg/fs/pull/73#discussion_r1011231194 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 avatar Feb 02 '23 18:02 a-sully

Hi all, I've updated this PR to make use of the various infrastructural changes that have merged recently (primarily https://github.com/whatwg/fs/pull/95 and https://github.com/whatwg/fs/pull/96). Please take a look at the updated PR. Thanks!

a-sully avatar Jul 17 '23 03:07 a-sully