admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing bug that results in incorrect ICE role with ICE lite endpoints.

Browse Source 
There's some code that resets the ICE role on an ICE restart (behavior
that's specified in ICE, but removed from ICEbis). And it wasn't taking
into account that the remote endpoint may be an ICE lite endpoint, in
which case the WebRTC endpoint's role should always be "controlling".

BUG=chromium:710760

Review-Url: https://codereview.webrtc.org/2812173003
Cr-Commit-Position: refs/heads/master@{#17779}
This commit is contained in:
deadbeef 2017-04-20 00:57:25 -07:00 committed by Commit bot
parent f963dda877
commit 897d08ef1b
2 changed files with 36 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
webrtc/p2p/base/transportcontroller.cc
View File

@ -602,7 +602,11 @@ bool TransportController::SetLocalTransportDescription_n(
if (redetermine_role_on_ice_restart_ && transport->local_description() &&
IceCredentialsChanged(transport->local_description()->ice_ufrag,
transport->local_description()->ice_pwd,
tdesc.ice_ufrag, tdesc.ice_pwd)) {
tdesc.ice_ufrag, tdesc.ice_pwd) &&
// Don't change the ICE role if the remote endpoint is ICE lite; we
// should always be controlling in that case.
(!transport->remote_description() ||
transport->remote_description()->ice_mode != ICEMODE_LITE)) {
IceRole new_ice_role =
(action == CA_OFFER) ? ICEROLE_CONTROLLING : ICEROLE_CONTROLLED;
SetIceRole(new_ice_role);

31
webrtc/p2p/base/transportcontroller_unittest.cc
View File

@ -834,6 +834,37 @@ TEST_F(TransportControllerTest, TestSetRemoteIceLiteInAnswer) {
EXPECT_EQ(ICEMODE_LITE, transport->fake_ice_transport()->remote_ice_mode());
}
// Tests that the ICE role remains "controlling" if a subsequent offer that
// does an ICE restart is received from an ICE lite endpoint. Regression test
// for: https://crbug.com/710760
TEST_F(TransportControllerTest,
IceRoleIsControllingAfterIceRestartFromIceLiteEndpoint) {
FakeDtlsTransport* transport = CreateFakeDtlsTransport("audio", 1);
ASSERT_NE(nullptr, transport);
std::string err;
// Initial offer/answer.
TransportDescription remote_desc(std::vector<std::string>(), kIceUfrag1,
kIcePwd1, ICEMODE_LITE,
CONNECTIONROLE_ACTPASS, nullptr);
TransportDescription local_desc(kIceUfrag1, kIcePwd1);
ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription(
"audio", remote_desc, CA_OFFER, &err));
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
"audio", local_desc, CA_ANSWER, nullptr));
// Subsequent ICE restart offer/answer.
remote_desc.ice_ufrag = kIceUfrag2;
remote_desc.ice_pwd = kIcePwd2;
local_desc.ice_ufrag = kIceUfrag2;
local_desc.ice_pwd = kIcePwd2;
ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription(
"audio", remote_desc, CA_OFFER, &err));
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
"audio", local_desc, CA_ANSWER, nullptr));
EXPECT_EQ(ICEROLE_CONTROLLING, transport->fake_ice_transport()->GetIceRole());
}
// Tests SetNeedsIceRestartFlag and NeedsIceRestart, setting the flag and then
// initiating an ICE restart for one of the transports.
TEST_F(TransportControllerTest, NeedsIceRestart) {