diff --git a/pc/channel.h b/pc/channel.h index dbcdf9d1d7..fe3778b6cd 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -133,16 +133,6 @@ class BaseChannel : public ChannelInterface, return rtp_transport_ && rtp_transport_->IsSrtpActive(); } - // Version of the above that can be called from any thread. - bool SrtpActiveForTesting() const { - if (!network_thread_->IsCurrent()) { - return network_thread_->Invoke(RTC_FROM_HERE, - [this] { return srtp_active(); }); - } - RTC_DCHECK_RUN_ON(network_thread()); - return srtp_active(); - } - // Set an RTP level transport which could be an RtpTransport without // encryption, an SrtpTransport for SDES or a DtlsSrtpTransport for DTLS-SRTP. // This can be called from any thread and it hops to the network thread @@ -154,16 +144,6 @@ class BaseChannel : public ChannelInterface, return rtp_transport_; } - // Version of the above that can be called from any thread. - webrtc::RtpTransportInternal* RtpTransportForTesting() const { - if (!network_thread_->IsCurrent()) { - return network_thread_->Invoke( - RTC_FROM_HERE, [this] { return rtp_transport(); }); - } - RTC_DCHECK_RUN_ON(network_thread()); - return rtp_transport(); - } - // Channel control bool SetLocalContent(const MediaContentDescription* content, webrtc::SdpType type, @@ -207,12 +187,6 @@ class BaseChannel : public ChannelInterface, // RtpPacketSinkInterface overrides. void OnRtpPacket(const webrtc::RtpPacketReceived& packet) override; - // Used by the RTCStatsCollector tests to set the transport name without - // creating RtpTransports. - void set_transport_name_for_testing(const std::string& transport_name) { - transport_name_ = transport_name; - } - MediaChannel* media_channel() const override { return media_channel_.get(); } diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 7ff25a9466..e85500ac70 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -510,12 +510,31 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Base implementation. } + // Utility method that calls BaseChannel::srtp_active() on the network thread + // and returns the result. The |srtp_active()| state is maintained on the + // network thread, which callers need to factor in. + bool IsSrtpActive(std::unique_ptr& channel) { + RTC_DCHECK(channel.get()); + return network_thread_->Invoke( + RTC_FROM_HERE, [&] { return channel->srtp_active(); }); + } + + // Returns true iff the transport is set for a channel and rtcp_mux_enabled() + // returns true. + bool IsRtcpMuxEnabled(std::unique_ptr& channel) { + RTC_DCHECK(channel.get()); + return network_thread_->Invoke(RTC_FROM_HERE, [&] { + return channel->rtp_transport() && + channel->rtp_transport()->rtcp_mux_enabled(); + }); + } + // Tests that can be used by derived classes. // Basic sanity check. void TestInit() { CreateChannels(0, 0); - EXPECT_FALSE(channel1_->SrtpActiveForTesting()); + EXPECT_FALSE(IsSrtpActive(channel1_)); EXPECT_FALSE(media_channel1_->sending()); if (verify_playout_) { EXPECT_FALSE(media_channel1_->playout()); @@ -857,14 +876,14 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Test setting up a call. void TestCallSetup() { CreateChannels(0, 0); - EXPECT_FALSE(channel1_->SrtpActiveForTesting()); + EXPECT_FALSE(IsSrtpActive(channel1_)); EXPECT_TRUE(SendInitiate()); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); } EXPECT_FALSE(media_channel1_->sending()); EXPECT_TRUE(SendAccept()); - EXPECT_FALSE(channel1_->SrtpActiveForTesting()); + EXPECT_FALSE(IsSrtpActive(channel1_)); EXPECT_TRUE(media_channel1_->sending()); EXPECT_EQ(1U, media_channel1_->codecs().size()); if (verify_playout_) { @@ -901,8 +920,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(IsRtcpMuxEnabled(channel1_)); + EXPECT_TRUE(IsRtcpMuxEnabled(channel2_)); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -925,13 +944,13 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { void SendDtlsSrtpToDtlsSrtp(int flags1, int flags2) { CreateChannels(flags1 | DTLS, flags2 | DTLS); - EXPECT_FALSE(channel1_->SrtpActiveForTesting()); - EXPECT_FALSE(channel2_->SrtpActiveForTesting()); + EXPECT_FALSE(IsSrtpActive(channel1_)); + EXPECT_FALSE(IsSrtpActive(channel2_)); EXPECT_TRUE(SendInitiate()); WaitForThreads(); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->SrtpActiveForTesting()); - EXPECT_TRUE(channel2_->SrtpActiveForTesting()); + EXPECT_TRUE(IsSrtpActive(channel1_)); + EXPECT_TRUE(IsSrtpActive(channel2_)); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -949,10 +968,10 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(SSRC_MUX | RTCP_MUX | DTLS, SSRC_MUX | RTCP_MUX | DTLS); EXPECT_TRUE(SendOffer()); EXPECT_TRUE(SendProvisionalAnswer()); - EXPECT_TRUE(channel1_->SrtpActiveForTesting()); - EXPECT_TRUE(channel2_->SrtpActiveForTesting()); - EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(IsSrtpActive(channel1_)); + EXPECT_TRUE(IsSrtpActive(channel2_)); + EXPECT_TRUE(IsRtcpMuxEnabled(channel1_)); + EXPECT_TRUE(IsRtcpMuxEnabled(channel2_)); WaitForThreads(); // Wait for 'sending' flag go through network thread. SendCustomRtp1(kSsrc1, ++sequence_number1_1); WaitForThreads(); @@ -965,8 +984,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Complete call setup and ensure everything is still OK. EXPECT_TRUE(SendFinalAnswer()); - EXPECT_TRUE(channel1_->SrtpActiveForTesting()); - EXPECT_TRUE(channel2_->SrtpActiveForTesting()); + EXPECT_TRUE(IsSrtpActive(channel1_)); + EXPECT_TRUE(IsSrtpActive(channel2_)); SendCustomRtp1(kSsrc1, ++sequence_number1_1); SendCustomRtp2(kSsrc2, ++sequence_number2_2); WaitForThreads(); @@ -995,8 +1014,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(IsRtcpMuxEnabled(channel1_)); + EXPECT_TRUE(IsRtcpMuxEnabled(channel2_)); SendRtp1(); SendRtp2(); WaitForThreads(); diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index 70f8dd50a1..f51a69a04c 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -75,6 +75,64 @@ class FakeVideoMediaChannelForStats : public cricket::FakeVideoMediaChannel { constexpr bool kDefaultRtcpMuxRequired = true; constexpr bool kDefaultSrtpRequired = true; +class VoiceChannelForTesting : public cricket::VoiceChannel { + public: + VoiceChannelForTesting(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr channel, + const std::string& content_name, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, + std::string transport_name) + : VoiceChannel(worker_thread, + network_thread, + signaling_thread, + std::move(channel), + content_name, + srtp_required, + std::move(crypto_options), + ssrc_generator), + test_transport_name_(std::move(transport_name)) {} + + private: + const std::string& transport_name() const override { + return test_transport_name_; + } + + const std::string test_transport_name_; +}; + +class VideoChannelForTesting : public cricket::VideoChannel { + public: + VideoChannelForTesting(rtc::Thread* worker_thread, + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + std::unique_ptr channel, + const std::string& content_name, + bool srtp_required, + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, + std::string transport_name) + : VideoChannel(worker_thread, + network_thread, + signaling_thread, + std::move(channel), + content_name, + srtp_required, + std::move(crypto_options), + ssrc_generator), + test_transport_name_(std::move(transport_name)) {} + + private: + const std::string& transport_name() const override { + return test_transport_name_; + } + + const std::string test_transport_name_; +}; + // This class is intended to be fed into the StatsCollector and // RTCStatsCollector so that the stats functionality can be unit tested. // Individual tests can configure this fake as needed to simulate scenarios @@ -140,11 +198,10 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { auto voice_media_channel = std::make_unique(); auto* voice_media_channel_ptr = voice_media_channel.get(); - voice_channel_ = std::make_unique( + voice_channel_ = std::make_unique( worker_thread_, network_thread_, signaling_thread_, std::move(voice_media_channel), mid, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_); - voice_channel_->set_transport_name_for_testing(transport_name); + webrtc::CryptoOptions(), &ssrc_generator_, transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) ->internal() ->SetChannel(voice_channel_.get()); @@ -158,11 +215,10 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { auto video_media_channel = std::make_unique(); auto video_media_channel_ptr = video_media_channel.get(); - video_channel_ = std::make_unique( + video_channel_ = std::make_unique( worker_thread_, network_thread_, signaling_thread_, std::move(video_media_channel), mid, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_); - video_channel_->set_transport_name_for_testing(transport_name); + webrtc::CryptoOptions(), &ssrc_generator_, transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) ->internal() ->SetChannel(video_channel_.get());