Update ChannelManagerTest suite to use separate threads.

Before the tests were using the current thread for three roles,
signaling, worker and network.

Also, removing redundant test and unnecessary setters for test.

Bug: none
Change-Id: Id132b6290b78765dc075ede9483dd2d12b201130
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/212615
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33612}
This commit is contained in:
Tomas Gunnarsson
2021-03-30 23:47:49 +02:00
committed by Commit Bot
parent 3278a71343
commit b620e2d3ec
3 changed files with 41 additions and 76 deletions

View File

@ -41,6 +41,7 @@ ChannelManager::ChannelManager(
}
ChannelManager::~ChannelManager() {
RTC_DCHECK_RUN_ON(main_thread_);
if (initialized_) {
Terminate();
}
@ -50,6 +51,7 @@ ChannelManager::~ChannelManager() {
}
bool ChannelManager::SetVideoRtxEnabled(bool enable) {
RTC_DCHECK_RUN_ON(main_thread_);
// To be safe, this call is only allowed before initialization. Apps like
// Flute only have a singleton ChannelManager and we don't want this flag to
// be toggled between calls or when there's concurrent calls. We expect apps
@ -119,7 +121,13 @@ void ChannelManager::GetSupportedDataCodecs(
*codecs = data_engine_->data_codecs();
}
bool ChannelManager::initialized() const {
RTC_DCHECK_RUN_ON(main_thread_);
return initialized_;
}
bool ChannelManager::Init() {
RTC_DCHECK_RUN_ON(main_thread_);
RTC_DCHECK(!initialized_);
if (initialized_) {
return false;
@ -171,6 +179,7 @@ ChannelManager::GetSupportedVideoRtpHeaderExtensions() const {
}
void ChannelManager::Terminate() {
RTC_DCHECK_RUN_ON(main_thread_);
RTC_DCHECK(initialized_);
if (!initialized_) {
return;
@ -206,7 +215,6 @@ VoiceChannel* ChannelManager::CreateVoiceChannel(
}
RTC_DCHECK_RUN_ON(worker_thread_);
RTC_DCHECK(initialized_);
RTC_DCHECK(call);
if (!media_engine_) {
return nullptr;
@ -241,8 +249,6 @@ void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel) {
return;
}
RTC_DCHECK(initialized_);
auto it = absl::c_find_if(voice_channels_,
[&](const std::unique_ptr<VoiceChannel>& p) {
return p.get() == voice_channel;
@ -279,7 +285,6 @@ VideoChannel* ChannelManager::CreateVideoChannel(
}
RTC_DCHECK_RUN_ON(worker_thread_);
RTC_DCHECK(initialized_);
RTC_DCHECK(call);
if (!media_engine_) {
return nullptr;
@ -315,8 +320,6 @@ void ChannelManager::DestroyVideoChannel(VideoChannel* video_channel) {
return;
}
RTC_DCHECK(initialized_);
auto it = absl::c_find_if(video_channels_,
[&](const std::unique_ptr<VideoChannel>& p) {
return p.get() == video_channel;
@ -346,7 +349,6 @@ RtpDataChannel* ChannelManager::CreateRtpDataChannel(
}
// This is ok to alloc from a thread other than the worker thread.
RTC_DCHECK(initialized_);
DataMediaChannel* media_channel = data_engine_->CreateChannel(media_config);
if (!media_channel) {
RTC_LOG(LS_WARNING) << "Failed to create RTP data channel.";
@ -377,8 +379,6 @@ void ChannelManager::DestroyRtpDataChannel(RtpDataChannel* data_channel) {
return;
}
RTC_DCHECK(initialized_);
auto it = absl::c_find_if(data_channels_,
[&](const std::unique_ptr<RtpDataChannel>& p) {
return p.get() == data_channel;

View File

@ -52,25 +52,8 @@ class ChannelManager final {
rtc::Thread* network_thread);
~ChannelManager();
// Accessors for the worker thread, allowing it to be set after construction,
// but before Init. set_worker_thread will return false if called after Init.
rtc::Thread* worker_thread() const { return worker_thread_; }
bool set_worker_thread(rtc::Thread* thread) {
if (initialized_) {
return false;
}
worker_thread_ = thread;
return true;
}
rtc::Thread* network_thread() const { return network_thread_; }
bool set_network_thread(rtc::Thread* thread) {
if (initialized_) {
return false;
}
network_thread_ = thread;
return true;
}
MediaEngineInterface* media_engine() { return media_engine_.get(); }
// Retrieves the list of supported audio & video codec types.
@ -88,7 +71,7 @@ class ChannelManager final {
GetSupportedVideoRtpHeaderExtensions() const;
// Indicates whether the media engine is started.
bool initialized() const { return initialized_; }
bool initialized() const;
// Starts up the media engine.
bool Init();
// Shuts down the media engine.
@ -165,10 +148,10 @@ class ChannelManager final {
private:
std::unique_ptr<MediaEngineInterface> media_engine_; // Nullable.
std::unique_ptr<DataEngineInterface> data_engine_; // Non-null.
bool initialized_ = false;
rtc::Thread* main_thread_;
rtc::Thread* worker_thread_;
rtc::Thread* network_thread_;
bool initialized_ RTC_GUARDED_BY(main_thread_) = false;
rtc::Thread* const main_thread_;
rtc::Thread* const worker_thread_;
rtc::Thread* const network_thread_;
// Vector contents are non-null.
std::vector<std::unique_ptr<VoiceChannel>> voice_channels_;

View File

@ -55,33 +55,39 @@ class ChannelManagerTest : public ::testing::Test {
cm_(new cricket::ChannelManager(
std::unique_ptr<MediaEngineInterface>(fme_),
std::unique_ptr<DataEngineInterface>(fdme_),
rtc::Thread::Current(),
rtc::Thread::Current())),
worker_.get(),
network_.get())),
fake_call_() {
fme_->SetAudioCodecs(MAKE_VECTOR(kAudioCodecs));
fme_->SetVideoCodecs(MAKE_VECTOR(kVideoCodecs));
network_->SetName("Network", this);
worker_->SetName("Worker", this);
network_->Start();
worker_->Start();
}
void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) {
cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel(
&fake_call_, cricket::MediaConfig(), rtp_transport,
rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired,
webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions());
EXPECT_TRUE(voice_channel != nullptr);
cricket::VideoChannel* video_channel = cm_->CreateVideoChannel(
&fake_call_, cricket::MediaConfig(), rtp_transport,
rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired,
webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(),
video_bitrate_allocator_factory_.get());
EXPECT_TRUE(video_channel != nullptr);
cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel(
cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(),
cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions(),
&ssrc_generator_);
EXPECT_TRUE(rtp_data_channel != nullptr);
cm_->DestroyVideoChannel(video_channel);
cm_->DestroyVoiceChannel(voice_channel);
cm_->DestroyRtpDataChannel(rtp_data_channel);
worker_->Invoke<void>(RTC_FROM_HERE, [this, rtp_transport]() {
cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel(
&fake_call_, cricket::MediaConfig(), rtp_transport,
rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired,
webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions());
EXPECT_TRUE(voice_channel != nullptr);
cricket::VideoChannel* video_channel = cm_->CreateVideoChannel(
&fake_call_, cricket::MediaConfig(), rtp_transport,
rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired,
webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(),
video_bitrate_allocator_factory_.get());
EXPECT_TRUE(video_channel != nullptr);
cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel(
cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(),
cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions(),
&ssrc_generator_);
EXPECT_TRUE(rtp_data_channel != nullptr);
cm_->DestroyVideoChannel(video_channel);
cm_->DestroyVoiceChannel(voice_channel);
cm_->DestroyRtpDataChannel(rtp_data_channel);
});
cm_->Terminate();
}
@ -100,7 +106,6 @@ class ChannelManagerTest : public ::testing::Test {
// Test that we startup/shutdown properly.
TEST_F(ChannelManagerTest, StartupShutdown) {
EXPECT_FALSE(cm_->initialized());
EXPECT_EQ(rtc::Thread::Current(), cm_->worker_thread());
EXPECT_TRUE(cm_->Init());
EXPECT_TRUE(cm_->initialized());
cm_->Terminate();
@ -109,19 +114,11 @@ TEST_F(ChannelManagerTest, StartupShutdown) {
// Test that we startup/shutdown properly with a worker thread.
TEST_F(ChannelManagerTest, StartupShutdownOnThread) {
network_->Start();
worker_->Start();
EXPECT_FALSE(cm_->initialized());
EXPECT_EQ(rtc::Thread::Current(), cm_->worker_thread());
EXPECT_TRUE(cm_->set_network_thread(network_.get()));
EXPECT_EQ(network_.get(), cm_->network_thread());
EXPECT_TRUE(cm_->set_worker_thread(worker_.get()));
EXPECT_EQ(worker_.get(), cm_->worker_thread());
EXPECT_TRUE(cm_->Init());
EXPECT_TRUE(cm_->initialized());
// Setting the network or worker thread while initialized should fail.
EXPECT_FALSE(cm_->set_network_thread(rtc::Thread::Current()));
EXPECT_FALSE(cm_->set_worker_thread(rtc::Thread::Current()));
cm_->Terminate();
EXPECT_FALSE(cm_->initialized());
}
@ -166,21 +163,6 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) {
}
TEST_F(ChannelManagerTest, CreateDestroyChannels) {
EXPECT_TRUE(cm_->Init());
auto rtp_dtls_transport = std::make_unique<FakeDtlsTransport>(
"fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP);
auto dtls_srtp_transport = std::make_unique<webrtc::DtlsSrtpTransport>(
/*rtcp_mux_required=*/true);
dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport.get(),
/*rtcp_dtls_transport=*/nullptr);
TestCreateDestroyChannels(dtls_srtp_transport.get());
}
TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) {
network_->Start();
worker_->Start();
EXPECT_TRUE(cm_->set_worker_thread(worker_.get()));
EXPECT_TRUE(cm_->set_network_thread(network_.get()));
EXPECT_TRUE(cm_->Init());
auto rtp_dtls_transport = std::make_unique<FakeDtlsTransport>(
"fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP,