Define a parallel queue for all file system operations

[a-sully]

Fixes #74 by specifying a parallel queue for all file system operations, not just locking (see https://github.com/whatwg/fs/pull/9/files#r1109679577)

• [x] At least two implementers are interested (and none opposed):
  • Chromium
  • WebKit
• [ ] Tests are written and can be reviewed and commented upon at:
  • N/A (unless there's a way to deterministically test race conditions that I don't know about)
• [ ] Implementation bugs are filed:
  • Chromium: ...
  • Gecko: …
  • WebKit: …
• [ ] MDN issue is filed: …

---

Preview | Diff

[a-sully]

cc @jesup @szewai

[a-sully]

Updated to post all file system operations to a "file system queue", as we discussed offline. Lock take/release is once again sync (and must be called from that queue). Please feel free to nit-pick this as much as you please, since this pattern will be applied to all the other methods. Thanks!

[a-sully]

The overall setup here seems sound from a quick skim. The main problem is that "reject", "resolve", and "create a new X" are operations that need to happen on the main thread. And as such you need to do the "queue a storage task" dance for them.

Oh I see... Question - does this work? If not, why not?

1. [=Queue a storage task=] with [=this=]'s [=relevant global object=] to
   [=enqueue the following steps=] to the [=file system queue=]:
   1. ...

Note that you don't necessarily need to replace each individually. You might be able to restructure things a bit so do all the in parallel work first and safe the results in variables and then queue a storage task that does the rejecting/resolving as per the variables.

Also, what kind of exception can "request access" throw, out of curiosity? Since we get that exception while in parallel, forwarding it probably doesn't entirely work. If we could normalize that to it not returning "granted" that would be better.

Unfortunately I think this is needed, since it can reject with a SecurityError. See https://wicg.github.io/file-system-access/#permissions

[annevk]

Question - does this work? If not, why not?

Because you still end up in parallel when doing "resolve", etc. You need some things to happen "in parallel" and then after those things you queue a task for things to happen on the main thread. Such as resolving a promise or creating a JS object.

Unfortunately I think this is needed, since it can reject with a SecurityError.

Hmm, I guess we should redesign this somewhat whereby "request access" returns some kind of internal value and we then later turn that into the appropriate exception.

[a-sully]

Question - does this work? If not, why not?

Because you still end up in parallel when doing "resolve", etc. You need some things to happen "in parallel" and then after those things you queue a task for things to happen on the main thread. Such as resolving a promise or creating a JS object.

Ah yes, looking at this with fresh eyes I see the problem. I've updated a few methods - if that pattern looks good, I'll copy it everywhere else

Unfortunately I think this is needed, since it can reject with a SecurityError.

Hmm, I guess we should redesign this somewhat whereby "request access" returns some kind of internal value and we then later turn that into the appropriate exception.

This is a bit tricky because the WICG spec considers file system access a "powerful feature", and powerful features have permission request algorithms that are allowed to throw. I've added an explicit warning that this spec doesn't comply with that pattern, which seems like it should be good enough?

Anyways, I've updated the access checks to return either a PermissionState or an error code that can be turned into a DomException (though I wonder if returning (as opposed to throwing) a DomException directly is an allowed pattern?)

If this all seems reasonable, I'll file an issue to update the WICG spec (and update all the instances in this spec which assume these algorithms can throw)

[a-sully]

if that pattern looks good, I'll copy it everywhere else

friendly ping @annevk. This infra PR is holding up the PRs specifying move() and remove()

[a-sully]

@annevk friendly ping (this is holding up a number of other changes). If this looks good, feel free to merge

[a-sully]

Thanks for the review! I'll merge this now so we can start making progress on the various PRs that are blocked on it. If you have more nits, I'm happy to address them in a follow-up

Just an FYI that I also updated the permission checks to use the new [=DOMException/name=] and "names table" added in https://github.com/whatwg/webidl/pull/1211 (which broke the refs we were adding)