Closed Bug 910810 Opened 6 years ago Closed 6 years ago

WebrtcAudioConduit uses preferences off the main thread

Categories

(Core :: WebRTC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: khuey, Assigned: jesup)

References

Details

(Whiteboard: [webrtc])

Attachments

(1 file)

This is caused by the unit tests having a faked-up "MainThread".  Actual browser code doesn't have this problem; the code is called from vcmTxStartICE_m(), which (as the _m implies in the webrtc_signaling code) runs on MainThread.

ekr, do you remember what the issues were that caused us to use a fake mainthread in the unit tests?
Component: WebRTC: Audio/Video → WebRTC
Flags: needinfo?(ekr)
Hey Randell, can we get someone to take this?
Flags: needinfo?(rjesup)
Assignee: nobody → rjesup
Flags: needinfo?(rjesup)
Whiteboard: [webrtc]
Target Milestone: --- → mozilla27
Comment on attachment 809591 [details] [diff] [review]
don't read prefs off "main" thread in unittests

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

Patch got uploaded but forgot to ask for review on it.  Pretty simple....  (Larger issue of not running these tests off-mainthread in c++ unit tests will be dealt with elsewhere (bug 925675))
Attachment #809591 - Flags: review?(khuey)
Comment on attachment 809591 [details] [diff] [review]
don't read prefs off "main" thread in unittests

Somebody familiar with this code should review this.
Attachment #809591 - Flags: review?(khuey)
Attachment #809591 - Flags: review?(adam)
Comment on attachment 809591 [details] [diff] [review]
don't read prefs off "main" thread in unittests

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

Seems reasonable, given that the constructor sets these to sane defaults.
Attachment #809591 - Flags: review?(adam) → review+
https://hg.mozilla.org/mozilla-central/rev/fa8afb1865a1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.