[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

 // controlled by 'init'/'done' messages.
 var resolveLockPromise;
 var port;
 
 self.addEventListener('message', function(event) {
     var waitPromise;
     var resolveTestPromise;
 
     switch (event.data.step) {
       case 'init':
@@ skipping 32 matching lines @@
         async_task_waituntil(event).then(reportResultExpecting('InvalidStateError'));
         break;
       case 'script-extendable-event':
         new_event_waituntil().then(reportResultExpecting('InvalidStateError'));
         break;
     }
     event.source.postMessage('ACK');
   });
 
 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) {
   return function (result) {
     port.postMessage({result : result, expected: expectedResult});
     return result;
   };
 }
 
 function sync_waituntil(event) {
@@ skipping 37 matching lines @@
     setTimeout(() => {
       try {
         event.waitUntil(Promise.resolve());
         res('OK');
       } catch (error) {
         res(error.name);
       }
     }, 0);
   });
 }