Overhaul how standards integrate with fetch #1165
Conversation
See whatwg/fetch#1165 for context.
There was a problem hiding this comment.
Overall looks good! I'd like to work on a PR for HTML to use these in the fetching scripts section to see what that looks like, although I don't want to block too long. I'll check out the XHR PR as well...
Requests no longer have a synchronous flag. Blocking a thread is now up to the caller.
This seems kind of inconvenient. What does it buy us? What would it look like? I guess I should go check out the XHR PR... Maybe you could work on a PR to HTML to update some of the synchronous fetches, to see what it looks like there?
| </ol> | ||
| <li><p>If <var>request</var>'s <a for=request>client</a> is non-null, then set <var>global</var> to | ||
| <var>request</var>'s <a for=request>client</a>'s | ||
| <a for="environment settings object">global object</a>. |
There was a problem hiding this comment.
Can we assert now that either global or algorithmQueue is non-null?
There was a problem hiding this comment.
No, for sync XHR both will have a value, for instance. And most things will need a non-null client (figuring out client more precisely is a separate can of worms).
There was a problem hiding this comment.
I meant, assert that at least one of them is non-null.
There was a problem hiding this comment.
That would not allow fire-and-forget (e.g., pings or reports).
There was a problem hiding this comment.
Actually, those would have a body prolly and would need to use useParallelQueue as otherwise transmit-request-body loop breaks. I'll add this.
There was a problem hiding this comment.
Hmm, but elsewhere we have callers that don't set this. They do go directly to main fetch but still... I'm going to add this as a comment as something to evaluate over time once we've upgraded more callers.
This makes some rather big changes: * Requests no longer have a synchronous flag. Blocking a thread is now up to the caller. * Fetch therefore no longer returns a response directly. In parallel callers will need to pass in "callbacks" that are either invoked on a parallel queue or on an event loop. * To hold onto these callbacks as well as some other information a fetch params struct is now passed around the various fetch algorithms. This will allow for cleanup around termination and aborting in the future. Potentially some bookkeeping state from request can move there going forward. * There's a dedicated navigate-redirect fetch algorithm for HTML as the HTTP-redirect fetch algorithm now wants a fetch params instance. * Some allowance for aborting early on in fetch was removed as all that is run synchronously with the code that invoked fetch to begin with. Closes #1164. * Algorithms that needed to be adjusted were changed to use the new Infra patterns for parameters. I also tried to improve naming, e.g., makeCORSPreflight rather than CORS-preflight flag. * "process response done" was removed as it seemed redundant with "response response end-of-body"; possible leftover from trailers. * Removed duplicate task queueing at the end of main fetch for request's body. Fixes #536.
annevk
force-pushed
the
annevk/formalize-callbacks
branch
from
ae4b46d
to
f5bac56
Compare
|
Do you know yet how much impact these changes will have on non-WHATWG specs that call into Fetch algorithms? I’m wondering in particular about WebAppSec specs. Regardless, I’m happy to help as needed both on assessing what other specs will be affected, and then also working on getting other specs updated as needed to align with these changes. |
|
It affects everything that uses Fetch to fetch something. Based on https://fetch.spec.whatwg.org/#concept-request-destination I'd say most of that is HTML, but there is probably some reporting in WebAppSec that would need updating. It should all be fairly minor and I'm happy to help. |
See whatwg/fetch#1165 for context.
See whatwg/fetch#1165 for context.
whatwg/fetch#1165 removed the flag.
whatwg/fetch#1165 removed the flag.
See whatwg/fetch#1165 for context.
This makes some rather big changes:
Fixes #536.
Preview | Diff