importScripts() on ServiceWorkerGlobalScope

[jungkees]

About importScripts() behavior called on various points of service workers, we'd had a long discussion in #1021. We didn't conclude that thread but found how browsers are implemented: #1021 (comment).

And the spec behavior hasn't changed since then. The latest changes around importScripts() just sorted out related issues on the byte-for-byte comparison matters. So, currently the spec flips the flag when the installation of a service worker version completes.

While discussing with my colleagues, I noticed Edge implemented the behavior conforms to the spec. And I checked out again that Chrome and Firefox don't throw when importScripts() is called in functional event tasks, but fetch the script from the network.

@mattto, @wanderview, I'd like to hear your thoughts on the implementation plan.

/cc @jakearchibald @slightlyoff @aliams @hober

[wanderview]

Which version of firefox did you test? I may have inadvertently made late calls to non-offlined importScripts() throw in firefox 61. See the changes here:

https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/dom/workers/ScriptLoader.cpp#1794

Added in:

https://bugzilla.mozilla.org/show_bug.cgi?id=1448141

[wanderview]

Hmm, looks like I got a couple things wrong:

• The spec says to allow importScripts() in parsed or installing states, but I am restricting to parsed.
• The spec says to throw a NetworkError, but I'm throwing InvalidStateError.

I can fix those things depending on what we decide here.

Edit: Filed bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1465670

[wanderview]

@jungkees Do you know if the edge team has a WPT test for this they could upstream?

[jungkees]

@wanderview, thanks for checking with FF. We found we already have a WPT test for this: https://w3c-test.org/service-workers/service-worker/import-scripts-updated-flag.https.html. (It seems we need to fix the error type in the test from TypeError to "NetworkError" DOMException.)

[jungkees]

/cc @josephliccini

[wanderview]

Thanks, although reading the test I think it might not work as intended. Every test case sends a postMessage which will try to trigger an importScripts() after activation, AFAICT. Maybe I'm misreading it, though. I'll have to look later.

[jungkees]

postMessage() can be posted before the install completes. But as you pointed, it seems all the postMessage()s are called after the SW's activated. I'm looking at it, too.

[jungkees]

test_import('root') and test_import('root-and-message') calls in the script actually capture the result of the initial importScripts(). And the messaging between the document and the SW is just to share the test results. The test seems OK.

[wanderview]

Oh, I see. The late importScripts is fine in those cases because the earlier importScripts offlined the result. Thanks for helping me understand.

[mfalken]

I'm planning to fix Chrome to align with the spec. I'm highly motivated because this is actually an annoying code path we are still supporting. But we haven't yet implemented byte-to-byte comparison of importScripts on update. I would like to implement that before deprecating new importScripts after installation, because I think sites use it as a workaround for not doing that.

[jungkees]

Thanks for sharing the plan, @mattto!

[wanderview]

I'm going to update the test to expect NetworkError.

[wanderview]

Also, the test has a bug where it leaves the echo_output from the previous test case set.

[jungkees]

Good catch! I made a PR: web-platform-tests/wpt#11315. @wanderview, PTAL.

[wanderview]

Firefox 62 should follow the spec regarding late importScripts() calls starting with today's nightly.

Edit: Ships in release at the beginning of September.

[mfalken]

Update: Chrome 71 should follow the spec about late importScripts().

[mfalken]

Firefox and Chrome now match the spec. I filed a bug for WebKit at https://bugs.webkit.org/show_bug.cgi?id=188495.

[tophf]

Asking for some guidance, please, now that #1585 tries to allow a dynamic import(): should I open a new issue for allowing a late importScripts or maybe someone else can do it? AFAICT the use case here is the same: importing an arbitrary local script on demand at a later point in time.

This is crucial for complex browser extensions based on a service worker (ManifestV3 in Chrome) and not using ES modules. Previously, extensions were using a standard Window environment for their background script and we could load arbitrary code via a DOM script element so an extension's main background script was small and fast to load on an event, it would then look at the message/event, dynamically load the necessary script, and let the latter handle the event. Complex scripts may have megabytes of code and to make things worse, Chrome doesn't allow code compilation cache for extensions to avoid privilege escalation attacks.

[asutherland]

There's now a WebExtensions Community Group at https://github.com/w3c/webextensions/ announced here that is likely a better place to raise issues with how WebExtensions integrate with ServiceWorkers. Especially as it relates to the implications of WebExtensions already being locally installed but ServiceWorkers being explicitly architected for data coming over the network that then needs to be locally cached.

Do note that the WECG is just getting started, though, so I don't know that they'll be getting to things like that yet.

[tophf]

