Remove device enumeration, caching and monitoring from MediaStreamManager.

Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
Committed: https://crrev.com/cc6d460ef322ee5b6eb57736cdf5bdfb8f5c73d2
Cr-Commit-Position: refs/heads/master@{#420735}

[Guido Urdaneta, 2016-09-19 12:35:35]

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

[commit-bot: I haz the power, 2016-09-19 12:35:44]

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

[Guido Urdaneta, 2016-09-19 13:12:24]

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

[Guido Urdaneta, 2016-09-19 13:12:25]

Hi, PTAL

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/audio_input_device_manager.h (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/audio_input_device_manager.h:17: #include
<vector>
This is a drive-by lint fix.

[commit-bot: I haz the power, 2016-09-19 13:39:20]

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

[commit-bot: I haz the power, 2016-09-19 13:39:22]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2016-09-20 08:44:31]

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

[commit-bot: I haz the power, 2016-09-20 08:44:43]

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

[tommi (sloooow) - chröme, 2016-09-20 08:47:15]

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_manager.cc:43:
base::StringPrintf("Getting devices for stream type %d:\n", device_type);
nit: do we want to have a line feed in there?  maybe just a couple of spaces
would be enough

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_manager.cc:145: bool
is_update_ongoing() { return is_update_ongoing_; }
const?

btw, would it make sense to have thread checks in this class?

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:423:
media::AudioDeviceDescription::kCommunicationsDeviceId)
nit: {}

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:214: for (const
auto& info : device_infos)
{}

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:225: for (const
auto& info : device_infos)
{}

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:758: {{false, false,
true /* audio output only*/}},
nit: since you've documented one param, what about doing the same for the
others?  It's a bit hard to read without knowing what this is.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:840: MediaDeviceType
device_type = MEDIA_DEVICE_AUDIO_CAPTURE
I'm not fully groking what's supposed to be checked here... feels like there's a
missing comparison operator... did you mean to check stream_type?

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:884: "Media input
device removed: type = %s, id = %s, name = %s ",
nit: the whitespace around '=' isn't needed

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:1063: for (const
auto& info : enumeration[MEDIA_DEVICE_TYPE_AUDIO_INPUT])
{}

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:1066: for (const
auto& info : enumeration[MEDIA_DEVICE_TYPE_VIDEO_INPUT])
{}

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:1380: //
TODO(guidou):
w00t!

[Guido Urdaneta, 2016-09-20 08:48:47]

The CQ bit was unchecked by guidou@chromium.org

[hta - Chromium, 2016-09-20 09:41:38]

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

[hta - Chromium, 2016-09-20 09:41:39]

