Skip to content

Commit

Permalink
Bug 1536619 [wpt PR 15852] - service worker: Improve WPT tests for as…
Browse files Browse the repository at this point in the history
…ync respondWith/waitUntil., a=testonly

Automatic update from web-platform-tests
service worker: Improve WPT tests for async respondWith/waitUntil. (#15852)

See discussion at [1] and [2].

This makes the following changes.

1.
Adds a test for:

self.addEventListener('fetch', e => {
  Promise.resolve().then(() => {
    e.respondWith(new Response('hi'));
  });
});

This should not throw because respondWith() is called while the event
dispatch flag is still set.

The microtask checkpoint is in "Cleanup After Running Scripts" here:
https://html.spec.whatwg.org/multipage/webappapis.html#clean-up-after-running-script

This is called from step 16.2 here:
https://heycam.github.io/webidl/#call-a-user-objects-operation

Which in turn is called from the DOM spec's "Inner Invoke" to call event
targets:
https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke

2.
Changes the expectation for:

addEventListener('message', event => {
  Promise.resolve().then(event.waitUntil(p));
});

From throws to not throws, for the same reasoning as above.

3.
Changes the expectation for:

addEventListener('message', event => {
  waitPromise = Promise.resolve();
  event.waitUntil(waitPromise);
  waitPromise.then(() => {
    Promise.resolve().then(() => {event.waitUntil();});
  });
});

From throws to not throws. This is subtle. Because all the promises
are just Promise.resolve(), the event dispatch flag is still set
by the time the second waitUntil() is called.

4.
To test what 3. originally intended, a new test is
added which makes waitPromise a promise that does not immediately
resolve.

5.
Changes the expectation for:

addEventListener(‘fetch’, event => {
  response = Promise.resolve(new Response('RESP'));
  event.respondWith(response);
  response.then(() => {
    Promise.resolve().then(() => {event.waitUntil();});
  })
});

Again this is because the promises used resolve immediately,
so the event dispatch flag is still set.

Similarly, a new test is added to cover the original intent.

These WPT changes appear to match the behavior of Safari and Edge while
diverging from Chrome and (partially) Firefox.

[1] w3c/ServiceWorker#1213
[2] w3c/ServiceWorker#1394

Bug: 942414
Change-Id: I9a4a56d71d3919ed614ff78df2bdc6cc0251dadd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524393
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641514}
--

wpt-commits: cecb3eba4dae3d795876f7b4be71bd49afa03356
wpt-pr: 15852
  • Loading branch information
chromium-wpt-export-bot authored and jgraham committed Apr 23, 2019
1 parent 7ce83e5 commit 43a34a8
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 74 deletions.
@@ -1,4 +1,5 @@
<!DOCTYPE html>
<meta name="timeout" content="long">
<script src="/resources/testharness.js"></script>
<script src="resources/testharness-helpers.js"></script>
<script src="/resources/testharnessreport.js"></script>
Expand Down Expand Up @@ -56,21 +57,25 @@
}

promise_test(msg_event_test.bind(this, 'no-current-extension-different-task'),
'Test calling waitUntil in a different task without an existing extension throws');
'Test calling waitUntil in a task at the end of the event handler without an existing extension throws');

promise_test(msg_event_test.bind(this, 'no-current-extension-different-microtask'),
'Test calling waitUntil in a different microtask without an existing extension throws');
'Test calling waitUntil in a microtask at the end of the event handler without an existing extension suceeds');

promise_test(msg_event_test.bind(this, 'current-extension-different-task'),
'Test calling waitUntil in a different task with an existing extension succeeds');
'Test calling waitUntil in a different task an existing extension succeeds');

promise_test(msg_event_test.bind(this, 'current-extension-expired-same-microtask-turn'),
'Test calling waitUntil with an existing extension promise handler succeeds');
promise_test(msg_event_test.bind(this, 'during-event-dispatch-current-extension-expired-same-microtask-turn'),
'Test calling waitUntil at the end of an existing extension promise handler succeeds (event is still being dispatched)');

