Add display detection to dom/ipc/ContentChild.cpp

RESOLVED FIXED in Firefox 54

Status

()

Core
IPC
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: stransky, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla54
All
Linux
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

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

Updated

a year ago
Attachment #8832837 - Flags: review?(mh+mozilla)
(Reporter)

Updated

a year 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

a year 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

a year 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

11 months 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

11 months ago
Keywords: checkin-needed

Comment 8

11 months 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

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8d482c6c4efb
Status: NEW → RESOLVED
Last Resolved: 11 months 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.