|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
40 bytes, text/x-review-board-request
|Details | Review|
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2499.0 Safari/537.36 Steps to reproduce: Call getUserMedia and then enumerate devices in the success callback right after stopping the resulting stream. https://jsfiddle.net/catr8ca3/10/ Actual results: When you delay the call to enumerate devices, it works but if you call it right away, nothing happens and the callback to enumerate the devices never fires. Expected results: Probably in any case the callback should still fire to enumerate the devices but that is a philosophical question left for the developers.
Status: UNCONFIRMED → NEW
Component: Untriaged → WebRTC: Audio/Video
Ever confirmed: true
Product: Firefox → Core
This seems timing related to me. I'm only able to reproduce it around 40% of the time and mostly on the first try after opening or refreshing (or hitting Run in) the fiddle. I have also seen it in 40, which I think means gcp is off the hook.
Interesting. For me it fails every time. Maybe also related to cpu speed or count/platform? Even 100 ms delay makes it work every time so it seems like the stream.stop and enumerate end up triggering async functions in varying order.
(In reply to Anthony Minessale from comment #2) > Even 100 ms delay makes it work every time so it seems like the stream.stop > and enumerate end up triggering async functions in varying order. Yes, I'm not able to reproduce without the stream.stop, or with a delay, so those definitely seem involved. Didn't mean to suggest otherwise.
Jib -- I've made this a P2/25. Do you think it should be higher? Have you had a chance to verify the likely impact? Thanks.
backlog: --- → webrtc/webaudio+
Priority: -- → P2
Yes I'm looking into this now, since it's happening in 40 for me. Marking as P1.
Priority: P2 → P1
Thanks, Jib. I'm assigning this to you until we know more.
Assignee: nobody → jib
Rank: 25 → 15
Created attachment 8656882 [details] MozReview Request: Bug 1201197 - add dedicated listener to enumerateDevices. Bug 1201197 - add dedicated listener to enumerateDevices.
Attachment #8656882 - Flags: review?(rjesup)
.stop() is a synchronous API, but its effect has a lag, which overlaps slightly here with enumerateDevices()'s setup. enumerateDevices marks the content window "active" in MediaManager early, but it wasn't holding any reference to anyting like a listener to make sure it stayed active for the duration of its request, so basically stop() takes the window off the "active" list, since there are no other listeners, leaving enumerateDevices with no active window when it finishes, which it silently fails on on the assumption that the user navigated away. enumerateDevices now holds on to a placeholder listener just like gUM does, to make sure the window is held active for the duration of the request.
Comment on attachment 8656882 [details] MozReview Request: Bug 1201197 - add dedicated listener to enumerateDevices. https://reviewboard.mozilla.org/r/18261/#review16403 Failure to get the maanger would seem to be impossible, given the impl, and other uses assume it's infallible (such as MediaManager::EnumerateDevicesImpl()). So being consistent would make sense, and you can add a comment to MediaManager::Get() to that effect.
Attachment #8656882 - Flags: review?(rjesup) → review+
Comment on attachment 8656882 [details] MozReview Request: Bug 1201197 - add dedicated listener to enumerateDevices. Bug 1201197 - add dedicated listener to enumerateDevices.
Version: 43 Branch → 39 Branch
jib: is there a try run for this change ?
Yes, in the mozReview link. https://reviewboard.mozilla.org/r/18259/
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.