[ServiceWorker] Allow waitUntil to be called multiple times asynchronously

third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js

Index: third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js
index fc64e6ce567e3371ea9996d46d1cad76a66e5c88..9daf3087982eb71f3e765bbfdcede6595d8dd074 100644
--- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js
+++ b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/extendable-event-async-waituntil.js
@@ -50,12 +50,24 @@ self.addEventListener('message', function(event) {
   });
 
 self.addEventListener('fetch', function(event) {
-    var resolveFetch;
-    let response = new Promise((res) => { resolveFetch = res; });
-    event.respondWith(response);
-    async_task_waituntil(event)
-      .then(reportResultExpecting('OK'))
-      .then(() => { resolveFetch(new Response('OK')); });
+    if (event.request.url.indexOf('pending-respondwith-async-waituntil') != -1) {
+      var resolveFetch;
+      let response = new Promise((res) => { resolveFetch = res; });
+      event.respondWith(response);
+      async_task_waituntil(event)
+        .then(reportResultExpecting('OK'))
+        .then(() => { resolveFetch(new Response('OK')); });
+    } else if (event.request.url.endsWith('respondwith-microtask-sync-waituntil') != -1) {
+      event.respondWith(Promise.resolve().then(() => {
+        sync_waituntil(event).then(reportResultExpecting('OK'));
+        return new Response('OK');
+        }));

[falken, 2017/05/24 08:24:18]

did these tests catch an implementation bug?

i wonder if it'd be more useful to test:

response = Promise.resolve(new Response('response'));
event.respondWith(response);
response.then(sync_waituntil); // ok
response.then(async_waituntil); // fails

this way it tests that you can do waitUntil() in the microtask for the promise
passed to respondWith. Maybe we want both?

It'd be good to have negative tests, i.e., that InvalidStateError is thrown.

i found the new Response('OK') confusing since the 'OK' in the response is not
actually used, AFAICT.

[leonhsl(Using Gerrit), 2017/05/25 00:16:39]

I thought test coverage for respondWith is not enough so I created such 2 new
tests.
Yes I do think your suggested tests are better, and sync_waituntil should be OK
and async_waituntil should throw, given the description in spec
https://w3c.github.io/ServiceWorker/#dom-fetchevent-respondwith:
'
Note: event.respondWith(r) extends the lifetime of the event by default as if
event.waitUntil(r) is called.
'

I'll revise the tests and try to solve potential bug around them. Thanks!

[leonhsl(Using Gerrit), 2017/05/25 06:43:55]

Done.

+    } else if (event.request.url.endsWith('respondwith-microtask-async-waituntil') != -1) {
+      event.respondWith(Promise.resolve().then(() => {
+        async_microtask_waituntil(event).then(reportResultExpecting('OK'));
+        return new Response('OK');
+        }));

[falken, 2017/05/24 08:24:18]

nit: indentation looks wrong, 62-63 and 67-68 should be indented an additional 2
spaces

[leonhsl(Using Gerrit), 2017/05/25 00:16:39]

Acknowledged.

+    }
   });
 
 function reportResultExpecting(expectedResult) {