Closed Bug 950833 Opened 6 years ago Closed 6 years ago

[e10s] Enable WebGL on Linux without forcing it

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch webgl-fix (obsolete) — Splinter Review
Right now we spawn a separate process to test whether WebGL is going to work. We're doing this in the main process, but not from content processes, where the WebGL code actually runs. I considered sending the data we need to the child via IPDL or something, but it's much easier to have each child re-run the test. From skimming bug 639842, it sounds like the test is pretty fast, so I don't see any reason not to take this approach.
Attachment #8348237 - Flags: review?(bjacob)
Attachment #8348237 - Flags: review?(benjamin)
Once we've run the program once, we know what it's result is presumably going to be and we should just send that result down to content processes when we create them. So this doesn't seem like the right solution, unless I'm missing something.

The content processes don't actually load the GL libs, do they? That would almost always break any kind of sandboxing.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Once we've run the program once, we know what it's result is presumably
> going to be and we should just send that result down to content processes
> when we create them. So this doesn't seem like the right solution, unless
> I'm missing something.

Well, I'm being lazy here, but I think it's simpler this way. The test that's being run collects 18 pieces of data. If we send that data over IPDL, we'll have to define a type in an .ipdlh file that lists those pieces, and we'll have to copy them in and out, so there will be three more copies of each field. Anyone who adds a new field or changes an existing one will have to update those three places.

Assuming that re-doing the check is fast enough, it just seems easier to me. We use the code that's already there, and there isn't really any cost to it.

> The content processes don't actually load the GL libs, do they? That would
> almost always break any kind of sandboxing.

My understanding is that the sandbox is only enforced later in the startup process, so loading the libraries seems fine. If you're wondering whether GL calls themselves break sandboxing, I'm not sure of the answer.
Attached patch webgl-fix2Splinter Review
Benjamin convinced me not to use the first approach, and I discovered a better way to do the IPDL stuff.
Attachment #8348237 - Attachment is obsolete: true
Attachment #8348237 - Flags: review?(bjacob)
Attachment #8348237 - Flags: review?(benjamin)
Attachment #8348390 - Flags: review?(bjacob)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Once we've run the program once, we know what it's result is presumably
> going to be and we should just send that result down to content processes
> when we create them.

That's correct.

> The content processes don't actually load the GL libs, do they?

They do, at least for WebGL and for Skia/GL 2D Canvas (the latter is not yet enabled by default on Linux, but the plan is for that to be enabled "soon").

Since content needs to access the GL API at least for WebGL, and in practice also for fast 2D canvas, the only alternative to loading GL libs in the content process would be to remote GL commands to another process, like Chrome does. We don't currently have code to do that, and last we looked at that possibility, there were arguments on both sides.
Comment on attachment 8348390 [details] [diff] [review]
webgl-fix2

Review of attachment 8348390 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine; just a couple of questions:

1. How slow is this sync message going to be? This is going to be called every time e.g. content does a canvas.getContext, which typically takes 5 ms. So if it takes less than a few ms, then it's fine, I suppose.

2. Is it a good idea to do this on all platforms, when this is only needed on Linux? I don't care strongly and can see the value of being consistent across platforms, but what we're doing here with the glxtest process on Linux is only needed because of a Linux-specific quirk (namely, that there is no safe, non-crashy way to get openGL info on Linux, so we have to do it in a separate process to avoid crashing).

Please check with :bsmedberg before landing --- I don't have a strong opinion either way.
Attachment #8348390 - Flags: review?(bjacob) → review+
My understanding from IRC is that we would get this information once at startup (or at first use) and then wouldn't send the message again. But that doesn't seem to match the implementation here.
What runs once at startup, is the forking an running the test on a separate process, and returning the results into a pipe.

What runs occasionally later, for instance once per canvas getContext call, is the blacklist lookup, GfxInfo::GetFeatureStatus. This is what is being changed to use a sync message here. The sync message is what gets me wondering, if this might be slow.
I changed the implementation since I last talked to Benjamin. I think that the new one is simpler even though it does a sync call for every getContext. I measured the cost of sync roundtrips about 6 months ago, and we get about 20000 messages/sec on my computer = 0.05 ms. Also, I was using the message manager; direct IPDL will be a little faster.
(Oh, and that was 20,000 roundtrips/sec, not just one way.)
OK, 0.05 ms is definitely too low to be a concern here. I hope that that figure will be as low in real-world page-loading scenarios when the main process might be busy.
https://hg.mozilla.org/mozilla-central/rev/78c499a96bd0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.