Add sinkId/setSinkId() extension to HTMLMediaElement.

Add sinkId/setSinkId() extension to HTMLMediaElement.

This is inteded to support the Audio Output Devices API.
http://w3c.github.io/mediacapture-output/
https://www.chromestatus.com/feature/4621603249848320

This implementation exposes the sinkId attribute and the setSinkId()
method to the web. sinkId always returns the default sink Id and
the setSinkId() method is not actually implemented and always returns
a rejected promise.

BUG=438023
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197229

[Guido Urdaneta, 2015-06-13 22:42:36]

guidou@chromium.org changed reviewers:
+ hta@chromium.org, mkwst@chromium.org, peter@chromium.org, tommi@chromium.org

[Guido Urdaneta, 2015-06-13 22:57:38]

Hi,
Please review

[Mike West, 2015-06-15 08:09:46]

What is the consent model for this feature? Are you working with felt@, et al.
to come up with a reasonable permission request? Or will we just be exposing the
available device IDs?

[Guido Urdaneta, 2015-06-15 08:52:34]

On 2015/06/15 08:09:46, Mike West wrote:
> What is the consent model for this feature? Are you working with felt@, et al.
> to come up with a reasonable permission request? Or will we just be exposing
the
> available device IDs?

The permission model we are using is the one that is already in place for
mediastreams,
which is the same that is being used for enumerating devices.
MediaStreamTrack.getSources() and getMediaDevices(), which are being superseded
by enumerateDevices().
The chromium-side implementation for this verifies that mediastreams permissions
are given before proceeding.
If this is not the case, then chromium refuses to perform the operation and the
promise returned by setSinkId() will get rejected with a SecurityError.
See https://codereview.chromium.org/1184473002/ (I just added you as reviewer)
implements the permissions checking.

The idea to reuse mediastream permissions is that this feature will be used
mainly in combination with mediastreams and enumerateDevices and having a single
permission model for all makes it simple for users and for the initial
implementation.
Now, output devices, device enumerations and mediastreams can be seen as
separate things which may have different permission models. Thus, we also have a
bug to better define the permission model/UI for these. See crbug.com/498675.

[Mike West, 2015-06-15 10:47:31]

On 2015/06/15 at 08:52:34, guidou wrote:
> On 2015/06/15 08:09:46, Mike West wrote:
> > What is the consent model for this feature? Are you working with felt@, et
al.
> > to come up with a reasonable permission request? Or will we just be exposing
the
> > available device IDs?
> 
> The permission model we are using is the one that is already in place for
mediastreams,
> which is the same that is being used for enumerating devices.
MediaStreamTrack.getSources() and getMediaDevices(), which are being superseded
by enumerateDevices().
> The chromium-side implementation for this verifies that mediastreams
permissions are given before proceeding.
> If this is not the case, then chromium refuses to perform the operation and
the promise returned by setSinkId() will get rejected with a SecurityError.
> See https://codereview.chromium.org/1184473002/ (I just added you as reviewer)
implements the permissions checking.
> 
> The idea to reuse mediastream permissions is that this feature will be used
mainly in combination with mediastreams and enumerateDevices and having a single
permission model for all makes it simple for users and for the initial
implementation.
> Now, output devices, device enumerations and mediastreams can be seen as
separate things which may have different permission models. Thus, we also have a
bug to better define the permission model/UI for these. See crbug.com/498675.

Ok. Assuming this will go through privacy/security review before launching, LGTM
to experiment with the implementation. I've added some relevant folks/labels to
that bug.

Thanks!

[Guido Urdaneta, 2015-06-15 10:59:12]

On 2015/06/15 10:47:31, Mike West wrote:
> On 2015/06/15 at 08:52:34, guidou wrote:
> > On 2015/06/15 08:09:46, Mike West wrote:
> > > What is the consent model for this feature? Are you working with felt@, et
> al.
> > > to come up with a reasonable permission request? Or will we just be
exposing
> the
> > > available device IDs?
> > 
> > The permission model we are using is the one that is already in place for
> mediastreams,
> > which is the same that is being used for enumerating devices.
> MediaStreamTrack.getSources() and getMediaDevices(), which are being
superseded
> by enumerateDevices().
> > The chromium-side implementation for this verifies that mediastreams
> permissions are given before proceeding.
> > If this is not the case, then chromium refuses to perform the operation and
> the promise returned by setSinkId() will get rejected with a SecurityError.
> > See https://codereview.chromium.org/1184473002/ (I just added you as
reviewer)
> implements the permissions checking.
> > 
> > The idea to reuse mediastream permissions is that this feature will be used
> mainly in combination with mediastreams and enumerateDevices and having a
single
> permission model for all makes it simple for users and for the initial
> implementation.
> > Now, output devices, device enumerations and mediastreams can be seen as
> separate things which may have different permission models. Thus, we also have
a
> bug to better define the permission model/UI for these. See crbug.com/498675.
> 
> Ok. Assuming this will go through privacy/security review before launching,
LGTM
> to experiment with the implementation. I've added some relevant folks/labels
to
> that bug.
> 
> Thanks!

Thanks!. Note that I gave you the wrong CL for the permissions checks. It was
https://codereview.chromium.org/1171953002/, which you already reviewed.

[Guido Urdaneta, 2015-06-15 10:59:28]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2015-06-15 10:59:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/40001

[commit-bot: I haz the power, 2015-06-15 12:28:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-15 12:28:05]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[Guido Urdaneta, 2015-06-15 14:20:17]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2015-06-15 14:20:18]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1182243005/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2015-06-15 14:20:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/60001

[commit-bot: I haz the power, 2015-06-15 16:24:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-15 16:24:44]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[Guido Urdaneta, 2015-06-15 19:19:08]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2015-06-15 19:19:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/60001

[commit-bot: I haz the power, 2015-06-15 21:09:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-15 21:09:59]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[Guido Urdaneta, 2015-06-16 09:53:54]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2015-06-16 09:54:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/60001

[commit-bot: I haz the power, 2015-06-16 11:36:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-16 11:36:18]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[Guido Urdaneta, 2015-06-16 13:05:05]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2015-06-16 13:05:05]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1182243005/#ps80001
(title: "Update expected value for public interface/properties tests")

[commit-bot: I haz the power, 2015-06-16 13:05:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/80001

[commit-bot: I haz the power, 2015-06-16 15:13:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-16 15:13:54]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/66800)

[Guido Urdaneta, 2015-06-17 00:30:55]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2015-06-17 00:31:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1182243005/80001

[commit-bot: I haz the power, 2015-06-17 02:53:33]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197229

[Peter Beverloo, 2015-06-18 14:21:20]

Thanks Guidou!

lgtm % comment

https://codereview.chromium.org/1182243005/diff/80001/Source/modules/audio_ou...
File Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp
(right):

https://codereview.chromium.org/1182243005/diff/80001/Source/modules/audio_ou...
Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp:22:
WTF_LOG(Media, __FUNCTION__);
Did you mean to land these WTF_LOG statements?

https://codereview.chromium.org/1182243005/diff/80001/Source/modules/audio_ou...
File Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.idl
(right):

https://codereview.chromium.org/1182243005/diff/80001/Source/modules/audio_ou...
Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.idl:5: //
http://w3c.github.io/mediacapture-output/#htmlmediaelement-extensions
micro nit: github.io links are also available over https :-).