Replace AudioManager::GetAudio*DeviceNames with AudioManager::GetAudio*DeviceDescriptions

Replace AudioManager::GetAudio*DeviceNames with AudioManager::GetAudio*DeviceDescriptions

see https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/media_devices_manager.cc?type=cs&sq=package:chromium&rcl=1481185182&l=68:

MediaDevicesManager now first asks AudioManager for device names, and then goes back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/87ea051559e9e40b7ee63db1d61fd0679f37d3e2
Cr-Commit-Position: refs/heads/master@{#439147}

[o1ka, 2016-12-08 13:56:21]

Description was changed from

==========
Replace AudioManager::GetAudio...DeviceNames with
AudioManager::GetAudio...DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
==========

to

==========
Replace AudioManager::GetAudio...DeviceNames with
AudioManager::GetAudio...DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[o1ka, 2016-12-08 13:56:34]

The CQ bit was checked by olka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 13:56:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[o1ka, 2016-12-08 13:59:12]

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

[o1ka, 2016-12-08 13:59:13]

PTAL

[o1ka, 2016-12-08 14:03:14]

Description was changed from

==========
Replace AudioManager::GetAudio...DeviceNames with
AudioManager::GetAudio...DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Replace AudioManager::GetAudio...DeviceNames with
AudioManager::GetAudio...DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-12-08 14:17:30]

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

