commit 5928e35abf3ad1f8f87fab21c5de3c426a4be800
author Florent Castelli <orphis@webrtc.org> Mon Mar 25 16:01:12 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Mar 25 18:36:09 2024
tree dc1fa6805395bf47cc56ad6a000d44d450e98d96
parent ff7a557f2e0712254f09bd8c18ece04074ca0b56

pc: Close the data channel association after sending messages in closing state

After we're done sending all the messages, if the channel was in closing
state, then we start closing the association at the SCTP level, which
allows transitioning to the closed state.

Bug: chromium:40072842
Change-Id: I81b26b4137593b8feeb4bd9a2563cdfd67e1049e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344421
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Auto-Submit: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41962}

pc/data_channel_integrationtest.cc

diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc
index a31481d..9afda05 100644
--- a/pc/data_channel_integrationtest.cc
+++ b/pc/data_channel_integrationtest.cc
@@ -253,6 +253,57 @@
     EXPECT_EQ_WAIT(data, caller()->data_observer()->last_message(),
                    kDefaultTimeout);
   }
+  caller()->data_channel()->Close();
+
+  EXPECT_EQ_WAIT(caller()->data_observer()->state(),
+                 webrtc::DataChannelInterface::kClosed, kDefaultTimeout);
+  EXPECT_EQ_WAIT(callee()->data_observer()->state(),
+                 webrtc::DataChannelInterface::kClosed, kDefaultTimeout);
+}
+
+// This test sets up a call between two parties with an SCTP
+// data channel only, and sends enough messages to fill the queue and then
+// closes on the caller. We expect the state to transition to closed on both
+// caller and callee.
+TEST_P(DataChannelIntegrationTest, EndToEndCallWithSctpDataChannelFullBuffer) {
+  ASSERT_TRUE(CreatePeerConnectionWrappers());
+  ConnectFakeSignaling();
+  // Expect that data channel created on caller side will show up for callee as
+  // well.
+  caller()->CreateDataChannel();
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  // Caller data channel should already exist (it created one). Callee data
+  // channel may not exist yet, since negotiation happens in-band, not in SDP.
+  ASSERT_NE(nullptr, caller()->data_channel());
+  ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout);
+  EXPECT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout);
+  EXPECT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout);
+
+  std::string data(256 * 1024, 'a');
+  for (size_t queued_size = 0;
+       queued_size < webrtc::DataChannelInterface::MaxSendQueueSize();
+       queued_size += data.size()) {
+    caller()->data_channel()->SendAsync(DataBuffer(data), nullptr);
+  }
+
+  caller()->data_channel()->Close();
+
+  DataChannelInterface::DataState expected_states[] = {
+      DataChannelInterface::DataState::kConnecting,
+      DataChannelInterface::DataState::kOpen,
+      DataChannelInterface::DataState::kClosing,
+      DataChannelInterface::DataState::kClosed};
+
+  EXPECT_EQ_WAIT(DataChannelInterface::DataState::kClosed,
+                 caller()->data_observer()->state(), kDefaultTimeout);
+  EXPECT_THAT(caller()->data_observer()->states(),
+              ::testing::ElementsAreArray(expected_states));
+
+  EXPECT_EQ_WAIT(DataChannelInterface::DataState::kClosed,
+                 callee()->data_observer()->state(), kDefaultTimeout);
+  EXPECT_THAT(callee()->data_observer()->states(),
+              ::testing::ElementsAreArray(expected_states));
 }
 
 // This test sets up a call between two parties with an SCTP

pc/sctp_data_channel.cc

diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index 6956cff..afc9488 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -663,6 +663,12 @@
 void SctpDataChannel::OnBufferedAmountLow() {
   RTC_DCHECK_RUN_ON(network_thread_);
   MaybeSendOnBufferedAmountChanged();
+
+  if (state_ == DataChannelInterface::kClosing && !started_closing_procedure_ &&
+      id_n_.has_value() && buffered_amount() == 0) {
+    started_closing_procedure_ = true;
+    controller_->RemoveSctpDataStream(*id_n_);
+  }
 }
 
 DataChannelStats SctpDataChannel::GetStats() const {