zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove ForTesting methods from BaseChannel

Browse Source 
The testing code prevents the production code from protecting the
member variables properly. The convenience methods for testing
purposes, can be located with the testing code.

Bug: none
Change-Id: Ieda248a199db84336dfafbd66c93c35508ab2582
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/213661
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33635}
This commit is contained in:
Tomas Gunnarsson
2021-04-06 11:26:47 +02:00
committed by Commit Bot
parent 3b4dd4c71a
commit 6cd508196a
3 changed files with 98 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
pc/channel.h
View File

@ -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<bool>(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<webrtc::RtpTransportInternal*>(
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();
}

53
pc/channel_unittest.cc
View File

@ -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<typename T::Channel>& channel) {
RTC_DCHECK(channel.get());
return network_thread_->Invoke<bool>(
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<typename T::Channel>& channel) {
RTC_DCHECK(channel.get());
return network_thread_->Invoke<bool>(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();

68
pc/test/fake_peer_connection_for_stats.h
View File

@ -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<cricket::VoiceMediaChannel> 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<cricket::VideoMediaChannel> 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<FakeVoiceMediaChannelForStats>();
auto* voice_media_channel_ptr = voice_media_channel.get();
voice_channel_ = std::make_unique<cricket::VoiceChannel>(
voice_channel_ = std::make_unique<VoiceChannelForTesting>(
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<FakeVideoMediaChannelForStats>();
auto video_media_channel_ptr = video_media_channel.get();
video_channel_ = std::make_unique<cricket::VideoChannel>(
video_channel_ = std::make_unique<VideoChannelForTesting>(
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());