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

Add parameter to TransportController to not change ICE role on restart.

Browse Source 
This will allow applications to opt in to this behavior before it's made
default.

R=skvlad@webrtc.org

Review URL: https://codereview.webrtc.org/2285453002 .

Cr-Commit-Position: refs/heads/master@{#13944}
This commit is contained in:
Taylor Brandstetter 2016-08-26 20:59:24 -07:00
parent 8cf8898973
commit f0bb360eca
4 changed files with 64 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
webrtc/p2p/base/faketransportcontroller.h
View File

@ -504,6 +504,13 @@ class FakeTransportController : public TransportController {
nullptr),
fail_create_channel_(false) {}
explicit FakeTransportController(bool redetermine_role_on_ice_restart)
: TransportController(rtc::Thread::Current(),
rtc::Thread::Current(),
nullptr,
redetermine_role_on_ice_restart),
fail_create_channel_(false) {}
explicit FakeTransportController(IceRole role)
: TransportController(rtc::Thread::Current(),
rtc::Thread::Current(),

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

@ -44,10 +44,20 @@ struct CandidatesData : public rtc::MessageData {
TransportController::TransportController(rtc::Thread* signaling_thread,
rtc::Thread* network_thread,
PortAllocator* port_allocator)
PortAllocator* port_allocator,
bool redetermine_role_on_ice_restart)
: signaling_thread_(signaling_thread),
network_thread_(network_thread),
port_allocator_(port_allocator) {}
port_allocator_(port_allocator),
redetermine_role_on_ice_restart_(redetermine_role_on_ice_restart) {}
TransportController::TransportController(rtc::Thread* signaling_thread,
rtc::Thread* network_thread,
PortAllocator* port_allocator)
: TransportController(signaling_thread,
network_thread,
port_allocator,
true) {}
TransportController::~TransportController() {
network_thread_->Invoke<void>(
@ -450,11 +460,12 @@ bool TransportController::SetLocalTransportDescription_n(
// 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.
// so for now we can't safely stop doing this, unless the application opts in
// by setting |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 (transport->local_description() &&
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)) {

9
webrtc/p2p/base/transportcontroller.h
View File

@ -31,6 +31,14 @@ namespace cricket {
class TransportController : public sigslot::has_slots<>,
public rtc::MessageHandler {
public:
// If |redetermine_role_on_ice_restart| is true, ICE role is redetermined
// upon setting a local transport description that indicates an ICE restart.
// For the constructor that doesn't take this parameter, it defaults to true.
TransportController(rtc::Thread* signaling_thread,
rtc::Thread* network_thread,
PortAllocator* port_allocator,
bool redetermine_role_on_ice_restart);
TransportController(rtc::Thread* signaling_thread,
rtc::Thread* network_thread,
PortAllocator* port_allocator);
@ -224,6 +232,7 @@ 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;
bool redetermine_role_on_ice_restart_;
uint64_t ice_tiebreaker_ = rtc::CreateRandomId64();
rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
rtc::AsyncInvoker invoker_;

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

@ -689,7 +689,7 @@ TEST_F(TransportControllerTest, TestSignalingOccursOnSignalingThread) {
// 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(TransportControllerTest, IceRoleRedeterminedOnIceRestart) {
TEST_F(TransportControllerTest, IceRoleRedeterminedOnIceRestartByDefault) {
FakeTransportChannel* channel = CreateChannel("audio", 1);
ASSERT_NE(nullptr, channel);
std::string err;
@ -717,4 +717,36 @@ TEST_F(TransportControllerTest, IceRoleRedeterminedOnIceRestart) {
EXPECT_EQ(ICEROLE_CONTROLLING, channel->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.
TEST_F(TransportControllerTest, IceRoleNotRedetermined) {
bool redetermine_role = false;
transport_controller_.reset(new TransportControllerForTest(redetermine_role));
FakeTransportChannel* channel = CreateChannel("audio", 1);
ASSERT_NE(nullptr, channel);
std::string err;
// Do an initial offer answer, so that the next offer is an ICE restart.
transport_controller_->SetIceRole(ICEROLE_CONTROLLED);
TransportDescription remote_desc(std::vector<std::string>(), kIceUfrag1,
kIcePwd1, ICEMODE_FULL,
CONNECTIONROLE_ACTPASS, nullptr);
EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription(
"audio", remote_desc, CA_OFFER, &err));
TransportDescription local_desc(std::vector<std::string>(), kIceUfrag2,
kIcePwd2, ICEMODE_FULL,
CONNECTIONROLE_ACTPASS, nullptr);
EXPECT_TRUE(transport_controller_->SetLocalTransportDescription(
"audio", local_desc, CA_ANSWER, &err));
EXPECT_EQ(ICEROLE_CONTROLLED, channel->GetIceRole());
// The endpoint that initiated an ICE restart should keep the existing role.
TransportDescription ice_restart_desc(std::vector<std::string>(), kIceUfrag3,
kIcePwd3, ICEMODE_FULL,
CONNECTIONROLE_ACTPASS, nullptr);
EXPECT_TRUE(transport_controller_->SetLocalTransportDescription(
"audio", ice_restart_desc, CA_OFFER, &err));
EXPECT_EQ(ICEROLE_CONTROLLED, channel->GetIceRole());
}
} // namespace cricket {