webrtc/p2p/base/p2ptransportchannel_unittest.cc - Issue 2099563004: Start ICE connectivity checks as soon as the first pair is pingable.

Unified Diff: webrtc/p2p/base/p2ptransportchannel_unittest.cc

Issue 2099563004: Start ICE connectivity checks as soon as the first pair is pingable. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master

Patch Set: Updating unit test failures. We ping too fast for our own good. Created 4 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc

diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc

index 98b0b8dae56e8938d547dda0c707e0272d784d76..214c95dbf761a020cac51eb5416c6c926a1b993a 100644

--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc

+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc

@@ -355,7 +355,6 @@ class P2PTransportChannelTestBase : public testing::Test,

}

channel->SetIceRole(GetEndpoint(endpoint)->ice_role());

channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker());

- channel->Connect();

return channel;

}

void DestroyChannels() {

@@ -370,6 +369,7 @@ class P2PTransportChannelTestBase : public testing::Test,

P2PTransportChannel* ep2_ch2() { return ep2_.cd2_.ch_.get(); }

TestTurnServer* test_turn_server() { return &turn_server_; }

+ rtc::VirtualSocketServer* virtual_socket_server() { return vss_.get(); }

// Common results.

static const Result kLocalUdpToLocalUdp;

@@ -487,8 +487,6 @@ class P2PTransportChannelTestBase : public testing::Test,

// on the local and remote candidate of ep2_ch1, match. This can be

// used in an EXPECT_TRUE_WAIT.

bool CheckCandidate2(const Result& expected) {

- const std::string& local_type = LocalCandidate(ep2_ch1())->type();

- // const std::string& remote_type = RemoteCandidate(ep2_ch1())->type();

const std::string& local_proto = LocalCandidate(ep2_ch1())->protocol();

const std::string& remote_proto = RemoteCandidate(ep2_ch1())->protocol();

// Removed remote_type comparision aginst selected connection remote

@@ -498,16 +496,19 @@ class P2PTransportChannelTestBase : public testing::Test,

// and in other cases like NAT -> NAT it will be LUSU. To avoid these

// mismatches and we are doing comparision in different way.

// i.e. when don't match its remote type is either local or stun.

+ //

+ // Update(deadbeef): Also had to remove local type comparison. There

+ // is currently an issue where the local type is not updated to stun.

+ // So one side may see local<->relay while the other sees relay<->stun.

+ // This mean the other side may prioritize prflx<->relay, and won't have

+ // a local type of relay no matter how long we wait.

+ // TODO(deadbeef): Fix this!! It's causing us to have very sparse test

+ // coverage and is a very real bug.

+ //

pthatcher1 2016/06/25 00:02:36 Who is going to fix this, and how soon?

Taylor Brandstetter 2016/06/27 22:19:34 Me, and this week if possible. I also plan to re-e

// TODO(ronghuawu): Refine the test criteria.

// https://code.google.com/p/webrtc/issues/detail?id=1953

return ((local_proto == expected.local_proto2 &&

- remote_proto == expected.remote_proto2) &&

- (local_type == expected.local_type2 ||

- // Sometimes we expect local -> prflx or prflx -> local

- // and instead get prflx -> local or local -> prflx, and

- // that's OK.

- (IsLocalToPrflxOrTheReverse(expected) &&

- local_type == expected.remote_type2)));

+ remote_proto == expected.remote_proto2));

}

// EXPECT_EQ on the approprite parts of the expected Result, based

@@ -1062,20 +1063,6 @@ const P2PTransportChannelTest::Result* P2PTransportChannelTest::kMatrix[NUM_CONF

P2P_TEST(x, PROXY_HTTPS) \

P2P_TEST(x, PROXY_SOCKS)

-#define FLAKY_P2P_TEST_SET(x) \

- P2P_TEST(x, OPEN) \

- P2P_TEST(x, NAT_FULL_CONE) \

- P2P_TEST(x, NAT_ADDR_RESTRICTED) \

- P2P_TEST(x, NAT_PORT_RESTRICTED) \

- P2P_TEST(x, NAT_SYMMETRIC) \

- P2P_TEST(x, NAT_DOUBLE_CONE) \

- P2P_TEST(x, NAT_SYMMETRIC_THEN_CONE) \

- P2P_TEST(x, BLOCK_UDP) \

- P2P_TEST(x, BLOCK_UDP_AND_INCOMING_TCP) \

- P2P_TEST(x, BLOCK_ALL_BUT_OUTGOING_HTTP) \

- P2P_TEST(x, PROXY_HTTPS) \

- P2P_TEST(x, PROXY_SOCKS)

P2P_TEST_SET(OPEN)

P2P_TEST_SET(NAT_FULL_CONE)

P2P_TEST_SET(NAT_ADDR_RESTRICTED)

@@ -1450,7 +1437,7 @@ TEST_F(P2PTransportChannelTest, MAYBE_TestIceConfigWillPassDownToPort) {

const std::vector<PortInterface*> ports_after = ep1_ch1()->ports();

for (size_t i = 0; i < ports_after.size(); ++i) {

EXPECT_EQ(ICEROLE_CONTROLLED, ports_before[i]->GetIceRole());

- // SetIceTiebreaker after Connect() has been called will fail. So expect the

+ // SetIceTiebreaker after ports have been created will fail. So expect the

// original value.

EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker());

}

@@ -1704,6 +1691,12 @@ TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) {

TEST_F(P2PTransportChannelTest, TurnToPrflxPresumedWritable) {

rtc::ScopedFakeClock fake_clock;

+ // We need to add artificial network delay to verify that the connection

+ // is presumed writable before it's actually writable. Without this delay

+ // it would become writable instantly.

+ virtual_socket_server()->set_delay_mean(50);

+ virtual_socket_server()->UpdateDelayDistribution();

ConfigureEndpoints(NAT_SYMMETRIC, NAT_SYMMETRIC, kDefaultPortAllocatorFlags,

kDefaultPortAllocatorFlags);

// We want the remote TURN candidate to show up as prflx. To do this we need

@@ -2291,7 +2284,6 @@ TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("trigger checks", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));

@@ -2316,7 +2308,6 @@ TEST_F(P2PTransportChannelPingTest, TestAllConnectionsPingedSufficiently) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("ping sufficiently", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));

