Closed Bug 1224904 (SharedWorkers-e10s) Opened 4 years ago Closed 2 years ago

support SharedWorker correctly in multi-e10s

Categories

(Core :: DOM: Workers, defect, P3)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1438945
Tracking Status
firefox45 --- affected

People

(Reporter: baku, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch WIP (obsolete) — Splinter Review
This is something we should discuss during the next work-week but I think it's a good idea to have a separate process to manage SharedWorkers and ServiceWorkers.
This is somehow needed when we will have multiple Content processes in Desktop, plus, it will be a big simplification of ServiceWorker code.

I wrote a small patch to see how complex this would be. My approach would be:

a. A single process built on top of PContent and PBackground.

b. Just because we don't have a way to make a communication between child processes without having the content process involved, we have to go from content process to the parent process via Pbackground and form the parent process to the worker process again using PBackground.

c. A child process will work with a PRemoteWorker actor.

d. The current SharedWorker map will be kept in the worker process using Origin and OriginAttributes as keys and for security checks.

d. For some privileged apps in b2g, we can still have the PRemoteWorker running the the 'local' PBackground thread and not in the worker process.
Attached patch WIPSplinter Review
Attachment #8687664 - Attachment is obsolete: true
There has been now some talk in, hmm, blink-dev or somewhere, that Apple and MS might not want to implement SharedWorkers. Don't quite understand the reasoning. But afaik FFOS really wants SharedWorkers and IMO it is rather sane feature.

bkelly had some argument (which I didn't understand :) Why doesn't multiple child processes but just one SeW lead to hard to reproduce races?) that service workers might be ok without using a separate process.
(In reply to Olli Pettay [:smaug] from comment #2)
> bkelly had some argument (which I didn't understand :) Why doesn't multiple
> child processes but just one SeW lead to hard to reproduce races?) that
> service workers might be ok without using a separate process.

I described this idea here:

  https://github.com/slightlyoff/ServiceWorker/issues/756

Basically, service workers are not supposed to rely on global state.  Script should always be reading from on-disk state.  So, in theory, we spinning up multiple service worker instances should just work.

Of course, code can try to observe global state in the service worker.  Also, I think this approach may be difficult when using the debugger.

The debugger is the main issue making me think we might need to support a single instance across all processes. :-\
(In reply to Olli Pettay [:smaug] from comment #2)
> There has been now some talk in, hmm, blink-dev or somewhere, that Apple and
> MS might not want to implement SharedWorkers. Don't quite understand the
> reasoning. But afaik FFOS really wants SharedWorkers and IMO it is rather
> sane feature.

I don't think we should remove SharedWorker, but at the same time I'd rather focus on ServiceWorker right now.  If SharedWorker will require a great deal of extra effort then we should consider not including it in these changes to start.
Duplicate of this bug: 1245087
Let's make this just about SharedWorker in multi-e10s.  I think we should wait for the ServiceWOrker multi-e10s to complete and then build on top of any infrastructure there.

Andrea, I'm clearing the assignment since I assume you are not actively working on this right now.
Assignee: amarchesini → nobody
Summary: Separate process for SharedWorkers and ServiceWorkers → Separate process for SharedWorkers
Summary: Separate process for SharedWorkers → support SharedWorker correctly in multi-e10s
Duplicate of this bug: 1419727
Alias: SharedWorkers-e10s
For example, once the ClientHandle is fully integrated into ServiceWorkerManager it should also be easy to replace the SharedWorker attached nsIDocument list with a ClientHandle list.  Hopefully we will also be able to re-use the code that picks/launches a process and starts the worker in it.
Duplicate of this bug: 1433788
Priority: -- → P3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1438945
You need to log in before you can comment on or make changes to this bug.