Created attachment 642256[details]
Misspelled (on purpose) call to mozGetUserMedia
Steps:
Sample code used is attached. Important part of it is below:
navigator.mozGetUserMedia({Video: true}, gotStream, noStream);
I purposely capitalized incorrectly to Video, instead of video, to see if I get an error callback (or something in the error console) indicating that the 1st parameter was incorrect.
Expected:
I'd expect an error callback to made to include an error indicating that the 1st parameter is incorrect. An error code is also sufficient.
Actual:
No error callback is made. Nothing shows up in the error console.

{} is valid input, both audio and video default to false, which means no media is requested.
{Video:true} is not valid input however, and we should pass a NOT_SUPPORTED_ERR to the errorCallback.
BTW you should be looking at http://dev.w3.org/2011/webrtc/editor/getusermedia.html#navigatorusermedia, not spec for the MediaStream constructor - that's a completely different thing.

Created attachment 656429[details][diff][review]
Patchv0
Sorry about the delay.
I havn't changed nsIDOMNavigatorUserMedia.idl and MediaManager.h
Could you tell me if this is the right way to solve the issue in MediaManager.cpp?

Comment on attachment 656429[details][diff][review]
Patchv0
Ok, there are a few things I want to point out in this patch that should be done differently.
* You'll want to add the [implicit_jscontext] annotation to the IDL definition at http://mxr.mozilla.org/mozilla-central/source/dom/media/nsIDOMNavigatorUserMedia.idl#36; this will make the mozGetUserMedia function take a JSContext* argument which you can pass to GetUserMedia and avoid the whole ScriptGlobalObject dance you're doing.
* The patch doesn't correctly deal with turning the strings into ids right now. It also won't build :) You should use the code at http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLCanvasElement.cpp#501 as your model: you want to have an array of the properties of the object you're examining (eg. you'll have an array of ["foo", "bar", "video"]), and check whether each element in that array is present in your array of expected names. You'll want to get an nsDependentJSString of the property name, and you can compare that to your nsStrings.
Once you make those changes, I think we'll be in a better position to comment on any remaining issues.

I assume to leverage WebIDL here, we'd have to move it to WebIDL from xpidl?
Also please define 'handled'. If "handled" means "ignored", well then it already does that. :-) And depending on the spec (dictionary vs object, etc), that might be correct. Anant?

Created attachment 658187[details][diff][review]
Patchv1
When this patch is applied Nightly crashes on running the attached test case.
Could you guys tell me why that happens and help me solve this problem?

We want the first parameter to be a jsval, not an object or dictionary. The reason is that the values can be any track type, not just audio, video or camera. That's the reason for enumerating the properties, but we should be enumerating a jsval instead of nsIMediaStreamOptions.
nsIMediaStreamOptions is a hack I used to get something quick up and running, let's get rid of it :)

(In reply to Anant Narayanan [:anant] from comment #11)
> We want the first parameter to be a jsval, not an object or dictionary. The
> reason is that the values can be any track type, not just audio, video or
> camera. That's the reason for enumerating the properties, but we should be
> enumerating a jsval instead of nsIMediaStreamOptions.
Don't understand a word of what you're saying here.

navigator.mozGetUserMedia({foo:true, bar:true}); is valid input. We need to enumerate the JS object first and then recognize the "track" types we support, where each object property is a track type. Currently, we only support "video", "audio" and "camera", but we may add some later.
The algorithm in the spec requires us to enumerate each property of the first parameter and invoke the error callback once we encounter a track type that we don't support.
In terms of what that means for this patch, nsIMediaStreamOptions should be removed from the IDL. I'm also arguing that enumerating the properties of the jsval is desired behaviour.

(In reply to Anant Narayanan [:anant] from comment #13)
> navigator.mozGetUserMedia({foo:true, bar:true}); is valid input. We need to
> enumerate the JS object first and then recognize the "track" types we
> support, where each object property is a track type. Currently, we only
> support "video", "audio" and "camera", but we may add some later.
>
> The algorithm in the spec requires us to enumerate each property of the
> first parameter and invoke the error callback once we encounter a track type
> that we don't support.
>
> In terms of what that means for this patch, nsIMediaStreamOptions should be
> removed from the IDL. I'm also arguing that enumerating the properties of
> the jsval is desired behaviour.
Then the algorithm in the spec is wrong. This was all discussed to death on public-script-coord, but apparently the people in webrtc prefer sitting in their own silo and ignoring what happens elsewhere in the platform.

The goal is to be flexible enough that we can allow new track types without having to change the spec. Could you link to the relevant discussion (or give me a rough time-frame so I can search) on public-script-coord? The WebRTC spec is far from done, we can definitely still make changes.

The summary here is very vague! Lets get it updated so it can be found by querying Bugzilla. I have already attached a mochitest update for a possible fix on bug 815582, but I'm not sure about the implementation details given the non-finished discussion on this bug. Once it's clear I can attach an updated fix for the test, and hope we can get it fixed in the code soon.
(In reply to :Ms2ger from comment #14)
> Then the algorithm in the spec is wrong. This was all discussed to death on
> public-script-coord, but apparently the people in webrtc prefer sitting in
> their own silo and ignoring what happens elsewhere in the platform.
It would be great to know how we have to handle that. Ms2ger, mind giving us the requested information? Thanks.

Flags: needinfo?(Ms2ger)

Summary: Incorrect 1st parameter to mozGetUserMedia results in no error callback, nothing in the Error Console either → Unknown media type in 1st parameter of mozGetUserMedia() call should throw a NS_ERROR_DOM_NOT_SUPPORTED_ERR

Most dictionary consumers expect that extra properties would be ignored as WebIDL currently spec'ed. The semantic of MediaStreamOptions contradicts their expectations.
Maybe a new extended attribute need to be added to the WebIDL spec to satisfy WebRTC's demand. Until then, it would be fine to operate on jsval directly.

(In reply to Henrik Skupin (:whimboo) from comment #19)
It doesn't look like that :infinity is still working on this bug. Putting it back into the pool.
> (In reply to :Ms2ger from comment #14)
> > Then the algorithm in the spec is wrong. This was all discussed to death on
> > public-script-coord, but apparently the people in webrtc prefer sitting in
> > their own silo and ignoring what happens elsewhere in the platform.
>
> It would be great to know how we have to handle that. Ms2ger, mind giving us
> the requested information? Thanks.
Ms2ger, would you mind to give us the requested information?