Safety versus simplicity in specification tests

[jugglinmike]

I'm working on the Service Worker tests in the Web Platform Tests Project, and I was hoping to get some advice from specification authors and implementers.

While upstreaming tests from the Chromium project, I ran across a Chromium bug report regarding the potential for instability in a testing pattern:

https://bugs.chromium.org/p/chromium/issues/detail?id=558244

In short: because an "idle" service worker may be terminated at any time, tests that track state between events via global variables are susceptible to intermittent failure.

It turns out that there are already tests in Web Platform Tests that suffer from this problem. This has led me to research mitigation techniques. I first used IndexedDB to store state, but I'm reluctant to introduce dependencies on unrelated technologies. More recently, I've been experimenting with a simple "keep alive" protocol where the client uses postMessage on a regular interval, and the worker uses waitUntil on the message events to prevent termination. I described this in more detail while asking for feedback on the public-webapps mailing list:

https://lists.w3.org/Archives/Public/public-webapps/2017JanMar/0035.html

@asutherland confirmed that the approach was sound, but he questioned whether it was necessary, even suggesting that implementing it broadly would degrade the value of the tests:

https://lists.w3.org/Archives/Public/public-webapps/2017JanMar/0040.html

Web reality may also have bearing here. At face value, the specification makes no guarantees about when worker termination may occur. But Andrew tells me that in practice, browsers are not expected to attempt to terminate often enough to impact short-lived tests. And even if this were expected to be a problem, it may be reasonable for the Web Platform Tests project to request that consumers take special precautions to avoid termination while executing the tests.

So I'm left wondering: what is the right answer for the Web Platform Tests project? On the one hand, requiring tests to initiate this "keep alive" protocol will promote test stability. On the other hand, it increases test complexity in a way that may make the them harder to consume, all to guard against an impractical circumstance.

I think the answer requires an understanding of the specification, of implementation reality, and of policy/philosophy for the Web Platform Tests project. That's why I'm asking here and also pinging @Ms2ger, @jgraham, @zcorpan, and @AnneKV.

[jungkees]

IMO, introducing a keep alive protocol across events seems not a good idea. It's against the original design decision that service workers are event-driven and short-running. User agents may also detect and terminate the execution anyway if they regard it an abnormal operation.

I think "3) sending results to the page on each event" in https://bugs.chromium.org/p/chromium/issues/detail?id=558244 is desirable. But if tests can't be designed in that way and need to span multiple ServiceWorkerGlobalScopes' creation/termination, using a storage would be a good direction I think. Can we think about adding some helper functions to make it easier to use a global storage instead of introducing a keep alive protocol?

[aliams]

Thank you @jugglinmike for reaching out about this!

Although I agree that it would make it easier to test certain aspects to have a way to keep service workers alive, I think that it would actually open us up to abuse that may outweigh the benefit. Perhaps @jungkees suggestion would satisfy your requirements?

[jugglinmike]

Thanks for your input, @jungkees and @aliams. To be clear: I am not suggesting
any modification to the specification. This may be a little confusing given
that I'm raising the discussion here, but my goal is to get feedback on a
testing strategy using the API as it is currently defined. (Apologies if this
was already apparent.)

So far, it's sounding like, "yes, worker termination is a practical
concern, so we do need to guard against it," but "no, we shouldn't do it with a
postMessage-based "keep alive" protocol." Maybe those initial experiments
with IndexedDB will come in handy, after all!

Interested to hear if others feel the same way.

[jungkees]

"yes, worker termination is a practical
concern, so we do need to guard against it," but "no, we shouldn't do it with a
postMessage-based "keep alive" protocol." Maybe those initial experiments
with IndexedDB will come in handy, after all!

Yes, that is what I thought. The keep alive protocol approach should be discouraged in practical use. So, if we introduce some helper function/testing strategy for that test pattern, one based on a persistent storage would be preferable in the first place IMO. Comments from other colleagues would be appreciated. Thanks for your contribution to service workers, @jugglinmike!

[asutherland]

For the current execution model of ServiceWorkers, I still think this is overkill that will complicate and slow down the tests given that the testing environment is controlled by the test authors. There is arguably a bug in any SW implementation that kills SW's so aggressively that they can't progress to activation or service multiple requests in rapid succession.

The caveat is #756 on supporting multiple SW instances which will potentially change the de facto execution model. At the July 2016 face-to-face, the Apple team seemed very interested in giving every event a fresh global, and in that case, global state is indeed impossible. I expect this issue to be discussed again at the Apr 3-4 face-to-face.

If multiple instances or fresh globals become a reality, SharedWorkers may also see adoption and they would be a great option as a place to post results to when the test involves multiple pages or otherwise does not want to be disturbed by direct results gathering and processing. They are ideal because of their lifetime that's tied to documents so they won't be arbitrarily terminated, and messages can be dispatched to them from all same-origin contexts in a single turn of the event loop. ex: (new SharedWorker('test.js', 'test-foo')).postMessage(data);. (BroadcastChannels can be used in a single turn too, but they don't support transferrables and are barely more available than SharedWorkers.)

[jugglinmike]

Thanks for the input, everyone! Although Shared Workers do seem like a good fit here, I think their current availability makes them a non-starter as a solution for now. Neither Edge nor Safari currently implement Shared Web Workers. Since they are currently working on their initial service worker implementation, they may have the most to gain from the Web Platform Tests. I'd hate to commit to an approach that shuts them out.

That's why I've proposed a mitigation technique that relies on IndexedDB, see: web-platform-tests/wpt#5206. As noted there, I only implemented the solution for a handful of tests because I'd like to get feedback before starting work on a complete fix. So please let me know what you
think!

[jakearchibald]

I'm happy with leaning on IndexedDB here.

[mfalken]

F2F: I'd also like to discuss how to properly tear down tests (removing iframes and unregistering service workers) in a simple and safe manner.

[jakearchibald]

F2F: Seems like we need async cleanup in WPT.

[jakearchibald]

Closing in favour of web-platform-tests/wpt#7188