Discard old-generation candidates when ICE restarts

The existing code only do so on the controlled side.

BUG=webrtc:5291
R=pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1496693002 .

Cr-Commit-Position: refs/heads/master@{#10993}
This commit is contained in:
Honghai Zhang 2015-12-11 14:26:22 -08:00
parent 43e4e23eeb
commit 04e9146e58
2 changed files with 69 additions and 6 deletions

View File

@ -492,9 +492,13 @@ class IceRestartAnswerLatch {
}
}
// This method has two purposes: 1. Return whether |new_desc| requests
// an ICE restart (i.e., new ufrag/pwd). 2. If it requests an ICE restart
// and it is an OFFER, remember this in |ice_restart_| so that the next
// Local Answer will be created with new ufrag and pwd.
bool CheckForRemoteIceRestart(const SessionDescriptionInterface* old_desc,
const SessionDescriptionInterface* new_desc) {
if (!old_desc || new_desc->type() != SessionDescriptionInterface::kOffer) {
if (!old_desc) {
return false;
}
const SessionDescription* new_sd = new_desc->description();
@ -520,7 +524,9 @@ class IceRestartAnswerLatch {
new_transport_desc->ice_ufrag,
new_transport_desc->ice_pwd)) {
LOG(LS_INFO) << "Remote peer request ice restart.";
ice_restart_ = true;
if (new_desc->type() == SessionDescriptionInterface::kOffer) {
ice_restart_ = true;
}
return true;
}
}

View File

@ -2806,10 +2806,9 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) {
EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error));
}
// Test that if the remote description indicates the peer requested ICE restart
// (via a new ufrag or pwd), the old ICE candidates are not copied,
// and vice versa.
TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithIceRestart) {
// Test that if the remote offer indicates the peer requested ICE restart (via
// a new ufrag or pwd), the old ICE candidates are not copied, and vice versa.
TEST_F(WebRtcSessionTest, TestSetRemoteOfferWithIceRestart) {
Init();
scoped_ptr<SessionDescriptionInterface> offer(CreateRemoteOffer());
@ -2863,6 +2862,64 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithIceRestart) {
EXPECT_EQ(0, session_->remote_description()->candidates(0)->count());
}
// Test that if the remote answer indicates the peer requested ICE restart (via
// a new ufrag or pwd), the old ICE candidates are not copied, and vice versa.
TEST_F(WebRtcSessionTest, TestSetRemoteAnswerWithIceRestart) {
Init();
SessionDescriptionInterface* offer = CreateOffer();
SetLocalDescriptionWithoutError(offer);
scoped_ptr<SessionDescriptionInterface> answer(CreateRemoteAnswer(offer));
// Create the first answer.
std::string sdp;
ModifyIceUfragPwdLines(answer.get(), "0123456789012345",
"abcdefghijklmnopqrstuvwx", &sdp);
SessionDescriptionInterface* answer1 =
CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL);
cricket::Candidate candidate1(1, "udp", rtc::SocketAddress("1.1.1.1", 5000),
0, "", "", "relay", 0, "");
JsepIceCandidate ice_candidate1(kMediaContentName0, kMediaContentIndex0,
candidate1);
EXPECT_TRUE(answer1->AddCandidate(&ice_candidate1));
SetRemoteDescriptionWithoutError(answer1);
EXPECT_EQ(1, session_->remote_description()->candidates(0)->count());
// The second answer has the same ufrag and pwd but different address.
sdp.clear();
ModifyIceUfragPwdLines(answer.get(), "0123456789012345",
"abcdefghijklmnopqrstuvwx", &sdp);
SessionDescriptionInterface* answer2 =
CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL);
candidate1.set_address(rtc::SocketAddress("1.1.1.1", 6000));
JsepIceCandidate ice_candidate2(kMediaContentName0, kMediaContentIndex0,
candidate1);
EXPECT_TRUE(answer2->AddCandidate(&ice_candidate2));
SetRemoteDescriptionWithoutError(answer2);
EXPECT_EQ(2, session_->remote_description()->candidates(0)->count());
// The third answer has a different ufrag and different address.
sdp.clear();
ModifyIceUfragPwdLines(answer.get(), "0123456789012333",
"abcdefghijklmnopqrstuvwx", &sdp);
SessionDescriptionInterface* answer3 =
CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL);
candidate1.set_address(rtc::SocketAddress("1.1.1.1", 7000));
JsepIceCandidate ice_candidate3(kMediaContentName0, kMediaContentIndex0,
candidate1);
EXPECT_TRUE(answer3->AddCandidate(&ice_candidate3));
SetRemoteDescriptionWithoutError(answer3);
EXPECT_EQ(1, session_->remote_description()->candidates(0)->count());
// The fourth answer has no candidate but a different ufrag/pwd.
sdp.clear();
ModifyIceUfragPwdLines(answer.get(), "0123456789012444",
"abcdefghijklmnopqrstuvyz", &sdp);
SessionDescriptionInterface* offer4 =
CreateSessionDescription(JsepSessionDescription::kPrAnswer, sdp, NULL);
SetRemoteDescriptionWithoutError(offer4);
EXPECT_EQ(0, session_->remote_description()->candidates(0)->count());
}
// Test that candidates sent to the "video" transport do not get pushed down to
// the "audio" transport channel when bundling.
TEST_F(WebRtcSessionTest, TestIgnoreCandidatesForUnusedTransportWhenBundling) {