diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index f40e5a3c41..27703c28b1 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -576,20 +576,17 @@ void TransportController::OnChannelCandidatesRemoved( void TransportController::OnChannelRoleConflict_n( TransportChannelImpl* channel) { RTC_DCHECK(network_thread_->IsCurrent()); - - if (ice_role_switch_) { - LOG(LS_WARNING) - << "Repeat of role conflict signal from TransportChannelImpl."; - return; - } - - ice_role_switch_ = true; + // Note: since the role conflict is handled entirely on the network thread, + // we don't need to worry about role conflicts occurring on two ports at once. + // The first one encountered should immediately reverse the role. IceRole reversed_role = (ice_role_ == ICEROLE_CONTROLLING) ? ICEROLE_CONTROLLED : ICEROLE_CONTROLLING; - for (const auto& kv : transports_) { - kv.second->SetIceRole(reversed_role); - } + LOG(LS_INFO) << "Got role conflict; switching to " + << (reversed_role == ICEROLE_CONTROLLING ? "controlling" + : "controlled") + << " role."; + SetIceRole_n(reversed_role); } void TransportController::OnChannelStateChanged_n( diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index 9996d7235d..da3bab3fe4 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -220,8 +220,6 @@ class TransportController : public sigslot::has_slots<>, // TODO(deadbeef): Move the fields below down to the transports themselves IceConfig ice_config_; IceRole ice_role_ = ICEROLE_CONTROLLING; - // Flag which will be set to true after the first role switch - bool ice_role_switch_ = false; uint64_t ice_tiebreaker_ = rtc::CreateRandomId64(); rtc::scoped_refptr certificate_; rtc::AsyncInvoker invoker_; diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc index 627975c90c..fdb99a733c 100644 --- a/webrtc/p2p/base/transportcontroller_unittest.cc +++ b/webrtc/p2p/base/transportcontroller_unittest.cc @@ -255,6 +255,12 @@ TEST_F(TransportControllerTest, TestIceRoleConflict) { channel1->SignalRoleConflict(channel1); EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel1->GetIceRole()); EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel2->GetIceRole()); + + // Should be able to handle a second role conflict. The remote endpoint can + // change its role/tie-breaker when it does an ICE restart. + channel2->SignalRoleConflict(channel2); + EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel1->GetIceRole()); + EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel2->GetIceRole()); } TEST_F(TransportControllerTest, TestGetSslRole) {