Refactor the VideoCaptureDevice::Name struct.

Refactor the VideoCaptureDevice::Name struct.

The main purpose of this change is to allow for future changes where platform specific properties can be stored with a device name.
An example of this is when we enumerate capture devices from more than one API (DirectShow vs Media Foundation on Windows) since we have to use different APIs to get access to different capture devices.

Additionally:
* Change Name from being a struct to a class.
* Only allow read-only access to the properties after construction.
* Change the "Names" typedef to a class to make looking a name up by device ID easier.

I'm make it harder to construct a 'Name' without going through the enumeration since the enumeration should be used to get the proper name, ID and other potential properties to go with them.
This change is related to another one that mcasas is working on: https://codereview.chromium.org/17402002/

(I'm TBR-ing fischman although he has already approved this CL.  There appears to be a problem with Rietveld)

BUG=144465
TBR=fischman

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209662

[tommi (sloooow) - chröme, 2013-06-26 13:53:05]

Fix MEDIA_SCREEN_VIDEO_CAPTURE handling

[tommi (sloooow) - chröme, 2013-06-26 14:20:15]

Fix TabCaptureApiTest tests.

[wjia(left Chromium), 2013-06-26 18:16:44]

https://codereview.chromium.org/17846002/diff/24001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/17846002/diff/24001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.cc:183: }
What's the reason to add this logic (of enumeration) here? If the device is
detached, it will error out when trying to open the device.

It seems even though the device can't be found, we will use
Name(device.device.name, device.device.id) anyway as below.

https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
media/video/capture/video_capture_device.h:64: Name* FindById(const std::string&
id) {
Do you need an inline function here? If not, it's better move this one into cc
file to cut compile time. Same applies above.

[tommi (sloooow) - chröme, 2013-06-27 14:46:02]

https://codereview.chromium.org/17846002/diff/24001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/17846002/diff/24001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.cc:183: }
On 2013/06/26 18:16:44, wjia wrote:
> What's the reason to add this logic (of enumeration) here? If the device is
> detached, it will error out when trying to open the device.
> 
> It seems even though the device can't be found, we will use
> Name(device.device.name, device.device.id) anyway as below.

Yes, currently that's going to be the case.  The reason this is like it is
though is to use the Name instance as it comes from the enumeration, where it
might have more information than just name and id.  On Windows we have an actual
need today to also store the API from which the enumeration comes.

Instead of doing this enumeration again here though, we can cache the video
capture device names in OnEnumerateDevices and refer to that list here.

Also, I've noticed that the 'use_fake_device_' flag should only apply to
MEDIA_DEVICE_VIDEO_CAPTURE, so I'm going to move that check to that switch case.

The vc_device_name variable should also only be used for
MEDIA_DEVICE_VIDEO_CAPTURE, so I'll reduce the scope of it to the same case
label.

[tommi (sloooow) - chröme, 2013-06-27 14:46:16]

Address comments

[mcasas, 2013-06-28 07:35:29]

LGTM for what's worth; my comments are just expressions of happiness.

Btw it would be good to beef up VideoCaptureManagerTest, more concretely
CreateAndClose test, perhaps by using the fake capture device.

https://codereview.chromium.org/17846002/diff/43001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_manager.h (right):

https://codereview.chromium.org/17846002/diff/43001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_manager.h:164: 
Exactly.

https://codereview.chromium.org/17846002/diff/43001/media/video/capture/mac/v...
File media/video/capture/mac/video_capture_device_mac.mm (right):

https://codereview.chromium.org/17846002/diff/43001/media/video/capture/mac/v...
media/video/capture/mac/video_capture_device_mac.mm:169: Name* found =
device_names.FindById(device_name_.id());
much cleaner!

[wjia(left Chromium), 2013-06-29 01:44:22]

>
https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
> File media/video/capture/video_capture_device.h (right):
> 
>
https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
> media/video/capture/video_capture_device.h:64: Name* FindById(const
std::string&
> id) {
> Do you need an inline function here? If not, it's better move this one into cc
> file to cut compile time. Same applies above.

Waiting for response on this one.

[tommi (sloooow) - chröme, 2013-07-01 13:32:09]

Address comments

[tommi (sloooow) - chröme, 2013-07-01 13:32:30]

https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/17846002/diff/24001/media/video/capture/video...
media/video/capture/video_capture_device.h:64: Name* FindById(const std::string&
id) {
On 2013/06/26 18:16:44, wjia wrote:
> Do you need an inline function here? If not, it's better move this one into cc
> file to cut compile time. Same applies above.

Sorry, missed this in the last round.  Added a new file.

[wjia(left Chromium), 2013-07-01 14:26:40]

lgtm

[commit-bot: I haz the power, 2013-07-01 14:35:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommi@chromium.org/17846002/66001

[commit-bot: I haz the power, 2013-07-01 14:50:31]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-07-01 16:10:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommi@chromium.org/17846002/66001

[commit-bot: I haz the power, 2013-07-01 16:21:25]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Ami GONE FROM CHROMIUM, 2013-07-01 16:52:22]

OWNERS-stamp for media.gyp

[commit-bot: I haz the power, 2013-07-02 07:57:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommi@chromium.org/17846002/66001

[tommi (sloooow) - chröme, 2013-07-02 07:57:28]

On 2013/07/01 16:52:22, Ami Fischman wrote:
> OWNERS-stamp for media.gyp

Thanks!

[commit-bot: I haz the power, 2013-07-02 08:09:17]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[tommi (sloooow) - chröme, 2013-07-02 08:31:27]

The presubmit step is still failing for media.gyp even though fischman@ is
clearly in the owners file.  For some reason Rietveld isn't returning Ami's
user name when the presubmit script checks the list of reviewers.
I'll ping Maruel.


On Tue, Jul 2, 2013 at 10:09 AM, <commit-bot@chromium.org> wrote:

> Retried try job too often on chromium_presubmit for step(s) presubmit
> http://build.chromium.org/p/**tryserver.chromium/**
>
buildstatus?builder=chromium_**presubmit&number=13402<http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=13402>
>
>
https://chromiumcodereview.**appspot.com/17846002/<https://chromiumcodereview...
>

[commit-bot: I haz the power, 2013-07-02 08:40:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommi@chromium.org/17846002/66001

[commit-bot: I haz the power, 2013-07-02 10:40:45]

Change committed as 209662

[Ami GONE FROM CHROMIUM, 2013-07-02 14:20:07]

lol apparently I forgot to say LGTM in my stamp email.  Sorry.
On Jul 2, 2013 1:31 AM, "Tommi" <tommi@chromium.org> wrote:

> The presubmit step is still failing for media.gyp even though fischman@is
clearly in the owners file.  For some reason Rietveld isn't returning
> Ami's user name when the presubmit script checks the list of reviewers.
> I'll ping Maruel.
>
>
> On Tue, Jul 2, 2013 at 10:09 AM, <commit-bot@chromium.org> wrote:
>
>> Retried try job too often on chromium_presubmit for step(s) presubmit
>> http://build.chromium.org/p/**tryserver.chromium/**
>>
buildstatus?builder=chromium_**presubmit&number=13402<http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=13402>
>>
>>
https://chromiumcodereview.**appspot.com/17846002/<https://chromiumcodereview...
>>
>
>

[tommi (sloooow) - chröme, 2013-07-02 14:26:46]

hah, I looked over it and couldn't figure out what was wrong. doh!


On Tue, Jul 2, 2013 at 4:20 PM, Ami Fischman <fischman@chromium.org> wrote:

> lol apparently I forgot to say LGTM in my stamp email.  Sorry.
> On Jul 2, 2013 1:31 AM, "Tommi" <tommi@chromium.org> wrote:
>
>> The presubmit step is still failing for media.gyp even though fischman@is
clearly in the owners file.  For some reason Rietveld isn't returning
>> Ami's user name when the presubmit script checks the list of reviewers.
>> I'll ping Maruel.
>>
>>
>> On Tue, Jul 2, 2013 at 10:09 AM, <commit-bot@chromium.org> wrote:
>>
>>> Retried try job too often on chromium_presubmit for step(s) presubmit
>>> http://build.chromium.org/p/**tryserver.chromium/**
>>>
buildstatus?builder=chromium_**presubmit&number=13402<http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=13402>
>>>
>>>
https://chromiumcodereview.**appspot.com/17846002/<https://chromiumcodereview...
>>>
>>
>>