Handle any number of role conflicts; not just the first.

I think the code that only allowed one role conflict was a workaround
for our old implementation (TransportChannelProxy, etc.). Not necessary
now, with TransportController.

BUG=webrtc:5470

Review-Url: https://codereview.webrtc.org/2004463003
Cr-Commit-Position: refs/heads/master@{#12949}
This commit is contained in:
deadbeef 2016-05-27 13:34:37 -07:00 committed by Commit bot
parent d7973ccdb5
commit 1c20610ede
3 changed files with 14 additions and 13 deletions

View File

@ -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(

View File

@ -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<rtc::RTCCertificate> certificate_;
rtc::AsyncInvoker invoker_;

View File

@ -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) {