1457407 - Application panel: tests should have an API to unregister all service workers

Status: RESOLVED FIXED

(DevTools :: Application Panel, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Issue raised by :ladybenko.

We are currently relying on the UI to unregister service workers at the end of a test. This is both inefficient and risky, and it would be better to have access to create a "unregisterAll" util.

We can add a method to application panel's head.js to:
- list all workers for the current DebuggerClient
- unregister each worker

But maybe there is another API we could use on the platform side to unregister everything at once?

Comment 1

[Julian Descottes [:jdescottes]]

Ben, do you know if there is any API available to unregister all service workers?

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

What do you mean "relying on the UI to unregister service workers"?  The test harness or something in devtools?

If unregistering is a feature of devtools you could add a check at the end that things have been unregistered.  We have something like that which runs between plain mochitest tests.

As far as an "unregister all", no we don't.  You could use quota manager to wipe the entire origin, though.  Something like:

https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/dom/cache/test/mochitest/test_cache_orphaned_body.html#25-33

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Hmm, although I guess QM wipe does not clear service workers registrations yet.  You would need to use whatever the firefox UI uses for clearing storage.  It has an extra bit that clears registrations.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Its probably easier to just do getRegistrations() and then call unregister() on each one.

Comment 5

[Julian Descottes [:jdescottes]]

Thanks for the feedback. 

> What do you mean "relying on the UI to unregister service workers"?  The test harness or something in devtools?

Yes when I refer to UI here, I refer to the application panel in devtools. It has "unregister" buttons next to each displayed worker, and so far that's what we use at the end of our tests to "cleanup" the state.

> Its probably easier to just do getRegistrations() and then call unregister() on each one.

Yes, that seems like the most straightforward option.

Comment 6

[Belén [:ladybenko]]

Attachment: Bug 1457407 - Application panel: common api for tests to unregister all workers.

Review commit: https://reviewboard.mozilla.org/r/241654/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/241654/

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8973148 [details]
Bug 1457407 - Application panel: common api for tests to unregister all workers.

https://reviewboard.mozilla.org/r/241654/#review247588

This is very nice, thanks! Just one small suggestion, not blocking. 
Try is green, so feel free to apply or discard my comment and ping me when you want to land this changeset!

::: devtools/client/application/test/browser_application_panel_list-several-workers.js:39
(Diff revision 1)
>    info("Wait until the unregister button is displayed for the service worker");
>    await waitUntil(() => getWorkerContainers(doc)[1].querySelector(".unregister-button"));
>  
>    ok(true, "Second service worker registration is displayed");
>  
>    info("Unregister all service workers");

Could we move the info() statement inside of the helper and remove it here? In general I find it helpful to have info() statements before we do anything asynchronous. 

This helps investigating test failures caused by timeouts, because you can easily see where the test stopped by looking at the logs. Helpful when the failure only happens on try for instance.

Comment 8

[Patrick Brosset <:pbro>]

Mass move to the new application panel component.

Comment 9

[Belén [:ladybenko]]

Comment on attachment 8973148 [details]
Bug 1457407 - Application panel: common api for tests to unregister all workers.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/241654/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1652bf99d12d
Application panel: common api for tests to unregister all workers. r=jdescottes

Comment 11

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/1652bf99d12d