generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Re-evalutes the ICE role on ICE restart.

Browse Source 
Also unifies the logic of ICE restart.

BUG=1775
R=juberti@google.com, juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/20619004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6510 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
jiayl@webrtc.org 2014-06-20 16:32:09 +00:00
parent 0b893b1e05
commit db397e5c6c
5 changed files with 131 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
talk/app/webrtc/webrtcsession.cc
View File

@ -430,8 +430,10 @@ class IceRestartAnswerLatch {
// No transport description exist. This is not an ice restart.
continue;
}
if (new_transport_desc->ice_pwd != old_transport_desc->ice_pwd &&
new_transport_desc->ice_ufrag != old_transport_desc->ice_ufrag) {
if (cricket::IceCredentialsChanged(old_transport_desc->ice_ufrag,
old_transport_desc->ice_pwd,
new_transport_desc->ice_ufrag,
new_transport_desc->ice_pwd)) {
LOG(LS_INFO) << "Remote peer request ice restart.";
ice_restart_ = true;
break;

3
talk/p2p/base/p2ptransportchannel.cc
View File

@ -259,7 +259,8 @@ void P2PTransportChannel::SetIceCredentials(const std::string& ice_ufrag,
if (!ice_ufrag_.empty() && !ice_pwd_.empty()) {
// Restart candidate allocation if there is any change in either
// ice ufrag or password.
ice_restart = (ice_ufrag_ != ice_ufrag) || (ice_pwd_!= ice_pwd);
ice_restart =
IceCredentialsChanged(ice_ufrag_, ice_pwd_, ice_ufrag, ice_pwd);
}
ice_ufrag_ = ice_ufrag;

27
talk/p2p/base/transport.cc
View File

@ -118,6 +118,23 @@ bool BadTransportDescription(const std::string& desc, std::string* err_desc) {
return false;
}
bool IceCredentialsChanged(const std::string& old_ufrag,
const std::string& old_pwd,
const std::string& new_ufrag,
const std::string& new_pwd) {
// TODO(jiayl): The standard (RFC 5245 Section 9.1.1.1) says that ICE should
// restart when both the ufrag and password are changed, but we do restart
// when either ufrag or passwrod is changed to keep compatible with GICE. We
// should clean this up when GICE is no longer used.
return (old_ufrag != new_ufrag) || (old_pwd != new_pwd);
}
static bool IceCredentialsChanged(const TransportDescription& old_desc,
const TransportDescription& new_desc) {
return IceCredentialsChanged(old_desc.ice_ufrag, old_desc.ice_pwd,
new_desc.ice_ufrag, new_desc.ice_pwd);
}
Transport::Transport(talk_base::Thread* signaling_thread,
talk_base::Thread* worker_thread,
const std::string& content_name,
@ -726,7 +743,17 @@ bool Transport::SetLocalTransportDescription_w(
error_desc);
}
if (local_description_ && IceCredentialsChanged(*local_description_, desc)) {
IceRole new_ice_role = (action == CA_OFFER) ? ICEROLE_CONTROLLING
: ICEROLE_CONTROLLED;
// It must be called before ApplyLocalTransportDescription_w, which may
// trigger an ICE restart and depends on the new ICE role.
SetIceRole_w(new_ice_role);
}
local_description_.reset(new TransportDescription(desc));
for (ChannelMap::iterator iter = channels_.begin();
iter != channels_.end(); ++iter) {
ret &= ApplyLocalTransportDescription_w(iter->second.get(), error_desc);

5
talk/p2p/base/transport.h
View File

@ -189,6 +189,11 @@ struct TransportStats {
bool BadTransportDescription(const std::string& desc, std::string* err_desc);
bool IceCredentialsChanged(const std::string& old_ufrag,
const std::string& old_pwd,
const std::string& new_ufrag,
const std::string& new_pwd);
class Transport : public talk_base::MessageHandler,
public sigslot::has_slots<> {
public:

93
talk/p2p/base/transport_unittest.cc
View File

@ -52,6 +52,9 @@ using talk_base::SocketAddress;
static const char kIceUfrag1[] = "TESTICEUFRAG0001";
static const char kIcePwd1[] = "TESTICEPWD00000000000001";
static const char kIceUfrag2[] = "TESTICEUFRAG0002";
static const char kIcePwd2[] = "TESTICEPWD00000000000002";
class TransportTest : public testing::Test,
public sigslot::has_slots<> {
public:
@ -184,6 +187,96 @@ TEST_F(TransportTest, TestChannelIceParameters) {
EXPECT_EQ(kIcePwd1, channel_->remote_ice_pwd());
}
// Verifies that IceCredentialsChanged returns true when either ufrag or pwd
// changed, and false in other cases.
TEST_F(TransportTest, TestIceCredentialsChanged) {
EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p2"));
EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p1"));
EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p2"));
EXPECT_FALSE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p1"));
}
// This test verifies that the callee's ICE role changes from controlled to
// controlling when the callee triggers an ICE restart.
TEST_F(TransportTest, TestIceControlledToControllingOnIceRestart) {
EXPECT_TRUE(SetupChannel());
transport_->SetIceRole(cricket::ICEROLE_CONTROLLED);
cricket::TransportDescription desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1);
ASSERT_TRUE(transport_->SetRemoteTransportDescription(desc,
cricket::CA_OFFER,
NULL));
ASSERT_TRUE(transport_->SetLocalTransportDescription(desc,
cricket::CA_ANSWER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLED, transport_->ice_role());
cricket::TransportDescription new_local_desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2);
ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc,
cricket::CA_OFFER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role());
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole());
}
// This test verifies that the caller's ICE role changes from controlling to
// controlled when the callee triggers an ICE restart.
TEST_F(TransportTest, TestIceControllingToControlledOnIceRestart) {
EXPECT_TRUE(SetupChannel());
transport_->SetIceRole(cricket::ICEROLE_CONTROLLING);
cricket::TransportDescription desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1);
ASSERT_TRUE(transport_->SetLocalTransportDescription(desc,
cricket::CA_OFFER,
NULL));
ASSERT_TRUE(transport_->SetRemoteTransportDescription(desc,
cricket::CA_ANSWER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role());
cricket::TransportDescription new_local_desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2);
ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc,
cricket::CA_ANSWER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLED, transport_->ice_role());
EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel_->GetIceRole());
}
// This test verifies that the caller's ICE role is still controlling after the
// callee triggers ICE restart if the callee's ICE mode is LITE.
TEST_F(TransportTest, TestIceControllingOnIceRestartIfRemoteIsIceLite) {
EXPECT_TRUE(SetupChannel());
transport_->SetIceRole(cricket::ICEROLE_CONTROLLING);
cricket::TransportDescription desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1);
ASSERT_TRUE(transport_->SetLocalTransportDescription(desc,
cricket::CA_OFFER,
NULL));
cricket::TransportDescription remote_desc(
cricket::NS_JINGLE_ICE_UDP, std::vector<std::string>(),
kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE,
cricket::CONNECTIONROLE_NONE, NULL, cricket::Candidates());
ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc,
cricket::CA_ANSWER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role());
cricket::TransportDescription new_local_desc(
cricket::NS_JINGLE_ICE_UDP, kIceUfrag2, kIcePwd2);
ASSERT_TRUE(transport_->SetLocalTransportDescription(new_local_desc,
cricket::CA_ANSWER,
NULL));
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role());
EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole());
}
// This test verifies that the Completed and Failed states can be reached.
TEST_F(TransportTest, TestChannelCompletedAndFailed) {
transport_->SetIceRole(cricket::ICEROLE_CONTROLLING);