// The promise handler will queue a new microtask after the check for new
// extensions was performed.
promise_test(msg_event_test.bind(this, 'current-extension-expired-same-microtask-turn-extra'),
'Test calling waitUntil at the end of the microtask turn throws');
promise_test(msg_event_test.bind(this, 'during-event-dispatch-current-extension-expired-same-microtask-turn-extra'),
'Test calling waitUntil in a microtask at the end of an existing extension promise handler succeeds (event is still being dispatched)');

promise_test(msg_event_test.bind(this, 'after-event-dispatch-current-extension-expired-same-microtask-turn'),
'Test calling waitUntil in an existing extension promise handler succeeds (event is not being dispatched)');

promise_test(msg_event_test.bind(this, 'after-event-dispatch-current-extension-expired-same-microtask-turn-extra'),
'Test calling waitUntil in a microtask at the end of an existing extension promise handler throws (event is not being dispatched)');

promise_test(msg_event_test.bind(this, 'current-extension-expired-different-task'),
'Test calling waitUntil after the current extension expired in a different task fails');
Expand All @@ -80,24 +85,36 @@

promise_test(function(t) {
var testBody = function(worker) {
return with_iframe('./resources/pending-respondwith-async-waituntil/dummy.html');
return with_iframe('./resources/pending-respondwith-async-waituntil');
}
return runTest(t, 'pending-respondwith-async-waituntil', testBody);
}, 'Test calling waitUntil asynchronously with pending respondWith promise.');

promise_test(function(t) {
var testBody = function(worker) {
return with_iframe('./resources/respondwith-microtask-sync-waituntil/dummy.html');
return with_iframe('./resources/during-event-dispatch-respondwith-microtask-sync-waituntil');
}
return runTest(t, 'respondwith-microtask-sync-waituntil', testBody);
}, 'Test calling waitUntil synchronously inside microtask of respondWith promise.');
return runTest(t, 'during-event-dispatch-respondwith-microtask-sync-waituntil', testBody);
}, 'Test calling waitUntil synchronously inside microtask of respondWith promise (event is being dispatched).');

promise_test(function(t) {
var testBody = function(worker) {
return with_iframe('./resources/respondwith-microtask-async-waituntil/dummy.html');
return with_iframe('./resources/during-event-dispatch-respondwith-microtask-async-waituntil');
}
return runTest(t, 'respondwith-microtask-async-waituntil', testBody);
}, 'Test calling waitUntil asynchronously inside microtask of respondWith promise.');
return runTest(t, 'during-event-dispatch-respondwith-microtask-async-waituntil', testBody);
}, 'Test calling waitUntil asynchronously inside microtask of respondWith promise (event is being dispatched).');

promise_test(function(t) {
var testBody = function(worker) {
return with_iframe('./resources/after-event-dispatch-respondwith-microtask-sync-waituntil');
}
return runTest(t, 'after-event-dispatch-respondwith-microtask-sync-waituntil', testBody);
}, 'Test calling waitUntil synchronously inside microtask of respondWith promise (event is not being dispatched).');

promise_test(function(t) {
var testBody = function(worker) {
return with_iframe('./resources/after-event-dispatch-respondwith-microtask-async-waituntil');
}
return runTest(t, 'after-event-dispatch-respondwith-microtask-async-waituntil', testBody);
}, 'Test calling waitUntil asynchronously inside microtask of respondWith promise (event is not being dispatched).');
</script>
@@ -1,36 +1,61 @@
<!DOCTYPE html>
<html>
<title>respondWith cannot be called asynchronously</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/test-helpers.sub.js"></script>
<script>
promise_test(function(t) {
var script = 'resources/fetch-event-async-respond-with-worker.js';
var scope = 'resources/simple.html';

return service_worker_unregister_and_register(t, script, scope)
.then(function(registration) {
t.add_cleanup(function() {
return service_worker_unregister(t, scope);
});

return wait_for_state(t, registration.installing, 'activated');
})
.then(function() {
return with_iframe(scope);
})
.then(function(frame) {
add_completion_callback(function() { frame.remove(); });
var channel = new MessageChannel();
var saw_message = new Promise(function(resolve) {
channel.port1.onmessage = function(e) { resolve(e.data); }
});
var worker = frame.contentWindow.navigator.serviceWorker.controller;

worker.postMessage({port: channel.port2}, [channel.port2]);
return saw_message;
})
.then(function(message) {
assert_equals(message, 'PASS');
})
}, 'Calling respondWith asynchronously throws an exception');
// This file has tests that call respondWith() asynchronously.

