Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RTCSctpTransportState #1620

Merged
merged 9 commits into from
Oct 12, 2017
Merged

Add RTCSctpTransportState #1620

merged 9 commits into from
Oct 12, 2017

Conversation

aboba
Copy link
Contributor

@aboba aboba commented Oct 5, 2017

Fix for Issue #1612


Preview | Diff

@aboba aboba changed the title pc.close() should close Data Channels and SctpTransports Add RTCSctpTransportState Oct 5, 2017
@aboba aboba requested a review from taylor-b October 5, 2017 18:49
transport.</p>
<div>
<pre class="idl">enum RTCSctpTransportState {
"new",
Copy link
Contributor

@taylor-b taylor-b Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need "new", since we decided the RTCSctpTransport is only created on applying an answer, in which case it already has enough information to connect? Though maybe we should revisit that decision.

Copy link
Contributor Author

@aboba aboba Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be an advantage to having the RTCSctpTransport created earlier, in which case the "new" state might be used. But I agree that if it is only created on applying an Answer it should be initialized to "connecting".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opening another issue relating to the transition text.

webrtc.html Outdated
"idlAttrType"><a>RTCSctpTransportState</a></span>, readonly</dt>
<dd>
<p>The current state of the SCTP transport. The attribute returns
the value of the <dfn>[[\SctpTransportState]]</dfn> internal slot.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comment on the PR that added [[DtlsTransportState]]: we need to either describe when the slot's value is changed, or not use a slot.

In this case it's probably pretty simple:

  1. When it's first created, the value is connecting? (See below comment)
  2. When the four-way startup handshake finishes, queue a task to update the state to connected
  3. When a SHUTDOWN or ABORT chunk is received, queue a task to update the state to closed
  4. Also can go to closed from applying a description that rejects the data m= section, or changes the sctp-port. Or if the application closes the PeerConnection,

@aboba aboba merged commit 3f3f496 into master Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants