1614713 - [meta] Convert Remote Page Manager and the about: pages that use it to be based on JSWindowActors

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P3)

Comment 1

[Neil Deakin]

The remote page manager provides an mechanism for internal pages, most about: pages such as about:plugins to be loaded in an unpriviledged content process, yet injects some special methods into the page that can be called to perform specific privileged operations.

This is done via loading RemotePageManagerParent.jsm within the parent process and invoking a constructor contained within for a given page, for example:
new RemotePages("about:plugins"). This sets up various things to listen to that about page being loaded, inject some 'special powers'-like functions into it and listen for messages from that page.

On the child side, a per-process frame script process-content.js listens to page loads and when one matches one of the remote page urls that was set up in RemotePageManagerParent.jsm, a corresponding RemotePageManagerChild.jsm is created and listeners set up to communicate between the parent and child for that about page.

This mechanism is similar to the way that JSWindowActors work. They create a parent and child actor autmatically when an attempt is made to access them. JSWindowActors have two features that allow much of the remote page manager work to be handled directly:

1. A 'matches' flag in the actor declaration that allows an actor to only apply to a specific URL. For example, the actor only applies to one specific about page.
2. The actor declaration can specify the DOMContentLoaded or DOMWindowCreated event and the actor will be created automatically when that event occurs.

The proposal here is to remove much of the remote page manager setup and replace this with actors. The existing MessagePort module will still be used to interface between both processes. Each about page will have its own actor pair, although the child side will mostly be boilerplate where the real work is done by the existing message port module.

The child will look something like the following:

class AboutPluginsChild extends JSWindowActorChild {
actorCreated() {
this.messagePort = new ChildMessagePort(this, this.contentWindow);
}

handleEvent() {
// Do nothing. The DOMContentLoaded/DOMWindowCreated event is just used to create
// the actor and set up the MessagePort.
}

receiveMessage(aMessage) {
// Pass messages off to the MessagePort.
this.messagePort.handleMessage(aMessage);
}
}

Messages can be sent to the parent from the child within the unprivileged page using RPMSendAsyncMessage and RPMSendQuery. The parent actor receives the message using its normal receiveMessage method, thus the parent can be written like any other actor. The parent can also send messages to the content page using the normal sendAsyncMessage method. Listeners do not need to be explicitly added or removed. If desired, the RPMAccessManager could be extended to limit the message strings that can be send from the child page.

There are currently eight places where RemotePages is used:

1. about:neterror (this work was already done in bug 1533951)
2. about:protections
3. about:privatebrowsing
4. about:plugins
5. about:tabcrashed
6. about:newtab
7. about:newinstall
8. talos tab switching tests (about:tabswitch)

The current step done by bug 1533951 allows the MessagePort to handle both message managers and actors. Once the eight pages above have been converted, then the message manager handling as well as support code in process-content.js can be removed.

Comment 2

[Johann Hofmann [:johannh]]

So, is the goal to remove RPM entirely? I think looking at the current situation I'd be fine with that, though I'm not sure if there's usage left that can't be removed...

Comment 3

[Neil Deakin]

No, just convert it to use actors instead of message managers, and remove some redundant code that the RPM does that actors have built-in.

Comment 4

[Neil Deakin]

Actually, I ended up just replacing the remote page manager. For now, I left the RPM in place for the two pieces I didn't work on (about:newtab and talos test) and created an actor based implementation for the rest. about:newtab I think would be best left to the work being done to improve performance (such as bug 1513045)

Patch coming soon.

Comment 5

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6)

Comment 6

[Neil Deakin]

Attachment: Bug 1614713, add a remote page manager like object that uses JSWindowActor instead. Move access management into a separate module that can be used within a child or parent process. Ensure all RPM calls go through the access manager, r=mossop

Comment 7

[Neil Deakin]

Attachment: Bug 1614713, move neterror actor over to the actor-based RemotePageChild, and move neterror-specific functions to the NetErrorChild subclass so that they cannot be accessed via other pages, r=johannh

Depends on D63713

Comment 8

[Neil Deakin]

This latest version moves the access manager parts into a separate module, so it could possibly be shared between the child and parent, altough, as the current code does, it only checks in the child process at present.

It also moves out the functions that are specific to certain pages (neterror, new install page, etc) so they can only be used by that page. To make this more feasible, the exporting functions code has been modified to make adding functions easier.

The second part, the neterror patch, shows how this works for the network error pages.

Gijs, since you commented on the last patch, would you mind commenting to see if this approach would work, or what further work might be needed?

Comment 9

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #8)

This latest version moves the access manager parts into a separate module, so it could possibly be shared between the child and parent, altough, as the current code does, it only checks in the child process at present.

It also moves out the functions that are specific to certain pages (neterror, new install page, etc) so they can only be used by that page. To make this more feasible, the exporting functions code has been modified to make adding functions easier.

The second part, the neterror patch, shows how this works for the network error pages.

Gijs, since you commented on the last patch, would you mind commenting to see if this approach would work, or what further work might be needed?

This looks reasonable to me. It'd be nice if we could push the access checks and the naming of the exported function into the wrapper you've created to avoid accidentally forgetting to check the access stuff on arguments etc.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/64e2b785794e
add a remote page manager like object that uses JSWindowActor instead. Move access management into a separate module that can be used within a child or parent process. Ensure all RPM calls go through the access manager, r=mossop
https://hg.mozilla.org/integration/autoland/rev/1e24982c7988
move neterror actor over to the actor-based RemotePageChild, and move neterror-specific functions to the NetErrorChild subclass so that they cannot be accessed via other pages, r=johannh,mossop

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/64e2b785794e
https://hg.mozilla.org/mozilla-central/rev/1e24982c7988