Julian Descottes [:jdescottes] Reporter
	Description • 7 years ago

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?

	Julian Descottes [:jdescottes] Reporter
	Comment 1 • 7 years ago

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

	Ben Kelly [:bkelly, not reviewing]
	Comment 2 • 7 years ago

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

	Ben Kelly [:bkelly, not reviewing]
	Comment 3 • 7 years ago

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.

	Ben Kelly [:bkelly, not reviewing]
	Comment 4 • 7 years ago

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

	Julian Descottes [:jdescottes] Reporter
	Comment 5 • 7 years ago

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.

	Julian Descottes [:jdescottes] Reporter
	Comment 7 • 7 years ago
mozreview-review

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.

	Patrick Brosset <:pbro>
	Comment 8 • 6 years ago

Mass move to the new application panel component.

	Pulsebot
	Comment 10 • 6 years ago

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1652bf99d12d
Application panel: common api for tests to unregister all workers. r=jdescottes

	Arthur Iakab [arthur_iakab]
	Comment 11 • 6 years ago
bugherder

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