Re: [w3c/ServiceWorker] Introduce Extend Service Worker Lifetime algorithm (#1010)

annevk requested changes on this pull request.



> @@ -168,7 +168,7 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-type">type</dfn> which is either "<code>classic</code>" or "<code>module</code>". Unless stated otherwise, it is "<code>classic</code>".</p>
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-containing-service-worker-registration">containing service worker registration</dfn> (a <a href="#dfn-service-worker-registration">service worker registration</a>), which contains itself.</p>
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-service-worker-id">id</dfn> (an opaque string), which uniquely identifies itself during the lifetime of its <a href="#dfn-containing-service-worker-registration">containing service worker registration</a>.</p>
-    <p>A <a href="#dfn-service-worker">service worker</a> is dispatched a set of <dfn id="dfn-lifecycle-events">lifecycle events</dfn>, <a href="#service-worker-global-scope-install-event">install</a> and <a href="#service-worker-global-scope-activate-event">activate</a>, and <dfn id="dfn-functional-events">functional events</dfn> including <a href="#service-worker-global-scope-fetch-event">fetch</a>.</p>
+    <p>A <a href="#dfn-service-worker">service worker</a> is dispatched a set of <dfn id="dfn-lifecycle-events">lifecycle events</dfn>, <a href="#service-worker-global-scope-install-event">install</a> and <a href="#service-worker-global-scope-activate-event">activate</a>, and <dfn id="dfn-functional-events" for="">functional events</dfn> including <a href="#service-worker-global-scope-fetch-event">fetch</a>.</p>

This should not need `for=""`.

> @@ -168,7 +168,7 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-type">type</dfn> which is either "<code>classic</code>" or "<code>module</code>". Unless stated otherwise, it is "<code>classic</code>".</p>
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-containing-service-worker-registration">containing service worker registration</dfn> (a <a href="#dfn-service-worker-registration">service worker registration</a>), which contains itself.</p>
     <p>A <a href="#dfn-service-worker">service worker</a> has an associated <dfn id="dfn-service-worker-id">id</dfn> (an opaque string), which uniquely identifies itself during the lifetime of its <a href="#dfn-containing-service-worker-registration">containing service worker registration</a>.</p>
-    <p>A <a href="#dfn-service-worker">service worker</a> is dispatched a set of <dfn id="dfn-lifecycle-events">lifecycle events</dfn>, <a href="#service-worker-global-scope-install-event">install</a> and <a href="#service-worker-global-scope-activate-event">activate</a>, and <dfn id="dfn-functional-events">functional events</dfn> including <a href="#service-worker-global-scope-fetch-event">fetch</a>.</p>
+    <p>A <a href="#dfn-service-worker">service worker</a> is dispatched a set of <dfn id="dfn-lifecycle-events">lifecycle events</dfn>, <a href="#service-worker-global-scope-install-event">install</a> and <a href="#service-worker-global-scope-activate-event">activate</a>, and <dfn id="dfn-functional-events" for="">functional events</dfn> including <a href="#service-worker-global-scope-fetch-event">fetch</a>.</p>

You should also try to migrate away from relying on `href="#..."` and instead use the linking mechanism of Bikeshed that does not require `href` but uses `lt` (or just the contents).

> @@ -3173,8 +3158,10 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
       <li>Invoke <a href="#run-service-worker-algorithm">Run Service Worker</a> algorithm with <var>installingWorker</var> as the argument.</li>
       <li><a>Queue a task</a> <var>task</var> to run the following substeps:
         <ol>
-          <li>Create a trusted event <var>e</var> that uses the {{InstallEvent}} interface, with the event type <code><a href="#service-worker-global-scope-install-event">install</a></code>, which does not bubble and is not cancelable.</li>
+          <li><a>Create an event</a> <var>e</var> with the {{InstallEvent}} interface.</li>

I think this phrasing is a bit confusing. "Let _e_ be the result of ..." would be clearer.

> @@ -3173,8 +3158,10 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
       <li>Invoke <a href="#run-service-worker-algorithm">Run Service Worker</a> algorithm with <var>installingWorker</var> as the argument.</li>
       <li><a>Queue a task</a> <var>task</var> to run the following substeps:
         <ol>
-          <li>Create a trusted event <var>e</var> that uses the {{InstallEvent}} interface, with the event type <code><a href="#service-worker-global-scope-install-event">install</a></code>, which does not bubble and is not cancelable.</li>
+          <li><a>Create an event</a> <var>e</var> with the {{InstallEvent}} interface.</li>
+          <li>Initialize <var>e</var>’s {{Event/type}} attribute to "<code>install</code>".</li>
           <li><a>Dispatch</a> <var>e</var> at <var>installingWorker</var>'s <a>environment settings object</a>'s <a for="environment settings object">global object</a> <var>globalObject</var>.</li>

Shouldn't this be "relevant global object" rather than "environment settings object's global object"?

> @@ -3359,6 +3348,31 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
     </ol>
   </section>
 
+  <section algorithm="extend-service-worker-lifetime-algorithm">
+    <h3 id="extend-service-worker-lifetime-algorithm" dfn>Extend Service Worker Lifetime</h3>
+
+    <dl>
+      <dt>Input</dt>
+        <dd><var>event</var>, an {{ExtendableEvent}} object</dd>
+      <dt>Output</dt>
+        <dd>None</dd>
+    </dl>
+    <ol>
+      <li>If <var>event</var>'s <a>extend lifetime promises</a> is empty, unset <var>event</var>'s <a>extensions allowed flag</a> and abort these steps.</li>
+      <li>Let <var>extendLifetimePromises</var> be an empty array.</li>
+      <li>Run the following substeps <a>in parallel</a>:
+        <ol>
+          <li><em>SetupPromiseArray</em>: Set <var>extendLifetimePromises</var> to a copy of <var>event</var>'s <a>extend lifetime promises</a>.</li>

I thought we had a better way of doing this? Just letting the promises decrease a counter.

> @@ -3426,13 +3440,16 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
       <li>Invoke <a href="#run-service-worker-algorithm">Run Service Worker</a> algorithm with <var>activeWorker</var> as the argument.</li>
       <li><a>Queue a task</a> <var>task</var> to run the following substeps:
         <ol>
-          <li>Create a trusted event <var>e</var> that uses the {{FetchEvent}} interface, with the event type <code><a href="#service-worker-global-scope-fetch-event">fetch</a></code>, which does not bubble.</li>
-          <li>Let the {{FetchEvent/request}} attribute of <var>e</var> be initialized to <var>r</var>.</li>
+          <li><a>Create an event</a> <var>e</var> with the {{FetchEvent}} interface.</li>
+          <li>Initialize <var>e</var>’s {{Event/type}} attribute to "<code>fetch</code>".</li>
+          <li>Initialize <var>e</var>’s {{Event/cancelable}} attribute to true.</li>

Why do you make it cancelable? That seems like a change.

You're not handling the return value of the dispatch algorithm though, so how does it matter?

> @@ -3509,10 +3526,13 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
       <li>Invoke [[#run-service-worker-algorithm]] algorithm with <var>activeWorker</var> as the argument.</li>
       <li><a>Queue a task</a> <var>task</var> to run the following substeps:
         <ol>
-          <li>Create a trusted event <var>e</var> that uses the {{ForeignFetchEvent}} interface, with the event type <code><a href="#service-worker-global-scope-foreignfetch-event">foreignfetch</a></code>, which does not bubble.</li>
-          <li>Let the {{ForeignFetchEvent/request}} attribute of <var>e</var> be initialized to <var>r</var>.</li>
-          <li>Let the {{ForeignFetchEvent/origin}} attribute of <var>e</var> be initialized to the  <a lt="Unicode serialization of an origin">Unicode serialization</a> of <var>request</var>'s <a for=request>origin</a>.</li>
+          <li><a>Create an event</a> <var>e</var> with the {{ForeignFetchEvent}} interface.</li>
+          <li>Initialize <var>e</var>’s {{Event/type}} attribute to "<code>foreignfetch</code>".</li>
+          <li>Initialize <var>e</var>’s {{Event/cancelable}} attribute to true.</li>

Same here.

> @@ -3582,8 +3602,9 @@ spec: rfc7231; urlPrefix: https://tools.ietf.org/html/rfc7231
     <ol>
       <li><a>Assert</a>: a <a>Record</a> with the \[[value]] equals to <var>registration</var> is contained in <a href="#dfn-scope-to-registration-map">scope to registration map</a>.</li>
       <li><a>Assert</a>: <var>registration</var>'s <a href="#dfn-active-worker">active worker</a> is not null.</li>
+      <li>Let <var>event</var> be the <a for="/">functional event</a> for which this algorithm was invoked.</li>

Shouldn't this be passed in as variable?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1010#pullrequestreview-9627777