[commit-bot: I haz the power, 2016-12-08 14:17:31]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Guido Urdaneta, 2016-12-08 15:53:37]

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:125:
new AudioDeviceDescriptions);
nit: I think base::MakeUnique is preferred over explicitly calling new like
this, but check the style guide

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc:341:
for (AudioDeviceDescriptions::const_iterator device = devices.begin();
nit: consider switching to range for

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
File
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:352:
for (; audio_it != physical_audio_devices_.end(); ++audio_it) {
nit: consider switching to range for

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:366:
for (; video_it != physical_video_devices_.end(); ++video_it) {
nit: If you switch the above loop to range for, switch this one as well

https://codereview.chromium.org/2563653002/diff/40001/content/common/media/me...
File content/common/media/media_devices.h (right):

https://codereview.chromium.org/2563653002/diff/40001/content/common/media/me...
content/common/media/media_devices.h:12: #include
"media/audio/audio_device_description.h"
include not necessary. just declaring media::AudioDeviceDescription should be
enough.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/android/aud...
File media/audio/android/audio_android_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/android/aud...
media/audio/android/audio_android_unittest.cc:120: DVLOG(2) << "Device ID(" <<
it->unique_id
Should you check anything about group ID here?

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
File media/audio/audio_device_description.h (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:69: typedef
std::list<AudioDeviceDescription> AudioDeviceDescriptions;
why list and not vector?

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:108: GetAudioInputDeviceNames(&device_names);
Should this have a CHECK(GetTaskRunner()->BelongToCurrentThread())?

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:318: EXPECT_FALSE(it->group_id.empty());
It looks like unique_id got renamed to group_id here.
Shouldn't you check both, or at least check unique_id?

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:520: for
(AudioDeviceDescriptions::iterator i = xp_device_descriptions.begin();
nit: consider switching to range for.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:633: for
(AudioDeviceDescriptions::iterator it = device_descriptions.begin();
nit: consider switching to range for

https://codereview.chromium.org/2563653002/diff/40001/media/audio/win/audio_l...
File media/audio/win/audio_low_latency_input_win_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/win/audio_l...
media/audio/win/audio_low_latency_input_win_unittest.cc:286: for
(media::AudioDeviceDescriptions::const_iterator it =
consider switching to range for

[mcasas, 2016-12-08 16:37:01]

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

[mcasas, 2016-12-08 16:37:01]

Drive-by

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
File media/audio/audio_device_description.h (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:66: std::string group_id;     // Group
identifier.
These three should be const, right?

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:69: typedef
std::list<AudioDeviceDescription> AudioDeviceDescriptions;
No typedef anymore, use "using"

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.h (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.h:111: static int GetUserBufferSize();
Why change the location of this method?
If it's to line up with the implementations,
I'd say it's best to do it in a separate cL with
no code changes. I bet this is not the only 
method out of order re. its declaration.

[mcasas, 2016-12-08 16:37:09]

mcasas@chromium.org changed reviewers:
- mcasas@chromium.org

[o1ka, 2016-12-09 14:07:16]

The CQ bit was checked by olka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-09 14:07:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 14:45:17]

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

[commit-bot: I haz the power, 2016-12-09 14:45:18]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[o1ka, 2016-12-09 14:56:31]

PTAL at nit fixes

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:125:
new AudioDeviceDescriptions);
On 2016/12/08 15:53:36, Guido Urdaneta wrote:
> nit: I think base::MakeUnique is preferred over explicitly calling new like
> this, but check the style guide

Done.

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc:341:
for (AudioDeviceDescriptions::const_iterator device = devices.begin();
On 2016/12/08 15:53:36, Guido Urdaneta wrote:
> nit: consider switching to range for

Done.

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
File
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:352:
for (; audio_it != physical_audio_devices_.end(); ++audio_it) {
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> nit: consider switching to range for

Done.

https://codereview.chromium.org/2563653002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:366:
for (; video_it != physical_video_devices_.end(); ++video_it) {
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> nit: If you switch the above loop to range for, switch this one as well

Done.

https://codereview.chromium.org/2563653002/diff/40001/content/common/media/me...
File content/common/media/media_devices.h (right):

https://codereview.chromium.org/2563653002/diff/40001/content/common/media/me...
content/common/media/media_devices.h:12: #include
"media/audio/audio_device_description.h"
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> include not necessary. just declaring media::AudioDeviceDescription should be
> enough.

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/android/aud...
File media/audio/android/audio_android_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/android/aud...
media/audio/android/audio_android_unittest.cc:120: DVLOG(2) << "Device ID(" <<
it->unique_id
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> Should you check anything about group ID here?

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
File media/audio/audio_device_description.h (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:66: std::string group_id;     // Group
identifier.
On 2016/12/08 16:37:01, mcasas wrote:
> These three should be const, right?
Not really. It's a standard Chrome-style structure, if we want it to be const
that we can declare the whole structure as const.
Also if they are const we can't store them in a vector, which seems to be an
unnecessary restriction.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:69: typedef
std::list<AudioDeviceDescription> AudioDeviceDescriptions;
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> why list and not vector?

Just because AudioDeviceNames are a list as well. Switched to vector.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_devic...
media/audio/audio_device_description.h:69: typedef
std::list<AudioDeviceDescription> AudioDeviceDescriptions;
On 2016/12/08 16:37:01, mcasas wrote:
> No typedef anymore, use "using"

"using declaration cannot refer to a template specialization"

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:108: GetAudioInputDeviceNames(&device_names);
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> Should this have a CHECK(GetTaskRunner()->BelongToCurrentThread())?

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.h (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.h:111: static int GetUserBufferSize();
On 2016/12/08 16:37:01, mcasas wrote:
> Why change the location of this method?
> If it's to line up with the implementations,
> I'd say it's best to do it in a separate cL with
> no code changes. I bet this is not the only 
> method out of order re. its declaration.

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:318: EXPECT_FALSE(it->group_id.empty());
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> It looks like unique_id got renamed to group_id here.
> Shouldn't you check both, or at least check unique_id?

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:520: for
(AudioDeviceDescriptions::iterator i = xp_device_descriptions.begin();
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> nit: consider switching to range for.

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:633: for
(AudioDeviceDescriptions::iterator it = device_descriptions.begin();
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> nit: consider switching to range for

Done.

https://codereview.chromium.org/2563653002/diff/40001/media/audio/win/audio_l...
File media/audio/win/audio_low_latency_input_win_unittest.cc (right):

https://codereview.chromium.org/2563653002/diff/40001/media/audio/win/audio_l...
media/audio/win/audio_low_latency_input_win_unittest.cc:286: for
(media::AudioDeviceDescriptions::const_iterator it =
On 2016/12/08 15:53:37, Guido Urdaneta wrote:
> consider switching to range for

Done.

[o1ka, 2016-12-09 16:12:52]

olka@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[o1ka, 2016-12-09 16:12:53]

dalecurtis@chromium.org: PTAL

[Guido Urdaneta, 2016-12-12 13:29:45]

lgtm

[Guido Urdaneta, 2016-12-12 13:29:56]

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:363:
// We need to get the output device ids, and calculate the HMAC
nit: ids -> IDs

[o1ka, 2016-12-12 22:03:17]

Dale - ping?

[DaleCurtis, 2016-12-12 22:13:45]

lgtm

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:379:
for (AudioDeviceDescriptions::const_iterator it =
auto here and elsewhere if you want.

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:64: for (const
media::AudioDeviceDescription& description : device_descriptions)
Does snapshot.insert(snapshot.end(), device_descriptions.begin(),
device_descriptions.end()) work?

https://codereview.chromium.org/2563653002/diff/80001/media/audio/audio_devic...
File media/audio/audio_device_description.h (right):

https://codereview.chromium.org/2563653002/diff/80001/media/audio/audio_devic...
media/audio/audio_device_description.h:56: AudioDeviceDescription(const
AudioDeviceDescription&) = default;
missing parameter name ?

[o1ka, 2016-12-13 09:33:57]

olka@chromium.org changed reviewers:
+ rockot@google.com

[o1ka, 2016-12-13 09:33:58]

rockot@ please RS chrome/browser/extensions/api/webrtc_audio_private/*

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
File
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
(right):

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:363:
// We need to get the output device ids, and calculate the HMAC
On 2016/12/12 13:29:56, Guido Urdaneta wrote:
> nit: ids -> IDs

Done.

https://codereview.chromium.org/2563653002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc:379:
for (AudioDeviceDescriptions::const_iterator it =
On 2016/12/12 22:13:44, DaleCurtis wrote:
> auto here and elsewhere if you want.

We'll be getting rid of these functions
(https://bugs.chromium.org/p/chromium/issues/detail?id=647185) so I'm not
cleaning this up now.

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:64: for (const
media::AudioDeviceDescription& description : device_descriptions)
On 2016/12/12 22:13:44, DaleCurtis wrote:
> Does snapshot.insert(snapshot.end(), device_descriptions.begin(),
> device_descriptions.end()) work?

No, unless we have a conversion from AudioDeviceDescription to MediaDeviceInfo.

https://codereview.chromium.org/2563653002/diff/80001/media/audio/audio_devic...
File media/audio/audio_device_description.h (right):

https://codereview.chromium.org/2563653002/diff/80001/media/audio/audio_devic...
media/audio/audio_device_description.h:56: AudioDeviceDescription(const
AudioDeviceDescription&) = default;
On 2016/12/12 22:13:44, DaleCurtis wrote:
> missing parameter name ?

Done.

[o1ka, 2016-12-13 09:34:05]

The CQ bit was checked by olka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 09:34:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[o1ka, 2016-12-13 09:48:43]

Description was changed from

==========
Replace AudioManager::GetAudio...DeviceNames with
AudioManager::GetAudio...DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Replace AudioManager::GetAudio*DeviceNames with
AudioManager::GetAudio*DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-12-13 12:07:24]

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

[commit-bot: I haz the power, 2016-12-13 12:07:25]

Dry run: This issue passed the CQ dry run.

[DaleCurtis, 2016-12-13 20:16:48]

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:64: for (const
media::AudioDeviceDescription& description : device_descriptions)
On 2016/12/13 at 09:33:58, o1ka wrote:
> On 2016/12/12 22:13:44, DaleCurtis wrote:
> > Does snapshot.insert(snapshot.end(), device_descriptions.begin(),
> > device_descriptions.end()) work?
> 
> No, unless we have a conversion from AudioDeviceDescription to
MediaDeviceInfo.

I was hoping the insert() operator would be smart enough to figure out the
conversion; but we probably have explicit on the constructor which prevents
this.

[o1ka, 2016-12-16 10:02:26]

On 2016/12/13 20:16:48, DaleCurtis_OOO_Dec14_Jan6 wrote:
>
https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
> File content/browser/renderer_host/media/media_devices_manager.cc (right):
> 
>
https://codereview.chromium.org/2563653002/diff/80001/content/browser/rendere...
> content/browser/renderer_host/media/media_devices_manager.cc:64: for (const
> media::AudioDeviceDescription& description : device_descriptions)
> On 2016/12/13 at 09:33:58, o1ka wrote:
> > On 2016/12/12 22:13:44, DaleCurtis wrote:
> > > Does snapshot.insert(snapshot.end(), device_descriptions.begin(),
> > > device_descriptions.end()) work?
> > 
> > No, unless we have a conversion from AudioDeviceDescription to
> MediaDeviceInfo.
> 
> I was hoping the insert() operator would be smart enough to figure out the
> conversion; but we probably have explicit on the constructor which prevents
> this.

Yes, style guide insists on 'explicit'.

[Ken Rockot(use gerrit already), 2016-12-16 16:04:09]

rockot@chromium.org changed reviewers:
+ rockot@chromium.org
- rockot@google.com

[Ken Rockot(use gerrit already), 2016-12-16 16:04:33]

Sorry, rockot@google.com -> rockot@chromium.org (this is why I didn't notice the
CL!)

[Ken Rockot(use gerrit already), 2016-12-16 16:05:25]

extensions RS LGTM

[o1ka, 2016-12-16 16:12:35]

The CQ bit was checked by olka@chromium.org

[o1ka, 2016-12-16 16:12:36]

The patchset sent to the CQ was uploaded after l-g-t-m from guidou@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2563653002/#ps100001
(title: "nit fixes")

[commit-bot: I haz the power, 2016-12-16 16:12:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[o1ka, 2016-12-16 16:14:01]

On 2016/12/16 16:04:33, Ken Rockot wrote:
> Sorry, mailto:rockot@google.com -> mailto:rockot@chromium.org (this is why I
didn't notice the
> CL!)

My bad!

Thanks everybody for review!

[commit-bot: I haz the power, 2016-12-16 18:14:24]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481904755662690,
"parent_rev": "d4e848b1ae84ecdc4a139af7fead18b0f3d3c75b", "commit_rev":
"4b11e4326cc0b600fc602fff4e19802c3f1a1acb"}

[commit-bot: I haz the power, 2016-12-16 18:14:52]

Description was changed from

==========
Replace AudioManager::GetAudio*DeviceNames with
AudioManager::GetAudio*DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Replace AudioManager::GetAudio*DeviceNames with
AudioManager::GetAudio*DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2563653002
==========

[commit-bot: I haz the power, 2016-12-16 18:14:55]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-16 18:16:43]

Description was changed from

==========
Replace AudioManager::GetAudio*DeviceNames with
AudioManager::GetAudio*DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2563653002
==========

to

==========
Replace AudioManager::GetAudio*DeviceNames with
AudioManager::GetAudio*DeviceDescriptions

see
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...:

MediaDevicesManager now first asks AudioManager for device names, and then goes
back to AudioManager for group ID of each device.
Since AudioManager will be moved to a separate process, we want to minimize the
number of roundtrips.

BUG=672467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/87ea051559e9e40b7ee63db1d61fd0679f37d3e2
Cr-Commit-Position: refs/heads/master@{#439147}
==========

[commit-bot: I haz the power, 2016-12-16 18:16:44]

Patchset 6 (id:??) landed as
https://crrev.com/87ea051559e9e40b7ee63db1d61fd0679f37d3e2
Cr-Commit-Position: refs/heads/master@{#439147}