Implement proper SCTP data channel closing procedure.
The proper closing procedure is:
1. Alice resets outgoing stream.
2. Bob receives incoming stream reset, resets his outgoing stream.
3. Alice receives incoming stream reset; channel closed!
4. Bob receives acknowledgement of reset; channel closed!
https://tools.ietf.org/html/draft-ietf-rtcweb-data-channel-13#section-6.7
However, up until now we've been sending both an incoming and outgoing reset
from the side initiating the closing procedure, and doing nothing on the remote
side.
This means that if you call "Close" and the remote endpoint is using an old
version of WebRTC, the channel's state will be stuck at "closing" since the
remote endpoint won't send a reset. Which is already what happens when Firefox
is talking to Chrome.
This CL also fixes an issue where the DataChannel's state prematurely went to
"closed" before the closing procedure was complete. Which could result in a
new DataChannel attempting to re-use the ID and failing.
TBR=magjed@webrtc.org
Bug: chromium:449934, webrtc:4453
Change-Id: Ic1ba813e46538c6c65868961aae6a9780d68a5e2
Reviewed-on: https://webrtc-review.googlesource.com/79061
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23478}diff --git a/pc/datachannel_unittest.cc b/pc/datachannel_unittest.cc
index f8e9880..4a713cb 100644
--- a/pc/datachannel_unittest.cc
+++ b/pc/datachannel_unittest.cc
@@ -66,6 +66,9 @@
size_t on_buffered_amount_change_count_;
};
+// TODO(deadbeef): The fact that these tests use a fake provider makes them not
+// too valuable. Should rewrite using the
+// peerconnection_datachannel_unittest.cc infrastructure.
class SctpDataChannelTest : public testing::Test {
protected:
SctpDataChannelTest()
@@ -560,29 +563,6 @@
webrtc_data_channel_->state());
}
-// Tests that a already closed DataChannel does not fire onStateChange again.
-TEST_F(SctpDataChannelTest, ClosedDataChannelDoesNotFireOnStateChange) {
- AddObserver();
- webrtc_data_channel_->Close();
- // OnStateChange called for kClosing and kClosed.
- EXPECT_EQ(2U, observer_->on_state_change_count());
-
- observer_->ResetOnStateChangeCount();
- webrtc_data_channel_->RemotePeerRequestClose();
- EXPECT_EQ(0U, observer_->on_state_change_count());
-}
-
-// Tests that RemotePeerRequestClose closes the local DataChannel.
-TEST_F(SctpDataChannelTest, RemotePeerRequestClose) {
- AddObserver();
- webrtc_data_channel_->RemotePeerRequestClose();
-
- // OnStateChange called for kClosing and kClosed.
- EXPECT_EQ(2U, observer_->on_state_change_count());
- EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
- webrtc_data_channel_->state());
-}
-
// Tests that the DataChannel is closed if the received buffer is full.
TEST_F(SctpDataChannelTest, ClosedWhenReceivedBufferFull) {
SetChannelReady();
