Closed Bug 1182117 Opened 4 years ago Closed 10 months ago

Move ServiceWorkerManager to the parent process in e10s

Categories

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

defect

Tracking

()

RESOLVED FIXED
Tracking Status
e10s + ---
firefox42 --- affected

People

(Reporter: catalinb, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [e10s-multi:M3], SW-MUST)

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.
Blocks: 1179191
Blocks: 1175949
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!
Flags: needinfo?(catalin.badea392)
(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.
Flags: needinfo?(catalin.badea392)
No longer blocks: 1181544
Blocks: 1178236
Blocks: 1181351
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?
Flags: needinfo?(jonas)
(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.
Blocks: 1181544
Sounds fine. Is there a reason we can't run about:serviceworkers in the parent process?
Flags: needinfo?(jonas)
(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.
Depends on: 1199036
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.
Depends on: 1226434
Moving the service worker manager to the parent process is part of our new overall e10s redesign.
Assignee: catalin.badea392 → nobody
Status: ASSIGNED → NEW
Blocks: e10s-multi
No longer blocks: e10s-multi
Whiteboard: [e10s-multi:M1]
Whiteboard: [e10s-multi:M1] → [e10s-multi:M3]
Priority: -- → P2
Whiteboard: [e10s-multi:M3] → [e10s-multi:M3] [SW-MUST]
Whiteboard: [e10s-multi:M3] [SW-MUST] → [e10s-multi:M3], SW-MUST

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

Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.