Re: [whatwg/fetch] Deferred fetching (PR #1647)

@mingyc commented on this pull request.

PTAL

> +  throw a {{NotAllowedError}}.
+
+  <p class=note>User-agents can, for example, decide that deferred fetching is not allowed for
+  subframes or gated behind some permission.
+ </li>
+
+ <li><p>Let <var>requestObject</var> be the result of invoking the initial value of {{Request}} as
+ constructor with <var>input</var> and <var>init</var> as arguments. This may throw an exception.
+
+ <li><p>If <var>requestObject</var>'s <a for=Request>signal</a> is <a for=AbortSignal>aborted</a>,
+ then throw <a for=Request>signal</a>'s <a for=AbortSignal>abort reason</a>.
+
+ <li><p>Let <var>request</var> be <var>requestObject</var>'s <a for=Request>request</a>.
+
+ <li><p>If <var>request</var>'s <a for=request>URL</a>'s <a for=url>scheme</a> is not an
+ <a>HTTP(S) scheme</a> then throw a {{TypeError}}.

```
If request’s URL’s scheme is not an HTTP(S) scheme then throw a TypeError.
```

Should this be HTTPS-only? (although there's a potentially trustworthy check below)

According to our previous [discussion] and https://github.com/WICG/pending-beacon/issues/27, our new API should enforce security requests.


> +  <p><a for=list>For each</a> <a for="fetch group">deferred fetch record</a>
+  <var>deferredRecord</var> of <var>fetchGroup</var>'s
+  <a for="fetch group">deferred fetch records</a>:
+
+  <ol>
+   <li><p>Set <var>deferredRecord</var>'s <a for="deferred fetch record">invoke state</a> to
+   "<code>terminated</code>".
+
+   <li><p><a for=/>Fetch</a> <var>deferredRecord</var>'s
+   <a for="deferred fetch record">request</a>.
+  </ol>
+ </li>
+</ol>
+
+<p>When a <a for=fetch>fetch group</a> <var>fetchGroup</var> is
+<dfn export for="fetch group" id=concept-fetch-group-deactivate>deactivated</dfn>:

```
When a fetch group fetchGroup is deactivated:
...
1. Set deferredRecord’s [ invoke state ] to "  scheduled  ".
...
The user agent should wait until deferredRecord’s [ inactivity delay ] have passed or until deferredRecord’s [ invoke state ] is not "scheduled"
```

It does not talk about the case when the fetch group transition from `deactivated` -> `activated` again while the inactivity delay has not passed. For example, `deactivated -> 10 second -> activated` while inactivity delay=10 minutes.

In such case, should the invoke state be set back to `deferred` such that the inactivity delay timer can know it should stop?


> +
+ <li><p>Let <var>request</var> be <var>requestObject</var>'s <a for=Request>request</a>.
+
+ <li><p>If <var>request</var>'s <a for=request>URL</a>'s <a for=url>scheme</a> is not an
+ <a>HTTP(S) scheme</a> then throw a {{TypeError}}.
+
+ <li><p>If <var>request</var>'s <a for=request>URL</a> is not a
+ [=potentially trustworthy url=], then throw a {{SecurityError}}.
+
+ <li><p>Let <var>backgroundTimeout</var> be null.
+
+ <li><p>If <var>init</var> is given and <var>init</var>["<code>backgroundTimeout</code>"]
+ <a for=map>exists</a> then set <var>backgroundTimeout</var> to
+ <var>init</var>["<code>backgroundTimeout</code>"].
+
+ <li><p>If <var>backgroundTimeout</var> is not a {{DOMHighResTimeStamp}} then throw a {{TypeError}}.

Thanks!

In addition, there was a long privacy-related [discussion](https://github.com/WICG/pending-beacon/issues/3) around the max we should allow for backgroundTimeout. The conclusion was that browser can internally enforce a time limit (but the value is not specified). In Chrome's case we set it to 30 minutes (> BFCache's 10 minutes TTL), while FF's BFCache along is 40 minutes.

I guess it's somehow implemented by the step `The user agent may wait for a longer or shorter period time, e.g., to optimize batching of deferred fetches. ` in [fetch groups](https://whatpr.org/fetch/1647/53e4c3d...71fd383.html#fetch-groups), but should we also explicitly mention this behavior here?




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

Message ID: <whatwg/fetch/pull/1647/review/1592785018@github.com>