Re: [whatwg/fetch] Partition HTTP Connections (#1063)

@annevk commented on this pull request.



> @@ -2123,11 +2125,11 @@ clearly stipulates that <a>connections</a> are keyed on
      WebSocket saner -->
 
 
-<h3 id=http-cache-partitions>HTTP cache partitions</h3>
+<h3 id=http-state-partition>Network partition key</h3>
 

Let's add a definition here of a network partition key as a tuple consisting of a site and null or an implementation-defined value.

> @@ -2158,12 +2160,31 @@ given <var>request</var>, run these steps:
  <li><p>Let <var>topLevelSite</var> be the result of <a lt="obtain a site">obtaining a site</a>,
  given <var>topLevelOrigin</var>.
 
- <li>
-  <p>Return the HTTP cache associated with <var>topLevelSite</var> and, possibly, a second key.
-  [[!HTTP-CACHING]]
+ <li><p>Let <var>secondKey</var> be the second key, or null.

Let's rephrase this to

> `be null or an <a>implementation-defined</a> value.`

to make it even more explicit.

>  
   <p class=XXX>The second key is intentionally a little vague as the finer points are still
   evolving. See <a href=https://github.com/whatwg/fetch/issues/1035>issue #1035</a>.
+
+ <li><p>If <var>secondKey</var> is not null, return (<var>topLevelSite</var>, <var>secondKey</var>).

Let's remove the conditional and the then unneeded step below. Implementations that do not have a second key can use a simpler data structure.

> @@ -2071,15 +2071,16 @@ for each associated <a for="fetch group">fetch record</a> whose
 <a for=connection>connection pool</a> consists of zero or more
 <dfn lt=connection export id=concept-connection>connections</dfn>. Each
 <a>connection</a> is identified by an <b>origin</b> (an
-<a for=/>origin</a>) and <b>credentials</b> (a boolean).
+<a for=/>origin</a>), <b>credentials</b> (a boolean), and
+<b>networkPartitionKey</b> (a <a for=/>site</a> list or null).

I suggest we call this key and lead with it. So key, origin, credentials. And per below its type can be network partition key.

> @@ -2123,11 +2125,11 @@ clearly stipulates that <a>connections</a> are keyed on
      WebSocket saner -->
 
 
-<h3 id=http-cache-partitions>HTTP cache partitions</h3>
+<h3 id=http-state-partition>Network partition key</h3>

The ID here needs updating and the title should be plural per convention.

> +
+ <li><p>If <var>secondKey</var> is not null, return (<var>topLevelSite</var>, <var>secondKey</var>).
+
+ <li><p>Return (<var>topLevelSite</var>)
+
+</ol>
+
+
+<h3 id=http-cache-partitions>HTTP cache partitions</h3>
+
+<p>To
+<dfn lt="determine the HTTP cache partition|determining the HTTP cache partition">determine the HTTP cache partition</dfn>,
+given <var>request</var>, run these steps:
+
+<ol>
+ <li><p>Set <var>networkPartitionKey</var> to the result of <a>determining the network partition key</a>.

Use "Let" when initializing. See https://infra.spec.whatwg.org/#variables.

> +
+ <li><p>Return (<var>topLevelSite</var>)
+
+</ol>
+
+
+<h3 id=http-cache-partitions>HTTP cache partitions</h3>
+
+<p>To
+<dfn lt="determine the HTTP cache partition|determining the HTTP cache partition">determine the HTTP cache partition</dfn>,
+given <var>request</var>, run these steps:
+
+<ol>
+ <li><p>Set <var>networkPartitionKey</var> to the result of <a>determining the network partition key</a>.
+
+ <li><p>If <var>networkPartitionKey</var> is non-null, return null.

then return null.

But also, the conditional seems wrong?

> +
+</ol>
+
+
+<h3 id=http-cache-partitions>HTTP cache partitions</h3>
+
+<p>To
+<dfn lt="determine the HTTP cache partition|determining the HTTP cache partition">determine the HTTP cache partition</dfn>,
+given <var>request</var>, run these steps:
+
+<ol>
+ <li><p>Set <var>networkPartitionKey</var> to the result of <a>determining the network partition key</a>.
+
+ <li><p>If <var>networkPartitionKey</var> is non-null, return null.
+
+ <li><p>Otherwise, Return the unique HTTP cache associated with the <var>networkPartitionKey</var>.

No need for otherwise.

-- 
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/1063#pullrequestreview-461467230