We should be implementing https://w3c.github.io/ServiceWorker/#importscripts basically.
<rdar://problem/37164835>
I've been having problems with service worker generated using Google's Workbox tool not working offline after a period of time (on iOS). I believe it's because Workbox splits its functionality into several small .js files which loaded by importScripts. The offline functionality seems to work for while, and then it doesn't. I believe it's because the workbox-v3.2.0/*.js requests in the cache are getting evicted, and they aren't stored otherwise. When if fails, I get "Safari cannot open the page because your iPhone is not connected to the Internet." The app is simple and completely open source: https://dat-shopping-list.glitch.me/ The code to generate the service worker is here: https://glitch.com/edit/#!/dat-shopping-list?path=server/makeServiceWorker.js:1:0 I've tried adding a cache-control http header to the /workbox-* files of 'public,max-age=31536000,immutable' but it seems the scripts still get evicted after a short period of time. The http headers on google's CDN for workbox only has a one hour expiry.
Created attachment 341102 [details] WIP
Created attachment 341764 [details] Patch
Comment on attachment 341764 [details] Patch Attachment 341764 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7926412 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/registration-mime-types.https.html
Created attachment 341770 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 341771 [details] Patch
Comment on attachment 341771 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341771&action=review > Source/WebCore/workers/WorkerScriptLoader.cpp:57 > + bool isServiceWorkerGlobalScope = workerGlobalScope.isServiceWorkerGlobalScope(); is<ServiceWorkerGlobalScope>(workerGlobalScope) is the preferred way. > Source/WebCore/workers/WorkerScriptLoader.cpp:235 > + return !m_cachedScript.isNull() ? m_cachedScript : m_script.toString(); I am not sure how useful m_cachedScript is here. I think we could reuse m_script and merely append() the script to it, no? Looking at the implementation of StringBuilder, a single append(String) then toString() should be pretty efficient. > Source/WebCore/workers/service/ServiceWorkerContextData.cpp:35 > + HashMap<URL, ImportedScript> map; Could you please add a CrossThreadCopierBase specialization for HashMap to CrossThreadCopier.h? (we already have one for Vector and HashSet). Then you can use crossThreadCopy(scriptResourceMap) here. > Source/WebCore/workers/service/ServiceWorkerContextData.h:46 > + String responseURL; Why isn't this stored as a URL type? > Source/WebCore/workers/service/ServiceWorkerContextData.h:145 > + // FIXME: Add a templated version of HashMap encode/decode What's wrong with the one in ArgumentCoders.h? We already encode / decode HashMap, there's got to be a better way than duplicating this code here. > Source/WebCore/workers/service/ServiceWorkerGlobalScope.cpp:133 > +std::optional<ServiceWorkerContextData::ImportedScript> ServiceWorkerGlobalScope::scriptResource(const URL& url) const Cannot we return a ServiceWorkerContextData::ImportedScript* to avoid copying? > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:50 > +static const int schemaVersion = 2; do we need to rename all the v1 methods below to v2? also, there is an ASSERT_NOT_REACHED() below when this is not true: if (currentSchema == v1RecordsTableSchema() || currentSchema == v1RecordsTableSchemaAlternate()) Wouldn't people with an existing database hit it? > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:355 > + HashMap<URL, ServiceWorkerContextData::ImportedScript> scriptResourceMap; Where is this populated? > Source/WebCore/workers/service/server/RegistrationDatabase.cpp:360 > + extra blank line?
Comment on attachment 341771 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341771&action=review >> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:50 >> +static const int schemaVersion = 2; > > do we need to rename all the v1 methods below to v2? > > also, there is an ASSERT_NOT_REACHED() below when this is not true: > if (currentSchema == v1RecordsTableSchema() || currentSchema == v1RecordsTableSchemaAlternate()) > > Wouldn't people with an existing database hit it? I would be good if Brady could take a look at the changes to this file.
Created attachment 341942 [details] Patch
Created attachment 341952 [details] Patch
Created attachment 341966 [details] Patch
Comment on attachment 341966 [details] Patch Clearing flags on attachment: 341966 Committed r232516: <https://trac.webkit.org/changeset/232516>
All reviewed patches have been landed. Closing bug.
Sorry to respond to a close ticket but I can't find a way to know when this fix will be integrated into official (non- technical preview) version of Safari iOS. More generally, is there a table mapping official Safari release and WebKit preview releases ?
(In reply to bertrand from comment #15) > Sorry to respond to a close ticket but I can't find a way to know when this > fix will be integrated into official (non- technical preview) version of > Safari iOS. More generally, is there a table mapping official Safari release > and WebKit preview releases ? Hi Bertrand, I believe Mojave or iOS12 should have the fix.
Thanks Youenn. I'll try Workbox again with ios 12.1.4 safari then. Thus, I assume there is no way to know for sure when a Webkit fix is embedded into an official release... That's a problem.
(In reply to bertrand from comment #17) > Thanks Youenn. I'll try Workbox again with ios 12.1.4 safari then. > > Thus, I assume there is no way to know for sure when a Webkit fix is > embedded into an official release... That's a problem. It's because after we branch, we merge many fixes. So we can't say we included revision X of WebKit in version Y of Safari. Here's some trick. If you open Safari's version number, it'd say something like 14607.1.~. Strip "14" or whatever leading two digits, and then look for that branch in: https://trac.webkit.org/browser/webkit/branches e.g. https://trac.webkit.org/browser/webkit/branches/safari-607-branch in this case. Some tag/branch created off of this branch is probably what Safari includes.