MediaStream API: Update getUserMedia to match the spec

MediaStream API: Update getUserMedia to match the spec
Both the success and the fail callbacks are mandatory and may not be null.
Also taking the opportunity to clean up the code and add proper
exceptions where needed as promised.

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

[Tommy Widenflycht, 2014-02-21 10:14:10]

As promised I cleaned up getUserMedias use of exceptions :)

[Mike West, 2014-02-21 14:59:11]

LGTM, so long as the bots are happy (they're mostly red at the moment; I've
triggered new runs).

[sof, 2014-02-21 15:15:58]

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

https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
Source/modules/mediastream/UserMediaRequest.cpp:71:
exceptionState.throwDOMException(TypeMismatchError,
ExceptionMessages::argumentNullOrIncorrectType(2,
"NavigatorUserMediaSuccessCallback"));
This isn't correct, I think - shouldn't it be TypeError?

[Tommy Widenflycht, 2014-02-21 15:40:57]

The CQ bit was checked by tommyw@chromium.org

[Tommy Widenflycht, 2014-02-21 15:40:59]

The CQ bit was unchecked by tommyw@chromium.org

[Tommy Widenflycht, 2014-02-21 15:43:57]

On 2014/02/21 15:15:58, sof wrote:
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> File Source/modules/mediastream/UserMediaRequest.cpp (right):
> 
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> Source/modules/mediastream/UserMediaRequest.cpp:71:
> exceptionState.throwDOMException(TypeMismatchError,
> ExceptionMessages::argumentNullOrIncorrectType(2,
> "NavigatorUserMediaSuccessCallback"));
> This isn't correct, I think - shouldn't it be TypeError?


It's a bit confusing:

TypeMismatchError, // Historical; use TypeError instead

vs

// WebIDL exception types, handled by the binding layer.
// FIXME: Add GeneralError, EvalError, etc. when implemented in the bindings.
TypeError,

[sof, 2014-02-21 15:48:56]

On 2014/02/21 15:43:57, Tommy Widenflycht wrote:
> On 2014/02/21 15:15:58, sof wrote:
> >
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> > File Source/modules/mediastream/UserMediaRequest.cpp (right):
> > 
> >
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> > Source/modules/mediastream/UserMediaRequest.cpp:71:
> > exceptionState.throwDOMException(TypeMismatchError,
> > ExceptionMessages::argumentNullOrIncorrectType(2,
> > "NavigatorUserMediaSuccessCallback"));
> > This isn't correct, I think - shouldn't it be TypeError?
> 
> 
> It's a bit confusing:
> 
> TypeMismatchError, // Historical; use TypeError instead
> 
> vs
> 
> // WebIDL exception types, handled by the binding layer.
> // FIXME: Add GeneralError, EvalError, etc. when implemented in the bindings.
> TypeError,

A lot of details to navigate in this area, but
http://heycam.github.io/webidl/#es-callback-function says that TypeError should
be thrown for ill-formed callback values.

[Tommy Widenflycht, 2014-02-21 16:01:02]

On 2014/02/21 15:48:56, sof wrote:
> On 2014/02/21 15:43:57, Tommy Widenflycht wrote:
> > On 2014/02/21 15:15:58, sof wrote:
> > >
> >
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> > > File Source/modules/mediastream/UserMediaRequest.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/173893009/diff/1/Source/modules/mediastream/U...
> > > Source/modules/mediastream/UserMediaRequest.cpp:71:
> > > exceptionState.throwDOMException(TypeMismatchError,
> > > ExceptionMessages::argumentNullOrIncorrectType(2,
> > > "NavigatorUserMediaSuccessCallback"));
> > > This isn't correct, I think - shouldn't it be TypeError?
> > 
> > 
> > It's a bit confusing:
> > 
> > TypeMismatchError, // Historical; use TypeError instead
> > 
> > vs
> > 
> > // WebIDL exception types, handled by the binding layer.
> > // FIXME: Add GeneralError, EvalError, etc. when implemented in the
bindings.
> > TypeError,
> 
> A lot of details to navigate in this area, but
> http://heycam.github.io/webidl/#es-callback-function says that TypeError
should
> be thrown for ill-formed callback values.

Removed the null checks since the binding handle that already (and with correct
type)

[sof, 2014-02-21 20:44:26]

On 2014/02/21 16:01:02, Tommy Widenflycht wrote:
..
> 
> Removed the null checks since the binding handle that already (and with
correct
> type)

And that's the only way it is called. lgtm.

[Tommy Widenflycht, 2014-02-24 07:36:12]

The CQ bit was checked by tommyw@chromium.org

[commit-bot: I haz the power, 2014-02-24 07:36:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommyw@chromium.org/173893009/180001

[Mike West, 2014-02-24 07:50:49]

still LGTM, FWIW. :)

[jochen (gone - plz use gerrit), 2014-02-24 11:23:39]

lgtm

[commit-bot: I haz the power, 2014-02-24 12:54:13]

Change committed as 167685