From 6da271844c21462ab732fb78909615a071c722e5 Mon Sep 17 00:00:00 2001
From: Tomas Gunnarsson <tommi@webrtc.org>
Date: Sat, 12 Sep 2020 23:10:17 +0200
Subject: [PATCH] Avoid deallocating the async invoker when clearing the
 transport.

Deallocating the async invoker is a costly operation
but it's also unnecessary and could cause us to miss signal
events.

The data_channel_transport and data_channel_transport_invoker
are (despite the name) not related, since the latter is
used to signal events on the signaling thread whereas the
former deals with the data.

Bug: webrtc:11908
Change-Id: I37b345476a6381aef5d87807877ec1e05b380137
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184062
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32096}
---
 pc/data_channel_controller.cc | 14 ++++++--------
 pc/data_channel_controller.h  |  5 +++--
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index 04a4bb6245..9fabe13cc7 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -129,7 +129,7 @@ void DataChannelController::OnDataReceived(
   cricket::ReceiveDataParams params;
   params.sid = channel_id;
   params.type = ToCricketDataMessageType(type);
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this, params, buffer] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         // TODO(bugs.webrtc.org/11547): The data being received should be
@@ -148,7 +148,7 @@ void DataChannelController::OnDataReceived(
 
 void DataChannelController::OnChannelClosing(int channel_id) {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this, channel_id] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         SignalDataChannelTransportChannelClosing_s(channel_id);
@@ -157,7 +157,7 @@ void DataChannelController::OnChannelClosing(int channel_id) {
 
 void DataChannelController::OnChannelClosed(int channel_id) {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this, channel_id] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         SignalDataChannelTransportChannelClosed_s(channel_id);
@@ -166,7 +166,7 @@ void DataChannelController::OnChannelClosed(int channel_id) {
 
 void DataChannelController::OnReadyToSend() {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         data_channel_transport_ready_to_send_ = true;
@@ -177,7 +177,7 @@ void DataChannelController::OnReadyToSend() {
 
 void DataChannelController::OnTransportClosed() {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         OnTransportChannelClosed();
@@ -186,7 +186,6 @@ void DataChannelController::OnTransportClosed() {
 
 void DataChannelController::SetupDataChannelTransport_n() {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_ = std::make_unique<rtc::AsyncInvoker>();
 
   // There's a new data channel transport.  This needs to be signaled to the
   // |sctp_data_channels_| so that they can reopen and reconnect.  This is
@@ -196,7 +195,6 @@ void DataChannelController::SetupDataChannelTransport_n() {
 
 void DataChannelController::TeardownDataChannelTransport_n() {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_ = nullptr;
   if (data_channel_transport()) {
     data_channel_transport()->SetDataSink(nullptr);
   }
@@ -592,7 +590,7 @@ bool DataChannelController::DataChannelSendData(
 
 void DataChannelController::NotifyDataChannelsOfTransportCreated() {
   RTC_DCHECK_RUN_ON(network_thread());
-  data_channel_transport_invoker_->AsyncInvoke<void>(
+  data_channel_transport_invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, signaling_thread(), [this] {
         RTC_DCHECK_RUN_ON(signaling_thread());
         for (const auto& channel : sctp_data_channels_) {
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 3daee11381..6759288825 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -221,8 +221,9 @@ class DataChannelController : public RtpDataChannelProviderInterface,
   sigslot::signal1<SctpDataChannel*> SignalSctpDataChannelCreated_
       RTC_GUARDED_BY(signaling_thread());
 
-  // Used to invoke data channel transport signals on the signaling thread.
-  std::unique_ptr<rtc::AsyncInvoker> data_channel_transport_invoker_
+  // Used from the network thread to invoke data channel transport signals on
+  // the signaling thread.
+  rtc::AsyncInvoker data_channel_transport_invoker_
       RTC_GUARDED_BY(network_thread());
 
   // Owning PeerConnection.