From 598d330d529480e748ecc00c415fe7e57faf4e00 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Wed, 1 May 2019 12:16:25 -0700 Subject: [PATCH] Fix service worker cache response for other origin The service worker should not only be deciding what to cache, but what to pull from cache. A recent refactor to this file overlooked this, which caused resources from other origins not to be available offline, even though they were cached. Change-Id: I62d7c2f96827e778a991b06d0bd222c4f0cb336b --- demo/index.html | 2 +- demo/service_worker.js | 23 ++++++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/demo/index.html b/demo/index.html index c309397d5f..fd06f3e594 100644 --- a/demo/index.html +++ b/demo/index.html @@ -30,7 +30,7 @@ - + diff --git a/demo/service_worker.js b/demo/service_worker.js index 7439fb22b5..ad5e1b3c51 100644 --- a/demo/service_worker.js +++ b/demo/service_worker.js @@ -116,6 +116,8 @@ const OPTIONAL_RESOURCES = [ */ const CACHEABLE_URL_PREFIXES = [ // Anything associated with this application is fair game to cache. + // This would not be necessary if this demo were always served from the same + // location and always used absolute URLs in the resources lists above. location.origin, // Google Web Fonts should be cached when first seen, without being explicitly @@ -187,14 +189,29 @@ function onActivate(event) { function onFetch(event) { // Make sure this is a request we should be handling in the first place. // If it's not, it's important to leave it alone and not call respondWith. - let cacheable = false; + let useCache = false; for (const prefix of CACHEABLE_URL_PREFIXES) { if (event.request.url.startsWith(prefix)) { - cacheable = true; + useCache = true; + break; } } - if (cacheable) { + // Now we need to check our resource lists. The list of prefixes above won't + // cover everything that was installed initially, and those things still need + // to be read from cache. So we check if this request URL matches one of + // those lists. + // The resource lists contain some relative URLs and some absolute URLs. The + // check here will only be able to match the absolute ones, but that's enough, + // because the relative ones are covered by the loop above. + if (!useCache) { + if (CRITICAL_RESOURCES.includes(event.request.url) || + OPTIONAL_RESOURCES.includes(event.request.url)) { + useCache = true; + } + } + + if (useCache) { event.respondWith(fetchCacheableResource(event.request)); } }