Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Editorial: adopt Fetch's new approach to callbacks #311

Merged
merged 3 commits into from
Feb 15, 2021
Merged

Editorial: adopt Fetch's new approach to callbacks #311

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9525e59
Editorial: adopt Fetch's new approach to callbacks
annevk Feb 10, 2021
2e25443
no longer set request's synchronous flag
annevk Feb 15, 2021
0afb2ba
nit
annevk Feb 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 56 additions & 35 deletions xhr.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,6 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<dd><a>This</a>'s <a>request body</a>.
<dt><a for=request>client</a>
<dd><a>This</a>'s <a>relevant settings object</a>.
<dt><a for=request>synchronous flag</a>
<dd>Set if <a>this</a>'s <a>synchronous flag</a> is set.
<dt><a for=request>mode</a>
<dd>"<code>cors</code>".
<dt><a for=request>use-CORS-preflight flag</a>
Expand Down Expand Up @@ -794,27 +792,10 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<a><code>send()</code> flag</a> is unset, then return.

<li>
<p><a for=/>Fetch</a> <var>req</var>.
Handle the <a>tasks</a>
<a lt="queue a task">queued</a> on the
<a>networking task source</a> per below.

<p>Run these steps <a>in parallel</a>:
<p>Let <var>processRequestBody</var>, given a <var>request</var>, be these steps:

<ol>
<li><p>Wait until either <var>req</var>'s <a for=request>done flag</a> is set or <a>this</a>'s
<a>timeout</a> is not 0 and <a>this</a>'s <a>timeout</a> milliseconds have passed since these
steps started.

<li><p>If <var>req</var>'s <a for=request>done flag</a> is unset, then set <a>this</a>'s
<a>timed out flag</a> and <a for=fetch>terminate</a> <a for=/>fetching</a>.
</ol>

<p>To <a>process request body</a> for <var>request</var>, run these steps:

<ol>
<li><p>If not roughly 50ms have passed since these steps were last invoked,
terminate these steps.
<li><p>If not roughly 50ms have passed since these steps were last invoked, then return.

<li><p>If <a>this</a>'s <a>upload listener flag</a> is set, then <a>fire a progress event</a>
named <a event><code>progress</code></a> at <a>this</a>'s <a>upload object</a> with
Expand All @@ -825,12 +806,13 @@ return <a>this</a>'s <a>cross-origin credentials</a>.

<p class=note>These steps are only invoked when new bytes are transmitted.

<p>To <a>process request end-of-body</a> for <var>request</var>, run these steps:
<li>
<p>Let <var>processRequestEndOfBody</var>, given a <var>request</var>, be these steps:

<ol>
<li><p>Set <a>this</a>'s <a>upload complete flag</a>.
annevk marked this conversation as resolved.
Show resolved Hide resolved

<li><p>If <a>this</a>'s <a>upload listener flag</a> is unset, then terminate these steps.
<li><p>If <a>this</a>'s <a>upload listener flag</a> is unset, then return.

<li><p>Let <var>transmitted</var> be <var>request</var>'s
<a for=request>body</a>'s
Expand All @@ -851,12 +833,14 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
</ol>
<!-- upload complete flag can never be set here I hope -->

<p>To <a>process response</a> for <var>response</var>, run these steps:
<li>
<p>Let <var>processResponse</var>, given a <var>response</var>, be these steps:
annevk marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some steps use "terminate these steps" to bail, others use "return". Probably "abort these steps" is most common and best?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I would kinda prefer if we could use return everywhere as it's a lot more convenient. And some of these algorithms can be invoked from in parallel and main thread alike.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a matter of parallel or main thread. It's a matter of which algorithm they terminate. "Return" as we use it today terminates the main algorithm...

Maybe it's unambiguous to allow it to terminate the innermost algorithm, but it seems a bit tricky.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing I don't understand is why a step in a callback or a step in in parallel steps could ever terminate the main algorithm. It seems you would always have to handle that with some kind of flag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, some callbacks are called synchronously (e.g. readable stream read request steps), and so could theoretically return. But I agree it's an unlikely reading.

It's more that, when I go to scan an algorithm, I want to look for all "return"/"throw" lines and say "that's where the algorithm could terminate". If we allow re-using return to break out of a collection of steps, then I have to mentally discard those in my scan.

I don't have a strong argument for why this is more problematic for me in specs than it is in programming languages. Maybe the lack of syntax highlighting, or the fact that substeps for callbacks look very similar to substeps for if statements. But I hope this help explains my mild preference.


<ol>
<li><p>Set <a>this</a>'s <a for=XMLHttpRequest>response</a> to <var>response</var>.

<li><p><a>Handle errors</a> fo <a>this</a> and <var>response</var>.
<li><p><a>Handle errors</a> for <a>this</a> and <a>this</a>'s
<a for=XMLHttpRequest>response</a>.

