Remove SctpDataChannel::Init()

This is a small tweak to explicitly remove this second construction
step from SctpDataChannel (async call to OnTransportReady) and move
it over to DataChannelController, which is where OnTransportReady()
is called from otherwise.

Bug: webrtc:11547
Change-Id: Ie86fa85cbb79b405248f88b47d5920c7f163dba6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297921
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39627}

pc/data_channel_controller.cc

@@ -260,6 +260,12 @@ DataChannelController::InternalCreateSctpDataChannel(
     const std::string& label,
     const InternalDataChannelInit* config) {
   RTC_DCHECK_RUN_ON(signaling_thread());
+  if (config && !config->IsValid()) {
+    RTC_LOG(LS_ERROR) << "Failed to initialize the SCTP data channel due to "
+                         "invalid DataChannelInit.";
+    return nullptr;
+  }
+
   InternalDataChannelInit new_config =
       config ? (*config) : InternalDataChannelInit();
   StreamId sid(new_config.id);
@@ -294,10 +300,21 @@ DataChannelController::InternalCreateSctpDataChannel(
   rtc::scoped_refptr<SctpDataChannel> channel(SctpDataChannel::Create(
       weak_factory_.GetWeakPtr(), label, data_channel_transport() != nullptr,
       new_config, signaling_thread(), network_thread()));
-  if (!channel) {
+  RTC_DCHECK(channel);
-    sid_allocator_.ReleaseSid(sid);
+
-    return nullptr;
+  if (ReadyToSendData()) {
+    // Checks if the transport is ready to send because the initial channel
+    // ready signal may have been sent before the DataChannel creation.
+    // This has to be done async because the upper layer objects (e.g.
+    // Chrome glue and WebKit) are not wired up properly until after this
+    // function returns.
+    signaling_thread()->PostTask(
+        SafeTask(signaling_safety_.flag(), [channel = channel] {
+          if (channel->state() != DataChannelInterface::DataState::kClosed)
+            channel->OnTransportReady();
+        }));
   }
+
   sctp_data_channels_.push_back(channel);
   has_used_data_channels_ = true;
   return channel;
@@ -353,6 +370,12 @@ void DataChannelController::OnTransportChannelClosed(RTCError error) {
   RTC_DCHECK_RUN_ON(signaling_thread());
   // Use a temporary copy of the SCTP DataChannel list because the
   // DataChannel may callback to us and try to modify the list.
+  // TODO(tommi): `OnTransportChannelClosed` is called from
+  // `SdpOfferAnswerHandler::DestroyDataChannelTransport` just before
+  // `TeardownDataChannelTransport_n` is called (but on the network thread) from
+  // the same function. Once `sctp_data_channels_` moves to the network thread,
+  // we can get rid of this function (OnTransportChannelClosed) and run this
+  // loop from within the TeardownDataChannelTransport_n callback.
   std::vector<rtc::scoped_refptr<SctpDataChannel>> temp_sctp_dcs;
   temp_sctp_dcs.swap(sctp_data_channels_);
   for (const auto& channel : temp_sctp_dcs) {

pc/sctp_data_channel.cc

@@ -148,18 +148,10 @@ rtc::scoped_refptr<SctpDataChannel> SctpDataChannel::Create(
     rtc::Thread* signaling_thread,
     rtc::Thread* network_thread) {
   RTC_DCHECK(controller);
-
+  RTC_DCHECK(config.IsValid());
-  if (!config.IsValid()) {
+  return rtc::make_ref_counted<SctpDataChannel>(
-    RTC_LOG(LS_ERROR) << "Failed to initialize the SCTP data channel due to "
-                         "invalid DataChannelInit.";
-    return nullptr;
-  }
-
-  auto channel = rtc::make_ref_counted<SctpDataChannel>(
       config, std::move(controller), label, connected_to_transport,
       signaling_thread, network_thread);
-  channel->Init();
-  return channel;
 }
 
 // static
@@ -215,26 +207,6 @@ SctpDataChannel::SctpDataChannel(
   }
 }
 
-void SctpDataChannel::Init() {
-  RTC_DCHECK_RUN_ON(signaling_thread_);
-
-  // Checks if the transport is ready to send because the initial channel
-  // ready signal may have been sent before the DataChannel creation.
-  // This has to be done async because the upper layer objects (e.g.
-  // Chrome glue and WebKit) are not wired up properly until after this
-  // function returns.
-  if (controller_->ReadyToSendData()) {
-    RTC_DCHECK(connected_to_transport_);
-    AddRef();
-    absl::Cleanup release = [this] { Release(); };
-    rtc::Thread::Current()->PostTask([this, release = std::move(release)] {
-      RTC_DCHECK_RUN_ON(signaling_thread_);
-      if (state_ != kClosed)
-        OnTransportReady();
-    });
-  }
-}
-
 SctpDataChannel::~SctpDataChannel() {
   RTC_DCHECK_RUN_ON(signaling_thread_);
 }

pc/sctp_data_channel.h

@@ -221,7 +221,6 @@ class SctpDataChannel : public DataChannelInterface {
     kHandshakeReady
   };
 
-  void Init();
   void UpdateState();
   void SetState(DataState state);
 

pc/test/fake_data_channel_controller.h

@@ -36,10 +36,20 @@ class FakeDataChannelController
       absl::string_view label,
       webrtc::InternalDataChannelInit init,
       rtc::Thread* network_thread = rtc::Thread::Current()) {
+    rtc::Thread* signaling_thread = rtc::Thread::Current();
     rtc::scoped_refptr<webrtc::SctpDataChannel> channel =
         webrtc::SctpDataChannel::Create(weak_ptr(), std::string(label),
                                         transport_available_, init,
-                                        rtc::Thread::Current(), network_thread);
+                                        signaling_thread, network_thread);
+    if (ReadyToSendData()) {
+      signaling_thread->PostTask(
+          SafeTask(signaling_safety_.flag(), [channel = channel] {
+            if (channel->state() !=
+                webrtc::DataChannelInterface::DataState::kClosed) {
+              channel->OnTransportReady();
+            }
+          }));
+    }
     connected_channels_.insert(channel.get());
     return channel;
   }
@@ -160,5 +170,6 @@ class FakeDataChannelController
   std::set<webrtc::SctpDataChannel*> connected_channels_;
   std::set<webrtc::StreamId> known_stream_ids_;
   rtc::WeakPtrFactory<FakeDataChannelController> weak_factory_{this};
+  webrtc::ScopedTaskSafety signaling_safety_;
 };
 #endif  // PC_TEST_FAKE_DATA_CHANNEL_CONTROLLER_H_