From 5928e35abf3ad1f8f87fab21c5de3c426a4be800 Mon Sep 17 00:00:00 2001
From: Florent Castelli <orphis@webrtc.org>
Date: Mon, 25 Mar 2024 16:01:12 +0000
Subject: [PATCH] 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 | 51 ++++++++++++++++++++++++++++++
 pc/sctp_data_channel.cc            |  6 ++++
 2 files changed, 57 insertions(+)

diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc
index a31481d634..9afda05090 100644
--- a/pc/data_channel_integrationtest.cc
+++ b/pc/data_channel_integrationtest.cc
@@ -253,6 +253,57 @@ TEST_P(DataChannelIntegrationTest,
     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
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index 6956cffa75..afc9488e63 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -663,6 +663,12 @@ void SctpDataChannel::OnTransportChannelClosed(RTCError error) {
 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 {