<li><p>If <a>this</a>'s <a for=XMLHttpRequest>response</a> is a <a>network error</a>, then
return.
Expand All @@ -876,7 +860,7 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<a>this</a>'s <a for=XMLHttpRequest>response</a>'s <a for=response>body</a>'s
<a for=body>stream</a>.

<p><span class="note no-backref">This operation will not throw an exception.</span>
<p class=note>This operation will not throw an exception.

<li>
<p>Let <var>readRequest</var> be the following <a>read request</a>:
Expand All @@ -887,8 +871,7 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<ol>
<li><p>Append <var>chunk</var> to <a>this</a>'s <a>received bytes</a>.

<li><p>If not roughly 50ms have passed since these steps were last invoked, then abort
these steps.
<li><p>If not roughly 50ms have passed since these steps were last invoked, then return.

<li><p>If <a>this</a>'s <a>state</a> is <i>headers received</i>, then set <a>this</a>'s
<a>state</a> to <i>loading</i>.
Expand Down Expand Up @@ -920,6 +903,26 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<li><p><a for=ReadableStreamDefaultReader>Read a chunk</a> from <var>reader</var> given
<var>readRequest</var>.
</ol>

<li><p><a for=/>Fetch</a> <var>req</var> with <a for=fetch><i>processRequestBody</i></a> set to
<var>processRequestBody</var>, <a for=fetch><i>processRequestEndOfBody</i></a> set to
<var>processRequestEndOfBody</var>, and <a for=fetch><i>processResponse</i></a> set to
<var>processResponse</var>.

<li><p>Let <var>now</var> be the present time.
<!-- Eventually this should use some kind of time standard. -->

<li>
<p>Run these steps <a>in parallel</a>:

<ol>
<li><p>Wait until either <var>req</var>'s <a for=request>done flag</a> is set or <a>this</a>'s
<a>timeout</a> is not 0 and <a>this</a>'s <a>timeout</a> milliseconds have passed since
<var>now</var>.

<li><p>If <var>req</var>'s <a for=request>done flag</a> is unset, then set <a>this</a>'s
<a>timed out flag</a> and <a for=fetch>terminate</a> <a for=/>fetching</a>.
</ol>
</ol>

<li>
Expand All @@ -930,13 +933,31 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
is not <a>allowed to use</a> the "<code><a>sync-xhr</a></code>" feature, then run
<a>handle response end-of-body</a> for <a>this</a> and a <a>network error</a>, and then return.

<li><p>Let <var>response</var> be a <a for=/>network error</a>.

<li><p>Let <var>processedResponse</var> be false.

<li>
<p>Let <var>response</var> be the result of
<a for=/>fetching</a> <var>req</var>.
<p>Let <var>processResponse</var>, given a <var>fetchResponse</var>, be these steps:

<ol>
<li><p>Set <var>response</var> to <var>fetchResponse</var>.

<li><p>Set <var>processedResponse</var> to true.
</ol>

<li><p><a for=/>Fetch</a> <var>req</var> with <a for=fetch><i>processResponse</i></a>
set to <var>processResponse</var> and <a for=fetch><i>useParallelQueue</i></a> set to true.

<li><p>Let <var>now</var> be the present time.
<!-- Eventually this should use some kind of time standard. -->

<li><p><a>Pause</a> until either <var>processedResponse</var> is true or <a>this</a>'s
<a>timeout</a> is not 0 and <a>this</a>'s <a>timeout</a> milliseconds have passed since
<var>now</var>.

<p>If <a>this</a>'s <a>timeout</a> is not 0, then set <a>this</a>'s <a>timed out flag</a> and
<a for=fetch>terminate</a> <a for=/>fetching</a> if it has not returned within <a>this</a>'s
<a>timeout</a> milliseconds.
<li><p>If <var>processedResponse</var> is false, then set <a>this</a>'s <a>timed out flag</a> and
<a for=fetch>terminate</a> <a for=/>fetching</a>.

<li><p>If <var>response</var>'s <a for=response>body</a> is null, then run
<a>handle response end-of-body</a> for <a>this</a> and <var>response</var>, and then return.
Expand All @@ -945,12 +966,12 @@ return <a>this</a>'s <a>cross-origin credentials</a>.
<p>Let <var>reader</var> be the result of <a for=ReadableStream>getting a reader</a> for
<var>response</var>'s <a for=response>body</a>'s <a for=body>stream</a>.

<p><span class="note no-backref">This operation will not throw an exception.</span>
<p class=note>This operation will not throw an exception.

<li><p>Let <var>promise</var> be the result of
<a for=ReadableStreamDefaultReader>reading all bytes</a> from <var>reader</var>.

<li><p>Wait for <var>promise</var> to be fulfilled or rejected.
<li><p><a>Pause</a> until <var>promise</var> is fulfilled or rejected.

<li><p>If <var>promise</var> is fulfilled with <var>bytes</var>, then append <var>bytes</var>
to <a>this</a>'s <a>received bytes</a>.
Expand Down