Add support for switching the audio output device for HTMLMediaElements.

Add support for switching the audio output device for HTMLMediaElements.

This is a first implementation of Chromium support for the
Audio Output Devices API (see http://w3c.github.io/mediacapture-output).
This implementation supports:
* Output device switching for media elements. 
* Permission check based on MediaStream devices permissions.
* Sending a reply from the browser process to the renderer process such that it can be used by Blink to reject or accept a JavaScript Promise returned by the setSinkId()
method.

BUG=438023
TEST=manually tested

[Guido Urdaneta, 2015-05-12 14:54:49]

guidou@chromium.org changed reviewers:
+ tommi@chromium.org

[Guido Urdaneta, 2015-05-12 14:56:08]

Hi,

Please review.

[Guido Urdaneta, 2015-06-03 18:34:07]

Hi,

This is the first version that behaves according to spec.
Please take a look.

Note that this is still not landable because some bots will fail, as I haven't
updated a couple implementations of the interfaces that don't get built in my
local installation (e.g., mojo renderer implementations). 

Also, we have to replace base::Unretained pointers with weak pointers in
callbacks.

Otherwise, this version should be close to the final version.

[miu, 2015-06-03 21:00:59]

miu@chromium.org changed reviewers:
+ miu@chromium.org

[miu, 2015-06-03 21:01:02]

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
File content/browser/renderer_host/media/audio_renderer_host.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
content/browser/renderer_host/media/audio_renderer_host.cc:451: // We want to
check that MediaStream device permissions have been granted,
nit: Please rewrite all code comments in this CL without using the words "we" or
"you."  ;-)

Example: For this code comment just remove the words "We want to."

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
content/browser/renderer_host/media/audio_renderer_host.cc:563: DVLOG(1) <<
__FUNCTION__ << "(" << stream_id << ", " << raw_device_id << ", "
nit: Add DCHECK_CURRENTLY_ON(...)

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
content/browser/renderer_host/media/audio_renderer_host.cc:582: DVLOG(1) <<
__FUNCTION__ << "(" << stream_id << ", " << request_id << ")";
nit: Add DCHECK_CURRENTLY_ON(...)

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
File content/browser/renderer_host/media/audio_renderer_host_unittest.cc
(right):

https://codereview.chromium.org/1122393004/diff/160001/content/browser/render...
content/browser/renderer_host/media/audio_renderer_host_unittest.cc:350: 
Please add one or more unit tests for this new functionality.

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
File media/audio/audio_output_device.h (right):

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
media/audio/audio_output_device.h:187: std::map<int, base::Callback<void(int)>>
switch_requests_;
When is it appropriate to have multiple switch requests in-progress?  It seems
only the last switch request should succeed, so maybe just keep track of that
one and run the callbacks with an abort status code for all the prior ones?

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
File media/audio/audio_output_device_unittest.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
media/audio/audio_output_device_unittest.cc:246: 
Please write one or more unit tests to test new functionality.

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
File media/audio/audio_output_stream_sink.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/audio/audio_outp...
media/audio/audio_output_stream_sink.cc:65: const base::Callback<void(int)>&
callback) { }
All the other default method implementations run their callback.  You should
always run the callback passed in here with some kind of failure code.

https://codereview.chromium.org/1122393004/diff/160001/media/audio/clockless_...
File media/audio/clockless_audio_sink.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/audio/clockless_...
media/audio/clockless_audio_sink.cc:113: const base::Callback<void(int)>&
callback) { }
ditto: Run the callback with a failure code here.

https://codereview.chromium.org/1122393004/diff/160001/media/audio/null_audio...
File media/audio/null_audio_sink.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/audio/null_audio...
media/audio/null_audio_sink.cc:78: }
ditto: Run callback.  Maybe the NullAudioSink should just "fake out" a success
code here?

https://codereview.chromium.org/1122393004/diff/160001/media/base/audio_rende...
File media/base/audio_renderer.h (right):

https://codereview.chromium.org/1122393004/diff/160001/media/base/audio_rende...
media/base/audio_renderer.h:75: // Switches the output device.
Please do the same here as I suggested in AudioRendererSink (re: documentation,
naming the type of the callback, and using an enum).

https://codereview.chromium.org/1122393004/diff/160001/media/base/audio_rende...
File media/base/audio_renderer_sink.h (right):

https://codereview.chromium.org/1122393004/diff/160001/media/base/audio_rende...
media/base/audio_renderer_sink.h:63: // Switches the audio output device.
The documentation here is insufficient: Please clarify what the integer argument
to the callback is supposed to be.  Better yet, maybe you should:

  // Callback run with <<EXPLANATION OF INT HERE>> when the output device switch
request has completed.
  using SwitchOutputDeviceDoneCallback = base::Callback<void(int)>;

Also, the int argument should probably be an enum instead...

https://codereview.chromium.org/1122393004/diff/160001/media/base/fake_audio_...
File media/base/fake_audio_renderer_sink.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/base/fake_audio_...
media/base/fake_audio_renderer_sink.cc:58: const base::Callback<void(int)>&
callback) { }
ditto: Run the callback to "fake" that something happened.

https://codereview.chromium.org/1122393004/diff/160001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/base/pipeline.cc...
media/base/pipeline.cc:600: // TODO(guidou): Check if there are any values of
state_ for which switching
Please do this in the current CL, or submit a crbug and note the bug number
here.

https://codereview.chromium.org/1122393004/diff/160001/media/blink/webmediapl...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/1122393004/diff/160001/media/blink/webmediapl...
media/blink/webmediaplayer_impl.cc:389: // TODO(guidou): Change base::Unretained
to a weak pointer
This TODO needs to be resolved before committing.  Also, is the base::Unretained
in the setAudioOutputDevice() method correct?.

[Guido Urdaneta, 2015-06-09 07:58:37]

I will replace this CL with a set of smaller CLs.
All unaddressed comments will be addressed there.