Rewrite FileReader definitions. #118
Conversation
Make algorithms more imperative to make it clearer what is going on. No intended behavioral changes, but making it clearer that the behavior from #79 is the expected behavior.
|
This isn't supposed to change behavior, but I should still write tests to actually verify the behavior, as there are things in this area (like #79) that aren't currently tested, and where browsers do diverge. |
| 1. Let |bytes| by an empty [=byte sequence=]. | ||
| 1. Let |chunk| be the result of [=read a chunk|reading a chunk=] from |stream| with |reader|. | ||
| 1. Let |isFirstChunk| be true. | ||
| 1. [=In parallel=], while true: |
There was a problem hiding this comment.
This isn't really in parallel since you operate on promises from the main thread. Maybe it's okay for now though, but doesn't seem like the correct setup long term.
cc @domenic
There was a problem hiding this comment.
This is exactly what fetch does in for example the "transmit body" algorithm. But yeah, not sure what the best spec language is these days to "wait" for promises to resolve without blocking the main thread of execution. Open to suggestions.
|
Thanks for the review. Yeah, I tried to mostly just mimic what fetch (and sometimes XHR) are doing. Corresponding tests are in web-platform-tests/wpt#7494 (and unfortunately not a single browser currently passes those tests... Firefox is close (it is synchronously firing the loadstart event rather than posting a task, and it set result too soon for readAsBinaryString). Chromium/Edge have various other issues, but we already have a bug for at least some of those, and I don't see any problems with fixing those. Safari seems to mostly currently match Chromium in behavior. |
| 1. Let |stream| be the result of calling [=get stream=] on |blob|. | ||
| 1. Let |reader| be the result of [=get a reader|getting a reader=] from |stream|. | ||
| 1. Let |promise| be the result of [=read all bytes|reading all bytes=] from |stream| with |reader|. | ||
| 1. Wait for |promise| to be fulfilled or rejected. |
There was a problem hiding this comment.
Not the most sound thing to do with real promises, but let's pretend these are spec promises, like they really should be if streams had a better infrastructure :).
There was a problem hiding this comment.
Ah yes, this of course only sort of works because we control the stream, and we know no script has to execute for this particular stream to be drained and the promise to resolve... That's not very nice (but then neither are sync APIs in general :) )
Co-Authored-By: mkruisselbrink <[email protected]>
Make algorithms more imperative to make it clearer what is going on.
No intended behavioral changes, but making it clearer that the behavior
from #79 is the expected behavior.
Preview | Diff