Re: some comments on the IDL in the WebRTC spec

Cameron,

Thanks for the comments! Most of them have been incorporated into the 
latest ED.

Regards,
-Anant

On 06/20/2012 10:22 PM, Cameron McCormack wrote:
> Hi,
>
> Below are some comments on the IDL currently in the spec after a brief
> look.  I haven't reviewed any prose.
>
>
> 4.1.1 SdpType
> 4.1.3 SessionDescriptionCallback
> 4.1.4 PeerConnectionErrorCallback
> 4.1.5 PeerState Enum
> 4.1.6 IceState Enum
> 4.1.7 IceCandidate Type
> 4.1.8 IceCandidateCallback
>
> The definitions in these sections are all missing semicolons at the end.
>
>
> 4.1.2 SessionDescription Class
>
> If you want, you can just write "stringifier;" rather than "stringifier
> DOMString ();", since they are equivalent.
>
> Why do you want these to be stringifiable, by the way?
>
>
> 4.1.9 IceServers Type - Option 1
>
> The attribute needs to be written as:
>
>    attribute DOMString[][] servers;
>
> Similarly for 4.1.10 (square brackets as part of the type).
>
>
> 4.1.11 PeerConnection interface
>
> s/Boolean/boolean/g in the IDL block.
>
> If you specify default values for optional arguments, then all preceding
> optional arguments must also have default values.  So I would either
> rewrite createAnswer to:
>
>    void createAnswer
>      (SessionDescription offer,
>       SessionDescriptionCallback successCallback,
>       optional PeerConnectionErrorCallback? failureCallback = null,
>       optional MediaConstraints? constraints = null,
>       optional boolean createProvisionalAnswer = false);
>
> or
>
>    void createAnswer
>      (SessionDescription offer,
>       SessionDescriptionCallback successCallback,
>       optional PeerConnectionErrorCallback failureCallback,
>       optional MediaConstraints constraints,
>       optional boolean createProvisionalAnswer);
>
> and just describe the behaviour when createProvisionalAnswer is not
> specified in prose.  Similarly for updateIce.
>
> You can't use [TreatNullAs=EmptyString] on a type that already accepts
> null, like createDataChannel's label argument does.  Either remove the
> "?" from the type or drop the [TreatNullAs=EmptyString] and describe in
> prose what happens when you pass null.
>
> Please put [TreatNonCallableAsNull] on all of the event listener
> attributes, like you do for DataChannel.  We want the behaviour of all
> event listener attributes across the platform to be consistent.  (There
> should hopefully be a simpler definition to reference at some point in
> the future that won't look as awkward as "[TreatNonCallableAsNull]
> attribute Function?".)
>
>
> 8.1 DataChannel
>
> I recommend using enums instead of integer constants.  I see there is a
> PeerState enum defined earlier in the document -- can you use this instead?
>
> It might be good to support all the same kinds of types being passed to
> send() that XHR allows.
>
>
> 10.3 MediaStreamTrackEvent
>
> The "readonly" should be removed from the dictionary member.
>
> It seems like the attribute on MediaStreamTrackEvent should be of type
> "MediaStreamTrack?" rather than "MediaStreamTrack", if you can use null
> in the event initialisation dictionary.  (Similarly for MediaStreamEvent
> in the following section.)
>
>
>

Received on Thursday, 16 August 2012 18:23:26 UTC