@@ -2345,7 +2336,6 @@ TEST_F(P2PTransportChannelPingTest, TestStunPingIntervals) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("TestChannel", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -2423,7 +2413,6 @@ TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("trigger checks", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));

@@ -2449,7 +2438,6 @@ TEST_F(P2PTransportChannelPingTest, TestFailedConnectionNotPingable) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("Do not ping failed connections", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

@@ -2467,7 +2455,6 @@ TEST_F(P2PTransportChannelPingTest, TestSignalStateChanged) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("state change", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -2488,7 +2475,6 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("add candidate", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

// Add a candidate with a future ufrag.

ch.AddRemoteCandidate(

@@ -2541,7 +2527,6 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("connection resurrection", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

// Create conn1 and keep track of original candidate priority.

@@ -2601,7 +2586,6 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {

ch.SetIceConfig(CreateIceConfig(500, false));

EXPECT_EQ(500, ch.receiving_timeout());

EXPECT_EQ(50, ch.check_receiving_interval());

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -2625,7 +2609,6 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {

P2PTransportChannel ch("receiving state change", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -2704,7 +2687,6 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {

P2PTransportChannel ch("receiving state change", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

// A minimal STUN message with prflx priority.

IceMessage request;

@@ -2785,7 +2767,6 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {

P2PTransportChannel ch("receiving state change", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 10));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -2838,7 +2819,6 @@ TEST_F(P2PTransportChannelPingTest,

P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

// The connections have decreasing priority.

Connection* conn1 =

@@ -2878,7 +2858,6 @@ TEST_F(P2PTransportChannelPingTest,

P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

// The connections have decreasing priority.

Connection* conn1 =

@@ -2924,7 +2903,6 @@ TEST_F(P2PTransportChannelPingTest,

P2PTransportChannel ch("SwitchSelectedConnection", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

// The connections have decreasing priority.

Connection* conn1 =

@@ -2963,7 +2941,6 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithAddressReuse) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("candidate reuse", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

const std::string host_address = "1.1.1.1";

const int port_num = 1;

@@ -3003,7 +2980,6 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -3041,7 +3017,6 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneHighPriorityConnections) {

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

Connection* conn1 =

CreateConnectionWithCandidate(ch, clock, "1.1.1.1", 1, 100, true);

@@ -3063,7 +3038,6 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

EXPECT_EQ(TransportChannelState::STATE_INIT, ch.GetState());

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));

@@ -3090,7 +3064,6 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

ch.SetIceConfig(CreateIceConfig(1000, false));

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -3132,7 +3105,6 @@ TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {

FakePortAllocator pa(rtc::Thread::Current(), nullptr);

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

// Have one connection only but later becomes write-time-out.

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));

@@ -3166,7 +3138,6 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {

P2PTransportChannel ch("test channel", 1, &pa);

PrepareChannel(&ch);

ch.SetIceConfig(CreateIceConfig(2000, false));

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));

Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

@@ -3204,7 +3175,6 @@ TEST_F(P2PTransportChannelPingTest,

PrepareChannel(&ch);

IceConfig config = CreateIceConfig(1000, true);

ch.SetIceConfig(config);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

@@ -3227,7 +3197,6 @@ TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnPortAfterIceRestart) {

P2PTransportChannel ch("test channel", ICE_CANDIDATE_COMPONENT_DEFAULT, &pa);

// Starts with ICEROLE_CONTROLLING.

PrepareChannel(&ch);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

@@ -3252,7 +3221,6 @@ TEST_F(P2PTransportChannelPingTest, TestPortDestroyedAfterTimeout) {

PrepareChannel(&ch);

// Only a controlled channel should expect its ports to be destroyed.

ch.SetIceRole(ICEROLE_CONTROLLED);

- ch.Connect();

ch.MaybeStartGathering();

ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));

@@ -3297,7 +3265,6 @@ class P2PTransportChannelMostLikelyToWorkFirstTest

stable_writable_connection_ping_interval;

channel_->SetIceConfig(config);

PrepareChannel(channel_.get());

- channel_->Connect();

channel_->MaybeStartGathering();

return *channel_.get();

}

« webrtc/p2p/base/p2ptransportchannel.cc ('K') | « webrtc/p2p/base/p2ptransportchannel.cc ('k') | webrtc/p2p/base/transport.h » ('j') | no next file with comments »