1541186 - Stop using TYPE_MOZILLA_NONMAINUITHREAD thread type for the background media manager thread

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P1)

Description

[Jim Mathies [:jimm]]

This was introduced back in 2014 when we were doing everything in content, including capturing window contents and various other UI type activity.

We should be able to switch to TYPE_MOZILLA_NONMAINTHREAD here.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db8054e9f57e6c2947c68e158138dc07b90dbe44

Comment 1

[Jim Mathies [:jimm]]

s/everything in content/everything in a single app/

Comment 2

[Jim Mathies [:jimm]]

Pushed a try build with asserts in a couple media related native event processing routines - didn't get any hits for content. I think it's safe to remove the ui loop here.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6876287fb194c18a96d5f922c0671913c1577fd

Comment 3

[Jim Mathies [:jimm]]

Attachment: Bug 1541186 - Switch to using TYPE_MOZILLA_NONMAINTHREAD for MediaManager. r=jesup

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

jimm - so the mediamanager thread is made a non-UI thread by this, but it appears to do so universally, not just in the content process. The main question is if this affects GetUserMedia streams initiated by Chrome content (which Hello used to do), and if this affects the remote (in the Master process) capture of screen data on behalf of the Content process.

Someone should set up these two experiments (screen capture from content, and screen capture from Chrome, which would require code insertion to do) and verify if the actual calls that caused us to switch to a UI thread are either a) being done from a UI thread, or b) aren't being done anymore. The original failures that required this were very random and not easily reproducible.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

We have tests in the tree for verifying that screen capture from content works. Screen capture is easier to test than cameras.

Our screen-capture prompts' preview area is implemented using screen capture from chrome, i.e. verifiable through manual testing—We have automated tests for these capture calls succeeding, but not for verifying that actual pixels appear. In any case, it should be doable without code insertion.

I don't know which calls specifically needed to be on the UI thread, maybe others recall?

Comment 6

[Jim Mathies [:jimm]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #5)

We have [tests in the
tree](https://searchfox.org/mozilla-central/rev/
d302c3058330a57f238be4062fddea629311ce66/dom/media/tests/mochitest/
test_getUserMedia_basicScreenshare.html#100,119) for verifying that screen
capture from content works. Screen capture is easier to test than cameras.

Our screen-capture prompts' preview area is implemented using screen capture
from chrome, i.e. verifiable through manual testing—We have automated tests
for these capture calls succeeding, but not for verifying that actual pixels
appear. In any case, it should be doable without code insertion.

I don't know which calls specifically needed to be on the UI thread, maybe
others recall?

I don't think any of these calls need to be on a ui thread. We originally implemented this thinking some of this capture stuff needed a ui pump. After implementing it we realized the real culprit was the compositor background thread, which created windows but didn't pump.

Comment 7

[Dan Minor [:dminor]]

Even with these changes, the desktop capture code will still be running on a UI Thread due to our PlatformUIThread implementation [1]. As far as I know, all capture calls still end up spinning up that thread, so I think these changes should be safe.

[1] https://searchfox.org/mozilla-central/rev/ee3905439acbf81e9c829ece0b46d09d2fa26c5c/dom/media/systemservices/video_engine/desktop_capture_impl.cc#411

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13d43fe66cae
Switch to using TYPE_MOZILLA_NONMAINTHREAD for MediaManager. r=jesup

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/13d43fe66cae