From b1a51bdbf4f8a1eb4c92d9108a10b77005a07e96 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 24 Sep 2020 12:47:09 +0000 Subject: [PATCH 1/5] [FIX] website_event_track: listen for pwa install event earlier To display the PWA's install banner, the browser triggers a `beforeinstallprompt` event when he estimates that all the requirements for installation are fulfilled (e.g. Service-worker, manifest...). But we have observed that the browser can trigger this event really early in the loading of the page and even before our listener has been registered, failing to display the banner. This commit moves the event listener's registration out of the lifecycle of our widget to allow it to be registered as soon as possible, drastically reducing the risk of missed-events. X-original-commit: a014a6c9e3baced22b158d6d8b4199bdae7562b9 --- .../static/src/js/website_event_pwa_widget.js | 74 ++++++++++--------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/addons/website_event_track/static/src/js/website_event_pwa_widget.js b/addons/website_event_track/static/src/js/website_event_pwa_widget.js index 1eb93a9cc22..7953c9c132b 100644 --- a/addons/website_event_track/static/src/js/website_event_pwa_widget.js +++ b/addons/website_event_track/static/src/js/website_event_pwa_widget.js @@ -1,6 +1,21 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { "use strict"; + /* + * The "deferredPrompt" Promise will resolve only if the "beforeinstallprompt" event + * has been triggered. It allows to register this listener as soon as possible + * to avoid missed-events (as the browser can trigger it very early in the page lifecycle). + */ + var deferredPrompt = new Promise(function (resolve, reject) { + if (!("serviceWorker" in navigator)) { + return reject(); + } + window.addEventListener("beforeinstallprompt", function (ev) { + ev.preventDefault(); + resolve(ev); + }); + }); + var config = require("web.config"); var publicWidget = require("web.public.widget"); var utils = require("web.utils"); @@ -35,23 +50,21 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { prompt_close_bar: "_onPromptCloseBar", }, - /** - * - * @override - */ - init: function () { - this._super.apply(this, arguments); - this.beforeInstallPromptHandler = this._onBeforeInstallPrompt.bind(this); - }, - /** * * @override */ start: function () { - var superProm = this._super.apply(this, arguments); - window.addEventListener("beforeinstallprompt", this.beforeInstallPromptHandler); - return superProm.then(this._registerServiceWorker.bind(this)).then(this._prefetch.bind(this)); + var self = this; + return this._super.apply(this, arguments) + .then(this._registerServiceWorker.bind(this)) + .then(function () { + // Don't wait for the prompt's Promise as it may never resolve. + deferredPrompt.then(self._showInstallBanner.bind(self)).catch(function () { + console.log("ServiceWorker not supported"); + }); + }) + .then(this._prefetch.bind(this)); }, /** @@ -59,7 +72,6 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { * @override */ destroy: function () { - window.removeEventListener("beforeinstallprompt", this.beforeInstallPromptHandler); this._super.apply(this, arguments); }, @@ -156,6 +168,9 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { * @private */ _showInstallBanner: function () { + if (!config.device.isMobile) { + return; + } var self = this; this.installBanner = new PWAInstallBanner(this); this.installBanner.appendTo(this.$el).then(function () { @@ -169,18 +184,6 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { // Handlers //-------------------------------------------------------------------------- - /** - * @private - * @param ev {Event} - */ - _onBeforeInstallPrompt: function (ev) { - if (!config.device.isMobile) { - return; - } - ev.preventDefault(); - this.deferredPrompt = ev; - this._showInstallBanner(); - }, /** * @private * @param ev {Event} @@ -195,15 +198,20 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { */ _onPromptInstall: function (ev) { ev.stopPropagation(); - this.deferredPrompt.prompt(); this._hideInstallBanner(); - this.deferredPrompt.userChoice.then(function (choiceResult) { - if (choiceResult.outcome === "accepted") { - console.log("User accepted the install prompt"); - } else { - console.log("User dismissed the install prompt"); - } - }); + deferredPrompt.then(function (prompt) { + prompt.prompt(); + prompt.userChoice.then(function (choiceResult) { + if (choiceResult.outcome === "accepted") { + console.log("User accepted the install prompt"); + } else { + console.log("User dismissed the install prompt"); + } + }); + }) + .catch(function () { + console.log("ServiceWorker not supported"); + }); }, }); From 92aee2732dcb38e5a5299bb31f298a8d11ada830 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 24 Sep 2020 12:50:33 +0000 Subject: [PATCH 2/5] [FIX] website_event_track: properly match custom lang in url In multilanguage website, URLs are automatically prefixed with the language (beside the default language) but only the default language-slug were supported (e.g. "en", "en_US"...). This commit supports also customized language-slug, which can actually be any string, for the ServiceWorker's scope. X-original-commit: 20eddda77d0ed6f2bb9b61d8ae4194e960ccc4ff --- .../static/src/js/website_event_pwa_widget.js | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/addons/website_event_track/static/src/js/website_event_pwa_widget.js b/addons/website_event_track/static/src/js/website_event_pwa_widget.js index 7953c9c132b..09a3f635417 100644 --- a/addons/website_event_track/static/src/js/website_event_pwa_widget.js +++ b/addons/website_event_track/static/src/js/website_event_pwa_widget.js @@ -79,32 +79,21 @@ odoo.define("website_event_track.website_event_pwa_widget", function (require) { // Private //-------------------------------------------------------------------------- - /** - * Returns the website's language from the URL - * - * Note: this method performs a matching against both the simple (eg. "en") and complete (eg. "en_US") - * language formatting in the url. This is required as the cookie always returns the complete - * form but the URL can use both, depending on the language (eg. "en_US" => "en" but "en_UK" => "en_UK"). - * @private - * @return {String} - */ - _getLangPrefix: function () { - var lang = utils.get_cookie("frontend_lang"); - var path = window.location.pathname; - var matches = path.match(new RegExp("^\/?(" + lang + "|" + lang.slice(0, 2) + ")\/")); - if (lang && matches && matches[1]) { - return "/" + matches[1]; - } - return ""; - }, - /** * Returns the PWA's scope + * + * Note: this method performs a matching to handle URLs with the language prefix. + * Typically this prefix is in the form of "en" or "en_US" but it can also be + * any string using the customization options in the Website's settings. * @private * @returns {String} */ _getScope: function () { - return this._getLangPrefix() + "/event"; + var matches = window.location.pathname.match(/^(\/(?:event|[^/]+\/event))\/?/); + if (matches && matches[1]) { + return matches[1]; + } + return "/event"; }, /** From 65c65b6c1999283a211f94205019aa381e64e6d1 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 24 Sep 2020 12:51:45 +0000 Subject: [PATCH 3/5] [FIX] website_event_track: don't clutter the ServiceWorker cache This commit aims at reducing the requests stored in the cache by filtering out the error-like requests (404, 500...) and the "opaque" requests. Also this commit adds a check for the cache size to prevent it from filling up, resulting into a locked out cache (blocking certain requests). X-original-commit: ef2d323c3a029cb58ee219f06a9809aa52d108b3 Co-authored-by: Olivier Dony --- .../static/src/js/service_worker.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/addons/website_event_track/static/src/js/service_worker.js b/addons/website_event_track/static/src/js/service_worker.js index aec317435ef..4b8772de3c3 100644 --- a/addons/website_event_track/static/src/js/service_worker.js +++ b/addons/website_event_track/static/src/js/service_worker.js @@ -3,6 +3,8 @@ importScripts("/website_event_track/static/lib/idb-keyval/idb-keyval.js"); const PREFIX = "odoo-event"; const SYNCABLE_ROUTES = ["/event/track/toggle_reminder"]; const CACHABLE_ROUTES = ["/web/webclient/version_info"]; +const MAX_CACHE_SIZE = 512 * 1024 * 1024; // 500 MB +const MAX_CACHE_QUOTA = 0.5; const { Store, set, get, del } = idbKeyval; const pendingRequestsQueueName = `${PREFIX}-pending-requests`; @@ -44,6 +46,18 @@ const isSyncableURL = canHandleRoutes(SYNCABLE_ROUTES); */ const isCachableURL = canHandleRoutes(CACHABLE_ROUTES); +/** + * + * @returns {boolean} true if navigator has a quota we can read and we reached it + */ +const isCacheFull = async () => { + if (!("storage" in navigator && "estimate" in navigator.storage)) { + return false; + } + const { usage, quota } = await navigator.storage.estimate(); + return usage / quota > MAX_CACHE_QUOTA || usage > MAX_CACHE_SIZE; +}; + /** * * @return {Promise} @@ -133,6 +147,26 @@ const buildEmptyResponse = () => new Response(JSON.stringify({ jsonrpc: "2.0", i * @returns {Promise} */ const cacheRequest = async (request, response) => { + // don't even attempt to cache: + // - error pages (why cache that?) + // - non-"basic" response types, which include tracker 1-time opaque requests + // that are consuming cache space for no reason (namely due to padding MBs accounted for + // each opaque request) + if (!response || !response.ok || response.type !== "basic") { + console.error(`ignoring cache for ${request.url} => ${response.type}, mode: ${request.mode}, cache: ${request.cache}`); + return; + } + + // never blow up cache quota, as it will break things, and the space + // is shared with cookies and localStorage + if (await isCacheFull()) { + // TODO: clear some part of the cache to free older/less-relevant content + console.log("Cache full, not caching!"); + return; + } + + console.log(`grant cache for ${request.url} => ${response.type}, mode: ${request.mode}, cache: ${request.cache}, + isGet: ${isGET(request)}, isCachable: ${isCachableURL(request.url)}`); if (isGET(request)) { const cache = await caches.open(cacheName); await cache.put(request, response.clone()); From 0e56f7ce7df9b30eff7e1c09232eb1199700804d Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 24 Sep 2020 12:52:39 +0000 Subject: [PATCH 4/5] [FIX] website_event_track: reuse fetch logic for prefetching urls Before this commit, when prefetching urls if one of them fails, the following are dropped. This commit fixes it by handling each url seperately instead of sending all of them to `cache.addAll()`. It also reuse the same logic as the regular FetchEvent for consistency. X-original-commit: 47fdfb8ae73ab79100fb83751ff0f132b4a5c628 Co-authored-by: Olivier Dony --- .../static/src/js/service_worker.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/addons/website_event_track/static/src/js/service_worker.js b/addons/website_event_track/static/src/js/service_worker.js index 4b8772de3c3..c6b6800aacf 100644 --- a/addons/website_event_track/static/src/js/service_worker.js +++ b/addons/website_event_track/static/src/js/service_worker.js @@ -225,7 +225,7 @@ const matchCache = async (request) => { * @param {FetchEvent} param0 * @returns {Promise} */ -const processFetchEvent = async ({ request }) => { +const processFetchRequest = async ({ request }) => { const requestCopy = request.clone(); let response; try { @@ -288,11 +288,17 @@ const processPendingRequests = async () => { */ const prefetchUrls = async (urls = []) => { const cache = await caches.open(cacheName); - let urlsToCache = new Set(urls); - for (let url of urlsToCache) { - (await cache.match(url)) ? urlsToCache.delete(url) : undefined; + const uniqUrls = new Set(urls); + for (let url of uniqUrls) { + if (await cache.match(url)) { + continue; + } + try { + await processFetchRequest({ request: new Request(url) }); + } catch (error) { + console.error(`fail to prefetch ${url} : ${error}`); + } } - return cache.addAll([...urlsToCache]); }; /** @@ -327,7 +333,7 @@ const processMessage = (data) => { }; self.addEventListener("fetch", (event) => { - event.respondWith(processFetchEvent(event)); + event.respondWith(processFetchRequest(event)); }); self.addEventListener("sync", (event) => { From dc95d43efbfbbe646e41ab5b7d4f5e2431f39c91 Mon Sep 17 00:00:00 2001 From: Pierre Paridans Date: Thu, 24 Sep 2020 12:55:54 +0000 Subject: [PATCH 5/5] [FIX] website_event_track: only cache local & cdn based urls This commit allows to filter out the requests to local-ish domains (cf. the current host and an optionally configured CDN for the current website) and preventing from cluttering the cache used by the ServiceWorker with tracking files, scripts or other non-necessary assets. This allows to even further reduce the possible "opaque" requests from being stored in the cache (as they use way more space than regular/basic responses). X-original-commit: 052bc7665e89426fb51928e43bd4a15dbf6cb705 --- addons/website_event_track/controllers/webmanifest.py | 7 ++++++- .../static/src/js/service_worker.js | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/addons/website_event_track/controllers/webmanifest.py b/addons/website_event_track/controllers/webmanifest.py index 9f4967ed70d..627c2581577 100644 --- a/addons/website_event_track/controllers/webmanifest.py +++ b/addons/website_event_track/controllers/webmanifest.py @@ -48,8 +48,13 @@ class TrackManifest(http.Controller): """ Returns a ServiceWorker javascript file scoped for website_event """ sw_file = get_module_resource('website_event_track', 'static/src/js/service_worker.js') - with open(sw_file, 'rb') as fp: + with open(sw_file, 'r') as fp: body = fp.read() + js_cdn_url = 'undefined' + if request.website.cdn_activated: + cdn_url = request.website.cdn_url.replace('"','%22').replace('\x5c','%5C') + js_cdn_url = '"%s"' % cdn_url + body = body.replace('__ODOO_CDN_URL__', js_cdn_url) response = request.make_response(body, [ ('Content-Type', 'text/javascript'), ('Service-Worker-Allowed', url_for('/event')), diff --git a/addons/website_event_track/static/src/js/service_worker.js b/addons/website_event_track/static/src/js/service_worker.js index c6b6800aacf..41ce7e54297 100644 --- a/addons/website_event_track/static/src/js/service_worker.js +++ b/addons/website_event_track/static/src/js/service_worker.js @@ -5,6 +5,7 @@ const SYNCABLE_ROUTES = ["/event/track/toggle_reminder"]; const CACHABLE_ROUTES = ["/web/webclient/version_info"]; const MAX_CACHE_SIZE = 512 * 1024 * 1024; // 500 MB const MAX_CACHE_QUOTA = 0.5; +const CDN_URL = __ODOO_CDN_URL__; // {string|undefined} the cdn_url configured for the website if activated const { Store, set, get, del } = idbKeyval; const pendingRequestsQueueName = `${PREFIX}-pending-requests`; @@ -12,6 +13,8 @@ const cacheName = `${PREFIX}-cache`; const syncStore = new Store(`${PREFIX}-sync-db`, `${PREFIX}-sync-store`); const cacheStore = new Store(`${PREFIX}-cache-db`, `${PREFIX}-cache-store`); const offlineRoute = `${self.registration.scope}/offline`; +const scopeURL = new URL(self.registration.scope); +const cdnURL = CDN_URL ? (CDN_URL.startsWith("http") ? new URL(CDN_URL) : new URL(`http:${CDN_URL}`)) : undefined; /** * @@ -147,6 +150,13 @@ const buildEmptyResponse = () => new Response(JSON.stringify({ jsonrpc: "2.0", i * @returns {Promise} */ const cacheRequest = async (request, response) => { + // only attempts to cache local or cdn delivered urls + const url = new URL(request.url); + if (url.hostname !== scopeURL.hostname && (!cdnURL || url.hostname !== cdnURL.hostname)) { + console.error(`ignoring cache for ${request.url} => ${url.hostname}, local: ${scopeURL.hostname}, cdn: ${cdnURL ? cdnURL.hostname : cdnURL}`); + return; + } + // don't even attempt to cache: // - error pages (why cache that?) // - non-"basic" response types, which include tracker 1-time opaque requests