1488872 - Launcher process: Treat --marionette as implicit --wait-for-browser

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[(No longer employed by Mozilla) Aaron Klotz]

This fixes the marionette failures that I was seeing when I tried to land launcher by default on inbound.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1488872: Make -marionette implcitly wait-for-browser in the launcher process; r=mhowell!

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hi Aaron, would you mind to explain a bit what exactly this launcher process is and what this patch accomplishes for Marionette. It's not that clear to me. I'm also asking because of bug 1438830, and I wonder if this could help us in any way. thanks.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Hi Henrik,

I'll paste in here the description that I'm working on for a future dev-platform post:

> Simply, on Windows builds, the launcher process is an initial process that is responsible for configuring and starting the
> "real" browser process. Such configurations include early initialization of the DLL blocklist, as well as the setting of
> other Windows process mitigations.

As for what this patch accomplishes:

> by default, the launcher process only lives long enough to stand up a browser process and transfer foreground to the new browser's window.

So any type of test automation that follows the pattern of starting firefox, then waiting for the child firefox process to finish running, needs to communicate this intention to the launcher process. Then the launcher process will wait for the browser to finish and return the browser process's error code as its own.

The following conditions already trigger the "wait for the browser" condition in the launcher:

1. Passing --wait-for-browser in the command line;
2. Setting MOZ_AUTOMATION=1 in the environment;

this patch adds a third case:

3. Implicitly set --wait-for-browser when --marionette is present.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, and how does it work with restarts of Firefox? Given that above mentioned bug we have problems on Windows right now to follow the process after a restart when triggered inside the application (eg. update, about:support, addo-ons disabled, etc). Does this launcher process always know the process id of Firefox and will kill all the childs?

If that works with `--wait-for-browser` as command line argument that would be fantastic.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> Ok, and how does it work with restarts of Firefox? Given that above
> mentioned bug we have problems on Windows right now to follow the process
> after a restart when triggered inside the application (eg. update,
> about:support, addo-ons disabled, etc). Does this launcher process always
> know the process id of Firefox and will kill all the childs?

It knows the ID of the parent process, but not all the children.

The right way to do this is probably to start Firefox inside a job object. That groups all the Firefox processes into one "job," such that shutting down the job kills all processes inside it.

We actually do that already in a C++ regression test: https://searchfox.org/mozilla-central/source/mozglue/tests/interceptor/TestDllInterceptorCrossProcess.cpp#48

Anyway, that sounds like something we should discuss in bug 1438830. Feel free to ni? me there.

Comment 6

[Molly Howell (she/her) (no longer active)]

Comment on attachment 9006649 [details]
Bug 1488872: Make -marionette implcitly wait-for-browser in the launcher process; r=mhowell!

Matt Howell [:mhowell] has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a76bc2779d81
Make -marionette implcitly wait-for-browser in the launcher process; r=mhowell

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a76bc2779d81