1519354 - All commands except those for navigation can hang when unexpected remoteness changes occur

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, enhancement, P2)

Description

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

As seen on bug 1512336 we have a kinda huge problem in Marionette with every command beside the navigation commands. All of them do not use the message command queue, and as such will hang forever if an unexpected remoteness change occurs, eg. something triggered a navigation request.

To at least prevent the infinite hang, a global timeout for each command could help.

But to really fix it we would have to change the way how the parent and the content processes communicate with each other. Maybe this can be all done with the Fission work for Marionette.

Comment 1

[Andreas Tolfsen ❲:ato❳]

The problem with the message command queue is that it’s not very
generic as you need to hard-code every decision to recover when the
first message makes a remoteness change occurs. This is easy with
navigation because we know what has caused the remoteness change
and what to do after to recover. With other commands it might not
be so easy.

Going into the future I don’t think we can expect calls from driver.js
through proxy.js to listener.js to be, or act as if they are,
synchronous. For example I don’t think this.listener.navigate(…)
is the right level of granularity. Specifically for navigation we
should have one IPC message to trigger the navigation, then an event
emitter in the frame script that sends us updates about the state
of the document. Alternatively, if it is possible, use the web
progress listener entirely in chrome space.

Incidentally this approach is similar to what we need to do for the
new remote agent, which I’ve outlined in greater detail in
https://bugzilla.mozilla.org/show_bug.cgi?id=1518468#c3.

Comment 2

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

By using the JSWindowActor implementation these hangs should no longer occur.

Comment 3

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

For several Fission only test failures it has been proven that enabling actors will solve this problem. Hopefully we can enable actors for all builds later this week.

Comment 4

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

We have never seen a hang with the JSWindowActor implementation, and given that this is the default architecture in Marionette now I'm going to wontfix this bug.