let frame;
let worker;
const script = 'resources/fetch-event-async-respond-with-worker.js';
const scope = 'resources/simple.html';

// Global setup: this must be the first promise_test.
promise_test(async (t) => {
const registration =
await service_worker_unregister_and_register(t, script, scope);
worker = registration.installing;
await wait_for_state(t, worker, 'activated');
frame = await with_iframe(scope);
}, 'global setup');

// Does one test case. It fetches |url|. The service worker gets a fetch event
// for |url| and attempts to call respondWith() asynchronously. It reports back
// to the test whether an exception was thrown.
async function do_test(url) {
// Send a message to tell the worker a new test case is starting.
const sawMessage = new Promise(resolve => {
navigator.serviceWorker.onmessage = (event) => {
resolve(event.data);
};
worker.postMessage('');
});

// Start a fetch.
frame.contentWindow.fetch(url);

// Receive the test result from the service worker.
return await sawMessage;
};

promise_test(async (t) => {
const result = await do_test('respondWith-in-task');
assert_true(result.didThrow, 'should throw');
assert_equals(result.error, 'InvalidStateError');
}, 'respondWith in a task throws InvalidStateError');

promise_test(async (t) => {
const result = await do_test('respondWith-in-microtask');
assert_equals(result.didThrow, false, 'should not throw');
}, 'respondWith in a microtask does not throw');

// Global cleanup: the final promise_test.
promise_test(async (t) => {
if (frame)
frame.remove();
await service_worker_unregister(t, scope);
}, 'global cleanup');
</script>
</html>
@@ -1,4 +1,12 @@
// controlled by 'init'/'done' messages.
// This worker calls waitUntil() and respondWith() asynchronously and
// reports back to the test whether they threw.
//
// These test cases are confusing. Bear in mind that the event is active
// (calling waitUntil() is allowed) if:
// * The pending promise count is not 0, or
// * The event dispatch flag is set.

// Controlled by 'init'/'done' messages.
var resolveLockPromise;
var port;

