From faf33878efcd7b2e06eb63ad8b829936733620d1 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 16 Mar 2023 09:25:29 +0100 Subject: [PATCH] Always require a valid controller when constructing an SctpDataChannel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All tests do this already except for RTCStatsCollectorTest. Bug: none Change-Id: I318f45a2c79b3d07ca6c92902ebb4f0622ec3200 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297862 Reviewed-by: Henrik Boström Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#39576} --- pc/rtc_stats_collector_unittest.cc | 26 +++++++++++++++----------- pc/sctp_data_channel.cc | 1 + pc/test/mock_data_channel.h | 12 +++++++++--- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index d4c03de1b9..423b9c7247 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -669,7 +669,8 @@ class RTCStatsCollectorTest : public ::testing::Test { public: RTCStatsCollectorTest() : pc_(rtc::make_ref_counted()), - stats_(new RTCStatsCollectorWrapper(pc_)) {} + stats_(new RTCStatsCollectorWrapper(pc_)), + data_channel_controller_(new FakeDataChannelController()) {} void ExpectReportContainsCertificateInfo( const rtc::scoped_refptr& report, @@ -955,6 +956,7 @@ class RTCStatsCollectorTest : public ::testing::Test { rtc::AutoThread main_thread_; rtc::scoped_refptr pc_; std::unique_ptr stats_; + std::unique_ptr data_channel_controller_; }; TEST_F(RTCStatsCollectorTest, SingleCallback) { @@ -1592,9 +1594,11 @@ TEST_F(RTCStatsCollectorTest, CertificateStatsCache) { TEST_F(RTCStatsCollectorTest, CollectTwoRTCDataChannelStatsWithPendingId) { pc_->AddSctpDataChannel(rtc::make_ref_counted( - /*id=*/-1, DataChannelInterface::kConnecting)); + data_channel_controller_->weak_ptr(), /*id=*/-1, + DataChannelInterface::kConnecting)); pc_->AddSctpDataChannel(rtc::make_ref_counted( - /*id=*/-1, DataChannelInterface::kConnecting)); + data_channel_controller_->weak_ptr(), /*id=*/-1, + DataChannelInterface::kConnecting)); rtc::scoped_refptr report = stats_->GetStatsReport(); } @@ -1605,8 +1609,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCDataChannelStats) { // the test, we reset the ID allocator at test start. SctpDataChannel::ResetInternalIdAllocatorForTesting(-1); pc_->AddSctpDataChannel(rtc::make_ref_counted( - 0, "MockSctpDataChannel0", DataChannelInterface::kConnecting, "udp", 1, 2, - 3, 4)); + data_channel_controller_->weak_ptr(), 0, "MockSctpDataChannel0", + DataChannelInterface::kConnecting, "udp", 1, 2, 3, 4)); RTCDataChannelStats expected_data_channel0("D0", Timestamp::Zero()); expected_data_channel0.label = "MockSctpDataChannel0"; expected_data_channel0.protocol = "udp"; @@ -1618,8 +1622,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCDataChannelStats) { expected_data_channel0.bytes_received = 4; pc_->AddSctpDataChannel(rtc::make_ref_counted( - 1, "MockSctpDataChannel1", DataChannelInterface::kOpen, "tcp", 5, 6, 7, - 8)); + data_channel_controller_->weak_ptr(), 1, "MockSctpDataChannel1", + DataChannelInterface::kOpen, "tcp", 5, 6, 7, 8)); RTCDataChannelStats expected_data_channel1("D1", Timestamp::Zero()); expected_data_channel1.label = "MockSctpDataChannel1"; expected_data_channel1.protocol = "tcp"; @@ -1631,8 +1635,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCDataChannelStats) { expected_data_channel1.bytes_received = 8; pc_->AddSctpDataChannel(rtc::make_ref_counted( - 2, "MockSctpDataChannel2", DataChannelInterface::kClosing, "udp", 9, 10, - 11, 12)); + data_channel_controller_->weak_ptr(), 2, "MockSctpDataChannel2", + DataChannelInterface::kClosing, "udp", 9, 10, 11, 12)); RTCDataChannelStats expected_data_channel2("D2", Timestamp::Zero()); expected_data_channel2.label = "MockSctpDataChannel2"; expected_data_channel2.protocol = "udp"; @@ -1644,8 +1648,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCDataChannelStats) { expected_data_channel2.bytes_received = 12; pc_->AddSctpDataChannel(rtc::make_ref_counted( - 3, "MockSctpDataChannel3", DataChannelInterface::kClosed, "tcp", 13, 14, - 15, 16)); + data_channel_controller_->weak_ptr(), 3, "MockSctpDataChannel3", + DataChannelInterface::kClosed, "tcp", 13, 14, 15, 16)); RTCDataChannelStats expected_data_channel3("D3", Timestamp::Zero()); expected_data_channel3.label = "MockSctpDataChannel3"; expected_data_channel3.protocol = "tcp"; diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index 9c8be692bc..908e82567e 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -190,6 +190,7 @@ SctpDataChannel::SctpDataChannel( RTC_DCHECK_RUN_ON(signaling_thread_); RTC_UNUSED(network_thread_); RTC_DCHECK(config.IsValid()); + RTC_DCHECK(controller_); switch (config.open_handshake_role) { case InternalDataChannelInit::kNone: // pre-negotiated diff --git a/pc/test/mock_data_channel.h b/pc/test/mock_data_channel.h index eb63d3b9e4..659988d3d1 100644 --- a/pc/test/mock_data_channel.h +++ b/pc/test/mock_data_channel.h @@ -12,6 +12,7 @@ #define PC_TEST_MOCK_DATA_CHANNEL_H_ #include +#include #include "pc/sctp_data_channel.h" #include "test/gmock.h" @@ -20,8 +21,12 @@ namespace webrtc { class MockSctpDataChannel : public SctpDataChannel { public: - MockSctpDataChannel(int id, DataState state) - : MockSctpDataChannel(id, + MockSctpDataChannel( + rtc::WeakPtr controller, + int id, + DataState state) + : MockSctpDataChannel(std::move(controller), + id, "MockSctpDataChannel", state, "udp", @@ -30,6 +35,7 @@ class MockSctpDataChannel : public SctpDataChannel { 0, 0) {} MockSctpDataChannel( + rtc::WeakPtr controller, int id, const std::string& label, DataState state, @@ -42,7 +48,7 @@ class MockSctpDataChannel : public SctpDataChannel { rtc::Thread* signaling_thread = rtc::Thread::Current(), rtc::Thread* network_thread = rtc::Thread::Current()) : SctpDataChannel(config, - rtc::WeakPtr(), + std::move(controller), label, signaling_thread, network_thread) {