Bug 1638099 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

This patch does prevent a service worker previously registered by a webextension to be spawned
if the webextension isn't enabled.

Instead of marking the service worker as disabled (e.g. as part of the registration data that we
store on disk), this patch is currently adding an additional check in
ServiceWorkerPrivateImpl::SpawnIfNeeded to make it early exit with an error if:
- the worker principal has a moz-extension url
- and it does not have a WebExtensionPolicy or the WebExtensionPolicy is not active
  (which would mean that the extension was being uninstalled or disabled)

This additional check along with:
- as part of the extension shutdown the extension background workers are explicitly terminated
  (and so there should be any still running after that)
- as part of the extension uninstall flow all the extension background workers are being unregistered

Should prevent the WebExtension background service worker from being spawned after the extension
has been unable and disabled.

As a side note, "allowing an extension to register other service workers besides the background
service worker defined in the extension manifest.json file" is locked behind a separate pref,
and we are not yet planning to enable that.

Before enabling that, we should also make sure to terminate also all other service workers that
the same extension may have registered (I'll file a separate follow up for that and make it a
blocker for Bug 1344561, which is tracking general service worker API support in extensions).

TODO:
- [ ] figure out a reasonable testing strategy to cover this behavior
- [ ] double-check with Andrew assertion crash triggered by manually forcing a SpawnWorkerIfNeeded failure
      (due to ServiceWorkerUpdateJob::Finish being called twice when SpawnWorkerIfNeeded aborts
      the ServiceWorkerUpdateJob, called once from ServiceWorkerUpdateJob::Update and once more
      from ServiceWorkerUpdateJob::ComparisonResult)

Depends on D119531
This patch does prevent a service worker previously registered by a webextension to be spawned
if the webextension isn't enabled.

Instead of marking the service worker as disabled (e.g. as part of the registration data that we
store on disk), this patch is currently adding an additional check in
ServiceWorkerPrivateImpl::SpawnIfNeeded to make it early exit with an error if:
- the worker principal has a moz-extension url
- and it does not have a WebExtensionPolicy or the WebExtensionPolicy is not active
  (which would mean that the extension was being uninstalled or disabled)

This additional check along with:
- as part of the extension shutdown the extension background workers are explicitly terminated
  (and so there should be any still running after that)
- as part of the extension uninstall flow all the extension background workers are being unregistered

Should prevent the WebExtension background service worker from being spawned after the extension
has been unable and disabled.

As a side note, "allowing an extension to register other service workers besides the background
service worker defined in the extension manifest.json file" is locked behind a separate pref,
and we are not yet planning to enable that.

Before enabling that, we should also make sure to terminate also all other service workers that
the same extension may have registered (I'll file a separate follow up for that and make it a
blocker for Bug 1344561, which is tracking general service worker API support in extensions).

TODO:
- [x] figure out a reasonable testing strategy to cover this behavior
- [ ] double-check with Andrew assertion crash triggered by manually forcing a SpawnWorkerIfNeeded failure
      (due to ServiceWorkerUpdateJob::Finish being called twice when SpawnWorkerIfNeeded aborts
      the ServiceWorkerUpdateJob, called once from ServiceWorkerUpdateJob::Update and once more
      from ServiceWorkerUpdateJob::ComparisonResult)

Depends on D119531

Back to Bug 1638099 Comment 2