From 0d5ce62d015cdb69a21d11171e9f01f2d32bfce7 Mon Sep 17 00:00:00 2001 From: Tomas Gunnarsson Date: Fri, 18 Mar 2022 15:57:15 +0100 Subject: [PATCH] Make RtpTransceiver not inherit from RefCountedObject. Also update API proxy Create() factory functions to accept the inner reference counted object via scoped_refptr instead of a raw pointer. This is to avoid accidentally creating and deleting an object when passing an inner object to a proxy class. Consider something like: auto proxy = MyProxy::Create( signaling_thread(), make_ref_counted()); Bug: webrtc:13464, webrtc:12701 Change-Id: I55ccfff43bbc164a5e909b2c9020e306ebb09075 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256010 Reviewed-by: Niels Moller Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#36261} --- pc/peer_connection.cc | 6 +- pc/proxy.h | 58 ++++----- pc/proxy_unittest.cc | 6 +- pc/rtp_transceiver.h | 6 +- pc/rtp_transceiver_unittest.cc | 157 ++++++++++++----------- pc/rtp_transmission_manager.cc | 2 +- pc/sctp_data_channel.cc | 7 +- pc/test/fake_peer_connection_for_stats.h | 3 +- pc/video_track.cc | 5 +- pc/video_track_source_proxy.cc | 4 +- 10 files changed, 130 insertions(+), 124 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index fb1f86c007..e5a83b0a18 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -649,11 +649,13 @@ RTCError PeerConnection::Initialize( rtp_manager()->transceivers()->Add( RtpTransceiverProxyWithInternal::Create( signaling_thread(), - new RtpTransceiver(cricket::MEDIA_TYPE_AUDIO, channel_manager()))); + rtc::make_ref_counted(cricket::MEDIA_TYPE_AUDIO, + channel_manager()))); rtp_manager()->transceivers()->Add( RtpTransceiverProxyWithInternal::Create( signaling_thread(), - new RtpTransceiver(cricket::MEDIA_TYPE_VIDEO, channel_manager()))); + rtc::make_ref_counted(cricket::MEDIA_TYPE_VIDEO, + channel_manager()))); } int delay_ms = configuration.report_usage_pattern_delay_ms diff --git a/pc/proxy.h b/pc/proxy.h index feeb06338e..2c347d6249 100644 --- a/pc/proxy.h +++ b/pc/proxy.h @@ -227,26 +227,26 @@ class ConstMethodCall : public QueuedTask { constexpr char class_name##ProxyWithInternal::proxy_name_[]; // clang-format on -#define PRIMARY_PROXY_MAP_BOILERPLATE(class_name) \ - protected: \ - class_name##ProxyWithInternal(rtc::Thread* primary_thread, \ - INTERNAL_CLASS* c) \ - : primary_thread_(primary_thread), c_(c) {} \ - \ - private: \ +#define PRIMARY_PROXY_MAP_BOILERPLATE(class_name) \ + protected: \ + class_name##ProxyWithInternal(rtc::Thread* primary_thread, \ + rtc::scoped_refptr c) \ + : primary_thread_(primary_thread), c_(std::move(c)) {} \ + \ + private: \ mutable rtc::Thread* primary_thread_; -#define SECONDARY_PROXY_MAP_BOILERPLATE(class_name) \ - protected: \ - class_name##ProxyWithInternal(rtc::Thread* primary_thread, \ - rtc::Thread* secondary_thread, \ - INTERNAL_CLASS* c) \ - : primary_thread_(primary_thread), \ - secondary_thread_(secondary_thread), \ - c_(c) {} \ - \ - private: \ - mutable rtc::Thread* primary_thread_; \ +#define SECONDARY_PROXY_MAP_BOILERPLATE(class_name) \ + protected: \ + class_name##ProxyWithInternal(rtc::Thread* primary_thread, \ + rtc::Thread* secondary_thread, \ + rtc::scoped_refptr c) \ + : primary_thread_(primary_thread), \ + secondary_thread_(secondary_thread), \ + c_(std::move(c)) {} \ + \ + private: \ + mutable rtc::Thread* primary_thread_; \ mutable rtc::Thread* secondary_thread_; // Note that the destructor is protected so that the proxy can only be @@ -284,15 +284,15 @@ class ConstMethodCall : public QueuedTask { void DestroyInternal() { delete c_; } \ INTERNAL_CLASS* c_; -#define BEGIN_PRIMARY_PROXY_MAP(class_name) \ - PROXY_MAP_BOILERPLATE(class_name) \ - PRIMARY_PROXY_MAP_BOILERPLATE(class_name) \ - REFCOUNTED_PROXY_MAP_BOILERPLATE(class_name) \ - public: \ - static rtc::scoped_refptr Create( \ - rtc::Thread* primary_thread, INTERNAL_CLASS* c) { \ - return rtc::make_ref_counted( \ - primary_thread, c); \ +#define BEGIN_PRIMARY_PROXY_MAP(class_name) \ + PROXY_MAP_BOILERPLATE(class_name) \ + PRIMARY_PROXY_MAP_BOILERPLATE(class_name) \ + REFCOUNTED_PROXY_MAP_BOILERPLATE(class_name) \ + public: \ + static rtc::scoped_refptr Create( \ + rtc::Thread* primary_thread, rtc::scoped_refptr c) { \ + return rtc::make_ref_counted( \ + primary_thread, std::move(c)); \ } #define BEGIN_PROXY_MAP(class_name) \ @@ -302,9 +302,9 @@ class ConstMethodCall : public QueuedTask { public: \ static rtc::scoped_refptr Create( \ rtc::Thread* primary_thread, rtc::Thread* secondary_thread, \ - INTERNAL_CLASS* c) { \ + rtc::scoped_refptr c) { \ return rtc::make_ref_counted( \ - primary_thread, secondary_thread, c); \ + primary_thread, secondary_thread, std::move(c)); \ } #define PROXY_PRIMARY_THREAD_DESTRUCTOR() \ diff --git a/pc/proxy_unittest.cc b/pc/proxy_unittest.cc index ab02359d53..48c087f690 100644 --- a/pc/proxy_unittest.cc +++ b/pc/proxy_unittest.cc @@ -98,7 +98,7 @@ class SignalingProxyTest : public ::testing::Test { ASSERT_TRUE(signaling_thread_->Start()); fake_ = Fake::Create(); fake_signaling_proxy_ = - FakeSignalingProxy::Create(signaling_thread_.get(), fake_.get()); + FakeSignalingProxy::Create(signaling_thread_.get(), fake_); } protected: @@ -186,8 +186,8 @@ class ProxyTest : public ::testing::Test { ASSERT_TRUE(signaling_thread_->Start()); ASSERT_TRUE(worker_thread_->Start()); fake_ = Fake::Create(); - fake_proxy_ = FakeProxy::Create(signaling_thread_.get(), - worker_thread_.get(), fake_.get()); + fake_proxy_ = + FakeProxy::Create(signaling_thread_.get(), worker_thread_.get(), fake_); } protected: diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index e7e3fb9be1..e71fdc1695 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h @@ -39,7 +39,6 @@ #include "pc/rtp_sender_proxy.h" #include "pc/rtp_transport_internal.h" #include "pc/session_description.h" -#include "rtc_base/ref_counted_object.h" #include "rtc_base/task_utils/pending_task_safety_flag.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread_annotations.h" @@ -76,9 +75,8 @@ namespace webrtc { // MediaType specified in the constructor. Audio RtpTransceivers will have // AudioRtpSenders, AudioRtpReceivers, and a VoiceChannel. Video RtpTransceivers // will have VideoRtpSenders, VideoRtpReceivers, and a VideoChannel. -class RtpTransceiver final - : public rtc::RefCountedObject, - public sigslot::has_slots<> { +class RtpTransceiver : public RtpTransceiverInterface, + public sigslot::has_slots<> { public: // Construct a Plan B-style RtpTransceiver with no senders, receivers, or // channel set. diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index e5c7733178..9c2a0461b3 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc @@ -52,7 +52,8 @@ class ChannelManagerForTest : public cricket::ChannelManager { TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) { ChannelManagerForTest cm; const std::string content_name("my_mid"); - RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_AUDIO, &cm); + auto transceiver = rtc::make_ref_counted( + cricket::MediaType::MEDIA_TYPE_AUDIO, &cm); cricket::MockChannelInterface channel1; EXPECT_CALL(channel1, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); @@ -60,35 +61,37 @@ TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) { EXPECT_CALL(channel1, SetFirstPacketReceivedCallback(_)); EXPECT_CALL(channel1, SetRtpTransport(_)).WillRepeatedly(Return(true)); - transceiver.SetChannel(&channel1, [&](const std::string& mid) { + transceiver->SetChannel(&channel1, [&](const std::string& mid) { EXPECT_EQ(mid, content_name); return nullptr; }); - EXPECT_EQ(&channel1, transceiver.channel()); + EXPECT_EQ(&channel1, transceiver->channel()); // Stop the transceiver. - transceiver.StopInternal(); - EXPECT_EQ(&channel1, transceiver.channel()); + transceiver->StopInternal(); + EXPECT_EQ(&channel1, transceiver->channel()); cricket::MockChannelInterface channel2; EXPECT_CALL(channel2, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); // Channel can no longer be set, so this call should be a no-op. - transceiver.SetChannel(&channel2, [](const std::string&) { return nullptr; }); - EXPECT_EQ(&channel1, transceiver.channel()); + transceiver->SetChannel(&channel2, + [](const std::string&) { return nullptr; }); + EXPECT_EQ(&channel1, transceiver->channel()); // Clear the current channel before `transceiver` goes out of scope. EXPECT_CALL(channel1, SetFirstPacketReceivedCallback(_)); EXPECT_CALL(cm, DestroyChannel(&channel1)).WillRepeatedly(testing::Return()); - transceiver.SetChannel(nullptr, nullptr); + transceiver->SetChannel(nullptr, nullptr); } // Checks that a channel can be unset on a stopped `RtpTransceiver` TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) { ChannelManagerForTest cm; const std::string content_name("my_mid"); - RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_VIDEO, &cm); + auto transceiver = rtc::make_ref_counted( + cricket::MediaType::MEDIA_TYPE_VIDEO, &cm); cricket::MockChannelInterface channel; EXPECT_CALL(channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_VIDEO)); @@ -98,34 +101,35 @@ TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) { EXPECT_CALL(channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); EXPECT_CALL(cm, DestroyChannel(&channel)).WillRepeatedly(testing::Return()); - transceiver.SetChannel(&channel, [&](const std::string& mid) { + transceiver->SetChannel(&channel, [&](const std::string& mid) { EXPECT_EQ(mid, content_name); return nullptr; }); - EXPECT_EQ(&channel, transceiver.channel()); + EXPECT_EQ(&channel, transceiver->channel()); // Stop the transceiver. - transceiver.StopInternal(); - EXPECT_EQ(&channel, transceiver.channel()); + transceiver->StopInternal(); + EXPECT_EQ(&channel, transceiver->channel()); // Set the channel to `nullptr`. - transceiver.SetChannel(nullptr, nullptr); - EXPECT_EQ(nullptr, transceiver.channel()); + transceiver->SetChannel(nullptr, nullptr); + EXPECT_EQ(nullptr, transceiver->channel()); } class RtpTransceiverUnifiedPlanTest : public ::testing::Test { public: RtpTransceiverUnifiedPlanTest() - : transceiver_(RtpSenderProxyWithInternal::Create( - rtc::Thread::Current(), - sender_), - RtpReceiverProxyWithInternal::Create( - rtc::Thread::Current(), - rtc::Thread::Current(), - receiver_), - &channel_manager_, - channel_manager_.GetSupportedAudioRtpHeaderExtensions(), - /* on_negotiation_needed= */ [] {}) {} + : transceiver_(rtc::make_ref_counted( + RtpSenderProxyWithInternal::Create( + rtc::Thread::Current(), + sender_), + RtpReceiverProxyWithInternal::Create( + rtc::Thread::Current(), + rtc::Thread::Current(), + receiver_), + &channel_manager_, + channel_manager_.GetSupportedAudioRtpHeaderExtensions(), + /* on_negotiation_needed= */ [] {})) {} static rtc::scoped_refptr MockReceiver() { auto receiver = rtc::make_ref_counted(); @@ -144,7 +148,7 @@ class RtpTransceiverUnifiedPlanTest : public ::testing::Test { rtc::scoped_refptr receiver_ = MockReceiver(); rtc::scoped_refptr sender_ = MockSender(); ChannelManagerForTest channel_manager_; - RtpTransceiver transceiver_; + rtc::scoped_refptr transceiver_; }; // Basic tests for Stop() @@ -154,16 +158,16 @@ TEST_F(RtpTransceiverUnifiedPlanTest, StopSetsDirection) { EXPECT_CALL(*sender_.get(), SetTransceiverAsStopped()); EXPECT_CALL(*sender_.get(), Stop()); - EXPECT_EQ(RtpTransceiverDirection::kInactive, transceiver_.direction()); - EXPECT_FALSE(transceiver_.current_direction()); - transceiver_.StopStandard(); - EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_.direction()); - EXPECT_FALSE(transceiver_.current_direction()); - transceiver_.StopTransceiverProcedure(); - EXPECT_TRUE(transceiver_.current_direction()); - EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_.direction()); + EXPECT_EQ(RtpTransceiverDirection::kInactive, transceiver_->direction()); + EXPECT_FALSE(transceiver_->current_direction()); + transceiver_->StopStandard(); + EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_->direction()); + EXPECT_FALSE(transceiver_->current_direction()); + transceiver_->StopTransceiverProcedure(); + EXPECT_TRUE(transceiver_->current_direction()); + EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_->direction()); EXPECT_EQ(RtpTransceiverDirection::kStopped, - *transceiver_.current_direction()); + *transceiver_->current_direction()); } class RtpTransceiverTestForHeaderExtensions : public ::testing::Test { @@ -182,16 +186,17 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test { RtpHeaderExtensionCapability(RtpExtension::kVideoRotationUri, 4, RtpTransceiverDirection::kSendRecv)}), - transceiver_(RtpSenderProxyWithInternal::Create( - rtc::Thread::Current(), - sender_), - RtpReceiverProxyWithInternal::Create( - rtc::Thread::Current(), - rtc::Thread::Current(), - receiver_), - &channel_manager_, - extensions_, - /* on_negotiation_needed= */ [] {}) {} + transceiver_(rtc::make_ref_counted( + RtpSenderProxyWithInternal::Create( + rtc::Thread::Current(), + sender_), + RtpReceiverProxyWithInternal::Create( + rtc::Thread::Current(), + rtc::Thread::Current(), + receiver_), + &channel_manager_, + extensions_, + /* on_negotiation_needed= */ [] {})) {} static rtc::scoped_refptr MockReceiver() { auto receiver = rtc::make_ref_counted(); @@ -212,7 +217,7 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test { EXPECT_CALL(mock_channel, SetFirstPacketReceivedCallback(_)); EXPECT_CALL(channel_manager_, DestroyChannel(&mock_channel)) .WillRepeatedly(testing::Return()); - transceiver_.SetChannel(nullptr, nullptr); + transceiver_->SetChannel(nullptr, nullptr); } rtc::scoped_refptr receiver_ = MockReceiver(); @@ -220,7 +225,7 @@ class RtpTransceiverTestForHeaderExtensions : public ::testing::Test { ChannelManagerForTest channel_manager_; std::vector extensions_; - RtpTransceiver transceiver_; + rtc::scoped_refptr transceiver_; }; TEST_F(RtpTransceiverTestForHeaderExtensions, OffersChannelManagerList) { @@ -229,7 +234,7 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, OffersChannelManagerList) { EXPECT_CALL(*sender_.get(), SetTransceiverAsStopped()); EXPECT_CALL(*sender_.get(), Stop()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), extensions_); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); } TEST_F(RtpTransceiverTestForHeaderExtensions, ModifiesDirection) { @@ -241,20 +246,20 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, ModifiesDirection) { auto modified_extensions = extensions_; modified_extensions[0].direction = RtpTransceiverDirection::kSendOnly; EXPECT_TRUE( - transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions).ok()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), modified_extensions); + transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions).ok()); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), modified_extensions); modified_extensions[0].direction = RtpTransceiverDirection::kRecvOnly; EXPECT_TRUE( - transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions).ok()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), modified_extensions); + transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions).ok()); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), modified_extensions); modified_extensions[0].direction = RtpTransceiverDirection::kSendRecv; EXPECT_TRUE( - transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions).ok()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), modified_extensions); + transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions).ok()); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), modified_extensions); modified_extensions[0].direction = RtpTransceiverDirection::kInactive; EXPECT_TRUE( - transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions).ok()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), modified_extensions); + transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions).ok()); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), modified_extensions); } TEST_F(RtpTransceiverTestForHeaderExtensions, AcceptsStoppedExtension) { @@ -266,8 +271,8 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, AcceptsStoppedExtension) { auto modified_extensions = extensions_; modified_extensions[0].direction = RtpTransceiverDirection::kStopped; EXPECT_TRUE( - transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions).ok()); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), modified_extensions); + transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions).ok()); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), modified_extensions); } TEST_F(RtpTransceiverTestForHeaderExtensions, RejectsUnsupportedExtension) { @@ -279,9 +284,9 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, RejectsUnsupportedExtension) { std::vector modified_extensions( {RtpHeaderExtensionCapability("uri3", 1, RtpTransceiverDirection::kSendRecv)}); - EXPECT_THAT(transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions), + EXPECT_THAT(transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions), Property(&RTCError::type, RTCErrorType::UNSUPPORTED_PARAMETER)); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), extensions_); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); } TEST_F(RtpTransceiverTestForHeaderExtensions, @@ -294,15 +299,15 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, std::vector modified_extensions = extensions_; // Attempting to stop the mandatory MID extension. modified_extensions[2].direction = RtpTransceiverDirection::kStopped; - EXPECT_THAT(transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions), + EXPECT_THAT(transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions), Property(&RTCError::type, RTCErrorType::INVALID_MODIFICATION)); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), extensions_); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); modified_extensions = extensions_; // Attempting to stop the mandatory video orientation extension. modified_extensions[3].direction = RtpTransceiverDirection::kStopped; - EXPECT_THAT(transceiver_.SetOfferedRtpHeaderExtensions(modified_extensions), + EXPECT_THAT(transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions), Property(&RTCError::type, RTCErrorType::INVALID_MODIFICATION)); - EXPECT_EQ(transceiver_.HeaderExtensionsToOffer(), extensions_); + EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); } TEST_F(RtpTransceiverTestForHeaderExtensions, @@ -311,7 +316,7 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_CALL(*receiver_.get(), SetMediaChannel(_)); EXPECT_CALL(*sender_.get(), SetTransceiverAsStopped()); EXPECT_CALL(*sender_.get(), Stop()); - EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), ElementsAre()); + EXPECT_THAT(transceiver_->HeaderExtensionsNegotiated(), ElementsAre()); } TEST_F(RtpTransceiverTestForHeaderExtensions, @@ -329,9 +334,9 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_CALL(mock_channel, media_channel()).WillRepeatedly(Return(nullptr)); EXPECT_CALL(mock_channel, mid()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); - transceiver_.SetChannel(&mock_channel, - [](const std::string&) { return nullptr; }); - EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), ElementsAre()); + transceiver_->SetChannel(&mock_channel, + [](const std::string&) { return nullptr; }); + EXPECT_THAT(transceiver_->HeaderExtensionsNegotiated(), ElementsAre()); ClearChannel(mock_channel); } @@ -356,11 +361,11 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, ReturnsNegotiatedHdrExts) { webrtc::RtpExtension("uri2", 2)}; cricket::AudioContentDescription description; description.set_rtp_header_extensions(extensions); - transceiver_.OnNegotiationUpdate(SdpType::kAnswer, &description); + transceiver_->OnNegotiationUpdate(SdpType::kAnswer, &description); - transceiver_.SetChannel(&mock_channel, - [](const std::string&) { return nullptr; }); - EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), + transceiver_->SetChannel(&mock_channel, + [](const std::string&) { return nullptr; }); + EXPECT_THAT(transceiver_->HeaderExtensionsNegotiated(), ElementsAre(RtpHeaderExtensionCapability( "uri1", 1, RtpTransceiverDirection::kSendRecv), RtpHeaderExtensionCapability( @@ -380,9 +385,9 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, webrtc::RtpExtension("uri2", 2)}; cricket::AudioContentDescription description; description.set_rtp_header_extensions(extensions); - transceiver_.OnNegotiationUpdate(SdpType::kAnswer, &description); + transceiver_->OnNegotiationUpdate(SdpType::kAnswer, &description); - EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), + EXPECT_THAT(transceiver_->HeaderExtensionsNegotiated(), ElementsAre(RtpHeaderExtensionCapability( "uri1", 1, RtpTransceiverDirection::kSendRecv), RtpHeaderExtensionCapability( @@ -391,9 +396,9 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, extensions = {webrtc::RtpExtension("uri3", 4), webrtc::RtpExtension("uri5", 6)}; description.set_rtp_header_extensions(extensions); - transceiver_.OnNegotiationUpdate(SdpType::kAnswer, &description); + transceiver_->OnNegotiationUpdate(SdpType::kAnswer, &description); - EXPECT_THAT(transceiver_.HeaderExtensionsNegotiated(), + EXPECT_THAT(transceiver_->HeaderExtensionsNegotiated(), ElementsAre(RtpHeaderExtensionCapability( "uri3", 4, RtpTransceiverDirection::kSendRecv), RtpHeaderExtensionCapability( diff --git a/pc/rtp_transmission_manager.cc b/pc/rtp_transmission_manager.cc index 5dbb765098..538fa62fe7 100644 --- a/pc/rtp_transmission_manager.cc +++ b/pc/rtp_transmission_manager.cc @@ -271,7 +271,7 @@ RtpTransmissionManager::CreateAndAddTransceiver( RTC_DCHECK(!FindSenderById(sender->id())); auto transceiver = RtpTransceiverProxyWithInternal::Create( signaling_thread(), - new RtpTransceiver( + rtc::make_ref_counted( sender, receiver, channel_manager(), sender->media_type() == cricket::MEDIA_TYPE_AUDIO ? channel_manager()->GetSupportedAudioRtpHeaderExtensions() diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index c63f820acf..356493658a 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -158,11 +158,8 @@ rtc::scoped_refptr SctpDataChannel::Create( rtc::scoped_refptr SctpDataChannel::CreateProxy( rtc::scoped_refptr channel) { // TODO(bugs.webrtc.org/11547): incorporate the network thread in the proxy. - // Also, consider allowing the proxy object to own the reference (std::move). - // As is, the proxy has a raw pointer and no reference to the channel object - // and trusting that the lifetime management aligns with the - // sctp_data_channels_ array in SctpDataChannelController. - return DataChannelProxy::Create(channel->signaling_thread_, channel.get()); + auto* signaling_thread = channel->signaling_thread_; + return DataChannelProxy::Create(signaling_thread, std::move(channel)); } SctpDataChannel::SctpDataChannel(const InternalDataChannelInit& config, diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index 4c1f73af8a..7f8559d4de 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -406,7 +406,8 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { } } auto transceiver = RtpTransceiverProxyWithInternal::Create( - signaling_thread_, new RtpTransceiver(media_type, &channel_manager_)); + signaling_thread_, + rtc::make_ref_counted(media_type, &channel_manager_)); transceivers_.push_back(transceiver); return transceiver; } diff --git a/pc/video_track.cc b/pc/video_track.cc index 744800c9f3..aa8e0df922 100644 --- a/pc/video_track.cc +++ b/pc/video_track.cc @@ -136,8 +136,9 @@ rtc::scoped_refptr VideoTrack::Create( rtc::Thread* worker_thread) { rtc::scoped_refptr< VideoTrackSourceProxyWithInternal> - source_proxy = VideoTrackSourceProxy::Create(rtc::Thread::Current(), - worker_thread, source); + source_proxy = VideoTrackSourceProxy::Create( + rtc::Thread::Current(), worker_thread, + rtc::scoped_refptr(source)); return rtc::make_ref_counted(id, std::move(source_proxy), worker_thread); diff --git a/pc/video_track_source_proxy.cc b/pc/video_track_source_proxy.cc index 26f0ecec98..c3e95e23cc 100644 --- a/pc/video_track_source_proxy.cc +++ b/pc/video_track_source_proxy.cc @@ -21,7 +21,9 @@ rtc::scoped_refptr CreateVideoTrackSourceProxy( rtc::Thread* signaling_thread, rtc::Thread* worker_thread, VideoTrackSourceInterface* source) { - return VideoTrackSourceProxy::Create(signaling_thread, worker_thread, source); + return VideoTrackSourceProxy::Create( + signaling_thread, worker_thread, + rtc::scoped_refptr(source)); } } // namespace webrtc