1182117 - Move ServiceWorkerManager to the parent process in e10s

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Cătălin Badea (:catalinb)]

We want to move the SWM and all service worker data to the parent process and spawn service workers in the child process when needed.

Comment 1

[Noemí Freire (:noemi)]

Hi Catalin,

Some bugs we need to work on in b2g depends on this one, in order to properly organize the workload for FxOS-S3 sprint starting today, is there an estimate ETA for this?. Thanks!

Comment 2

[Cătălin Badea (:catalinb)]

(In reply to Noemí Freire (:noemi) from comment #1)
> Hi Catalin,
> 
> Some bugs we need to work on in b2g depends on this one, in order to
> properly organize the workload for FxOS-S3 sprint starting today, is there
> an estimate ETA for this?. Thanks!

Hi Noemi,

So this is supposed to be my internship project for the summer. It will take some time, because there are quite a few details that need to be figured out before implementing the multi-process ServiceWorkerManager. Nikhil estimated this will take two months, but I'd like to get it done sooner than that.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Catalin, you were asking me today about how strict you should be in checking principals:

I spoke with Jonas and understand things a little bit better.

I think the short answer is that you should be strictly checking the principal of the page using service workers with AssertAppPrincipal() in the parent process.

I think the difficult thing to answer is what to do for utilities like about:serviceworkers, the settings app panel, and devtools.

Generally we should not allow chrome script to run in the child process with the system principal.  This is because we can't trust the system principal in IPC messages since it could be spoofed.

Its easiest to answer this for devtools.  Its chrome script and can run in the parent process.  Its easy to provide an API there.  This should be our long term solution.

For about:serviceworkers and settings app, its unclear there is a good answer.  Any API we could provide could be spoofed.  I think the only sane thing to do is to provide this API in a very restrictive manner and then remove it as soon as devtools support lands.  So the API would only allow listing, updating, and removing registrations.

If we can, we should verify this is only coming from the settings appid and the child process matches as well.

Jonas, is this a reasonable short term solution to accommodate our poor-mans-devtools solutions?

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Ben Kelly [:bkelly] from comment #3)
> If we can, we should verify this is only coming from the settings appid and
> the child process matches as well.

Note that for updates we need to trigger them from the appropriate child process according to https://bugzilla.mozilla.org/show_bug.cgi?id=1156092#c8 (bug 1181544) so if I am not wrong the appId won't be always the settings app's one.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sounds fine. Is there a reason we can't run about:serviceworkers in the parent process?

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Jonas Sicking (:sicking) from comment #5)
> Sounds fine. Is there a reason we can't run about:serviceworkers in the
> parent process?

Once the SWM is in the parent, it could easily provide all information and some observer subsystem to about:serviceworkers and other devtools to hook into this. worker debugging would probably need its own IPC mechanism i guess.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Note, in bug 1225121 we are seeing updates running in the wrong process due to the PropagateSoftUpdate() mechanism.  Part of the refactor here should be to ensure we only run an update once and in the correct process.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Moving the service worker manager to the parent process is part of our new overall e10s redesign.

Comment 9

[Andrew Overholt [:overholt]]

Andrew Sutherland advised this is done and it's not worth tracking independently of flipping the pref.