From f21354ce0efaa51bb60b0a98b63f2584852cbfc2 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Tue, 7 Mar 2023 08:43:24 +0100
Subject: [PATCH] Transition data channnel to kClosed when not connected to a
 transport.

If a data channel object was closed (via calling Close()) right after
construction and before attaching to a transport, it would never
transition to the `kClosed` state. This addresses that corner case,
which caused a DCHECK to trigger but might also cause a situation
whereby more than one DC instance existed for a given sctp sid.

Bug: chromium:1421534
Change-Id: Id757c0528f929f2e2daa5343236d7f62e309f6cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296341
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39513}
---
 pc/data_channel_unittest.cc |  9 +++++++++
 pc/sctp_data_channel.cc     | 29 ++++++++++++++++++-----------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc
index 80997161cf..d9575f0312 100644
--- a/pc/data_channel_unittest.cc
+++ b/pc/data_channel_unittest.cc
@@ -618,6 +618,15 @@ TEST_F(SctpDataChannelTest, NeverOpened) {
   webrtc_data_channel_->Close();
 }
 
+// Tests that a data channel that's not connected to a transport can transition
+// directly to the `kClosed` state when closed.
+// See also chromium:1421534.
+TEST_F(SctpDataChannelTest, UnusedTransitionsDirectlyToClosed) {
+  webrtc_data_channel_->Close();
+  EXPECT_EQ(webrtc::DataChannelInterface::kClosed,
+            webrtc_data_channel_->state());
+}
+
 // Test that the data channel goes to the "closed" state (and doesn't crash)
 // when its transport goes away, even while data is buffered.
 TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index cb7c1c31d9..d41baafc9d 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -539,18 +539,25 @@ void SctpDataChannel::UpdateState() {
       break;
     }
     case kClosing: {
-      // Wait for all queued data to be sent before beginning the closing
-      // procedure.
-      if (queued_send_data_.Empty() && queued_control_data_.Empty()) {
-        // For SCTP data channels, we need to wait for the closing procedure
-        // to complete; after calling RemoveSctpDataStream,
-        // OnClosingProcedureComplete will end up called asynchronously
-        // afterwards.
-        if (connected_to_transport_ && !started_closing_procedure_ &&
-            controller_ && config_.id >= 0) {
-          started_closing_procedure_ = true;
-          controller_->RemoveSctpDataStream(config_.id);
+      if (connected_to_transport_) {
+        // Wait for all queued data to be sent before beginning the closing
+        // procedure.
+        if (queued_send_data_.Empty() && queued_control_data_.Empty()) {
+          // For SCTP data channels, we need to wait for the closing procedure
+          // to complete; after calling RemoveSctpDataStream,
+          // OnClosingProcedureComplete will end up called asynchronously
+          // afterwards.
+          if (!started_closing_procedure_ && controller_ && config_.id >= 0) {
+            started_closing_procedure_ = true;
+            controller_->RemoveSctpDataStream(config_.id);
+          }
         }
+      } else {
+        // When we're not connected to a transport, we'll transition
+        // directly to the `kClosed` state from here.
+        queued_send_data_.Clear();
+        queued_control_data_.Clear();
+        SetState(kClosed);
       }
       break;
     }