1368625 - ServiceWorkerManagerService could provide an API to simplify picking a ServiceWorkerManagerParent to use for a scope for PBackground consumers

Status: RESOLVED WONTFIX

(Core :: DOM: Service Workers, enhancement, P3)

Description

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Pick a parent v1, with broken AbstractThread::GetCurrent() usage.

I've been cleaning up/enhancing the BackgroundSync API implementation in bug 1217544, most recently simplifying things via the great MozPromise.  (Unfortunately it's currently the great and terrible MozPromise[1] until bug 1366072 stops compelling adoption of the impedence-mismatching AbstractThread.)

The on-PBackground BackgroundSyncService dispatches its "sync" events by adding a new method to PServiceWorkerManager.  It originally just sent these to all ServiceWorkerManagerChild instances (including the parent process main thread one).  To minimize breakage in the "sync registered while online" case, I added some source-actor tracking so that we could send the request back to the same content process it came from.  This was still less than optimal since it meant multiple SW instances are possible when an offline-to-online transition occurred.  Tests can handle this if they use a BroadcastChannel, but some of the demo apps out there don't have this level of paranoia.  (And BroadcastChannel is not universally supported...)

Since bug 1293277 will soon provide us with omniscience about where SW's are running, I decided to try and improve things by migrating the decision process about which SWM to talk to (via SWMC) into the ServiceWorkerManagerService.  (This includes the logic to spin up a new content process if e10s is enabled and one doesn't exist.)  This reduces coupling since the BackgroundSyncService can avoid needing to track ServiceWorkerManagerParent instances and generally simplifies future migration.  There's more on that in the patch comments.

The Push Service can also potentially get in on the improvement (it currently picks the first content process it finds), although things are complicated there by most push logic being on the main thread and the general permissions-broadcasting snafu.  (BackgroundSync depends on the same wait-for-permissions approach used by fetch because the separate PContent and PBackground channels mean that we have no ordering guarantees about the relative arrivals of messages sent over PContent and PBackground unless we ping-pong something over PContent, in which case we absolutely know the thing has reached the child process main thread.)

I'm attaching the current patch for potential discussion, but I need to fix the AbstractThread::GetCurrent() usage which will fail and return null on PBackground.  I also have a test in process which is where I discovered more of what :bkelly was saying about AbstractThread...  Note that my test strategy may be too hacky since I'm trying to bypass IPDL and have the nsIServiceWorkerManager-triggered logic jump directly to the parent-process PBackground thread via MozPromise.  (My rationale is that there's non-trivial boilerplate to using IPDL for same-process-only test logic and that we don't want to clutter our existing PServiceWorkerManager protocol for that or risk granting super-powers to the content processes.  The counter-argument is that I could create an additional protocol that is only allowed to come from within the same process.)

1: This is a Wizard of Oz reference which makes it okay to use the word terrible.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Pick a Parent v2, mozpromise issues resolved

The test infra patch compiles, still finishing up the browser mochitest to test the logic.

Comment 2

[Marion Daly [:mdaly]]

:asuth, what do you think we should do with this bug?

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

We punted on this in favor of just finishing up the e10s work.  Given our closeness to that and other considerations surrounding BackgroundSync, I'm going to WONTFIX this.