Sending comments on the tests. Will return to comments on the code.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager_unittest.cc:162: for
(int i = 0; i < kNumCalls; i++) {
Why is running this loop 10 times a correct number? Are there 10 code paths, or
do we need to try 10 times in order to exercise 2 different paths?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_manager_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager_unittest.cc:47: //
testing.
Does this mean that we mock a method that isn't provided on the interface we're
mocking? This seems weird - should at least give pointers - "this is added as a
mock, and dispatched to in HandleEnumerationResult, because it's easier to write
tests in terms of a mocked DevicesEnumerated function than with other ways to
write the same thing".

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager_unittest.cc:574:
vcm_->EnumerateDevices(base::Bind(&HandleEnumerationResult, listener_.get()));
With the number of calls to this particular function, should it be an utility in
the test fixture? This could allow you to hide HandleEnmuerationResult too - it
seems from a quick read that all that DevicesEnumerated does is to save its
argument in &devices, which means that if you put |devices| into the fixture
too, you can avoid the strange mock completely.

[hta - Chromium, 2016-09-20 10:55:30]

Comments on the new implementation. Some thoughts.
Still not reviewed the changed files.

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

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:210:
DoEnumerateDevices(type);
If OS_MACOSX and type = MEDIA_DEVICE_TYPE_OUTPUT, should this fail?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:247: //
TODO(erikchen): Remove ScopedTracker below once crbug.com/458404 is fixed.
Is this code copied from elsewhere, including TODOs?
Is there a possibility of reusing that implementation rather than recoding it
here?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:421:
requests_.erase(std::remove_if(requests_.begin(), requests_.end(),
I'm confused by this construct. It seems to me that we first remove all the
ready requests from requests_ by remove_if, and then erase the remaining
members. But remove_if is supposed to return a value pointing past the end, so
request_.erase() should then be called on (equivalent) (request_.end(),
request_.end()), which erases nothing. Are you sure we have test coverage for
all the combinations of ready and non-ready requests?

std::remove_if is a C++ 17 feature according to
http://en.cppreference.com/w/cpp/algorithm/remove - is it approved for use in
Chromium?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:470: 
Why isn't the "if media_stream_manager_" test up here with a "return"?

Is the function doing anything but notifying the media stream manager?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:497: :
MEDIA_DEVICE_AUDIO_CAPTURE;
This will return MEDIA_DEVICE_AUDIO_CAPTURE for MEDIA_DEVICE_TYPE_AUDIO_OUTPUT.
Suggest that you DCHECK that the argument is either
MEDIA_DEVICE_TYPE_VIDEO_INPUT or MEDIA_DEVICE_TYPE_AUDIO_INPUT before doing this
switch.

Or even more explicit:

switch(type) {
  case MEDIA_DEVICE_TYPE_VIDEO_INPUT:
     ....
  case MEDIA_DEVICE_TYPE_AUDIO_INPUT:
     ....
  default:
     NOTREACHED()
}

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager.h (right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.h:73: void
SetCachePolicy(MediaDeviceType type, CachePolicy policy);
Why not return a failure when SYSTEM_MONITOR is set manually?

A return bool saying "this succeeded/failed" should be enough.

Given that the only values are SYSTEM_MONITOR (set by StartMonitoring() if
available) and NO_CACHE (set by StopMonitoring()), the utility of this function
seems doubtful.
Is it for testing?

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.h:128: using
CachePolicies = std::array<CachePolicy, NUM_MEDIA_DEVICE_TYPES>;
This is a pattern I haven't seen before. Is it a newer version of "typedef"?

[Guido Urdaneta, 2016-09-20 13:59:28]

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_manager.cc:43:
base::StringPrintf("Getting devices for stream type %d:\n", device_type);
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> nit: do we want to have a line feed in there?  maybe just a couple of spaces
> would be enough

There is a newline because it lists each device in a new line (this is from the
original MediaStreamManager).
I can change it to have all devices in a single line. Would you prefer it that
way?
I think it's easier to read with the newlines.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_manager.cc:145: bool
is_update_ongoing() { return is_update_ongoing_; }
On 2016/09/20 08:47:14, tommi (chrömium) wrote:
> const?
> 
> btw, would it make sense to have thread checks in this class?

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc:423:
media::AudioDeviceDescription::kCommunicationsDeviceId)
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> nit: {}

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:214: for (const
auto& info : device_infos)
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> {}

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:225: for (const
auto& info : device_infos)
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> {}

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:758: {{false, false,
true /* audio output only*/}},
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> nit: since you've documented one param, what about doing the same for the
> others?  It's a bit hard to read without knowing what this is.

Done. Since this is a bit confusing, do you think using three separate
parameters instead of the array is a better idea?
If so, enums instead of bool?
The array is nice for iterating and easy indexing, but that can be done achieved
without exposing it to callers.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:840: MediaDeviceType
device_type = MEDIA_DEVICE_AUDIO_CAPTURE
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> I'm not fully groking what's supposed to be checked here... feels like there's
a
> missing comparison operator... did you mean to check stream_type?

It's actually a type conversion from MediaStreamType to MediaDeviceType.
Added a function to make the conversion clearer.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:884: "Media input
device removed: type = %s, id = %s, name = %s ",
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> nit: the whitespace around '=' isn't needed

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:1063: for (const
auto& info : enumeration[MEDIA_DEVICE_TYPE_AUDIO_INPUT])
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> {}

Done.

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:1066: for (const
auto& info : enumeration[MEDIA_DEVICE_TYPE_VIDEO_INPUT])
On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> {}

Done.

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

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:210:
DoEnumerateDevices(type);
On 2016/09/20 10:55:29, hta - Chromium wrote:
> If OS_MACOSX and type = MEDIA_DEVICE_TYPE_OUTPUT, should this fail?

This just changes the policy, so there is no failure there.
If it's Mac, the policy is set to NO_CACHE for audio output in
StartMonitoring().
Since SetCachePolicy is now private, this can no longer be accidentally misused.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:247: //
TODO(erikchen): Remove ScopedTracker below once crbug.com/458404 is fixed.
On 2016/09/20 10:55:29, hta - Chromium wrote:
> Is this code copied from elsewhere, including TODOs?
> Is there a possibility of reusing that implementation rather than recoding it
> here?

This is code removed from MediaStreamManager.
It is practically unmodified so that it behaves exactly the same.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:421:
requests_.erase(std::remove_if(requests_.begin(), requests_.end(),
On 2016/09/20 10:55:29, hta - Chromium wrote:
> I'm confused by this construct. It seems to me that we first remove all the
> ready requests from requests_ by remove_if, and then erase the remaining
> members. But remove_if is supposed to return a value pointing past the end, so
> request_.erase() should then be called on (equivalent) (request_.end(),
> request_.end()), which erases nothing. Are you sure we have test coverage for
> all the combinations of ready and non-ready requests?
> 
> std::remove_if is a C++ 17 feature according to
> http://en.cppreference.com/w/cpp/algorithm/remove - is it approved for use in
> Chromium?

This version of remove_if is from C++98.
It is indeed a confusing construct, but it is the way one is supposed to remove
elements from an STL collection while iterating it.
It's so peculiar that it has its own Wikipedia page :-) 
https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:470: 
On 2016/09/20 10:55:29, hta - Chromium wrote:
> Why isn't the "if media_stream_manager_" test up here with a "return"?
> 
> Is the function doing anything but notifying the media stream manager?

Done.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.cc:497: :
MEDIA_DEVICE_AUDIO_CAPTURE;
Unfortunately, it is that way because there is no separate permission for audio
output devices yet.
Added a comment explaining the situation and renamed the variable to
|permission_type| to make it clearer.
Did add a DCHECK to make sure it's a valid device type.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager.h (right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.h:73: void
SetCachePolicy(MediaDeviceType type, CachePolicy policy);
On 2016/09/20 10:55:29, hta - Chromium wrote:
> Why not return a failure when SYSTEM_MONITOR is set manually?
> 
> A return bool saying "this succeeded/failed" should be enough.
> 
> Given that the only values are SYSTEM_MONITOR (set by StartMonitoring() if
> available) and NO_CACHE (set by StopMonitoring()), the utility of this
function
> seems doubtful.
> Is it for testing?

I originally made it public mainly for testing purposes. Since in practice it's
used only by the unit test, I just made it private and made the unit test a
friend.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager.h:128: using
CachePolicies = std::array<CachePolicy, NUM_MEDIA_DEVICE_TYPES>;
On 2016/09/20 10:55:29, hta - Chromium wrote:
> This is a pattern I haven't seen before. Is it a newer version of "typedef"?

Yes, it is. It's preferred over typedef, according to
http://chromium-cpp.appspot.com

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_devices_manager_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_devices_manager_unittest.cc:162: for
(int i = 0; i < kNumCalls; i++) {
On 2016/09/20 09:41:38, hta - Chromium wrote:
> Why is running this loop 10 times a correct number? Are there 10 code paths,
or
> do we need to try 10 times in order to exercise 2 different paths?
> 

The goal of using a relatively large number greater was to make sure that the
cache in cache-enabled tests did not become invalidated due to some bug.
Lowered the from 10 to 3.

For the no-cache tests 2 is enough to show that there is no caching, but keeping
kNumCalls for convenience.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_manager_unittest.cc
(right):

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager_unittest.cc:47: //
testing.
On 2016/09/20 09:41:39, hta - Chromium wrote:
> Does this mean that we mock a method that isn't provided on the interface
we're
> mocking? This seems weird - should at least give pointers - "this is added as
a
> mock, and dispatched to in HandleEnumerationResult, because it's easier to
write
> tests in terms of a mocked DevicesEnumerated function than with other ways to
> write the same thing".
> 

Removed the method.

https://codereview.chromium.org/2350693002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager_unittest.cc:574:
vcm_->EnumerateDevices(base::Bind(&HandleEnumerationResult, listener_.get()));
On 2016/09/20 09:41:39, hta - Chromium wrote:
> With the number of calls to this particular function, should it be an utility
in
> the test fixture? This could allow you to hide HandleEnmuerationResult too -
it
> seems from a quick read that all that DevicesEnumerated does is to save its
> argument in &devices, which means that if you put |devices| into the fixture
> too, you can avoid the strange mock completely.
> 

Done. The only slightly ugly thing is that HandleEnumerationResult is public so
that it can be used by the test that simulates device removals and additions.

[Guido Urdaneta, 2016-09-20 14:10:32]

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

[commit-bot: I haz the power, 2016-09-20 14:10:39]

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

[tommi (sloooow) - chröme, 2016-09-20 14:51:09]

lgtm

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_devices_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_devices_manager.cc:43:
base::StringPrintf("Getting devices for stream type %d:\n", device_type);
On 2016/09/20 13:59:26, Guido Urdaneta wrote:
> On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> > nit: do we want to have a line feed in there?  maybe just a couple of spaces
> > would be enough
> 
> There is a newline because it lists each device in a new line (this is from
the
> original MediaStreamManager).
> I can change it to have all devices in a single line. Would you prefer it that
> way?
> I think it's easier to read with the newlines.

ok as is then

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/2350693002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/media_stream_manager.cc:758: {{false, false,
true /* audio output only*/}},
On 2016/09/20 13:59:26, Guido Urdaneta wrote:
> On 2016/09/20 08:47:15, tommi (chrömium) wrote:
> > nit: since you've documented one param, what about doing the same for the
> > others?  It's a bit hard to read without knowing what this is.
> 
> Done. Since this is a bit confusing, do you think using three separate
> parameters instead of the array is a better idea?
> If so, enums instead of bool?
> The array is nice for iterating and easy indexing, but that can be done
achieved
> without exposing it to callers.

This is OK with me as is since the comments explain what's going on.

[Guido Urdaneta, 2016-09-20 14:55:30]

jochen@chromium.org: Please review content/public and the BUILD.gn files.

[Guido Urdaneta, 2016-09-20 14:56:57]

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

[Guido Urdaneta, 2016-09-20 14:56:58]

jochen@chromium.org: Please review changes in content/public and the BUILD.gn
files.

[hta - Chromium, 2016-09-20 15:05:51]

lgtm

[commit-bot: I haz the power, 2016-09-20 15:10:29]

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

[commit-bot: I haz the power, 2016-09-20 15:10:30]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2016-09-23 17:06:08]

Description was changed from

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
==========

to

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
==========

[Guido Urdaneta, 2016-09-23 17:06:09]

guidou@chromium.org changed reviewers:
- jochen@chromium.org

[Guido Urdaneta, 2016-09-23 17:07:48]

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

[Guido Urdaneta, 2016-09-23 17:07:49]

avi@: Can you rs this?

[Avi (use Gerrit), 2016-09-23 17:40:32]

lgtm

stampity stamp

[Guido Urdaneta, 2016-09-23 19:05:47]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2016-09-23 19:06:33]

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-09-23 21:29:01]

Description was changed from

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
==========

to

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
==========

[commit-bot: I haz the power, 2016-09-23 21:29:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-23 21:32:33]

Description was changed from

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660
==========

to

==========
Remove device enumeration, caching and monitoring from MediaStreamManager.

This CL introduces MediaDevicesManager, which is now responsible for low-level
device enumerations, including using device monitoring to maintain updated
caches.

This CL also removes the enumeration functionality from the MediaStreamProvider
interface, as MediaDevicesManager is the new class resnposible for providing
enumeration results to be used to handle renderer-generated enumeration requests
and device-change notifications. Due to this, relatively minor changes are made
to AudioInputDeviceManager and VideoCaptureManager.

BUG=647660

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

[commit-bot: I haz the power, 2016-09-23 21:32:34]

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