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

Remove DetermineIceRole workaround.

Browse Source 
Bug: webrtc:10198, chromium:628676
Change-Id: I65a57a2d23b714f9cdddc9122f4b50d523d04dfa
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173337
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31101}
This commit is contained in:
Mirko Bonadei 2020-04-11 12:54:45 +02:00 committed by Commit Bot
parent 1d76654e21
commit ee0864364d
2 changed files with 0 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
pc/jsep_transport_controller.cc
View File

@ -1319,28 +1319,6 @@ cricket::IceRole JsepTransportController::DetermineIceRole(
tdesc.ice_mode == cricket::ICEMODE_FULL) {
ice_role = cricket::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 by setting |config_.redetermine_role_on_ice_restart_| to false. See:
// https://bugs.chromium.org/p/chromium/issues/detail?id=628676
// TODO(deadbeef): Remove this when these old versions of Chrome reach a low
// enough population.
if (config_.redetermine_role_on_ice_restart &&
jsep_transport->local_description() &&
cricket::IceCredentialsChanged(
jsep_transport->local_description()->transport_desc.ice_ufrag,
jsep_transport->local_description()->transport_desc.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.
(!jsep_transport->remote_description() ||
jsep_transport->remote_description()->transport_desc.ice_mode !=
cricket::ICEMODE_LITE)) {
ice_role = (type == SdpType::kOffer) ? cricket::ICEROLE_CONTROLLING
: cricket::ICEROLE_CONTROLLED;
}
} else {
// If our role is cricket::ICEROLE_CONTROLLED and the remote endpoint
// supports only ice_lite, this local endpoint should take the CONTROLLING

43
pc/jsep_transport_controller_unittest.cc
View File

@ -1000,49 +1000,6 @@ TEST_F(JsepTransportControllerTest, IceSignalingOccursOnSignalingThread) {
EXPECT_TRUE(!signaled_on_non_signaling_thread_);
}
// 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.
// See: https://bugs.chromium.org/p/chromium/issues/detail?id=628676
// TODO(deadbeef): Remove this when these old versions of Chrome reach a low
// enough population.
TEST_F(JsepTransportControllerTest, IceRoleRedeterminedOnIceRestartByDefault) {
CreateJsepTransportController(JsepTransportController::Config());
// Let the |transport_controller_| be the controlled side initially.
auto remote_offer = std::make_unique<cricket::SessionDescription>();
AddAudioSection(remote_offer.get(), kAudioMid1, kIceUfrag1, kIcePwd1,
cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
nullptr);
auto local_answer = std::make_unique<cricket::SessionDescription>();
AddAudioSection(local_answer.get(), kAudioMid1, kIceUfrag2, kIcePwd2,
cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_PASSIVE,
nullptr);
EXPECT_TRUE(transport_controller_
->SetRemoteDescription(SdpType::kOffer, remote_offer.get())
.ok());
EXPECT_TRUE(transport_controller_
->SetLocalDescription(SdpType::kAnswer, local_answer.get())
.ok());
auto fake_dtls = static_cast<FakeDtlsTransport*>(
transport_controller_->GetDtlsTransport(kAudioMid1));
EXPECT_EQ(cricket::ICEROLE_CONTROLLED,
fake_dtls->fake_ice_transport()->GetIceRole());
// New offer will trigger the ICE restart.
auto restart_local_offer = std::make_unique<cricket::SessionDescription>();
AddAudioSection(restart_local_offer.get(), kAudioMid1, kIceUfrag3, kIcePwd3,
cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_ACTPASS,
nullptr);
EXPECT_TRUE(
transport_controller_
->SetLocalDescription(SdpType::kOffer, restart_local_offer.get())
.ok());
EXPECT_EQ(cricket::ICEROLE_CONTROLLING,
fake_dtls->fake_ice_transport()->GetIceRole());
}
// Test that if the TransportController was created with the
// |redetermine_role_on_ice_restart| parameter set to false, the role is *not*
// redetermined on an ICE restart.