Bug 205005 – enumerateDevices should not expose devices by default if capture persistent setting is set

• Component: WebRTC
• Status: NEW

[youenn fablet]

enumerateDevices should not expose devices by default if capture persistent setting is set.
Instead we should wait for an actual capture to expose devices.
This will simplify the implementation and we will no longer leak whether a user set persistent access to camera/microphone.

[youenn fablet]

Created attachment 385140 [details]
Patch

[youenn fablet]

Created attachment 385143 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Eric Carlson]

Comment on attachment 385143 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385143&action=review

> Source/WebCore/dom/Document.cpp:7815
> +    if (this == &topDocument || topDocument.securityOrigin().isSameOriginAs(securityOrigin())) {

Nit: the "this == &topDocument" isn't necessary

> Source/WebCore/dom/Document.cpp:7825
> +    if (this == &topDocument || topDocument.securityOrigin().isSameOriginAs(securityOrigin()))

Ditto.

> Tools/ChangeLog:9
> [phone]  youenn fablet  <[e-mail]>
> +
> +        enumerateDevices should not expose devices by default if capture persistent setting is set
> +        https://bugs.webkit.org/show_bug.cgi?id=205005
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +
> [phone]  youenn fablet  <[e-mail]>

Nit: two entries

[Dag-Inge Aas]

The presence of labels in enumerateDevices is often used to determine if getUserMedia has been granted (persistent or not). Will this patch impact that? 

Currently I really like iOS's approach where you ask once per session and the prompt is "remembered" for the remainder of that session. We use enumerate devices (as does appear.in and others) to help give us a hint if we've already gotten access and don't have to prime the user to press the right buttons.

[youenn fablet]

(In reply to daginge from comment #5)
> The presence of labels in enumerateDevices is often used to determine if
> getUserMedia has been granted (persistent or not). Will this patch impact
> that?

It does impact if the user gave persistent access.
Otherwise, it should have no impact.

If user gave persistent access to camera/microphone, enumerateDevices will not expose labels until after the first successful getUserMedia call.

> Currently I really like iOS's approach where you ask once per session and
> the prompt is "remembered" for the remainder of that session. We use
> enumerate devices (as does appear.in and others) to help give us a hint if
> we've already gotten access and don't have to prime the user to press the
> right buttons.

Note though that this behavior is limited in case the website first asks for microphone and then camera.

This patch might impact if you do not want to show this UI to a user that gave persistent access.

[Dag-Inge Aas]

A typical WebRTC app flow is the following:

// User has not granted access before
- enumerateDevices, see if labels is falsy
- Show primer to user that they have to grant permission and how to do it and what to expect
- Call getUserMedia on user action
- Done

// User has granted persistent access and is accessing the page for the second time
- enumerateDevices, see if labels are falsy
- Call getUserMedia directly without user action

Due to getUserMedia not being included in the PermissionAPI we have no other way to detect and skip the primer needed. I consider the enumerateDevices way a hack, and I wish there was a better approach, but currently we have none. This has worked in all WebRTC-based browsers since the beginning, so a lot of apps are dependent on this behaviour for the best user experience.

A change in this API (presumably for privacy reasons?) would be acceptable if there was another way to detect that persistent access had been granted, in which case we could skip the primer.

[Dag-Inge Aas]

Are there plans to support the Permissions API for Media Devices as part of this change? (I guess for Safari's sake the entire Permissions API)

https://w3c.github.io/permissions/#media-devices

This would remove the need for querying enumerateDevices as described in https://groups.google.com/d/msg/discuss-webrtc/qGdSM-Cs3bc/09trGbeCBwAJ

[youenn fablet]

(In reply to daginge from comment #8)
> Are there plans to support the Permissions API for Media Devices as part of
> this change? (I guess for Safari's sake the entire Permissions API)
> 
> https://w3c.github.io/permissions/#media-devices
> 
> This would remove the need for querying enumerateDevices as described in
> https://groups.google.com/d/msg/discuss-webrtc/qGdSM-Cs3bc/09trGbeCBwAJ

Right, I think we should expose some related information, something like:
- Expose that user granted access persistently (only for first party iframes)
- Do not expose that user denied access persistently (we would return prompt)
- In case getUserMedia was granted previously for the browsing context, expose grant/prompt based on our heuristics as well.

[Will Morgan]

This proposal is going to cause problems on multi camera setups.

It's valuable to give the user the choice of which camera to use by exposing the device's label. Once the user selects the device we can specify the device ID in the getUserMedia video constraints.

This proposed patch would actually make life more difficult and UX more cumbersome by forcing users to get a camera stream and then switch devices mid-stream. This isn't an intuitive pattern to follow.