1580448 - (JSProcessActor) Consider introducing JSProcessActors (JSWindowActor for PContent)

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, enhancement, P3)

Description

[Nika Layzell [:nika] (ni? for response)]

This would be like JSWindowActor, but for content processes, and would eliminate one of the remaining usecases for the old MessageChannel APIs.

Comment 1

[Chris Peterson [:cpeterson]]

kmag says this bug should block enabling Fission in Nightly (M6) because bug 1587058 blocks M6.

Comment 2

[Tomislav Jovanovic :zombie]

I believe this should be moved ahead to M5.

Unlike other (most of the?) work the Fission team is doing to replace features and primitives with OOPIF-compatible ones, this is introducing a new API that other teams (at least WebExt and DevTools, possibly others) need to build on top of.

Since we're working in parallel toward the same M6 goal, and since several of the things we need to build on top of this api are themselves non-trivial, I don't think it's reasonable for all of that to happen in the same milestone.

New APIs need to "bake" for some time before they could be reliable used. After landing, they need to be documented, learned, tried, used and misunderstood, crashes found and fixed -- which I don't think is possible in the span of a couple of months (as history has shown).

Comment 3

[Tomislav Jovanovic :zombie]

New APIs need to "bake" for some time before they could be reliable used. After landing, they need to be documented, learned, tried, used and misunderstood, crashes found and fixed -- which I don't think is possible in the span of a couple of months (as the history has shown).

*If left in M6, this all needs to happen after designing, writing, reviewing, testing and landing the API itself during the same milestone.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to :Tomislav Jovanovic :zombie from comment #2)

I believe this should be moved ahead to M5.

In that case, moving to M5.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

I can take a look at this. What's the use case, exactly?

Comment 6

[Nika Layzell [:nika] (ni? for response)]

I'm not sure I understand why this is considered to block M5. My impression was that we weren't planning to add per-process JS actors until after Fission, and had no intention to make them block Fission. The existing per-process MessageManager system should work as-usual with Fission.

:zombie, why does this block the work you're doing?

Comment 7

[Tomislav Jovanovic :zombie]

Because we need to very thoroughly combine frame-level and process-level messaging in a lot of places in the webextensions framework, and for those two to be using similar yet different APIs with different capabilities and guarantees is gonna be messy, and require keeping around thousands of lines of (old, crufty, special-case) code to deal with message managers.

If there was a change of plans regarding M5, I didn't see that communicated anywhere.

And more generally, I don't understand the point of adding process level actors after fission is "complete". It seems unlikely people are gonna have the opportunity to go back just to "improve" things after they get it working once (code complexity and cruft be damned).

Overall, it seems like a very inefficient way to coordinate our work.

Comment 8

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6). JSContentActors do not need to block the Fission dogfooding milestone (M5).

Comment 9

[Tomislav Jovanovic :zombie]

JSWindowActors, as currently implemented, are not a complete/finished fission-compatible replacement for all the functionality provided by the family of MessageManagers. Several aspects are still missing, and workarounds using process-level MessageManagers are somewhere between very inefficient and hard to impossible.

A few examples off the top of my head:

• Ability to send Queries in addition to one way messages
• Ability to efficiently broadcast the exact same message to the whole BrowsingContext subtree
• Ability to enumerate, identify and reference content processes from the parent
• Ability to link which processes a JSWindowActors belongs to, and vice versa
• A persistent top-level BC "id" (bug 1580766)

Though I'm sure there will be others to discover, since I didn't expect we'll need to make parts of the web extensions framework fission compatible without ProcessActors.

Comment 10

[Tomislav Jovanovic :zombie]

I'm working on bug 1316748, which should be least impacted by the above issues, and came across one more potential pitfall:

• message ordering between JSWindowActors and process level MessageManagers might not be guaranteed in all cases (specifically, for the in-parent documents, according to Nika)

Unfortunately, we still need to support GeckoView, which doesn't have support for out-of-process extensions, so all extensions pages run in the parent process on android.

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - JSProcessActor API;r?nika!

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - nsIContent{Child, Parent}::getActor;r?nika!

Depends on D64595

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - ChromeUtils.{register, unregister}ProcessActor;r?nika!

Depends on D69784

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Could we find a better name than JSContentActor{...}? Semantically, Content and Window are near-synonyms.

Can we rename them JSProcessActor, as someone suggested previously?

