Re: [whatwg/fetch] Call `finalize and report timing` automatically on end-of-body (PR #1413)

@annevk commented on this pull request.

Thanks, I think we're close, but there's at least one somewhat involved problem left.

> +   <li><p>If <var>fetchParams</var>'s <a for="fetch params">request</a>'s
+   <a for="request">initiator type</a> is not null and <var>client</var> is not null, then
+   <a>queue a fetch task</a> to run <var>fetchParams</var>'s <a for="fetch params">controller</a>'s
+   <a for="fetch controller">report timing steps</a> given <var>client</var>'s
+   <a for="environment settings object">global object</a> and <a for="fetch params">request</a>'s
+   <a for="request">initiator type</a>, with <var>client</var>'s
+   <a for="environment settings object">global object</a>.
+
    <li><p>If <var>fetchParams</var>'s <a for="fetch params">process response end-of-body</a> is not
    null, then <a>queue a fetch task</a> to run <var>fetchParams</var>'s
    <a for="fetch params">process response end-of-body</a> given <var>response</var> with

The problem is that we are queuing two tasks instead of one here. Can we instead check that fetchParams's task destination is fetchParams's request's client to avoid the need to have distinct destinations?

And then we could define _processResponseEndOfBodyTask_ steps as doing both of these. Probably first invoke fetchParams's process response end-of-body if non-null and then invoke the report timing steps if the task destination matches the request's client. And this set of steps would also be the correct place to flip the done flag on request I think.

> @@ -5278,8 +5298,8 @@ steps. They return a <a for=/>response</a>.
       <p class=note>If <var>forwardResponse</var> is a <a>network error</a>, this effectively caches
       the network error, which is sometimes known as "negative caching".
 
-      <p class=note>The associated <a for=response>timing info</a> is stored in the cache
-      alongside the response.
+      <p>The user agent should store <var>forwardResponse</var>'s <a for=response>body info</a>
+      as part of the response.

I don't understand why this was changed into a normative requirement from a note. In principle we expect everything to be stored unless stated otherwise.

> +"<code>embed</code>",
+"<code>fetch</code>",
+"<code>font</code>",
+"<code>frame</code>",
+"<code>iframe</code>",
+"<code>image</code>",
+"<code>img</code>",
+"<code>input</code>",
+"<code>link</code>",
+"<code>object</code>",
+"<code>ping</code>",
+"<code>script</code>",
+"<code>track</code>",
+"<code>video</code>",
+"<code>xmlhttprequest</code>", or
+"<code>other</code>". Unless stated otherwise it is null. [[RESOURCE-TIMING]]

We need a follow-up issue to add this to the table we have earlier around request's destination, CSP, etc.

> @@ -1404,6 +1420,30 @@ this flag set are subject to additional processing requirements.
 <p>A <a for=/>request</a> has an associated <dfn for=request export>service-workers mode</dfn>, that
 is "<code>all</code>" or "<code>none</code>". Unless stated otherwise it is "<code>all</code>".
 
+<p>A <a for=/>request</a> has an associated
+<dfn for=request export>initiator type</dfn>, which is null,
+"<code>audio</code>",
+"<code>beacon</code>",
+"<code>body</code>",
+"<code>css</code>",
+"<code>early-hint</code>",

Isn't this a little weird? Early Hints can result in all kinds of requests.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1413#pullrequestreview-1005488661
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1413/review/1005488661@github.com>