AFAICT, the final decision will be made in this specification anyway because browser makers really don't like to add nontrivial special cases on top of a complex spec to accommodate just one use case that's not even particularly common. So my reasoning is that since the dynamic import() is being allowed in a precisely defined situation in #1585 that helps WebExtensions among other things, the same reasoning should automatically translate to importScripts when being used in the same situation. I've tried asking in #1585 but apparently I failed to convey the sameness of the use case so I had to delete my comments there as the OP seemed irritated.

[jakearchibald]

I had to delete my comments there as the OP seemed irritated

I asked you to create a new issue to discuss the changes you want made to importScripts, I didn't ask you to delete your comments. I'd still like you to create a new issue fwiw.

So my reasoning is that since the dynamic import() is being allowed in a precisely defined situation in #1585 that helps WebExtensions

The changes in #1585 are designed to make import() somewhat similar to importScripts. What ability do you feel import() is getting in #1585 that doesn't already exist in importScripts? If possible please demonstrate with a reduced runnable example of the issue.

[tophf]

I'd still like you to create a new issue fwiw.

It's a steep curve for me. I thought I'd find a contributor who understands the thing I'm talking about first.

What ability do you feel import() is getting in #1585 that doesn't already exist in importScripts?

AFAICT, it's the ability to load local/offline code dynamically on demand. Currently, we can't use a dynamic import() and we can't use a dynamic (aka postponed/async/late) importScripts. Either of them would allow us to have a tiny fast-to-load service worker that runs only the bare minimum of code on an event, imports another specialized script for that type of event/route, and passes the event to that script to handle.

Theoretically, just having a dynamic import() solves the problem for modern codebases but an async/late importScripts is still useful to import legacy code that exposes global variables/functions without going through the hassle of wrapping it into a module using a WebPack plugin, for example. There are thousands of such libraries around that are still useful and not maintained.

If possible please demonstrate with a reduced runnable example of the issue.

A contrived example:

self.addEventListener('fetch', e => {
  e.respondWith(import(getScriptNameForRequest(e)).then(m => m.default(e)));
});

Similarly for importScripts:

self.addEventListener('fetch', e => {
  if (someCondition(e)) {
    importScripts('./foo.js');
    e.respondWith(foo(e));
  }
});

In case of Chrome extensions ManifestV3, the only difference is that they have lots of API events.

[jakearchibald]

I thought I'd find a contributor who understands the thing I'm talking about first.

An issue is where you explain the thing to (potentially) all contributors. You're currently sticking a vaguely related discussion on the end of an existing issue.

This discussion will be hard to find in future, because the topic is only vaguely related to the current discussion. We've already lost part of the conversation because it happened on yet another issues, and you deleted your comments. That's why I was asking you to create an issue specifically for this discussion. I guess I can only keep asking 😞

we can't use a dynamic (aka postponed/async/late) importScripts

Yes you can.

Demo: https://static-misc-3.glitch.me/late-loading-importscripts/
Code: https://glitch.com/edit/#!/static-misc-3?path=late-loading-importscripts%2Findex.html%3A13%3A37

I asked you for a reduced runnable example of the issue. You didn't do that, you just asserted that it can't be done. It's disappointing that I had to create the demo for you.

If you're still saying that what you want isn't possible, you need to show me what it is you want. I want to see a demo of the thing failing, along with a statement of what you expect to happen.

Please, meet me half way here.

[tophf]

Ah, now I see why we didn't understand each other. The original problem I came here with is that a late importScripts doesn't work in a ManifestV3 extension's service worker because that worker is installed automatically and the browser doesn't run importScripts inside install event, of course, because it can't know which scripts we will use.

So now that the misunderstanding is cleared, hopefully, do you think it'd be theoretically possible to allow a late importScripts without its clone inside the install event if the script is local? Or such a check is impossible to perform dynamically so I shouldn't even bother opening an issue?

[jakearchibald]

importScripts doesn't work in a ManifestV3 extension's service worker because that worker is installed automatically and the browser doesn't run importScripts inside install event

This seems like something for them to fix.

So now that the misunderstanding is cleared

This would have been cleared much sooner if you'd created a reduced runnable example of the issue you had, like I asked.

[tophf]

Sorry for the confusion. To me the "runnable example" was just a single self-explanatory importScripts('foo.js') inside a service worker of an extension so it didn't occur to me to describe this explicitly.

[jakearchibald]

A runnable example is one I can run. Eg https://static-misc-3.glitch.me/late-loading-importscripts/ which you click and it runs.

Your examples weren't runnable, since they didn't contain code to register the service worker etc etc. They were an incomplete picture, and therefore I didn't know what you were on about.