From 5d71b655adf4a2044586e9dd69e4172c813a9d33 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 18 Mar 2021 19:34:41 +0100 Subject: [PATCH 1/3] halt any fetch request while waiting for new service worker to activate this make updates apply instantly rather than sometimes being stalled for seconds or minutes. --- src/platform/web/Platform.js | 2 +- src/platform/web/dom/ServiceWorkerHandler.js | 46 +++++++++++++------- src/platform/web/dom/request/fetch.js | 9 +++- src/platform/web/service-worker.template.js | 37 +++++++++++----- 4 files changed, 64 insertions(+), 30 deletions(-) diff --git a/src/platform/web/Platform.js b/src/platform/web/Platform.js index a1e8e52c..7682dfd3 100644 --- a/src/platform/web/Platform.js +++ b/src/platform/web/Platform.js @@ -107,7 +107,7 @@ export class Platform { this.sessionInfoStorage = new SessionInfoStorage("hydrogen_sessions_v1"); this.estimateStorageUsage = estimateStorageUsage; if (typeof fetch === "function") { - this.request = createFetchRequest(this.clock.createTimeout); + this.request = createFetchRequest(this.clock.createTimeout, this._serviceWorkerHandler); } else { this.request = xhrRequest; } diff --git a/src/platform/web/dom/ServiceWorkerHandler.js b/src/platform/web/dom/ServiceWorkerHandler.js index b05505ea..668c7d64 100644 --- a/src/platform/web/dom/ServiceWorkerHandler.js +++ b/src/platform/web/dom/ServiceWorkerHandler.js @@ -26,6 +26,7 @@ export class ServiceWorkerHandler { this._registration = null; this._registrationPromise = null; this._currentController = null; + this.haltRequests = false; } setNavigation(navigation) { @@ -39,10 +40,12 @@ export class ServiceWorkerHandler { this._registration = await navigator.serviceWorker.register(path); await navigator.serviceWorker.ready; this._currentController = navigator.serviceWorker.controller; - this._registrationPromise = null; - console.log("Service Worker registered"); this._registration.addEventListener("updatefound", this); - this._tryActivateUpdate(); + this._registrationPromise = null; + if (this._registration.waiting) { + this._proposeUpdate(); + } + console.log("Service Worker registered"); })(); } @@ -61,6 +64,10 @@ export class ServiceWorkerHandler { this._closeSessionIfNeeded(sessionId).finally(() => { event.source.postMessage({replyTo: data.id}); }); + } else if (data.type === "haltRequests") { + // this flag is read in fetch.js + this.haltRequests = true; + event.source.postMessage({replyTo: data.id}); } } @@ -82,15 +89,19 @@ export class ServiceWorkerHandler { } } - async _tryActivateUpdate() { - // we don't do confirm when the tab is hidden because it will block the event loop and prevent - // events from the service worker to be processed (like controllerchange when the visible tab applies the update). - if (!document.hidden && this._registration.waiting && this._registration.active) { - this._registration.waiting.removeEventListener("statechange", this); - const version = await this._sendAndWaitForReply("version", null, this._registration.waiting); - if (confirm(`Version ${version.version} (${version.buildHash}) is ready to install. Apply now?`)) { - this._registration.waiting.postMessage({type: "skipWaiting"}); // will trigger controllerchange event - } + async _proposeUpdate() { + if (document.hidden) { + return; + } + const version = await this._sendAndWaitForReply("version"); + if (confirm(`Version ${version.version} (${version.buildHash}) is available. Reload to apply?`)) { + // prevent any fetch requests from going to the service worker + // from any client, so that it is not kept active + // when calling skipWaiting on the new one + await this._sendAndWaitForReply("haltRequests"); + // only once all requests are blocked, ask the new + // service worker to skipWaiting + this._send("skipWaiting", null, this._registration.waiting); } } @@ -101,11 +112,14 @@ export class ServiceWorkerHandler { break; case "updatefound": this._registration.installing.addEventListener("statechange", this); - this._tryActivateUpdate(); break; - case "statechange": - this._tryActivateUpdate(); + case "statechange": { + if (event.target.state === "installed") { + this._proposeUpdate(); + event.target.removeEventListener("statechange", this); + } break; + } case "controllerchange": if (!this._currentController) { // Clients.claim() in the SW can trigger a controllerchange event @@ -115,7 +129,7 @@ export class ServiceWorkerHandler { } else { // active service worker changed, // refresh, so we can get all assets - // (and not some if we would not refresh) + // (and not only some if we would not refresh) // up to date from it document.location.reload(); } diff --git a/src/platform/web/dom/request/fetch.js b/src/platform/web/dom/request/fetch.js index dd3b7949..1222c41b 100644 --- a/src/platform/web/dom/request/fetch.js +++ b/src/platform/web/dom/request/fetch.js @@ -51,8 +51,15 @@ class RequestResult { } } -export function createFetchRequest(createTimeout) { +export function createFetchRequest(createTimeout, serviceWorkerHandler) { return function fetchRequest(url, requestOptions) { + if (serviceWorkerHandler?.haltRequests) { + // prevent any requests while waiting + // for the new service worker to get activated. + // Once this happens, the page will be reloaded + // by the serviceWorkerHandler so this is fine. + return new RequestResult(new Promise(() => {}), {}); + } // fetch doesn't do upload progress yet, delegate to xhr if (requestOptions?.uploadProgress) { return xhrRequest(url, requestOptions); diff --git a/src/platform/web/service-worker.template.js b/src/platform/web/service-worker.template.js index e3fa4651..396783e4 100644 --- a/src/platform/web/service-worker.template.js +++ b/src/platform/web/service-worker.template.js @@ -37,6 +37,13 @@ self.addEventListener('install', function(e) { })()); }); +self.addEventListener('activate', (event) => { + // on a first page load/sw install, + // start using the service worker on all pages straight away + self.clients.claim(); + event.waitUntil(purgeOldCaches()); +}); + async function purgeOldCaches() { // remove any caches we don't know about const keyList = await caches.keys(); @@ -60,15 +67,6 @@ async function purgeOldCaches() { } } -self.addEventListener('activate', (event) => { - event.waitUntil(Promise.all([ - purgeOldCaches(), - // on a first page load/sw install, - // start using the service worker on all pages straight away - self.clients.claim() - ])); -}); - self.addEventListener('fetch', (event) => { event.respondWith(handleRequest(event.request)); }); @@ -85,9 +83,11 @@ function isCacheableThumbnail(url) { } const baseURL = new URL(self.registration.scope); +let pendingFetchAbortController = new AbortController(); async function handleRequest(request) { try { const url = new URL(request.url); + // rewrite / to /index.html so it hits the cache if (url.origin === baseURL.origin && url.pathname === baseURL.pathname) { request = new Request(new URL("index.html", baseURL.href)); } @@ -96,15 +96,15 @@ async function handleRequest(request) { // use cors so the resource in the cache isn't opaque and uses up to 7mb // https://developers.google.com/web/tools/chrome-devtools/progressive-web-apps?utm_source=devtools#opaque-responses if (isCacheableThumbnail(url)) { - response = await fetch(request, {mode: "cors", credentials: "omit"}); + response = await fetch(request, {signal: pendingFetchAbortController.signal, mode: "cors", credentials: "omit"}); } else { - response = await fetch(request); + response = await fetch(request, {signal: pendingFetchAbortController.signal}); } await updateCache(request, response); } return response; } catch (err) { - if (!(err instanceof TypeError)) { + if (err.name !== "TypeError" && err.name !== "AbortError") { console.error("error in service worker", err); } throw err; @@ -172,6 +172,9 @@ self.addEventListener('message', (event) => { case "skipWaiting": self.skipWaiting(); break; + case "haltRequests": + event.waitUntil(haltRequests().then(() => reply())); + break; case "closeSession": event.waitUntil( closeSession(event.data.payload.sessionId, event.source.id) @@ -192,6 +195,16 @@ async function closeSession(sessionId, requestingClientId) { })); } +async function haltRequests() { + // first ask all clients to block sending any more requests + const clients = await self.clients.matchAll({type: "window"}); + await Promise.all(clients.map(client => { + return sendAndWaitForReply(client, "haltRequests"); + })); + // and only then abort the current requests + pendingFetchAbortController.abort(); +} + const pendingReplies = new Map(); let messageIdCounter = 0; function sendAndWaitForReply(client, type, payload) { From 017d3818eb874e91dabeb7243241246961ee5b76 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 18 Mar 2021 19:49:06 +0100 Subject: [PATCH 2/3] always reply here --- src/platform/web/service-worker.template.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/web/service-worker.template.js b/src/platform/web/service-worker.template.js index 396783e4..ebb31cdb 100644 --- a/src/platform/web/service-worker.template.js +++ b/src/platform/web/service-worker.template.js @@ -173,12 +173,12 @@ self.addEventListener('message', (event) => { self.skipWaiting(); break; case "haltRequests": - event.waitUntil(haltRequests().then(() => reply())); + event.waitUntil(haltRequests().finally(() => reply())); break; case "closeSession": event.waitUntil( closeSession(event.data.payload.sessionId, event.source.id) - .then(() => reply()) + .finally(() => reply()) ); break; } From ffdec1607635a150d3dafb0885d7b1caf3254dbb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 18 Mar 2021 19:58:50 +0100 Subject: [PATCH 3/3] don't show the update dialog if we open the app for the first time and don't have a service worker yet --- src/platform/web/dom/ServiceWorkerHandler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/web/dom/ServiceWorkerHandler.js b/src/platform/web/dom/ServiceWorkerHandler.js index 668c7d64..f5bae8d0 100644 --- a/src/platform/web/dom/ServiceWorkerHandler.js +++ b/src/platform/web/dom/ServiceWorkerHandler.js @@ -42,7 +42,8 @@ export class ServiceWorkerHandler { this._currentController = navigator.serviceWorker.controller; this._registration.addEventListener("updatefound", this); this._registrationPromise = null; - if (this._registration.waiting) { + // do we have a new service worker waiting to activate? + if (this._registration.waiting && this._registration.active) { this._proposeUpdate(); } console.log("Service Worker registered");