From 246724b0fe1ff48263039acb776dcba89b990bc0 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 3 Dec 2019 22:31:42 +0100 Subject: [PATCH] Move messaging -> PostTask for freeing datachannels I could find no reason for the extra complexity of doing messaging in order to schedule a task to be done after the current cycle. It also simplifies the peerconnection/datachannelcontroller coupling. Bug: webrtc:11146 Change-Id: I68f45059b9f4a6869fb44b856e05a480f4652365 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161232 Commit-Queue: Harald Alvestrand Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#29997} --- pc/data_channel_controller.cc | 8 +++++++- pc/data_channel_controller.h | 8 ++------ pc/peer_connection.cc | 9 --------- pc/peer_connection.h | 2 -- 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index cc9f149364..0eaf44d09f 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc @@ -348,7 +348,13 @@ void DataChannelController::OnSctpDataChannelClosed(DataChannel* channel) { // we can't free it directly here; we need to free it asynchronously. sctp_data_channels_to_free_.push_back(*it); sctp_data_channels_.erase(it); - pc_->SignalFreeDataChannels(); + signaling_thread()->PostTask( + RTC_FROM_HERE, [self = weak_factory_.GetWeakPtr()] { + if (self) { + RTC_DCHECK_RUN_ON(self->signaling_thread()); + self->sctp_data_channels_to_free_.clear(); + } + }); return; } } diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h index bfce16c10c..91bba66066 100644 --- a/pc/data_channel_controller.h +++ b/pc/data_channel_controller.h @@ -18,6 +18,7 @@ #include "pc/channel.h" #include "pc/data_channel.h" +#include "rtc_base/weak_ptr.h" namespace webrtc { @@ -77,12 +78,6 @@ class DataChannelController : public DataChannelProviderInterface, return !rtp_data_channels_.empty(); } - // Called when it's appropriate to delete released datachannels. - void FreeDataChannels() { - RTC_DCHECK_RUN_ON(signaling_thread()); - sctp_data_channels_to_free_.clear(); - } - void UpdateLocalRtpDataChannels(const cricket::StreamParamsVec& streams); void UpdateRemoteRtpDataChannels(const cricket::StreamParamsVec& streams); @@ -207,6 +202,7 @@ class DataChannelController : public DataChannelProviderInterface, // Owning PeerConnection. PeerConnection* const pc_; + rtc::WeakPtrFactory weak_factory_{this}; }; } // namespace webrtc diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 855df75718..1339638f9f 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -136,7 +136,6 @@ enum { MSG_SET_SESSIONDESCRIPTION_FAILED, MSG_CREATE_SESSIONDESCRIPTION_FAILED, MSG_GETSTATS, - MSG_FREE_DATACHANNELS, MSG_REPORT_USAGE_PATTERN, }; @@ -4496,10 +4495,6 @@ void PeerConnection::OnMessage(rtc::Message* msg) { delete param; break; } - case MSG_FREE_DATACHANNELS: { - data_channel_controller_.FreeDataChannels(); - break; - } case MSG_REPORT_USAGE_PATTERN: { ReportUsagePattern(); break; @@ -5676,10 +5671,6 @@ void PeerConnection::OnSctpDataChannelClosed(DataChannel* channel) { data_channel_controller_.OnSctpDataChannelClosed(channel); } -void PeerConnection::SignalFreeDataChannels() { - signaling_thread()->Post(RTC_FROM_HERE, this, MSG_FREE_DATACHANNELS, nullptr); -} - rtc::scoped_refptr> PeerConnection::GetAudioTransceiver() const { // This method only works with Plan B SDP, where there is a single diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 941b744b13..0e1a1f8ac3 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -317,8 +317,6 @@ class PeerConnection : public PeerConnectionInternal, bool GetSctpSslRole(rtc::SSLRole* role); // Handler for the "channel closed" signal void OnSctpDataChannelClosed(DataChannel* channel); - // Sends the MSG_FREE_DATACHANNELS signal - void SignalFreeDataChannels(); // Functions made public for testing. void ReturnHistogramVeryQuicklyForTesting() {