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.