Re: RTCPeerConnection webidl notes

Also, there's no way currently on the spec to get a list of the
DataChannels that habe been created on this PeerConnectiom, in the same way
there's a getLocalStreams() method or an iceServers attribute...
El 14/05/2013 03:42, "Jan-Ivar Bruaroey" <jib@mozilla.com> escribió:

> I took some notes while moving our http://dev.w3.org/2011/webrtc/**
> editor/webrtc.html#idl-def-**RTCPeerConnection<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 <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 :.
>
>
>