Re: [whatwg/fetch] Initial resource timing integration. (#1185)

@yoavweiss commented on this pull request.



>   <li>
   <p>Run these steps, but <a>abort when</a> the ongoing fetch is <a for=fetch>terminated</a>:
 
   <ol>
    <li>
     <p>Set <var>connection</var> to the result of establishing an HTTP connection to
-    <var>origin</var>. [[!HTTP]] [[!HTTP-SEMANTICS]] [[!HTTP-COND]] [[!HTTP-CACHING]] [[!HTTP-AUTH]]
-    [[!TLS]]
+    <var>origin</var>, while <a>recording connnection timing info</a> for <var>connection</var>.

Nit: Spurious "n" in "connnection"

> @@ -2215,6 +2288,86 @@ clearly stipulates that <a>connections</a> are keyed on
 <!-- See https://github.com/whatwg/fetch/issues/114#issuecomment-143500095 for when we make
      WebSocket saner -->
 
+<p>When <dfn>recording connnection timing info</dfn> given a <var>connection</var>, perform the

Same nit on extra "n"

> @@ -2215,6 +2288,86 @@ clearly stipulates that <a>connections</a> are keyed on
 <!-- See https://github.com/whatwg/fetch/issues/114#issuecomment-143500095 for when we make
      WebSocket saner -->
 
+<p>When <dfn>recording connnection timing info</dfn> given a <var>connection</var>, perform the
+following steps:
+<ol>
+  <li><p>Let <var>timingInfo</var> be <var>connection</var>'s <a for=connection>timingInfo</a>.
+<li>If the domain information is available in cache, then set <var>timingInfo</var>'s
+<a for="connection timing info">domain lookup start time</a> to the result of calling
+<a for=/>unsafe shared current time</a> immmediately after beginning retrieval of the
+information from cache.
+<li>Otherwise, set <var>timingInfo</var>'s
+<a for="connection timing info">domain lookup start time</a> to the result of calling
+<a for=/>unsafe shared current time</a> immmediately before starting the domain lookup.

Nit: extra "m" in immmediately here and elsewhere

> +  <dt><dfn for="connection timing info">connection end time</dfn> (default zero)
+  <dd>A <a for=/>DOMHighResTimeStamp</a>
+  <dt><dfn for="connection timing info">secure connection start time</dfn> (default zero)
+  <dd>A <a for=/>DOMHighResTimeStamp</a>
+  <dt><dfn for="connection timing info">alpn negotiated protocol</dfn> (default empty string)
+  <dd>A string.
+</dl>
+
+<p><dfn data-cite="hr-time-2#dom-domhighrestimestamp">DOMHighResTimeStamp</dfn> and
+<dfn data-cite="hr-time-2#dfn-unsafe-shared-current-time">unsafe shared current time</dfn> </code>
+are defined in [[HR-TIME]].
+
+<p class=note>Note that timestamps in this spec are usually unsafe, and are meant to be coarsened
+and normalized to a <a for=/>global object</a> prior to being exposed.
+
+<p>To <dfn>clamp connection timing info</dfn>, given <a for=/>connection timing info</a>

Might be worthwhile to clarify in the name that this ensures no negative timestamps, rather than e.g. protect exposure when TAO fails. So maybe "clamp negative timestamps from connection timing info"?

> +  <li>Let <var>timingInfo</var> be <var>fetchParams</var>'s <a for="fetch params">timing info</a>.
+  <li>Let <var>requestedURL</var> be <var>response</var>'s <a for=response>URL list</a>'s first
+  item.
+  <li>Return, if <var>request</var>'s <a for=request>destination</a> is one of the following:
+  <ul>
+    <li><code>"report"</code>
+    <li><code>"document"</code>
+    <li><code>"frame"</code>
+  </ul>
+
+  <li>If <var>response</var>'s <a for=response>timing allow passed flag</a> is not set, set
+  <var>timingInfo</var> to a new <a for=/>fetch timing info</a>, with its
+  <a for="fetch timing info">fetch start time</a> set to <var>timingInfo</var>'s
+  <a for="fetch timing info">fetch start time</a>,
+  and <a for="fetch timing info">response end time</a> set to <var>timingInfo</var>'s
+  <a for="fetch timing info">response end time</a>.

Also: TAO clamping of connection timing info?

> @@ -4773,6 +4977,9 @@ optional boolean <var>forceNewConnection</var> (default false), run these steps:
     `<code>Transfer-Encoding</code>`/`<code>chunked</code>` and <var>response</var> is transferred
     via HTTP/1.0 or older, then return a <a>network error</a>.
 
+    <li><p>Set <var>timingInfo</var>'s <a for="fetch timing info">response start time</a> to

That doesn't match how response start is [defined today](https://www.w3.org/TR/resource-timing-2/#dom-performanceresourcetiming-responsestart) (e.g. for 100 responses)

> @@ -4773,6 +4977,9 @@ optional boolean <var>forceNewConnection</var> (default false), run these steps:
     `<code>Transfer-Encoding</code>`/`<code>chunked</code>` and <var>response</var> is transferred
     via HTTP/1.0 or older, then return a <a>network error</a>.
 
+    <li><p>Set <var>timingInfo</var>'s <a for="fetch timing info">response start time</a> to

I'm not sure this is the right place for the response, as it's not mentioned anywhere that we're waiting for the response to start arriving and that it has started arriving here. @annevk - thoughts?

-- 
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/1185#pullrequestreview-607401172