From 476859d38bddb204640148a5c03aa845e26459a8 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 7 Dec 2020 11:16:45 +0000 Subject: [PATCH] Stop threadjumping to get RTP transport in channel.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This moves the code for threadjumping to get the RTP transport despite its thread guard from the main function to two functions marked especially "ForTesting". Bug: webrtc:12230 Change-Id: I4473ed38e6fdedb05e2fbc97c2521bc1993fdd1d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/196521 Commit-Queue: Harald Alvestrand Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/master@{#32792} --- pc/channel.h | 20 ++++++++++++++------ pc/channel_unittest.cc | 34 +++++++++++++++++----------------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/pc/channel.h b/pc/channel.h index d9e6d9b728..1fb2a3978c 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -106,14 +106,18 @@ class BaseChannel : public ChannelInterface, // This function returns true if using SRTP (DTLS-based keying or SDES). bool srtp_active() const { - // TODO(bugs.webrtc.org/12230): At least some tests call this function - // from other threads. + RTC_DCHECK_RUN_ON(network_thread()); + 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 rtp_transport_ && rtp_transport_->IsSrtpActive(); + return srtp_active(); } bool writable() const { return writable_; } @@ -125,14 +129,18 @@ class BaseChannel : public ChannelInterface, bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) override; webrtc::RtpTransportInternal* rtp_transport() const { - // TODO(bugs.webrtc.org/12230): At least some tests call this function - // from other threads. + RTC_DCHECK_RUN_ON(network_thread()); + 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_; + return rtp_transport(); } // Channel control diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 4eef70f4ea..c4071475d0 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -501,7 +501,7 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Basic sanity check. void TestInit() { CreateChannels(0, 0); - EXPECT_FALSE(channel1_->srtp_active()); + EXPECT_FALSE(channel1_->SrtpActiveForTesting()); EXPECT_FALSE(media_channel1_->sending()); if (verify_playout_) { EXPECT_FALSE(media_channel1_->playout()); @@ -843,14 +843,14 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // Test setting up a call. void TestCallSetup() { CreateChannels(0, 0); - EXPECT_FALSE(channel1_->srtp_active()); + EXPECT_FALSE(channel1_->SrtpActiveForTesting()); EXPECT_TRUE(SendInitiate()); if (verify_playout_) { EXPECT_TRUE(media_channel1_->playout()); } EXPECT_FALSE(media_channel1_->sending()); EXPECT_TRUE(SendAccept()); - EXPECT_FALSE(channel1_->srtp_active()); + EXPECT_FALSE(channel1_->SrtpActiveForTesting()); EXPECT_TRUE(media_channel1_->sending()); EXPECT_EQ(1U, media_channel1_->codecs().size()); if (verify_playout_) { @@ -887,8 +887,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -911,15 +911,15 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { void SendDtlsSrtpToDtlsSrtp(int flags1, int flags2) { CreateChannels(flags1 | DTLS, flags2 | DTLS); - EXPECT_FALSE(channel1_->srtp_active()); - EXPECT_FALSE(channel2_->srtp_active()); + EXPECT_FALSE(channel1_->SrtpActiveForTesting()); + EXPECT_FALSE(channel2_->SrtpActiveForTesting()); EXPECT_TRUE(SendInitiate()); WaitForThreads(); EXPECT_TRUE(channel1_->writable()); EXPECT_TRUE(channel2_->writable()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->srtp_active()); - EXPECT_TRUE(channel2_->srtp_active()); + EXPECT_TRUE(channel1_->SrtpActiveForTesting()); + EXPECT_TRUE(channel2_->SrtpActiveForTesting()); SendRtp1(); SendRtp2(); WaitForThreads(); @@ -937,10 +937,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_->srtp_active()); - EXPECT_TRUE(channel2_->srtp_active()); - EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel1_->SrtpActiveForTesting()); + EXPECT_TRUE(channel2_->SrtpActiveForTesting()); + EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); WaitForThreads(); // Wait for 'sending' flag go through network thread. SendCustomRtp1(kSsrc1, ++sequence_number1_1); WaitForThreads(); @@ -953,8 +953,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_->srtp_active()); - EXPECT_TRUE(channel2_->srtp_active()); + EXPECT_TRUE(channel1_->SrtpActiveForTesting()); + EXPECT_TRUE(channel2_->SrtpActiveForTesting()); SendCustomRtp1(kSsrc1, ++sequence_number1_1); SendCustomRtp2(kSsrc2, ++sequence_number2_2); WaitForThreads(); @@ -983,8 +983,8 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(RTCP_MUX, RTCP_MUX); EXPECT_TRUE(SendInitiate()); EXPECT_TRUE(SendAccept()); - EXPECT_TRUE(channel1_->rtp_transport()->rtcp_mux_enabled()); - EXPECT_TRUE(channel2_->rtp_transport()->rtcp_mux_enabled()); + EXPECT_TRUE(channel1_->RtpTransportForTesting()->rtcp_mux_enabled()); + EXPECT_TRUE(channel2_->RtpTransportForTesting()->rtcp_mux_enabled()); SendRtp1(); SendRtp2(); WaitForThreads();