This issue is part of the blockers for Bug 1573659 (tracker for the manifest v3 background.service_worker implementation).

Part of this issue goals:

• allow the WebExtensions Framework to ensure that the extension background.service_worker is registered (possibly without creating a webextension page to register it using the WebAPI exposed to the webpages, window.navigator.serviceWorker.register)

• add the background.service_worker property definition to the manifest's JSON schema, and lock it behind a preference (and set it to be disallowed by default)

• ensure that the moz-extension:// service worker are being spawned only in child processes with the "extension" remoteType

• allow the WebExtensions Framework to be sure that all the moz-extensions:// workers are being terminated while the related extension is shutting down

non-goals for this issue:

• allow an extension to register its own moz-extension:// service workers (tracked by Bug 1344561, will be evaluated separately, in the meantime this issue should lock that ability behind a separate about:config preference which disallow it by default)

• provide WebExtension APIs bindings to the background service worker (which is going to be tracked by a separate bugzilla issue, Bug 1609923)

• Adds the new about:config pref "extensions.backgroundServiceWorker.enabled" (currently defaults to false).
• Adds the background.service_worker property to the manifest JSON schema definition
• Locks background.service_worker manifest property behind the new preference
• Adds a new BackgroundWorker class to ext-backgroundPage.js (responsible for managing the background
  service worker for the extension, e.g. make sure that the expected worker script is registered
  as expected when the extension is starting up)
• Adds to the ServiceWorkerManager a new method to allow the WebExtension Framework to register the
  background service worker without an existing extension page
• Allows the "moz-extension" schema in the dom/serviceworkers and dom/cache internals

• Adds a new "extensions.serviceWorkerRegister.allowed" pref (defaults to false)
• Makes ServiceWorkerContainer::Register to throw NS_ERROR_DOM_SECURITY_ERR if the
  script url is a moz-extension url and the caller is a non-system caller
  (but do not throw NS_ERROR_DOM_SECURITY_ERR if the caller is a moz-extension
  and the new pref is set to true)

Depends on D60244

• Include a remoteType property to the RemoteWorkerData
• Changes to RemoteWorkerManager::LaunchNewContentProcess to launch a new process with remoteType "extension"
  when the service worker is from an extension
• Changes to RemoteWorkerManager::SelectTargetActorForServiceWorker to select an actor spawner in a child
  process with remoteType "extension" when the service worker is from an extension
• Changes to RemoteWorkerManager::RegisterActor does not launch an extension service worker
  in a non-"extension" remoteType" child process

Depends on D60245

:asuth :perry hi, do you have any idea on when you would have time to move forward with the review?

We're currently focused on a parent-intercept ServiceWorker regression on trunk (Bug 1622451) but when that's addressed there should be some bandwidth.

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:rpl, could you have a look please?
For more information, please visit auto_nag documentation.

The patches attached to this issue depend on the changes applied in Bug 1642676, which I pushed to autoland yesterday and they did reach mozilla-central today.
I'm going to land these on Nightly 80 as well.

Backed out 4 changesets (bug 1609920) for leaks in browser-chrome. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308197231&repo=autoland&lineNumber=27005

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=1c8faab05606ec8314fa008cfff15df65f304865

Backout:
https://hg.mozilla.org/integration/autoland/rev/728bf9ad480a59d7dfeacff4265c86e0990938af

