Shift media_stream::StreamOptions to align with the new getUserMedia spec

Shift media_stream::StreamOptions to align with the new getUserMedia spec

The new getUserMedia spec accept user media options like a dictionary format navigator.webkitGetUserMedia({audio:true, video:true}, ...) to replace its old style navigator.webkitGetUserMedia("audio, video", ...)

BUG=113673
TEST=content_unittests, media_unittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=134539

[Jeremy Mao, 2012-04-27 09:18:17]

Hi wjia, scherkus, tommi,

Could you please take a look at this?

Thanks & Best regards
Jeremy

[scherkus (not reviewing), 2012-04-27 17:52:40]

lgtm but I'm not an expert on getUserMedia

wjia/tommi: do you want to do a quick sanity check?

http://codereview.chromium.org/10177008/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

http://codereview.chromium.org/10177008/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/media/media_stream_manager.cc:48: (options.video
== true)) {
nit: avoid explicit comparison to "true/false" -- just use the boolean itself in
the comparison

http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
File content/renderer/media/media_stream_dispatcher_unittest.cc (right):

http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
content/renderer/media/media_stream_dispatcher_unittest.cc:93:
media_stream::StreamOptions components(
nit: this can fit on one line

http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
content/renderer/media/media_stream_dispatcher_unittest.cc:256:
media_stream::StreamOptions components(
nit: this can fit on one line

[wjia(left Chromium), 2012-04-27 18:22:01]

This involves API with WebKit. I'd refer to tommyw@ for comments on that front.
More specifically, the call to MediaStreamImpl::requestUserMedia().

If WebKit is ready for the new spec, then we can go ahead with the change.

[Jeremy Mao, 2012-04-27 22:32:10]

On 2012/04/27 17:52:40, scherkus wrote:
> lgtm but I'm not an expert on getUserMedia
> 
> wjia/tommi: do you want to do a quick sanity check?
> 
>
http://codereview.chromium.org/10177008/diff/1/content/browser/renderer_host/...
> File content/browser/renderer_host/media/media_stream_manager.cc (right):
> 
>
http://codereview.chromium.org/10177008/diff/1/content/browser/renderer_host/...
> content/browser/renderer_host/media/media_stream_manager.cc:48: (options.video
> == true)) {
> nit: avoid explicit comparison to "true/false" -- just use the boolean itself
in
> the comparison
fixed
> 
>
http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
> File content/renderer/media/media_stream_dispatcher_unittest.cc (right):
> 
>
http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
> content/renderer/media/media_stream_dispatcher_unittest.cc:93:
> media_stream::StreamOptions components(
> nit: this can fit on one line
fixed
> 
>
http://codereview.chromium.org/10177008/diff/1/content/renderer/media/media_s...
> content/renderer/media/media_stream_dispatcher_unittest.cc:256:
> media_stream::StreamOptions components(
> nit: this can fit on one line
fixed

[Jeremy Mao, 2012-04-27 22:35:14]

On 2012/04/27 18:22:01, wjia wrote:
> This involves API with WebKit. I'd refer to tommyw@ for comments on that
front.
> More specifically, the call to MediaStreamImpl::requestUserMedia().
> 
> If WebKit is ready for the new spec, then we can go ahead with the change.
Yes, let's wait for tommy's comments, I can see tommy is already changing the
WebKit's API. https://bugs.webkit.org/show_bug.cgi?id=84850

[Tommy Widenflycht, 2012-04-30 07:26:08]

lgtm++

[tommi (sloooow) - chröme, 2012-04-30 08:16:56]

lgtm

[commit-bot: I haz the power, 2012-04-30 14:49:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yujie.mao@intel.com/10177008/11002

[commit-bot: I haz the power, 2012-04-30 16:19:25]

Change committed as 134539