Expand All @@ -14,34 +22,72 @@ self.addEventListener('message', function(event) {
case 'done':
resolveLockPromise();
break;

// Throws because waitUntil() is called in a task after event dispatch
// finishes.
case 'no-current-extension-different-task':
async_task_waituntil(event).then(reportResultExpecting('InvalidStateError'));
break;

// OK because waitUntil() is called in a microtask that runs after the
// event handler runs, while the event dispatch flag is still set.
case 'no-current-extension-different-microtask':
async_microtask_waituntil(event).then(reportResultExpecting('InvalidStateError'));
async_microtask_waituntil(event).then(reportResultExpecting('OK'));
break;

// OK because the second waitUntil() is called while the first waitUntil()
// promise is still pending.
case 'current-extension-different-task':
event.waitUntil(new Promise((res) => { resolveTestPromise = res; }));
async_task_waituntil(event).then(reportResultExpecting('OK')).then(resolveTestPromise);
break;
case 'current-extension-expired-same-microtask-turn':

// OK because all promises involved resolve "immediately", so the second
// waitUntil() is called during the microtask checkpoint at the end of
// event dispatching, when the event dispatch flag is still set.
case 'during-event-dispatch-current-extension-expired-same-microtask-turn':
waitPromise = Promise.resolve();
event.waitUntil(waitPromise);
waitPromise.then(() => { return sync_waituntil(event); })
.then(reportResultExpecting('OK'))
break;
case 'current-extension-expired-same-microtask-turn-extra':
// The promise handler queues a new microtask *after* the check for new
// extensions was performed.

// OK for the same reason as above.
case 'during-event-dispatch-current-extension-expired-same-microtask-turn-extra':
waitPromise = Promise.resolve();
event.waitUntil(waitPromise);
waitPromise.then(() => { return async_microtask_waituntil(event); })
.then(reportResultExpecting('OK'))
break;


// OK because the pending promise count is decremented in a microtask
// queued upon fulfillment of the first waitUntil() promise, so the second
// waitUntil() is called while the pending promise count is still
// positive.
case 'after-event-dispatch-current-extension-expired-same-microtask-turn':
waitPromise = makeNewTaskPromise();
event.waitUntil(waitPromise);
waitPromise.then(() => { return sync_waituntil(event); })
.then(reportResultExpecting('OK'))
break;

// Throws because the second waitUntil() is called after the pending
// promise count was decremented to 0.
case 'after-event-dispatch-current-extension-expired-same-microtask-turn-extra':
waitPromise = makeNewTaskPromise();
event.waitUntil(waitPromise);
waitPromise.then(() => { return async_microtask_waituntil(event); })
.then(reportResultExpecting('InvalidStateError'))
break;

// Throws because the second waitUntil() is called in a new task, after
// first waitUntil() promise settled and the event dispatch flag is unset.
case 'current-extension-expired-different-task':
event.waitUntil(Promise.resolve());
async_task_waituntil(event).then(reportResultExpecting('InvalidStateError'));
break;

case 'script-extendable-event':
self.dispatchEvent(new ExtendableEvent('nontrustedevent'));
break;
Expand All @@ -51,25 +97,62 @@ self.addEventListener('message', function(event) {
});

self.addEventListener('fetch', function(event) {
if (event.request.url.indexOf('pending-respondwith-async-waituntil') != -1) {
const path = new URL(event.request.url).pathname;
const step = path.substring(path.lastIndexOf('/') + 1);
let response;
switch (step) {
// OK because waitUntil() is called while the respondWith() promise is still
// unsettled, so the pending promise count is positive.
case 'pending-respondwith-async-waituntil':
var resolveFetch;
let response = new Promise((res) => { resolveFetch = res; });
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.indexOf('respondwith-microtask-sync-waituntil') != -1) {
break;

// OK because all promises involved resolve "immediately", so waitUntil() is
// called during the microtask checkpoint at the end of event dispatching,
// when the event dispatch flag is still set.
case 'during-event-dispatch-respondwith-microtask-sync-waituntil':
response = Promise.resolve(new Response('RESP'));
event.respondWith(response);
response.then(() => { return sync_waituntil(event); })
.then(reportResultExpecting('OK'))
} else if (event.request.url.indexOf('respondwith-microtask-async-waituntil') != -1) {
.then(reportResultExpecting('OK'));
break;

// OK because all promises involved resolve "immediately", so waitUntil() is
// called during the microtask checkpoint at the end of event dispatching,
// when the event dispatch flag is still set.
case 'during-event-dispatch-respondwith-microtask-async-waituntil':
response = Promise.resolve(new Response('RESP'));
event.respondWith(response);
response.then(() => { return async_microtask_waituntil(event); })
.then(reportResultExpecting('OK'));
break;

// OK because the pending promise count is decremented in a microtask queued
// upon fulfillment of the respondWith() promise, so waitUntil() is called
// while the pending promise count is still positive.
case 'after-event-dispatch-respondwith-microtask-sync-waituntil':
response = makeNewTaskPromise().then(() => {return new Response('RESP');});
event.respondWith(response);
response.then(() => { return sync_waituntil(event); })
.then(reportResultExpecting('OK'));
break;


// Throws because waitUntil() is called after the pending promise count was
// decremented to 0.
case 'after-event-dispatch-respondwith-microtask-async-waituntil':
response = makeNewTaskPromise().then(() => {return new Response('RESP');});
event.respondWith(response);
response.then(() => { return async_microtask_waituntil(event); })
.then(reportResultExpecting('InvalidStateError'))
}
});
break;
}
});

self.addEventListener('nontrustedevent', function(event) {
sync_waituntil(event).then(reportResultExpecting('InvalidStateError'));
Expand Down Expand Up @@ -118,3 +201,10 @@ function async_task_waituntil(event) {
}, 0);
});
}

// Returns a promise that settles in a separate task.
function makeNewTaskPromise() {
return new Promise(resolve => {
setTimeout(resolve, 0);
});
}

0 comments on commit 43a34a8

Please sign in to comment.