(In reply to Dorel Luca [:dluca] from comment #10)

Backed out 4 changesets (bug 1609920) for leaks in browser-chrome. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308197231&repo=autoland&lineNumber=27005

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=1c8faab05606ec8314fa008cfff15df65f304865

Backout:
https://hg.mozilla.org/integration/autoland/rev/728bf9ad480a59d7dfeacff4265c86e0990938af

I did spent some time on looking into the failures triggered by LeakSanitizer in the asan build, but I'm not sure that they are really introduced or triggered by this set of patches.
As an example of something that suggest the opposite (the failure may be unrelated), the same leaks seems to be detected even without these patches by running those tests on the mozilla-central tip here:

• https://treeherder.mozilla.org/#/jobs?repo=try&revision=746c0bead63021f04707b6da2733d7d4af3c1e40&selectedTaskRun=LNhykVtXS_a1dJ2DFFPxMg.1

Besides that, by looking to the tests that are running as part of that job, I'm surprised that they would be able to trigger anything new that the attached patches may do (because most of the changes are actually locked behind a pref, which would keep most of the changes inactive in those set of tests, only a very small fragment of the changes may be able to actually be used while these tests are running)

would it be possible to double-check if I'm right (as the push to try linked above seems to suggest) and those failure are not really triggered by this patches?

Hi :mccr8 and :sfoster
The patches attached to this issue have been backed out for some LeakSanitizer failures in browser/components/downloads/test/browser/ tests (See link to the failures in comment 10).

As I did mention in comment 11, I don't see (at least not yet) how the patches backed out are able to trigger that failure (or make it more frequent), and it looks that I can trigger the same (or very similar) failures without any of those patches on a mozilla-central tip (see link to the try push in comment 11).

Could you take a look to the failure in comment 10 and see if you can spot anything that could give us some pointers to get it fixed (or how to investigate it further, or if you think that it is unrelated to the patches attached to this bugzilla issue)?

Thanks in advance for your help!

So it looks like it encountered problems running browser/components/downloads/tests/browser/browser_pdfjs_preview.js, which loads a minimal PDF document into a new tab using pdf.js, and that seems to be where at least some of the leaks are coming from. For context, that test opens "downloaded" PDF files into tabs by interacting with the various download lists - the downloads panel, the library -> all downloads window, the about:downloads view, in both non-private and private windows.

I have no idea how this service worker / moz-extension:// script work would have any impact on that.
I did see some asan test failures in try runs while I've been working on these tests over the last couple of weeks. It might be the most expedient step for now is just to disable this test for asan builds.

Brendan, do the logs from comment 10 mean anything more to you? The browser_pdfjs_preview.js test logs start around line 1740 I'm afraid most of the leak sanitizer output is gobbledy-gook to me.

The leak sanitizer output basically means that we're leaking lots of DOM stuff. This particular leak has been happening frequently but only intermittently for a few weeks now.

It does seem unambiguous that it became a permanent failure with your push, and it also seems to have gone away when your patch got backed out: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=ejWvA3HoRWaFEap_i_tMMA.0&searchStr=bc6%2Clinux&revision=728bf9ad480a59d7dfeacff4265c86e0990938af

....but then about 2 hours later it turned into a permafailure again, which seems to have made it all the way to mozilla-central. I'm not sure what is going on here.

I think I've found the second regressor for this leak. I'll ask sheriffs to back it out once I've got a bit more confirmation.

That was bug 1645538. I wonder if changes to prefs are somehow causing issues. I don't know how that would happen, exactly.

Ok, that wasn't actually a regression. It just added a test and caused the test failure to move from bc12 to bc6. Maybe your patch did something similar. You'd want to look for another ASan bc hitting that same failure on the changeset before you landed, and then look at the raw logs to double check that it contains a reference to browser/components/downloads/test/browser/browser_pdfjs_preview.js

Yeah, it looks like it was failing in bc12 before your patch landed, so it should be okay to reland your patch (assuming this was the only failure it was backed out for). I'm still not sure what was causing it, but it appears it wasn't your patch. Thanks to Aryx for reminding me about tests hopping chunks...

Note that the browser_pdfjs_preview.js test is relatively new: https://hg.mozilla.org/mozilla-central/log/tip/browser/components/downloads/test/browser/browser_pdfjs_preview.js - it only landed this May.

That test does set and unset a pref while it runs using the pushPrefEnv/popPrefEnv helpers, but it would be odd for bug 1645538 to have an impact on this one test that wasn't seen in the many 100s of other test that use them? Unless there is something particular about this test (which isn't impossible as it employs private & not-private windows and other details that combined could make it unique I guess?)

The failure doesn't have anything to do with the patch in this bug. It just added or removed a test, which caused the failure to move from bc12 to bc6, so it looked like a regression, but it was just shifting it.

Depends on D73326

https://hg.mozilla.org/mozilla-central/rev/f567e48e5957
https://hg.mozilla.org/mozilla-central/rev/3e35dbe7e66f
https://hg.mozilla.org/mozilla-central/rev/8ca84442f85d
https://hg.mozilla.org/mozilla-central/rev/fbcb0ee2d17a