Re: WebRTC spec errata (regarding states)

Thanks for making these changes. A few comments inline:


On Wed, Mar 6, 2013 at 10:04 AM, Adam Bergkvist <adam.bergkvist@ericsson.com
> wrote:

> Hi
>
> I've incorporated these changes into the "work in progress"-spec on github.
>
> https://github.com/fluffy/**webrtc-w3c/commit/**
> 700919bc2f35c86b5a72a1677efb50**ae0366357d<https://github.com/fluffy/webrtc-w3c/commit/700919bc2f35c86b5a72a1677efb50ae0366357d>
>
> Please review.
>
> /Adam
>
>
> On 2013-02-21 00:44, Justin Uberti wrote:
>
>> Some questions came up the other day regarding the current spec language
>> regarding the various state enums and callbacks. Looking it over, I see
>> that not all of what was agreed upon made it into the spec. Below is a
>> set of minor issues I noticed while reviewing.
>>
>> References:
>> - State proposal:
>> https://docs.google.com/**document/d/**13TYiNSEmFkB7IeNLEJFxI0xMNk8q_**
>> LhXE_hbvFbXRTU/edit<https://docs.google.com/document/d/13TYiNSEmFkB7IeNLEJFxI0xMNk8q_LhXE_hbvFbXRTU/edit>
>> - Dec teleconf minutes: http://www.w3.org/2012/12/13-**
>> webrtc-minutes.html <http://www.w3.org/2012/12/13-webrtc-minutes.html>
>>
>>  1. RTCSignalingState
>>      1. 4.3.1, list 3, bullet 4:
>>
>>         The transition to "stable" state, or any other signaling state,
>>         only occurs via calls to setLocalDescription or
>>         setRemoteDescription. It is not connected with the ICE state
>>         transitions.
>>
>
> Removed bullet.
>
>       2. 4.3.2.1: onopen
>>
>>         We agreed to remove this callback, as it is no longer
>>         meaningful, and redundant with onsignalingstatechange.
>>
>
> Removed.
>
>       3. 4.3.2.1: onstatechange
>>
>>         The correct name for this callback is onsignalingstatechange.
>>
>
> Updated.
>
>       4. 4.3.2.2: setLocalDescription
>>
>>         The description for this function should indicate that it
>>         triggers a signaling state change. It should also indicate that
>>         an INVALID_STATE exception is triggered if the wrong
>>         SessionDescription.type is supplied for the current state.
>>      5. 4.3.2.2: setRemoteDescription
>>
>>         The description for this function should indicate that it
>>         triggers a signaling state change. It should also indicate that
>>         an INVALID_STATE exception is triggered if the wrong
>>         SessionDescription.type is supplied for the current state.
>>
>
> Updated with the modifications we talked about in a prior reply to this
> mail. I've tried to address some stuff that was unspecified around these
> functions. We have a bit of redundancy with both a callback and an event
> when a description is applied successfully. I've picked an order for these
> which is event first, and callback second. Please review.
>

That ordering sounds good to me. I would expect that when I got the success
callback, I should be able to call other functions, so all states should
already be up-to-date.

I do think we need to have separately-specified processing for local and
remote descriptions. For example, the local description may influence the
gathering process, whereas the remote description does not.

>
>   2. RTCIceConnectionState
>>      1. 4.3.2: onicechange
>>
>>         The correct name for this callback is oniceconnectionstatechange.
>>
>
> Updated.
>
>       2. 4.4.3: RTCIceState
>>
>>         The enum name is incorrect; it should be RTCIceConnectionState.
>>         Also, the initial state should be "new", like
>>         RTCIceGatheringState, not "starting".
>>
>
> Updated.
>
>   3. RTCIceGatheringState
>>      1. 4.3.2.1: ongatheringchange
>>
>>         We decided that this callback is not needed; the necessary
>>         information can be gotten from onicecandidate.
>>
>
> Updated.
>
>       2. 4.3.2.1: onicecandidate
>>
>>         The description for this callback should indicate that it is
>>         invoked with a null argument when gathering completes. Prior to
>>         dispatching this null callback, the iceGatheringState will be
>>         set to "completed".
>>
>
> I've seen that we have descriptions on event handler attributes in the
> form "this callback should be invoked when...". There are other ways to
> listen for events so the attribute handler shouldn't be called for any
> special reason other then the event being fired. Anyways, to address this
> particular problem, I've added a paragraph about how the ICE Agent notifies
> the script about the candidate gathering process (new candidate or
> gathering done). Please review. Perhaps the ICE Agent should have it's own
> sub-section in the "4.3.1 Operation" section where we collect this kind of
> stuff.
>

I didn't quite follow what you were saying here. I do think we need some
way to specify when the gathering state transition to "completed" occurs.

>
>       3. 4.3.2.2: setLocalDescription
>>
>>         The description for this function should indicate that it causes
>>         a transition of the gathering state from "new", to "gathering",
>>         if that is the current state, and the local description contains
>>         at least one m= line. Also, if a local description is supplied
>>         that contains a different set of ICE credentials, this will
>>         trigger an ICE restart, and change the ICE gathering state to
>>         "gathering".
>>
>
> Updated.
>
>