diff --git a/pc/peerconnection_ice_unittest.cc b/pc/peerconnection_ice_unittest.cc index 3ab9acb267..52de1f65a4 100644 --- a/pc/peerconnection_ice_unittest.cc +++ b/pc/peerconnection_ice_unittest.cc @@ -12,6 +12,7 @@ #include "p2p/base/teststunserver.h" #include "p2p/client/basicportallocator.h" #include "pc/mediasession.h" +#include "pc/peerconnection.h" #include "pc/peerconnectionwrapper.h" #include "pc/sdputils.h" #ifdef WEBRTC_ANDROID @@ -19,6 +20,7 @@ #endif #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" +#include "api/peerconnectionproxy.h" #include "pc/test/fakeaudiocapturemodule.h" #include "rtc_base/fakenetwork.h" #include "rtc_base/gunit.h" @@ -152,6 +154,16 @@ class PeerConnectionIceUnitTest : public ::testing::Test { } } + // Set ICE mode on the given session description. + void SetIceMode(SessionDescriptionInterface* sdesc, + const cricket::IceMode ice_mode) { + auto* desc = sdesc->description(); + for (const auto& content : desc->contents()) { + auto* transport_info = desc->GetTransportInfoByName(content.name); + transport_info->description.ice_mode = ice_mode; + } + } + cricket::TransportDescription* GetFirstTransportDescription( SessionDescriptionInterface* sdesc) { auto* desc = sdesc->description(); @@ -172,6 +184,20 @@ class PeerConnectionIceUnitTest : public ::testing::Test { return &transport_info->description; } + // TODO(qingsi): Rewrite this method in terms of the standard IceTransport + // after it is implemented. + cricket::IceRole GetIceRole(const WrapperPtr& pc_wrapper_ptr) { + auto* pc_proxy = reinterpret_cast< + PeerConnectionProxyWithInternal*>( + pc_wrapper_ptr->pc()); + PeerConnection* pc = + reinterpret_cast(pc_proxy->internal()); + return pc->voice_channel() + ->rtp_dtls_transport() + ->ice_transport() + ->GetIceRole(); + } + bool AddCandidateToFirstTransport(cricket::Candidate* candidate, SessionDescriptionInterface* sdesc) { auto* desc = sdesc->description(); @@ -788,4 +814,53 @@ TEST_F(PeerConnectionIceUnitTest, local_transports[1].description.ice_pwd); } +// Test that when the initial offerer (caller) uses the lite implementation of +// ICE and the callee uses the full implementation, the caller takes the +// CONTROLLED role and the callee takes the CONTROLLING role. This is specified +// in RFC5245 Section 5.1.1. +TEST_F(PeerConnectionIceUnitTest, + OfferFromLiteIceControlledAndAnswerFromFullIceControlling) { + auto caller = CreatePeerConnectionWithAudioVideo(); + auto callee = CreatePeerConnectionWithAudioVideo(); + + auto offer = caller->CreateOffer(); + SetIceMode(offer.get(), cricket::IceMode::ICEMODE_LITE); + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); + + auto answer = callee->CreateAnswer(); + SetIceMode(answer.get(), cricket::IceMode::ICEMODE_FULL); + ASSERT_TRUE( + callee->SetLocalDescription(CloneSessionDescription(answer.get()))); + ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer))); + + EXPECT_EQ(cricket::ICEROLE_CONTROLLED, GetIceRole(caller)); + EXPECT_EQ(cricket::ICEROLE_CONTROLLING, GetIceRole(callee)); +} + +// Test that when the caller and the callee both use the lite implementation of +// ICE, the initial offerer (caller) takes the CONTROLLING role and the callee +// takes the CONTROLLED role. This is specified in RFC5245 Section 5.1.1. +TEST_F(PeerConnectionIceUnitTest, + OfferFromLiteIceControllingAndAnswerFromLiteIceControlled) { + auto caller = CreatePeerConnectionWithAudioVideo(); + auto callee = CreatePeerConnectionWithAudioVideo(); + + auto offer = caller->CreateOffer(); + SetIceMode(offer.get(), cricket::IceMode::ICEMODE_LITE); + ASSERT_TRUE( + caller->SetLocalDescription(CloneSessionDescription(offer.get()))); + ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer))); + + auto answer = callee->CreateAnswer(); + SetIceMode(answer.get(), cricket::IceMode::ICEMODE_LITE); + ASSERT_TRUE( + callee->SetLocalDescription(CloneSessionDescription(answer.get()))); + ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer))); + + EXPECT_EQ(cricket::ICEROLE_CONTROLLING, GetIceRole(caller)); + EXPECT_EQ(cricket::ICEROLE_CONTROLLED, GetIceRole(callee)); +} + } // namespace webrtc diff --git a/pc/transportcontroller.cc b/pc/transportcontroller.cc index 7cf9bbd3bd..d58564ce2f 100644 --- a/pc/transportcontroller.cc +++ b/pc/transportcontroller.cc @@ -597,6 +597,19 @@ bool TransportController::SetLocalTransportDescription_n( return true; } + // The initial offer side may use ICE Lite, in which case, per RFC5245 Section + // 5.1.1, the answer side should take the controlling role if it is in the + // full ICE mode. + // + // When both sides use ICE Lite, the initial offer side must take the + // controlling role, and this is the default logic implemented in + // SetLocalDescription in PeerConnection. + if (transport->remote_description() && + transport->remote_description()->ice_mode == ICEMODE_LITE && + ice_role_ == ICEROLE_CONTROLLED && tdesc.ice_mode == ICEMODE_FULL) { + SetIceRole_n(ICEROLE_CONTROLLING); + } + // Older versions of Chrome expect the ICE role to be re-determined when an // ICE restart occurs, and also don't perform conflict resolution correctly, // so for now we can't safely stop doing this, unless the application opts in @@ -645,6 +658,15 @@ bool TransportController::SetRemoteTransportDescription_n( return true; } + // If we use ICE Lite and the remote endpoint uses the full implementation of + // ICE, the local endpoint must take the controlled role, and the other side + // must be the controlling role. + if (transport->local_description() && + transport->local_description()->ice_mode == ICEMODE_LITE && + ice_role_ == ICEROLE_CONTROLLING && tdesc.ice_mode == ICEMODE_FULL) { + SetIceRole_n(ICEROLE_CONTROLLED); + } + RTC_LOG(LS_INFO) << "Set remote transport description on " << transport_name; return transport->SetRemoteTransportDescription(tdesc, action, err); }