Implement WebUserMediaClient::requestSources

Implement WebUserMediaClient::requestSources

This makes MediaStreamTrack::getSources go through UserMediaClient instead, needed to know which frame the call came from.

Blink CL: https://codereview.chromium.org/560843002/

BUG=406094
Committed: https://crrev.com/3a454d78eaab545bc976d9240b0e23019bcfbbcc
Cr-Commit-Position: refs/heads/master@{#294582}

[Henrik Grunell, 2014-09-10 12:56:14]

grunell@chromium.org changed reviewers:
+ perkj@chromium.org

[perkj_chrome, 2014-09-10 15:17:39]

Can you also add a unittests in user_media_client_unittests.

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:279: // TODO
need name name and fix indentation. Is this intentional?

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:437:
CancelAndDeleteMediaDevicesRequest(request);
code duplication. Can you move line 437 and 479 to line 699?
Maybe move both of these functions to line 700?

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:695: if
(audio_output_devices_requested)
why not check what type of request this is by looking at
request->media_devices_request.isNull?. Preferably by adding a method on the
bool MediaDevicesRequestInfo::IsGetSourcesRequest() .


Can you use that on line 683 as well?

[Henrik Grunell, 2014-09-12 10:58:26]

I'll add unit tests as well.

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:279: // TODO
On 2014/09/10 15:17:39, perkj wrote:
> need name name and fix indentation. Is this intentional?

Removed, it's counted in blink.

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:437:
CancelAndDeleteMediaDevicesRequest(request);
On 2014/09/10 15:17:39, perkj wrote:
> code duplication. Can you move line 437 and 479 to line 699?
> Maybe move both of these functions to line 700?

Well, it's not really duplicated. Above sets up a vector of WebMediaDeviceInfo,
below sets up a vector of WebSourceInfo. And they also have different
properties.

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:695: if
(audio_output_devices_requested)
On 2014/09/10 15:17:39, perkj wrote:
> why not check what type of request this is by looking at
> request->media_devices_request.isNull?. Preferably by adding a method on the
> bool MediaDevicesRequestInfo::IsGetSourcesRequest() .

Yes, that's better. Done.

> 
> 
> Can you use that on line 683 as well?

Done.

[Henrik Grunell, 2014-09-12 12:12:46]

Added unit test. Ptal.

[perkj_chrome, 2014-09-12 12:35:34]

lgtm provided the below is addressed.

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:437:
CancelAndDeleteMediaDevicesRequest(request);
On 2014/09/12 10:58:25, Henrik Grunell wrote:
> On 2014/09/10 15:17:39, perkj wrote:
> > code duplication. Can you move line 437 and 479 to line 699?
> > Maybe move both of these functions to line 700?
> 
> Well, it's not really duplicated. Above sets up a vector of
WebMediaDeviceInfo,
> below sets up a vector of WebSourceInfo. And they also have different
> properties.
CancelAndDeleteMediaDevicesRequest is duplicated and it should be possible to
move.

https://codereview.chromium.org/562643003/diff/60001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/60001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:285: // This is counted for UMA
stats in MediaStreamTrack::getSources in blink.
I don't understand this comment and I am not sure its relevant. Remove or
explain.

[Henrik Grunell, 2014-09-12 12:59:31]

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/20001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:437:
CancelAndDeleteMediaDevicesRequest(request);
On 2014/09/12 12:35:33, perkj wrote:
> On 2014/09/12 10:58:25, Henrik Grunell wrote:
> > On 2014/09/10 15:17:39, perkj wrote:
> > > code duplication. Can you move line 437 and 479 to line 699?
> > > Maybe move both of these functions to line 700?
> > 
> > Well, it's not really duplicated. Above sets up a vector of
> WebMediaDeviceInfo,
> > below sets up a vector of WebSourceInfo. And they also have different
> > properties.
> CancelAndDeleteMediaDevicesRequest is duplicated and it should be possible to
> move.

Done.

https://codereview.chromium.org/562643003/diff/60001/content/renderer/media/u...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/562643003/diff/60001/content/renderer/media/u...
content/renderer/media/user_media_client_impl.cc:285: // This is counted for UMA
stats in MediaStreamTrack::getSources in blink.
On 2014/09/12 12:35:33, perkj wrote:
> I don't understand this comment and I am not sure its relevant. Remove or
> explain.

Another shot at explaining.

[Henrik Grunell, 2014-09-12 12:59:38]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-09-12 12:59:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/562643003/100001

[commit-bot: I haz the power, 2014-09-12 14:00:02]

Committed patchset #6 (id:100001) as 87ff9a986ff3f97a4fe5980ad96d6c9904a2263f

[commit-bot: I haz the power, 2014-09-12 14:02:41]

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