1687853 - Stop creating default X11 connection in every content process

Status: RESOLVED DUPLICATE of bug 1635451

(Core :: Widget: Gtk, enhancement, P1)

Description

[Chris Peterson [:cpeterson]]

We don't want to ship Fission with one X11 connection per content process. The Fission team had hoped non-native theming alone (bug 1411425) would remove the default X11 connection from every content process, but Jed said in bug 1635451 comment 32:

If you just flip the widget.disable-native-theme-for-content pref, we'll still connect to the X server at content process startup, and have various dependencies on it.

and:

If ... we don't think out-of-process WebGL (bug 1638466) will be ready in time, then we'll need to set MOZ_HEADLESS on content processes add code to WebGL so that it can do its own XOpenDisplay (and ideally also close the display if it's unused — we might not want to tie up a connection for every domain that uses WebGL only for fingerprinting).

Comment 1

[Neha Kochar [:neha]]

I've discussed this last week with Nika. There needs to be work done, in addition to linux non-native-theming (NNT) and that is to ensure that we don't connect to the X server at content process init time i.e. we don't do this. For Fission beta criteria (but not for release), we are okay with at most one X connection only for webGL content. This means that we will need lazy loading of the gdk process only when webGL needs it. This will probably be needed wherever Graphics code relies on this being available for webGL content.

Triaging this for M7 as it blocks Fission Beta.

Comment 2

[Cameron McCormack (:heycam)]

(In reply to Chris Peterson [:cpeterson] from comment #0)

If ... we don't think out-of-process WebGL (bug 1638466) will be ready in time, then we'll need to set MOZ_HEADLESS on content processes add code to WebGL so that it can do its own XOpenDisplay (and ideally also close the display if it's unused — we might not want to tie up a connection for every domain that uses WebGL only for fingerprinting).

To indicate content processes should be headless, we have the security.sandbox.content.headless pref. Would it be better to adjust the gfxPlatform::IsHeadless checks to look at that pref too (or perhaps have a different function for checking content-headless), or is setting MOZ_HEADLESS=1 likely to just work?

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Chris, isn't this just a dupe of the original bug? https://bugzilla.mozilla.org/show_bug.cgi?id=1129492#c29

In fact, looking at the dependencies of that bug, I think you already filed this specific part before: https://bugzilla.mozilla.org/show_bug.cgi?id=1672013

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #3)

Chris, isn't this just a dupe of the original bug? https://bugzilla.mozilla.org/show_bug.cgi?id=1129492#c29

In fact, looking at the dependencies of that bug, I think you already filed this specific part before: https://bugzilla.mozilla.org/show_bug.cgi?id=1672013

I was thinking of bug 1129492 and bug 1672013 as meta bugs, but it looks like this bug might also be a meta bug (since the uses of gdk_display_get_default and DefaultXDisplay span different component directories). So I will close this bug as a duplicate of bug 1129492.

Bug 1672013 is a little different: it is about enabling the security.sandbox.content.headless pref to block all X11 in the content process. We won't be able to enable that for Fission MVP unless we get WebGL remoting. But in the meantime, we still want to remove the omnipresent X11 connection in every content process (bug 1129492) for Fission MVP.

Comment 6

[Chris Peterson [:cpeterson]]

I'm going to reopen and repurpose this bug to audit the gdk_display_get_default and DefaultXDisplay calls. We need to determine which are actually in the content process and need to be fixed (for bug 1635451). AFAIK, we don't need to remove gdk_display_get_default and DefaultXDisplay calls from non-content processes like GPU, RDD, and Network.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

We won't be able to enable that for Fission MVP unless we get WebGL remoting. But in the meantime, we still want to remove the omnipresent X11 connection in every content process (bug 1129492) for Fission MVP.

In other words, this is about duplicating the security.sandbox.content.headless functionality, but with the exception of WebGL, and adding infrastructure so any WebGL user will initialize (and free) the X11 connection on-demand.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #7)

In other words, this is about duplicating the security.sandbox.content.headless functionality, but with the exception of WebGL, and adding infrastructure so any WebGL user will initialize (and free) the X11 connection on-demand.

That's correct. Whoever makes this change will also need to audit gdk_display_get_default and DefaultXDisplay calls to see which will break then in the content process. Hopefully our tests will cover most of those calls. The person who fixes this bug won't be responsible for fixing broken gdk_display_get_default and DefaultXDisplay calls in other teams' components, but those calls will should probably be fixed before we can land this bug.

Neha said she will ask Jim Mathies about finding an engineer to implement this.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Whoever makes this change will also need to audit

Note one would expect these to be easy to find, as they're going to be places that have to check the headless flag right now, or at worst will check whether the X11 Socket is set (if that weren't the case, our headless mode would be broken).

Comment 10

[Neha Kochar [:neha]]

I think gcp's team have all the context for it to do this, instead of the Graphics team, so leaving a NI for gcp to get this resourced as soon as possible. Already discussed with gcp over slack to get this reviewed in their roadmap discussions.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

This is already included in the roadmap proposals.

Comment 12

[Chris Peterson [:cpeterson]]

Linux bugs don't block our Fission Beta experiment (on Windows and macOS), so I am moving Linux Fission bugs from Fission milestone M7 to M7a.

Comment 13

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I don't think we need to audit uses of the default X connection; they should all be guarded by checks for headless mode, and we do have test coverage for “full” headless mode that would hopefully detect missing checks.

However, I ran Try with security.sandbox.content.headless set and found two issues. The easier one is TEST-UNEXPECTED-PASS failures from a self-test of test infrastructure, which expects nsIWidget::SynthesizeNativeMouseEvent to be missing some features in headless mode which actually work in this configuration; it's simple enough to add a check for this pref, but that would regress the test in actual headless mode, so this needs a little thought.

The harder problem is that moz-icon loading is broken; see bug 1695381. For Fission it might suffice to turn off GTK only in content processes for HTTP origins — remote content can't link to moz-icon directly — and leave it active for the dedicated file:/// process (and for FTP origins if we still support FTP when this ships). That should work if there's nothing that could use icons from resource/chrome contexts in an HTTP content process. Note that the only test that failed because of this was a reftest for image downscaling, but the regression is user-visible in directory listings, so it's possible that there are other gaps in test coverage here.

Comment 14

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

As far as I can tell this bug is essentially the same as bug 1635451; there isn't anything else that we'd do for that bug that we wouldn't also need to do for this one.