Add display detection to dom/ipc/ContentChild.cpp

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stransky, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla54
All
Linux
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
We should use the same DISPLAY detection as we have in nsAppRunner.cpp.
Comment hidden (mozreview-request)
(Reporter)

Updated

2 years ago
Attachment #8832837 - Flags: review?(mh+mozilla)
(Reporter)

Updated

2 years ago
Severity: minor → normal
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

2 years ago
mozreview-review
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.
Attachment #8832837 - Flags: review?(mh+mozilla)
(Reporter)

Comment 4

2 years ago
(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 hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
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)
Attachment #8832837 - Flags: review?(mh+mozilla) → review+
Comment hidden (mozreview-request)
(Reporter)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d482c6c4efb
Add display detection to dom/ipc/ContentChild.cpp, r=glandium
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8d482c6c4efb
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.