Steps: 1. Call gUM with either video: true, audio: true, or video: true, audio: true with no camera and mic support Expected: I should get an error callback with NO_DEVICES_FOUND. Actual: No error callback is fired.

Apparently this also reproduces for me with using the steps from https://bugzilla.mozilla.org/show_bug.cgi?id=779039#c0.

Comment on attachment 678336 [details] [diff] [review] Deny request if no devices were found - v1 How feasible would it be for the backend to just bypass the UI invocation in this case, rather than letting UI code deny the request?

I'm going to add that code separately, but state may also change in between the messaging so it's better to handle this case in the UI code as well for completeness.

(In reply to Anant Narayanan [:anant] from comment #6) > I'm going to add that code separately, but state may also change in between > the messaging How exactly would the state change? Note that nsIObserverService notifications are synchronous.

Comment on attachment 678336 [details] [diff] [review] Deny request if no devices were found - v1 Also, comment 0 suggests that we don't want the PERMISSION_DENIED error for this.

(In reply to Dão Gottwald [:dao] from comment #7) > How exactly would the state change? Note that nsIObserverService > notifications are synchronous. But camera acquisition is not (and is done off the main thread). It is definitely possible for another tab to have grabbed the camera after we checked in C++ but not being available when JS enumerates. Besides, it is just wrong for the UI to simply return without sending back an observer notification when it receives one. We do not have a "NO_DEVICES_FOUND" error anywhere in the spec at the moment. It think PERMISSION_DENIED works just as well, atleast better than doing that nothing at all, though I agree we might want a better error name (perhaps HARDWARE_UNAVAILABLE).

(In reply to Anant Narayanan [:anant] from comment #9) > (In reply to Dão Gottwald [:dao] from comment #7) > > How exactly would the state change? Note that nsIObserverService > > notifications are synchronous. > > But camera acquisition is not (and is done off the main thread). It is > definitely possible for another tab to have grabbed the camera after we > checked in C++ but not being available when JS enumerates. In my experience the turnaround time is so short that it's impossible for the user to do anything in the meantime, so it seems like we're talking about an edge case here. A more realistic case would be the list of devices changing between the popup appearing the first time (it can be dismissed and re-opened multiple times) and the user granting the request. Your patch doesn't handle this, which seems inconsistent if we really care about this problem. It's also news to me that the same source can't be shared twice concurrently. Could this be made work on our side? > Besides, it is just wrong for the UI to simply return without sending back > an observer notification when it receives one. There's no actual contract requiring this.

(In reply to Dão Gottwald [:dao] from comment #11) > It's also news to me that the same source can't be shared twice > concurrently. Could this be made work on our side? Yes, but we couldn't solve the resulting privacy/UX issues satisfactorily so we've decided to disable that functionality for now. > > Besides, it is just wrong for the UI to simply return without sending back > > an observer notification when it receives one. > > There's no actual contract requiring this. Of course there is. If there is no corresponding message for an outgoing observer notification from the backend the gUM call will just hang. It is the UI's responsibility to send back a reasonable response for every incoming. Honestly, I don't see what the problem is here.

(In reply to Anant Narayanan [:anant] from comment #12) > > It's also news to me that the same source can't be shared twice > > concurrently. Could this be made work on our side? > > Yes, but we couldn't solve the resulting privacy/UX issues satisfactorily so > we've decided to disable that functionality for now. Ok, if that's just the interim solution, then I don't think the UI should worry about it especially if the proposed workaround only covers an edge case while leaving other more interesting cases unaddressed. (The privacy/UX implications aren't clear to me, by the way.) > > > Besides, it is just wrong for the UI to simply return without sending back > > > an observer notification when it receives one. > > > > There's no actual contract requiring this. > > Of course there is. If there is no corresponding message for an outgoing > observer notification from the backend the gUM call will just hang. It is > the UI's responsibility to send back a reasonable response for every > incoming. Popup notifications don't work like this. Users can generally leave them unanswered infinitely. Secondly, if the backend doesn't invoke the UI in the first place, sending a response in this case becomes moot. (I think the UI code should throw an exception rather than just returning, though.) > Honestly, I don't see what the problem is here. I don't see the point of adding this code. We should just take the backend fix which you said you're gonna add anyway.

Which bug caused this regression?

Henrik: Probably regressed when we did a major rewrite to make this non-synchronous. Dao: I have a problem with Anant's proposed backend fix to avoid invoking the UI if there are no devices to choose from. Right now we get the device list in response to a request (GetUserMediaDevices) from the UI. We'd have to do this before dispatching the UI request. Longer term, we need to support the usecases of "darn, I forgot to plug in my headphones/webcam" without cancelling/erroring out and re-invoking. That means allowing incomplete/empty lists, and allowing use to notify the UI if the device changes (hotplug events). So, the backend fix proposed will be wrong in that case and would need to be backed out anyways and go back to GetUserMediaDevices() or equivalent. So, my preference would be (for now) return an error or denial if the list is empty. Alternatively, I could return an error from GetUserMediaDevices if you prefer, but I'm not 100% certain that case is fully handled (and a denial returned) - it didn't look like it. I'll attach a patch that returns an error (the alternative solution); as mentioned I suspect this needs fixes in the JSM to complete.

Dao: which way do you want to go of the two alternatives? I'm guessing the second alternative is a little simpler.

Comment on attachment 694849 [details] [diff] [review] GetUserMediaDevices: Consider no devices available an error Review of attachment 694849 [details] [diff] [review]: ----------------------------------------------------------------- We should probably file a bug for the hotplug feature.

Can't automate this until we have bug 815002.

Needs bug 827145 to test. Will verify on that bug then.