From bad5dadef3cf90d8ba081e7f814e913ecd2ff83d Mon Sep 17 00:00:00 2001 From: deadbeef Date: Tue, 17 Jan 2017 18:32:35 -0800 Subject: [PATCH] More minor improvements to BaseChannel/transport code. Mostly from late comments on this CL: https://codereview.webrtc.org/2614263002/ Changes SetTransport to DCHECK instead of returning false. Renames it to SetTransports. Fixes some possible transport resource leaks. BUG=None Review-Url: https://codereview.webrtc.org/2637503003 Cr-Commit-Position: refs/heads/master@{#16130} --- webrtc/api/rtcstatscollector_unittest.cc | 58 +++++++++-------- webrtc/api/webrtcsession.cc | 56 ++++++++++------ webrtc/api/webrtcsession.h | 1 - webrtc/p2p/base/transportcontroller.cc | 8 +++ webrtc/p2p/base/transportcontroller.h | 2 + webrtc/pc/channel.cc | 38 ++++------- webrtc/pc/channel.h | 17 +++-- webrtc/pc/channel_unittest.cc | 81 ++++++++++++++++++++++++ 8 files changed, 179 insertions(+), 82 deletions(-) diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc index 5860a3ece7..f8efb24f82 100644 --- a/webrtc/api/rtcstatscollector_unittest.cc +++ b/webrtc/api/rtcstatscollector_unittest.cc @@ -315,11 +315,11 @@ class RTCStatsCollectorTestHelper : public SetSessionDescriptionObserver { RTCStatsCollectorTestHelper() : worker_thread_(rtc::Thread::Current()), network_thread_(rtc::Thread::Current()), + signaling_thread_(rtc::Thread::Current()), media_engine_(new cricket::FakeMediaEngine()), - channel_manager_( - new cricket::ChannelManager(media_engine_, - worker_thread_, - network_thread_)), + channel_manager_(new cricket::ChannelManager(media_engine_, + worker_thread_, + network_thread_)), media_controller_( MediaControllerInterface::Create(cricket::MediaConfig(), worker_thread_, @@ -349,6 +349,7 @@ class RTCStatsCollectorTestHelper : public SetSessionDescriptionObserver { rtc::ScopedFakeClock& fake_clock() { return fake_clock_; } rtc::Thread* worker_thread() { return worker_thread_; } rtc::Thread* network_thread() { return network_thread_; } + rtc::Thread* signaling_thread() { return signaling_thread_; } cricket::FakeMediaEngine* media_engine() { return media_engine_; } MockWebRtcSession& session() { return session_; } MockPeerConnection& pc() { return pc_; } @@ -442,6 +443,7 @@ class RTCStatsCollectorTestHelper : public SetSessionDescriptionObserver { RtcEventLogNullImpl event_log_; rtc::Thread* const worker_thread_; rtc::Thread* const network_thread_; + rtc::Thread* const signaling_thread_; cricket::FakeMediaEngine* media_engine_; std::unique_ptr channel_manager_; std::unique_ptr media_controller_; @@ -697,15 +699,15 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsSingle) { TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - test_->media_engine(), voice_media_channel, "VoiceContentName", - kDefaultRtcpMuxRequired, kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), test_->media_engine(), voice_media_channel, + "VoiceContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired); MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - video_media_channel, "VideoContentName", kDefaultRtcpMuxRequired, - kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), video_media_channel, "VideoContentName", + kDefaultRtcpMuxRequired, kDefaultSrtpRequired); // Audio cricket::VoiceMediaInfo voice_media_info; @@ -1550,9 +1552,9 @@ TEST_F(RTCStatsCollectorTest, TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - test_->media_engine(), voice_media_channel, "VoiceContentName", - kDefaultRtcpMuxRequired, kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), test_->media_engine(), voice_media_channel, + "VoiceContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired); test_->SetupRemoteTrackAndReceiver( cricket::MEDIA_TYPE_AUDIO, "RemoteAudioTrackID", 1); @@ -1629,9 +1631,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - video_media_channel, "VideoContentName", kDefaultRtcpMuxRequired, - kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), video_media_channel, "VideoContentName", + kDefaultRtcpMuxRequired, kDefaultSrtpRequired); test_->SetupRemoteTrackAndReceiver( cricket::MEDIA_TYPE_VIDEO, "RemoteVideoTrackID", 1); @@ -1714,9 +1716,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - test_->media_engine(), voice_media_channel, "VoiceContentName", - kDefaultRtcpMuxRequired, kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), test_->media_engine(), voice_media_channel, + "VoiceContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired); test_->SetupLocalTrackAndSender( cricket::MEDIA_TYPE_AUDIO, "LocalAudioTrackID", 1); @@ -1787,9 +1789,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - video_media_channel, "VideoContentName", kDefaultRtcpMuxRequired, - kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), video_media_channel, "VideoContentName", + kDefaultRtcpMuxRequired, kDefaultSrtpRequired); test_->SetupLocalTrackAndSender( cricket::MEDIA_TYPE_VIDEO, "LocalVideoTrackID", 1); @@ -1870,14 +1872,14 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Default) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - test_->media_engine(), voice_media_channel, "VoiceContentName", - kDefaultRtcpMuxRequired, kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), test_->media_engine(), voice_media_channel, + "VoiceContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired); MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( - test_->worker_thread(), test_->network_thread(), nullptr, - video_media_channel, "VideoContentName", kDefaultRtcpMuxRequired, - kDefaultSrtpRequired); + test_->worker_thread(), test_->network_thread(), + test_->signaling_thread(), video_media_channel, "VideoContentName", + kDefaultRtcpMuxRequired, kDefaultSrtpRequired); cricket::VoiceMediaInfo voice_media_info; voice_media_info.senders.push_back(cricket::VoiceSenderInfo()); diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index a0c72c853e..8487d2cee1 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -1092,18 +1092,16 @@ bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) { transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } - if (!ch->SetTransport(rtp_transport, rtcp_transport)) { - LOG(LS_WARNING) << "Failed to enable BUNDLE for " << ch->content_name(); - return false; - } + ch->SetTransports(rtp_transport, rtcp_transport); LOG(LS_INFO) << "Enabled BUNDLE for " << ch->content_name() << " on " << transport_name << "."; - DestroyTransport(old_transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + transport_controller_->DestroyTransportChannel( + old_transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); // If the channel needs rtcp, it means that the channel used to have a // rtcp transport which needs to be deleted now. if (need_rtcp) { - DestroyTransport(old_transport_name, - cricket::ICE_CANDIDATE_COMPONENT_RTCP); + transport_controller_->DestroyTransportChannel( + old_transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } return true; }; @@ -1805,6 +1803,12 @@ bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content, transport_controller_->signaling_thread(), content->name, bundle_transport, require_rtcp_mux, SrtpRequired(), audio_options_)); if (!voice_channel_) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + if (rtcp_transport) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + } return false; } @@ -1842,6 +1846,12 @@ bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content, bundle_transport, require_rtcp_mux, SrtpRequired(), video_options_)); if (!video_channel_) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + if (rtcp_transport) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + } return false; } @@ -1901,6 +1911,12 @@ bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content, bundle_transport, require_rtcp_mux, SrtpRequired())); if (!rtp_data_channel_) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + if (rtcp_transport) { + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + } return false; } @@ -2365,14 +2381,6 @@ const std::string WebRtcSession::GetTransportName( return channel->transport_name(); } -void WebRtcSession::DestroyTransport(const std::string& transport_name, - int component) { - network_thread_->Invoke( - RTC_FROM_HERE, - rtc::Bind(&cricket::TransportController::DestroyTransportChannel_n, - transport_controller_.get(), transport_name, component)); -} - void WebRtcSession::DestroyRtcpTransport_n(const std::string& transport_name) { RTC_DCHECK(network_thread()->IsCurrent()); transport_controller_->DestroyTransportChannel_n( @@ -2386,9 +2394,11 @@ void WebRtcSession::DestroyVideoChannel() { transport_name = video_channel_->rtp_transport()->transport_name(); bool need_to_delete_rtcp = (video_channel_->rtcp_transport() != nullptr); channel_manager_->DestroyVideoChannel(video_channel_.release()); - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); if (need_to_delete_rtcp) { - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } } @@ -2399,9 +2409,11 @@ void WebRtcSession::DestroyVoiceChannel() { transport_name = voice_channel_->rtp_transport()->transport_name(); bool need_to_delete_rtcp = (voice_channel_->rtcp_transport() != nullptr); channel_manager_->DestroyVoiceChannel(voice_channel_.release()); - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); if (need_to_delete_rtcp) { - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } } @@ -2412,9 +2424,11 @@ void WebRtcSession::DestroyDataChannel() { transport_name = rtp_data_channel_->rtp_transport()->transport_name(); bool need_to_delete_rtcp = (rtp_data_channel_->rtcp_transport() != nullptr); channel_manager_->DestroyRtpDataChannel(rtp_data_channel_.release()); - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); if (need_to_delete_rtcp) { - DestroyTransport(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); + transport_controller_->DestroyTransportChannel( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } } } // namespace webrtc diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index f75328f5bb..0030342ee6 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -553,7 +553,6 @@ class WebRtcSession : const std::string GetTransportName(const std::string& content_name); - void DestroyTransport(const std::string& transport_name, int component); void DestroyRtcpTransport_n(const std::string& transport_name); void DestroyVideoChannel(); void DestroyVoiceChannel(); diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index 810b96e8d1..5dc4b97a0e 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -296,6 +296,14 @@ TransportChannel* TransportController::CreateTransportChannel_n( return dtls; } +void TransportController::DestroyTransportChannel( + const std::string& transport_name, + int component) { + network_thread_->Invoke( + RTC_FROM_HERE, rtc::Bind(&TransportController::DestroyTransportChannel_n, + this, transport_name, component)); +} + void TransportController::DestroyTransportChannel_n( const std::string& transport_name, int component) { diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index 147d568a2e..6929a6e7f1 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -121,6 +121,8 @@ class TransportController : public sigslot::has_slots<>, // Decrements a channel's reference count, and destroys the channel if // nothing is referencing it. + virtual void DestroyTransportChannel(const std::string& transport_name, + int component); virtual void DestroyTransportChannel_n(const std::string& transport_name, int component); diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 4d3f616212..872ee7fcb6 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -228,11 +228,7 @@ bool BaseChannel::Init_w(TransportChannel* rtp_transport, bool BaseChannel::InitNetwork_n(TransportChannel* rtp_transport, TransportChannel* rtcp_transport) { RTC_DCHECK(network_thread_->IsCurrent()); - // const std::string& transport_name = - // (bundle_transport_name ? *bundle_transport_name : content_name()); - if (!SetTransport_n(rtp_transport, rtcp_transport)) { - return false; - } + SetTransports_n(rtp_transport, rtcp_transport); if (!SetDtlsSrtpCryptoSuites_n(rtp_transport_, false)) { return false; @@ -256,30 +252,27 @@ void BaseChannel::Deinit() { RTC_FROM_HERE, Bind(&BaseChannel::DisconnectTransportChannels_n, this)); } -bool BaseChannel::SetTransport(TransportChannel* rtp_transport, - TransportChannel* rtcp_transport) { - return network_thread_->Invoke( +void BaseChannel::SetTransports(TransportChannel* rtp_transport, + TransportChannel* rtcp_transport) { + network_thread_->Invoke( RTC_FROM_HERE, - Bind(&BaseChannel::SetTransport_n, this, rtp_transport, rtcp_transport)); + Bind(&BaseChannel::SetTransports_n, this, rtp_transport, rtcp_transport)); } -bool BaseChannel::SetTransport_n(TransportChannel* rtp_transport, - TransportChannel* rtcp_transport) { +void BaseChannel::SetTransports_n(TransportChannel* rtp_transport, + TransportChannel* rtcp_transport) { RTC_DCHECK(network_thread_->IsCurrent()); - if (!rtp_transport && !rtcp_transport) { - LOG(LS_ERROR) << "Setting nullptr to RTP Transport and RTCP Transport."; - return false; - } - + // Verify some assumptions (as described in the comment above SetTransport). + RTC_DCHECK(rtp_transport); + RTC_DCHECK(NeedsRtcpTransport() == (rtcp_transport != nullptr)); if (rtcp_transport) { RTC_DCHECK(rtp_transport->transport_name() == rtcp_transport->transport_name()); - RTC_DCHECK(NeedsRtcpTransport()); } if (rtp_transport->transport_name() == transport_name_) { // Nothing to do if transport name isn't changing. - return true; + return; } transport_name_ = rtp_transport->transport_name(); @@ -300,17 +293,13 @@ bool BaseChannel::SetTransport_n(TransportChannel* rtp_transport, LOG(LS_INFO) << "Setting RTCP Transport for " << content_name() << " on " << transport_name() << " transport " << rtcp_transport; SetTransportChannel_n(true, rtcp_transport); - if (!rtcp_transport_) { - return false; - } + RTC_DCHECK(rtcp_transport_); } LOG(LS_INFO) << "Setting non-RTCP Transport for " << content_name() << " on " << transport_name() << " transport " << rtp_transport; SetTransportChannel_n(false, rtp_transport); - if (!rtp_transport_) { - return false; - } + RTC_DCHECK(rtp_transport_); // Update aggregate writable/ready-to-send state between RTP and RTCP upon // setting new transport channels. @@ -328,7 +317,6 @@ bool BaseChannel::SetTransport_n(TransportChannel* rtp_transport, rtp_transport_ && rtp_transport_->writable()); SetTransportChannelReadyToSend( true, rtcp_transport_ && rtcp_transport_->writable()); - return true; } void BaseChannel::SetTransportChannel_n(bool rtcp, diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index c7749a9715..4c289a2345 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -107,8 +107,14 @@ class BaseChannel bool writable() const { return writable_; } - bool SetTransport(TransportChannel* rtp_transport, - TransportChannel* rtcp_transport); + // Set the transport(s), and update writability and "ready-to-send" state. + // |rtp_transport| must be non-null. + // |rtcp_transport| must be supplied if NeedsRtcpTransport() is true (meaning + // RTCP muxing is not fully active yet). + // |rtp_transport| and |rtcp_transport| must share the same transport name as + // well. + void SetTransports(TransportChannel* rtp_transport, + TransportChannel* rtcp_transport); bool PushdownLocalDescription(const SessionDescription* local_desc, ContentAction action, std::string* error_desc); @@ -194,11 +200,8 @@ class BaseChannel protected: virtual MediaChannel* media_channel() const { return media_channel_; } - // Sets the |rtp_transport_| (and |rtcp_transport_|, if - // |rtcp_enabled_| is true). - // This method also updates writability and "ready-to-send" state. - bool SetTransport_n(TransportChannel* rtp_transport, - TransportChannel* rtcp_transport); + void SetTransports_n(TransportChannel* rtp_transport, + TransportChannel* rtcp_transport); // This does not update writability or "ready-to-send" state; it just // disconnects from the old channel and connects to the new one. diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index 3970588e83..110ea04b7a 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -3577,4 +3577,85 @@ TEST_F(RtpDataChannelDoubleThreadTest, TestSendData) { EXPECT_EQ("foo", media_channel1_->last_sent_data()); } +#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) + +// Verifies some DCHECKs are in place. +// Uses VoiceChannel, but any BaseChannel subclass would work. +class BaseChannelDeathTest : public testing::Test { + public: + BaseChannelDeathTest() + : // RTCP mux not required, SRTP required. + voice_channel_( + rtc::Thread::Current(), + rtc::Thread::Current(), + rtc::Thread::Current(), + &fake_media_engine_, + new cricket::FakeVoiceMediaChannel(nullptr, + cricket::AudioOptions()), + cricket::CN_AUDIO, + false, + true) { + rtp_transport_ = fake_transport_controller_.CreateTransportChannel( + "foo", cricket::ICE_CANDIDATE_COMPONENT_RTP); + rtcp_transport_ = fake_transport_controller_.CreateTransportChannel( + "foo", cricket::ICE_CANDIDATE_COMPONENT_RTCP); + EXPECT_TRUE(voice_channel_.Init_w(rtp_transport_, rtcp_transport_)); + } + + protected: + cricket::FakeTransportController fake_transport_controller_; + cricket::FakeMediaEngine fake_media_engine_; + cricket::VoiceChannel voice_channel_; + // Will be cleaned up by FakeTransportController, don't need to worry about + // deleting them in this test. + cricket::TransportChannel* rtp_transport_; + cricket::TransportChannel* rtcp_transport_; +}; + +TEST_F(BaseChannelDeathTest, SetTransportWithNullRtpTransport) { + cricket::TransportChannel* new_rtcp_transport = + fake_transport_controller_.CreateTransportChannel( + "bar", cricket::ICE_CANDIDATE_COMPONENT_RTCP); + EXPECT_DEATH(voice_channel_.SetTransports(nullptr, new_rtcp_transport), ""); +} + +TEST_F(BaseChannelDeathTest, SetTransportWithMissingRtcpTransport) { + cricket::TransportChannel* new_rtp_transport = + fake_transport_controller_.CreateTransportChannel( + "bar", cricket::ICE_CANDIDATE_COMPONENT_RTP); + EXPECT_DEATH(voice_channel_.SetTransports(new_rtp_transport, nullptr), ""); +} + +TEST_F(BaseChannelDeathTest, SetTransportWithUnneededRtcpTransport) { + // Activate RTCP muxing, simulating offer/answer negotiation. + cricket::AudioContentDescription content; + content.set_rtcp_mux(true); + ASSERT_TRUE(voice_channel_.SetLocalContent(&content, CA_OFFER, nullptr)); + ASSERT_TRUE(voice_channel_.SetRemoteContent(&content, CA_ANSWER, nullptr)); + cricket::TransportChannel* new_rtp_transport = + fake_transport_controller_.CreateTransportChannel( + "bar", cricket::ICE_CANDIDATE_COMPONENT_RTP); + cricket::TransportChannel* new_rtcp_transport = + fake_transport_controller_.CreateTransportChannel( + "bar", cricket::ICE_CANDIDATE_COMPONENT_RTCP); + // After muxing is enabled, no RTCP transport should be passed in here. + EXPECT_DEATH( + voice_channel_.SetTransports(new_rtp_transport, new_rtcp_transport), ""); +} + +// This test will probably go away if/when we move the transport name out of +// the transport classes and into their parent classes. +TEST_F(BaseChannelDeathTest, SetTransportWithMismatchingTransportNames) { + cricket::TransportChannel* new_rtp_transport = + fake_transport_controller_.CreateTransportChannel( + "bar", cricket::ICE_CANDIDATE_COMPONENT_RTP); + cricket::TransportChannel* new_rtcp_transport = + fake_transport_controller_.CreateTransportChannel( + "baz", cricket::ICE_CANDIDATE_COMPONENT_RTCP); + EXPECT_DEATH( + voice_channel_.SetTransports(new_rtp_transport, new_rtcp_transport), ""); +} + +#endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) + // TODO(pthatcher): TestSetReceiver?