RTCPeerConnection webidl notes

I took some notes while moving our 
http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCPeerConnection 
implementation to our latest webidl compiler. Here are nine points I 
wanted to share. My apologies if any of these are repeats.

---

#1 - Proposal: I propose we make iceServers a sequence rather than a 
webidl array, because sequence is passed by value (represented in JS by 
an actual JS array), and [] is passed by reference (represented in JS by 
some weird array-like object, according to our webidl guy):

- RTCIceServer[] iceServers;
+ sequence<RTCIceServer> iceServers;

#2 - Proposal: Wherever EventInit is mentioned (like in 
MediaStreamEvent) it would be nice if the draft linked to 
http://dom.spec.whatwg.org/#eventinit instead of trying to re-describe 
what essentially is already the default inherited behavior for bubbles 
and cancelable. Unless I misunderstand, we want the implementer to add 
and subtract nothing to how bubbles and cancelable are dealt with by 
default here. I propose the best way to get this across is not to 
mention it at all.

dictionary EventInit {
boolean bubbles = false;
boolean cancelable = false;
};

#3 - Observation: In the draft, addStream talks about a failure callback 
it doesn't have. It says: "... If the constraints could not be 
successfully applied, provide an RTCError object of type 
INCOMPATIBLE_CONSTRAINTS to the failure callback.”.

It seems to me that we either must do constraint parsing synchronously 
(and throw on errors) or addStream needs success/failure callbacks here.

#4 - Proposal: Similarly, addIceCandidate needs a failure callback to 
handle bad passed-in SDP, which can happen in the field, and callers 
need a success callback to learn that the SDP did not fail. I'm assuming 
we've decided we can't parse SDP synchronously, so I propose we add:

void addIceCandidate (mozRTCIceCandidate candidate,
+ optional VoidFunction successCallback,
+ optional RTCPeerConnectionErrorCallback failureCallback);

#5 - Not a spec bug, but apprtc currently passes in null 
failureCallbacks to createOffer/Answer. Can someone with commit access 
bring it in line with spec, so we can remove workarounds?

void createOffer (RTCSessionDescriptionCallback successCallback,
RTCPeerConnectionErrorCallback? failureCallback, // for apprtc
optional object? constraints);
void createAnswer (RTCSessionDescriptionCallback successCallback,
RTCPeerConnectionErrorCallback? failureCallback, // for apprtc
optional object? constraints);

#6 - Proposal: I notice local/remoteDescription are not marked as 
nullable ('?') even though "A null object will be returned if the 
local/remote description has not yet been set." I think the wording 
makes sense and the webidl is wrong, so I propose this "null means 
unset" change:

- readonly attribute RTCSessionDescription localDescription;
- readonly attribute RTCSessionDescription remoteDescription;
+ readonly attribute RTCSessionDescription? localDescription;
+ readonly attribute RTCSessionDescription? remoteDescription;

#7 - In light of #6 ("null means unset"), allowing independently 
nullable type and sdp seems superfluous, so I was about to propose we 
make type and sdp non-nullable, as I assumed having people pass in null 
is undesirable. However, I then noticed that the 
constructor-init-dictionary itself is optional, and wonder what purpose 
does that serve? Seems like an undesirable variant. I wanted to propose:

-[Constructor (optional RTCSessionDescriptionInit descriptionInitDict)]
+[Constructor (RTCSessionDescriptionInit descriptionInitDict)]
interface RTCSessionDescription {
- attribute RTCSdpType? type;
- attribute DOMString? sdp;
+ attribute RTCSdpType type;
+ attribute DOMString sdp;

and have it throw if type is not specified (see next point).

#8 - It is hard to describe invariants with *Init dictionaries as 
constructor args, since dictionary keys are inherently optional. In 
other words, it is not clear from the webidl alone which 
constructor-init-dictionary-keys are required to avoid throwing and 
which are not.

For example:

dictionary RTCPeerConnectionIceEventInit : EventInit {
RTCIceCandidate candidate;
};

[Constructor(DOMString type, RTCPeerConnectionIceEventInit eventInitDict)]
interface RTCPeerConnectionIceEvent : Event {
readonly attribute RTCIceCandidate candidate;
};

Should new RTCPeerConnectionIceEventInit("icecandidate", { }); throw?

- If it should, there is nothing that states so (meaning: webidl 
wrappers wont throw for us), and we should document the need to throw.

- If it shouldn't, then what should the value of the candidate attribute 
be after creation? Null is not valid here, so maybe undefined or maybe 
an empty RTCIceCandidate object (which can be created from zero 
arguments)? There is no way to know.

#9 Proposal: For the latter case in #8 (optional 
constructor-init-dictionary-keys), once answers are found, I propose a 
convention where we specify default values in the dictionary whenever 
possible. This is descriptive, as readers learn both that the argument 
is optional and what value they'll get when omitting the key; The 
implementation benefits as well by gaining an invariant (see below); and 
we benefit by being forced to consider what the default value should be. 
For example:

webidl:
dictionary RTCIceCandidateInit {
- DOMString candidate;
- DOMString sdpMid;
+ DOMString? candidate = null;
+ DOMString? sdpMid = null;
unsigned short sdpMLineIndex;
};
[Constructor (optional RTCIceCandidateInit candidateInitDict)]
interface RTCIceCandidate {
attribute DOMString? candidate;
attribute DOMString? sdpMid;
attribute unsigned short? sdpMLineIndex;

Implementation effect:
RTCIceCandidate: function(dict) {
- this.candidate = ("candidate" in dict)? dict.candidate : null;
- this.sdpMid = ("sdpMid" in dict)? dict.sdpMid : null;
+ this.candidate = dict.candidate;
+ this.sdpMid = dict.sdpMid;
this.sdpMLineIndex = ("sdpMLineIndex" in dict)? dict.sdpMLineIndex+1 : null;
}

In general, such default values should be addable without making keys 
nullable where they weren't before (defaults can be any constant value). 
However, in this particular case, with the attributes themselves being 
defined as nullable already (why?), my best guess (since it is not 
specified) is that the intent here is that they also default to null, in 
which case a caller can already null them implicitly by omitting the 
corresponding keys, so this doesn't really expand the complexity even 
though it may appear to.

That's what I have. Thoughts?

.: Jan-Ivar :.

Received on Tuesday, 14 May 2013 01:40:33 UTC