Comment 15

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - Renaming Test{Child, Parent} => TestWindow{Child, Parent};r?nika!

Depends on D69786

Comment 16

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - JSProcessActor tests;r?nika!

Depends on D69818

Comment 17

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - Trivial unified build fixups;r?nika!

Comment 18

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - Rename JSWindowActor into JSActor;r?nika!

Depends on D70036

Comment 19

[David Teller [:Yoric] - still alive but not very active]

As per the vote, we're renaming them JSProcessActor.

Comment 20

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - Add NS_DECL_NSICONTENTPARENT to ContentParent;r?nika!

This is a fixup to Bug 1580447, which landed some time ago.

Depends on D69819

Comment 21

[David Teller [:Yoric] - still alive but not very active]

Attachment: Bug 1580448 - JSProcessActor{Child, Parent}::AfterDestroy;r?nika!

Depends on D72758

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/358f764ae48b
Trivial unified build fixups;r=nika
https://hg.mozilla.org/integration/autoland/rev/9b548bd38671
Rename JSWindowActor into JSActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/2a1701831a6a
JSProcessActor API;r=nika
https://hg.mozilla.org/integration/autoland/rev/dafa80c63322
nsIContent{Child, Parent}::getActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/bfbd3330b0a5
ChromeUtils.{register, unregister}ProcessActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/fd7527c86239
Renaming Test{Child, Parent} => TestWindow{Child, Parent};r=nika
https://hg.mozilla.org/integration/autoland/rev/6db8de5fc125
JSProcessActor tests;r=nika
https://hg.mozilla.org/integration/autoland/rev/677257a41457
Add NS_DECL_NSICONTENTPARENT to ContentParent;r=nika
https://hg.mozilla.org/integration/autoland/rev/6b4db1a501df
JSProcessActor{Child, Parent}::AfterDestroy;r=nika

Comment 23

[Stefan Hindli [:stefan_hindli]]

Backed out 9 changesets (Bug 1580448) for linux build bustages in /builds/worker/workspace/obj-build/dist/include/mozilla/dom/JSWindowActorChild.h CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=2a268a55b65452271999a81c15fc31543cb732ed

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedTaskRun=U31Pw7q1ReyHwcJMbevCIQ-0

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=300162333&repo=autoland&lineNumber=35117

Comment 24

[David Teller [:Yoric] - still alive but not very active]

Merge error. Fixed, will reland once I've confirmed that the fix is sufficient.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68a900996016
Trivial unified build fixups;r=nika
https://hg.mozilla.org/integration/autoland/rev/b49b7bbd25b9
Rename JSWindowActor into JSActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/9c7322618b18
JSProcessActor API;r=nika
https://hg.mozilla.org/integration/autoland/rev/08f0ecf2f308
nsIContent{Child, Parent}::getActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/7e0c0eacd079
ChromeUtils.{register, unregister}ProcessActor;r=nika
https://hg.mozilla.org/integration/autoland/rev/4de7ed8efac0
Renaming Test{Child, Parent} => TestWindow{Child, Parent};r=nika
https://hg.mozilla.org/integration/autoland/rev/2ba000294288
JSProcessActor tests;r=nika
https://hg.mozilla.org/integration/autoland/rev/e62bfdb50e18
Add NS_DECL_NSICONTENTPARENT to ContentParent;r=nika
https://hg.mozilla.org/integration/autoland/rev/c51bedeb809f
JSProcessActor{Child, Parent}::AfterDestroy;r=nika

Comment 26

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/68a900996016
https://hg.mozilla.org/mozilla-central/rev/b49b7bbd25b9
https://hg.mozilla.org/mozilla-central/rev/9c7322618b18
https://hg.mozilla.org/mozilla-central/rev/08f0ecf2f308
https://hg.mozilla.org/mozilla-central/rev/7e0c0eacd079
https://hg.mozilla.org/mozilla-central/rev/4de7ed8efac0
https://hg.mozilla.org/mozilla-central/rev/2ba000294288
https://hg.mozilla.org/mozilla-central/rev/e62bfdb50e18
https://hg.mozilla.org/mozilla-central/rev/c51bedeb809f

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

David, are you going to document that at https://firefox-source-docs.mozilla.org/dom/Fission.html?

Comment 28

[David Teller [:Yoric] - still alive but not very active]

Filed as bug 1635061. I think I'll wait a few days until I have a better idea of where bug 1633379 is heading, though.