1336048 - Add display detection to dom/ipc/ContentChild.cpp

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Martin Stránský [:stransky] (ni? me)]

We should use the same DISPLAY detection as we have in nsAppRunner.cpp.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1336048 - Add display detection to dom/ipc/ContentChild.cpp,

Review commit: https://reviewboard.mozilla.org/r/109088/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109088/

Comment 2

[Brendan Dahl [:bdahl]]

Sorry for the drive by review, but I'm working on headless browsing and was curious if some of the wayland work would conflict. Instead of copying the detectDisplay code couldn't we just set an environment variable with the result of detectDisplay in the parent process and then just read that variable in the child?

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8832837 [details]
Bug 1336048 - Add display detection to dom/ipc/ContentChild.cpp,

https://reviewboard.mozilla.org/r/109088/#review110502

Both this and nsAppRunner.cpp are in libxul. Just make the function in nsAppRunner.cpp non-static and use it.

Comment 4

[Martin Stránský [:stransky] (ni? me)]

(In reply to Brendan Dahl [:bdahl] from comment #2)
> Sorry for the drive by review, but I'm working on headless browsing and was
> curious if some of the wayland work would conflict. Instead of copying the
> detectDisplay code couldn't we just set an environment variable with the
> result of detectDisplay in the parent process and then just read that
> variable in the child?

I'd leave that for next iteration of this code. I think the shared detectDisplay() routine will work for now.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8832837 [details]
Bug 1336048 - Add display detection to dom/ipc/ContentChild.cpp,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109088/diff/1-2/

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8832837 [details]
Bug 1336048 - Add display detection to dom/ipc/ContentChild.cpp,

https://reviewboard.mozilla.org/r/109088/#review111424

::: dom/ipc/ContentChild.cpp:530
(Diff revision 2)
>      char* argv[] = {
>        // argv0 is unused because g_set_prgname() was called in
>        // XRE_InitChildProcess().
>        nullptr,
>        option_name,
> -      display_name,
> +      (char *)display_name,

const_cast<char*>(display_name)

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8832837 [details]
Bug 1336048 - Add display detection to dom/ipc/ContentChild.cpp,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109088/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d482c6c4efb
Add display detection to dom/ipc/ContentChild.cpp, r=glandium

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8d482c6c4efb