Refactor "options" argument to getUserMedia to be a dictionary

Refactor "options" argument to getUserMedia to be a dictionary

This argument was a Dictionary (unconstrained).
Now it is a WebIdl "dictionary" with members "video" and "audio".

Tests still pass.

BUG=522612
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200901

[hta - Chromium, 2015-08-19 19:46:40]

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

[commit-bot: I haz the power, 2015-08-19 19:46:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1302793002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1302793002/1

[commit-bot: I haz the power, 2015-08-19 22:22:36]

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

[commit-bot: I haz the power, 2015-08-19 22:22:37]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hta - Chromium, 2015-08-20 07:59:44]

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

[hta - Chromium, 2015-08-20 08:00:06]

Please take a look. The bot failure looks unrelated.

[tommi (sloooow) - chröme, 2015-08-20 08:05:28]

lgtm! one nit.

https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
File Source/modules/mediastream/UserMediaRequest.cpp (right):

https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
Source/modules/mediastream/UserMediaRequest.cpp:59: if (options.getAsBoolean())
{
} else if (options.getAsBoolean()) {

https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
Source/modules/mediastream/UserMediaRequest.cpp:62: }
} else {
  ASSERT(false);
}

(assuming options will always be boolean or a dictionary)

[hta - Chromium, 2015-08-20 08:20:33]

I think we're OK as it stands....

https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
File Source/modules/mediastream/UserMediaRequest.cpp (right):

https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
Source/modules/mediastream/UserMediaRequest.cpp:62: }
On 2015/08/20 08:05:28, tommi wrote:
> } else {
>   ASSERT(false);
> }
> 
> (assuming options will always be boolean or a dictionary)

options.getAsBoolean actually calls ASSERT(isBoolean()) already.

I find this a little hard to read, because the hidden inner "else" for
getAsBoolean returning false is "leave constraints alone, don't touch it", which
is logical, but not highly visible.

The argument for using nested rather than chained ifs is that the outer if is a
type switch, while the inner if is a switch on the value of getAsBoolean; I
don't like if/else chains that check on completely different properties.

[tommi (sloooow) - chröme, 2015-08-20 08:21:48]

On 2015/08/20 08:20:33, hta - Chromium wrote:
> I think we're OK as it stands....
> 
>
https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
> File Source/modules/mediastream/UserMediaRequest.cpp (right):
> 
>
https://codereview.chromium.org/1302793002/diff/1/Source/modules/mediastream/...
> Source/modules/mediastream/UserMediaRequest.cpp:62: }
> On 2015/08/20 08:05:28, tommi wrote:
> > } else {
> >   ASSERT(false);
> > }
> > 
> > (assuming options will always be boolean or a dictionary)
> 
> options.getAsBoolean actually calls ASSERT(isBoolean()) already.
> 
> I find this a little hard to read, because the hidden inner "else" for
> getAsBoolean returning false is "leave constraints alone, don't touch it",
which
> is logical, but not highly visible.
> 
> The argument for using nested rather than chained ifs is that the outer if is
a
> type switch, while the inner if is a switch on the value of getAsBoolean; I
> don't like if/else chains that check on completely different properties.

sgtm, lgtm++

[hta - Chromium, 2015-08-20 08:23:05]

The CQ bit was checked by hta@chromium.org

[commit-bot: I haz the power, 2015-08-20 08:23:17]

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

[commit-bot: I haz the power, 2015-08-20 11:26:52]

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

[commit-bot: I haz the power, 2015-08-20 11:26:53]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hta - Chromium, 2015-08-20 12:02:04]

The CQ bit was checked by hta@chromium.org

[commit-bot: I haz the power, 2015-08-20 12:02:11]

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

[commit-bot: I haz the power, 2015-08-20 14:22:47]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200901