Re: [whatwg/fetch] Move `finalize and report timing` to controller (PR #1413)

@annevk commented on this pull request.

This generally looks like an improvement to me, but there's a couple of things that still need to be worked out.

> @@ -225,8 +225,15 @@ lt="authentication entry">authentication entries</a> (for HTTP authentication).
 <a for=struct>items</a>:
 
 <dl>
- <dt><dfn for="fetch controller">state</dfn> (default "<code>ongoing</code>")
- <dd>"<code>ongoing</code>", "<code>terminated</code>", or "<code>aborted</code>"
+ <dt><dfn for="fetch controller">state</dfn> (default "<code>requesting</code>")
+ <dd>"<code>requesting</code>", "<code>responding</code>", "<code>concluded</code>",
+ "<code>terminated</code>", or "<code>aborted</code>"

I just realized this design maybe does not allow for full duplex HTTP implementations, which Deno aims to be.

See https://github.com/whatwg/fetch/issues/1254.

I think in practice this is only used for timing information so we should probably have some notes that timing information can be misleading for full duplex implementations. I think that might be a sufficiently good way to address this. And perhaps we should have a note near here about what state can be used for? Hmm...

cc @lucacasonato

> @@ -237,6 +244,32 @@ lt="authentication entry">authentication entries</a> (for HTTP authentication).
 <var>controller</var>, set <var>controller</var>'s <a for="fetch controller">state</a> to
 "<code>terminated</code>".
 
+<p>To <dfn export for="fetch controller" id="finalize-and-report-timing">conclude</dfn> a
+<a for=/>fetch controller</a> <var>controller</var>, perform the following steps given an optional
+string <var>initiatorType</var> (default "<code>other</code>"), an optional "<code>client</code>" or
+<a for=/>global object</a> <var>global</var> (default "<code>client</code>"), an optional
+"<code>original</code>" or <a for=/>response</a> <var>finalResponse</var> (default
+"<code>original</code>"), and an optional "<code>now</code>" or {{DOMHighResTimeStamp}}
+<var>unsafeResponseEndTime</var>:</p>

Looks like _unsafeResponseEndTime_ needs a default too.

> @@ -4292,22 +4320,56 @@ steps:
 <a for=/>response</a> <var>response</var>, run these steps:
 
 <ol>
+ <li><p>Let <var>timingInfo</var> be <var>fetchParams</var>'s
+ <a for="fetch params">timing info</a>.</p></li>

Nit: no closing tags.

> @@ -4292,22 +4320,56 @@ steps:
 <a for=/>response</a> <var>response</var>, run these steps:
 
 <ol>
+ <li><p>Let <var>timingInfo</var> be <var>fetchParams</var>'s
+ <a for="fetch params">timing info</a>.</p></li>
+
+ <li><p>Set <var>fetchParams</var>'s <a for="fetch params">controller</a>'s
+  <a for="fetch controller">state</a> to "<code>responding</code>".

Nit: indentation.

> +<p>A <dfn export>fetch resource info</dfn> is a <a for=/>struct</a> used to maintain
+information needed by <cite>Resource Timing</cite> and <cite>Navigation Timing</cite>. It has the
+following <a for=struct>items</a>: [[RESOURCE-TIMING]] [[NAVIGATION-TIMING]]
+
+<dl>
+ <dt><dfn export for="fetch resource info">encoded body size</dfn> (default 0)
+ <dt><dfn export for="fetch resource info">decoded body size</dfn> (default 0)
+ <dd>A number.
+</dl>

Let's call this "body info", "encoded size", and "decoded size"? We should also preserve the old IDs.

Though if we only track these for a response, perhaps we should simply move the two fields onto response directly?

I think we also want to preserve the notes in the caching section that we expect this information to be cached as caches normally don't handle that kind of thing.

> +<p>A <a for=/>response</a> has an associated
+<dfn export for=response id=concept-response-resource-info>resource info</dfn>
+(a <a for=/>fetch resource info</a>). Unlesss stated otherwise, it is a new
+<a for=/>fetch resource info</a>.

By placing this here the note below is no longer associated with the correct thing it seems.

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

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