From ee0864364d0daac7a62c4ed4229e62d4fa34349a Mon Sep 17 00:00:00 2001
From: Mirko Bonadei <mbonadei@webrtc.org>
Date: Sat, 11 Apr 2020 12:54:45 +0200
Subject: [PATCH] Remove DetermineIceRole workaround.

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}
---
 pc/jsep_transport_controller.cc          | 22 ------------
 pc/jsep_transport_controller_unittest.cc | 43 ------------------------
 2 files changed, 65 deletions(-)

diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 4a0df7a715..39451d5c06 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -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
diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc
index 7b18be8809..3fc6f8b7e5 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc
@@ -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.