Correctly handle enumerations for AudioManagers that report only the default audio output device.

Correctly handle enumerations for AudioManagers that report only the default audio output device.

BUG=578993
Committed: https://crrev.com/bbfc18f1577804ae0ee6f42d5866940116c97696
Cr-Commit-Position: refs/heads/master@{#370113}

[Guido Urdaneta, 2016-01-19 11:49:21]

Description was changed from

==========
Correctly handle enumerations for AudioManagers that report only the default
device.

BUG=578993
==========

to

==========
Correctly handle enumerations for AudioManagers that report only the default
audio output device.

BUG=578993
==========

[Guido Urdaneta, 2016-01-19 11:57:16]

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

[Guido Urdaneta, 2016-01-19 11:57:16]

Hi, PTAL

[tommi (sloooow) - chröme, 2016-01-19 13:03:37]

https://codereview.chromium.org/1602083003/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (left):

https://codereview.chromium.org/1602083003/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:744: if
(device_enumeration.size() > 1) {
instead of the 'has_actual_devices' member variable, could we have a method like
so:

bool has_actual_devices() const {
  // <include comment>
  return devices.size() > 1;
}

?

[Guido Urdaneta, 2016-01-19 13:41:10]

https://codereview.chromium.org/1602083003/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (left):

https://codereview.chromium.org/1602083003/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:744: if
(device_enumeration.size() > 1) {
On 2016/01/19 13:03:37, tommi-chromium wrote:
> instead of the 'has_actual_devices' member variable, could we have a method
like
> so:
> 
> bool has_actual_devices() const {
>   // <include comment>
>   return devices.size() > 1;
> }
> 
> ?

devices.size() > 1 is incorrect for AudioManagers that always report only one
device.
The has_actual_devices field allows differentiating that from the case where
there are no devices (but where we have one entry with the default parameters).
I updated the AudioOutputDeviceEnumerator::Enumerate() comment to explain that.

[tommi (sloooow) - chröme, 2016-01-19 14:53:46]

thanks for the explanation.  If you could include that somehow as a comment,
then maybe that will help someone understand the behavior down the line.  lgtm.

[Guido Urdaneta, 2016-01-19 15:03:33]

On 2016/01/19 14:53:46, tommi-chromium wrote:
> thanks for the explanation.  If you could include that somehow as a comment,
> then maybe that will help someone understand the behavior down the line. 
lgtm.

The comment for the AudioOutputDeviceEnumerator::Enumerate() method now contains
the explanation.

[Guido Urdaneta, 2016-01-19 15:27:22]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2016-01-19 15:27:54]

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

[commit-bot: I haz the power, 2016-01-19 15:34:29]

Description was changed from

==========
Correctly handle enumerations for AudioManagers that report only the default
audio output device.

BUG=578993
==========

to

==========
Correctly handle enumerations for AudioManagers that report only the default
audio output device.

BUG=578993
==========

[commit-bot: I haz the power, 2016-01-19 15:34:30]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-19 15:35:22]

Description was changed from

==========
Correctly handle enumerations for AudioManagers that report only the default
audio output device.

BUG=578993
==========

to

==========
Correctly handle enumerations for AudioManagers that report only the default
audio output device.

BUG=578993

Committed: https://crrev.com/bbfc18f1577804ae0ee6f42d5866940116c97696
Cr-Commit-Position: refs/heads/master@{#370113}
==========

[commit-bot: I haz the power, 2016-01-19 15:35:23]

Patchset 3 (id:??) landed as
https://crrev.com/bbfc18f1577804ae0ee6f42d5866940116c97696
Cr-Commit-Position: refs/heads/master@{#370113}