From e08f9a94fa5f2435fff92034ad5b82dc786d525f Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Tue, 28 Mar 2023 10:09:40 +0200
Subject: [PATCH] Remove SctpDataChannel::writable_

This flag isn't needed for sctp data channels.

Bug: none
Change-Id: I07b8ba2c5186729b8a5edb4d2bba7b800335ab5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299074
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39701}
---
 pc/sctp_data_channel.cc | 18 ++++--------------
 pc/sctp_data_channel.h  |  6 +++---
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index 2280174baa..caafa9d8ba 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -117,10 +117,6 @@ bool InternalDataChannelInit::IsValid() const {
   return true;
 }
 
-SctpSidAllocator::SctpSidAllocator() {
-  sequence_checker_.Detach();
-}
-
 StreamId SctpSidAllocator::AllocateSid(rtc::SSLRole role) {
   RTC_DCHECK_RUN_ON(&sequence_checker_);
   int potential_sid = (role == rtc::SSL_CLIENT) ? 0 : 1;
@@ -389,9 +385,7 @@ void SctpDataChannel::OnTransportChannelCreated() {
 
   connected_to_transport_ = true;
 
-  // The sid may have been unassigned when controller_->ConnectDataChannel was
-  // done. So always add the streams even if connected_to_transport_ is true.
-  if (id_.HasValue() && connected_to_transport_) {
+  if (id_.HasValue()) {
     network_thread_->BlockingCall(
         [c = controller_.get(), sid = id_] { c->AddSctpDataStream(sid); });
   }
@@ -475,9 +469,7 @@ void SctpDataChannel::OnDataReceived(DataMessageType type,
 void SctpDataChannel::OnTransportReady() {
   RTC_DCHECK_RUN_ON(signaling_thread_);
 
-  // TODO(tommi, hta): We don't need the `writable_` flag for SCTP datachannels.
-  // Remove it and just rely on `connected_to_transport_` instead.
-  // In practice the transport is configured inside
+  // TODO(bugs.webrtc.org/11547): The transport is configured inside
   // `PeerConnection::SetupDataChannelTransport_n`, which results in
   // `SctpDataChannel` getting the OnTransportChannelCreated callback, and then
   // that's immediately followed by calling `transport->SetDataSink` which is
@@ -488,7 +480,6 @@ void SctpDataChannel::OnTransportReady() {
   // be on for the below `Send*` calls, which currently do a BlockingCall
   // from the signaling thread to the network thread.
   RTC_DCHECK(connected_to_transport_);
-  writable_ = true;
 
   SendQueuedControlMessages();
   SendQueuedDataMessages();
@@ -544,8 +535,8 @@ void SctpDataChannel::UpdateState() {
           WriteDataChannelOpenAckMessage(&payload);
           SendControlMessage(payload);
         }
-        if (writable_ && (handshake_state_ == kHandshakeReady ||
-                          handshake_state_ == kHandshakeWaitingForAck)) {
+        if (handshake_state_ == kHandshakeReady ||
+            handshake_state_ == kHandshakeWaitingForAck) {
           SetState(kOpen);
           // If we have received buffers before the channel got writable.
           // Deliver them now.
@@ -717,7 +708,6 @@ void SctpDataChannel::QueueControlMessage(
 
 bool SctpDataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
   RTC_DCHECK_RUN_ON(signaling_thread_);
-  RTC_DCHECK(writable_);
   RTC_DCHECK(connected_to_transport_);
   RTC_DCHECK(id_.HasValue());
 
diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h
index 184603d63f..5cfa3649f7 100644
--- a/pc/sctp_data_channel.h
+++ b/pc/sctp_data_channel.h
@@ -86,7 +86,7 @@ struct InternalDataChannelInit : public DataChannelInit {
 // Helper class to allocate unique IDs for SCTP DataChannels.
 class SctpSidAllocator {
  public:
-  SctpSidAllocator();
+  SctpSidAllocator() = default;
   // Gets the first unused odd/even id based on the DTLS role. If `role` is
   // SSL_CLIENT, the allocated id starts from 0 and takes even numbers;
   // otherwise, the id starts from 1 and takes odd numbers.
@@ -101,7 +101,8 @@ class SctpSidAllocator {
 
  private:
   flat_set<StreamId> used_sids_ RTC_GUARDED_BY(&sequence_checker_);
-  RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_;
+  RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_{
+      SequenceChecker::kDetached};
 };
 
 // SctpDataChannel is an implementation of the DataChannelInterface based on
@@ -278,7 +279,6 @@ class SctpDataChannel : public DataChannelInterface {
   HandshakeState handshake_state_ RTC_GUARDED_BY(signaling_thread_) =
       kHandshakeInit;
   bool connected_to_transport_ RTC_GUARDED_BY(signaling_thread_) = false;
-  bool writable_ RTC_GUARDED_BY(signaling_thread_) = false;
   // Did we already start the graceful SCTP closing procedure?
   bool started_closing_procedure_ RTC_GUARDED_BY(signaling_thread_) = false;
   // Control messages that always have to get sent out before any queued