Re: [whatwg/fetch] processResponseDone should receive a response (#1202)

@annevk commented on this pull request.



> @@ -4017,9 +4032,10 @@ steps:
 
  <li><p>Set <var>response</var>'s <a for="response">timing info</a> to <var>timingInfo</var>.
 
- <li><p><a href="https://github.com/w3c/resource-timing/pull/261">Mark resource timing</a> for
- <var>timingInfo</var>, <var>originalURL</var>, <var>initiatorType</var>, and <var>global</var>.
- <!-- TODO -->
+ <li><p>
+ <a href="https://w3c.github.io/resource-timing/#dfn-mark-resource-timing">Mark resource timing</a>

Why is this needed, is it not exported? Let's export it first.

>   <li><p>Set <var>fetchParams</var>'s <a for="fetch params">request</a>'s
  <a for=request>done flag</a>.
 
+ <li><p>If <var>timingInfo</var> is null, then return.
+
+ <li><p>Set <var>timingInfo</var>'s <a for="fetch timing info">end time</a> to
+ the <a for=/>coarsened shared current time</a> given <var>fetchParams</var>'s
+ <a for="fetch params">cross-origin isolated capability</a>.

Why do we need to set end time twice?

> @@ -7217,8 +7233,9 @@ method steps are:
    <li><p><a lt=terminated for=fetch>Terminate</a> the ongoing fetch with the aborted flag set.
   </ol>
 
- <li><p>Let <var>handleFetchDone</var> be to <a>finalize and report timing</a> with
- <var>response</var>, <var>globalObject</var>, and "<code>fetch</code>".
+ <li><p>Let <var>handleFetchDone</var> given <a for=/>response</a> <var>response</var> be to
+ <a>finalize and report timing</a> with <var>response</var>, <var>globalObject</var>, and
+ "<code>fetch</code>".

I think you should rename the _response_ variable here as otherwise it's confusing with the other one, no?

> @@ -4017,9 +4032,10 @@ steps:
 
  <li><p>Set <var>response</var>'s <a for="response">timing info</a> to <var>timingInfo</var>.
 
- <li><p><a href="https://github.com/w3c/resource-timing/pull/261">Mark resource timing</a> for
- <var>timingInfo</var>, <var>originalURL</var>, <var>initiatorType</var>, and <var>global</var>.
- <!-- TODO -->
+ <li><p>
+ <a href="https://w3c.github.io/resource-timing/#dfn-mark-resource-timing">Mark resource timing</a>
+ for <var>timingInfo</var>, <var>originalURL</var>, <var>initiatorType</var>, <var>global</var> and

Oxford comma.

>   <li><p>If <var>timingInfo</var> is null, then return.
 
- <li><p>If <var>response</var>'s <a for=response>timing allow passed flag</a> is not set, then set
- <var>timingInfo</var> to a new <a for=/>fetch timing info</a> whose
- <a for="fetch timing info">start time</a> and
- <a for="fetch timing info">post-redirect start time</a> are <var>timingInfo</var>'s
- <a for="fetch timing info">start time</a>.
+ <li><p>If <var>response</var>'s <a for=response>timing allow passed flag</a> is not set, then

As this `<li>` contains multiple flow elements the `<p>` needs to indented on a newline.

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

Received on Wednesday, 14 April 2021 09:39:22 UTC