web-platform-tests
Service worker: Ensure preloadResponse can be read after responding with something else.
Firefox (nightly)
Testing web-platform-tests at revision f194925e6def470e5fbea4df8916a32c428d4444 Using browser at version BuildID 20170515100238; SourceStamp e66dedabe582ba7b394aee4f89ed70fe389b3c46 Starting 10 test iterations All results were stable
All results
1 test ran
/service-workers/service-worker/navigation-preload/allow-read-after-respondwith.https.html
| Subtest | Results | Messages |
|---|---|---|
| OK | ||
Allow read preloadResponse after respondWith |
FAIL | assert_true: Navigation preload supported expected true got false |
Sauce (safari)
Testing web-platform-tests at revision f194925e6def470e5fbea4df8916a32c428d4444 Using browser at version 10.0 Starting 10 test iterations All results were stable
All results
1 test ran
/service-workers/service-worker/navigation-preload/allow-read-after-respondwith.https.html
| Subtest | Results | Messages |
|---|---|---|
| OK | ||
Service Worker: Nav preload: Allow read after respondwith |
FAIL | SyntaxError: Unexpected keyword 'function' |
Sauce (MicrosoftEdge)
Testing web-platform-tests at revision f194925e6def470e5fbea4df8916a32c428d4444 Using browser at version 14.14393 Starting 10 test iterations All results were stable
All results
1 test ran
/service-workers/service-worker/navigation-preload/allow-read-after-respondwith.https.html
| Subtest | Results | Messages |
|---|---|---|
| OK | ||
Service Worker: Nav preload: Allow read after respondwith |
FAIL | Expected ';' |
Chrome (unstable)
Testing web-platform-tests at revision f194925e6def470e5fbea4df8916a32c428d4444 Using browser at version 60.0.3095.5 dev Starting 10 test iterations
@wanderview
Looks reasonable to me. Do we want to test the no-waitUntil behavior, though? It seems like a lot of people don't use waitUntil() in practice in this kind of code.
This is the big question I have – is it reasonable for browsers to cancel the fetch in this case?
This is the big question I have – is it reasonable for browsers to cancel the fetch in this case?
Cancelling the network load if the SW has indicated its not going to use it seems the right thing from a browser perf perspective.
If this encourages more people to use waitUntil(), that sounds like a win as well.
Maybe @annevk would have an opinion on if there is a reason to hide this behavior from script for some reason.
@annevk
addEventListener('fetch', async event => {
event.respondWith(new Response('whatever'));
const response = await event.preloadResponse;
if (!response) return;
const cache = await caches.open('dynamic-cache');
await cache.put(event.request, response);
});
The above is racy in Chrome. Chrome cancels the preload fetch once the fetch event is complete, assuming it isn't needed. You can avoid this by extending the fetch event using waitUntil, which is best practice.
The spec doesn't cover this, but it seems like a nice performance enhancement. However, I'm not sure we can spec it just yet, as preloadResponse should reject in the same manner as an aborted fetch.
That sounds reasonable to me. Browser aborted fetches are all over the map currently. I hope we can still fix that somehow. (E.g., when you navigate a document, subresource requests get aborted, probably resulting in TypeError exceptions?)
@annevk Well, it depends if we want it to look the same as a developer-aborted fetch.
True, I don't really know. I think we sorta decided that it would (because we were saying the abort signal could come from the browser as well), but we haven't written it down or shared that idea with a wide audience.
It looks like this is the first PR that has results from Edge and Safari via Sauce. Yay @bobholt!
It looks like this is the first PR that has results from Edge and Safari via Sauce. Yay @bobholt!
Is there a reason we are targeting safari 10.0 and edge 14?
I ask because this test is using async functions, but those are not supported until safari 10.1 and edge 15 (according to caniuse).
@wanderview it's been a while, but note that Edge and Safari are no longer running on PRs because of https://github.com/web-platform-tests/wpt/issues/9903. However, I've filed https://github.com/web-platform-tests/wpt/issues/13299 to perhaps bring them back.
This is still failing in Firefox (Nightly) – didn't test Safari yet. The preloadResponse can be accessed, but if respondWith() receives a response before the preload request has finished, the preloadResponse Promise will never resolve and you're stuck with a blank page that loads forever, which is pretty much the worst behavior possible.
Fails:
addEventListener('fetch', (event) => {
const responseStream = new TransformStream()
const response = new Response(responseStream.readable, { headers: { 'content-type': 'text/html' } })
// very high chance of putting the page in deadlock:
// if the request for the preloadResponse isn't done yet, preloadResponse will never resolve
// and the responseStream stays open (with no data written) until the SW is killed after ~5min
event.respondWith(response)
// wait until resolving to the response to give the preload request time to finish –
// if the request's done within the 5s we wait until responding, everything will work:
// event.respondWith(wait(5000).then(() => response))
event.waitUntil(event.preloadResponse.then(preload => preload.body.pipeTo(responseStream.writable)))
})
I have to work around that by racing the preload against a fetch so a stuck preload doesn't keep the whole pageload in deadlock, but that means I'm double-fetching:
const res = await Promise.any([
event.preloadResponse.then(preload => preload || Promise.reject()),
// in FF, preloadResponse can get stuck and NEVER resolves, so we must fetch even if a preloadResponse exists :(
// https://bugzilla.mozilla.org/show_bug.cgi?id=1495703, https://github.com/web-platform-tests/wpt/pull/5921
fetch(event.request),
])
Or maybe we need a different test "Ensure preloadResponse can be read while responding with something else"?
