1558520 - Make RemoteController.jsm work with OOP frames

Status: RESOLVED FIXED

(Toolkit :: General, defect, P3)

Comment 1

[Neil Deakin]

Attachment: Bug 1558520, allow controller commands to work for OOP frames

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hey Neil, is this looking for a reviewer? Or is this still a WIP?

Comment 3

[Neil Deakin]

I haven't tested it fully yet.

Comment 4

[Chris Peterson [:cpeterson]]

Neil, do you think this bug should block Fission's internal dogfooding (milestone M5)? Or can this fix wait until we're preparing to enable Fission on Nightly (M6)?

What happens if RemoteController.jsm doesn't work in OOP iframes?

Comment 5

[Neil Deakin]

It should be M5. The commands on the edit menu won't work otherwise.

Comment 6

[Neil Deakin]

The patch I had written long ago doesn't really work very well. The best solution I think would be to convert the controller into a JSWindowActor and sync the command state from the child to the parent actor when needed. Then, be able to access the currently active browsing context from the parent process and just use that in nsWindowRoot::GetEnabledDisabledCommands, which in turn would get the controller information from the actor. This should be straightforward once the general focus bug 1556627 is complete. That would also make this much easier to test.

Comment 7

[Neil Deakin]

Attachment: Bug 1558520, rework remote controller to use JSWindowActor instead of having the browser have a controller, r=smaug

When searching for the controller for a command in nsWindowRoot::GetControllerForCommand, look for a focused browsing context instead and get the controller through the Controllers actor associated with that browsing context. Similarly, when updating the commands in ChildCommandDispatcher, invoke a function on the Controllers actor associated with the window being updated. The enabled commands are synchronized through the child/parent actors. As long as we can get the right currently focused browsing context descendant, we can get the correct command state and invoke commands through the right actor.

Comment 8

[Neil Deakin]

Attachment: Bug 1558520, test that controllers work with out of process iframes of various arrangements, r=smaug

Depends on D58794

Comment 9

[:Gijs (he/him)]

Should we dupe bug 1534172 over to this bug - or perhaps just close as invalid once this lands?

Comment 10

[Neil Deakin]

Attachment: Bug 1558520, rework remote controller to use JSWindowActor instead of having the browser have a controller, r=smaug

When searching for the controller for a command in nsWindowRoot::GetControllerForCommand, look for a focused browsing context instead and get the controller through the Controllers actor associated with that browsing context. When a command update occurs in a window in the child process, send the list of commands to the parent process along with the browsing context for that window. The parent will pass this information to the controllers actor. As long as we can get the right currently focused browsing context descendant, we can get the correct command state and invoke commands through the right actor.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/044a624353ea
rework remote controller to use JSWindowActor instead of having the browser have a controller, r=smaug
https://hg.mozilla.org/integration/autoland/rev/181227bb4222
test that controllers work with out of process iframes of various arrangements, r=smaug

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/044a624353ea
https://hg.mozilla.org/mozilla-central/rev/181227bb4222