webrtc/p2p/base/p2ptransportchannel_unittest.cc - Issue 2069493002: Do not switch best connection on the controlled side too frequently

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

Issue 2069493002: Do not switch best connection on the controlled side too frequently (Closed) Base URL: https://chromium.googlesource.com/external/webrtc@master

Patch Set: Updates a comment 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 cbf5e76a7ae23a13376e6f8aa76ad10559978fbe..e4cccf75b9ca8ece3708c232578b137e326b081b 100644

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

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

@@ -529,8 +529,7 @@ class P2PTransportChannelTestBase : public testing::Test,

// Allow a few turns of the crank for the best connections to emerge.

// This may take up to 2 seconds.

- if (ep1_ch1()->best_connection() &&

- ep2_ch1()->best_connection()) {

+ if (ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection()) {

int64_t converge_start = rtc::TimeMillis();

int64_t converge_time;

int64_t converge_wait = 2000;

@@ -640,8 +639,8 @@ class P2PTransportChannelTestBase : public testing::Test,

ep2_ch1()->writable(),

1000);

- EXPECT_TRUE(ep1_ch1()->best_connection() &&

- ep2_ch1()->best_connection());

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection());

TestSendRecv(1);

}

@@ -752,13 +751,15 @@ class P2PTransportChannelTestBase : public testing::Test,

}

static const cricket::Candidate* LocalCandidate(

cricket::P2PTransportChannel* ch) {

- return (ch && ch->best_connection()) ?

- &ch->best_connection()->local_candidate() : NULL;

+ return (ch && ch->selected_connection())

+ ? &ch->selected_connection()->local_candidate()

+ : NULL;

}

static const cricket::Candidate* RemoteCandidate(

cricket::P2PTransportChannel* ch) {

- return (ch && ch->best_connection()) ?

- &ch->best_connection()->remote_candidate() : NULL;

+ return (ch && ch->selected_connection())

+ ? &ch->selected_connection()->remote_candidate()

+ : NULL;

}

Endpoint* GetEndpoint(cricket::TransportChannel* ch) {

if (ep1_.HasChannel(ch)) {

@@ -1116,16 +1117,19 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {

// The caller should have the best connection connected to the peer reflexive

Taylor Brandstetter 2016/06/21 18:33:26 s/best connection/selected connnection/g

honghaiz3 2016/06/22 08:03:16 Done.

// candidate.

- const cricket::Connection* best_connection = NULL;

- WAIT((best_connection = ep1_ch1()->best_connection()) != NULL, 2000);

- EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());

+ const cricket::Connection* selected_connection = NULL;

+ WAIT((selected_connection = ep1_ch1()->selected_connection()) != NULL, 2000);

+ EXPECT_EQ("prflx",

+ ep1_ch1()->selected_connection()->remote_candidate().type());

// Because we don't have a remote pwd, we don't ping yet.

EXPECT_EQ(kIceUfrag[1],

- ep1_ch1()->best_connection()->remote_candidate().username());

- EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());

+ ep1_ch1()->selected_connection()->remote_candidate().username());

+ EXPECT_EQ("",

+ ep1_ch1()->selected_connection()->remote_candidate().password());

// Because we don't have ICE credentials yet, we don't know the generation.

- EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());

+ EXPECT_EQ(0u,

+ ep1_ch1()->selected_connection()->remote_candidate().generation());

EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());

// Add two sets of remote ICE credentials, so that the ones used by the

@@ -1135,19 +1139,19 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) {

// After setting the remote ICE credentials, the password and generation

// of the peer reflexive candidate should be updated.

EXPECT_EQ(kIcePwd[1],

- ep1_ch1()->best_connection()->remote_candidate().password());

- EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());

+ ep1_ch1()->selected_connection()->remote_candidate().password());

+ EXPECT_EQ(1u,

+ ep1_ch1()->selected_connection()->remote_candidate().generation());

EXPECT_TRUE(nullptr != ep1_ch1()->FindNextPingableConnection());

ResumeCandidates(1);

- WAIT(ep2_ch1()->best_connection() != NULL, 2000);

+ WAIT(ep2_ch1()->selected_connection() != NULL, 2000);

// Verify ep1's best connection is updated to use the 'local' candidate.

- EXPECT_EQ_WAIT(

- "local",

- ep1_ch1()->best_connection()->remote_candidate().type(),

- 2000);

- EXPECT_EQ(best_connection, ep1_ch1()->best_connection());

+ EXPECT_EQ_WAIT("local",

+ ep1_ch1()->selected_connection()->remote_candidate().type(),

+ 2000);

+ EXPECT_EQ(selected_connection, ep1_ch1()->selected_connection());

DestroyChannels();

}

@@ -1167,15 +1171,18 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {

// The caller should have the best connection connected to the peer reflexive

// candidate.

- WAIT(ep1_ch1()->best_connection() != NULL, 2000);

- EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());

+ WAIT(ep1_ch1()->selected_connection() != NULL, 2000);

+ EXPECT_EQ("prflx",

+ ep1_ch1()->selected_connection()->remote_candidate().type());

// Because we don't have a remote pwd, we don't ping yet.

EXPECT_EQ(kIceUfrag[1],

- ep1_ch1()->best_connection()->remote_candidate().username());

- EXPECT_EQ("", ep1_ch1()->best_connection()->remote_candidate().password());

+ ep1_ch1()->selected_connection()->remote_candidate().username());

+ EXPECT_EQ("",

+ ep1_ch1()->selected_connection()->remote_candidate().password());

// Because we don't have ICE credentials yet, we don't know the generation.

- EXPECT_EQ(0u, ep1_ch1()->best_connection()->remote_candidate().generation());

+ EXPECT_EQ(0u,

+ ep1_ch1()->selected_connection()->remote_candidate().generation());

EXPECT_TRUE(nullptr == ep1_ch1()->FindNextPingableConnection());

// Add two sets of remote ICE credentials, so that the ones used by the

@@ -1185,18 +1192,20 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) {

// After setting the remote ICE credentials, the password and generation

// of the peer reflexive candidate should be updated.

EXPECT_EQ(kIcePwd[1],

- ep1_ch1()->best_connection()->remote_candidate().password());

- EXPECT_EQ(1u, ep1_ch1()->best_connection()->remote_candidate().generation());

+ ep1_ch1()->selected_connection()->remote_candidate().password());

+ EXPECT_EQ(1u,

+ ep1_ch1()->selected_connection()->remote_candidate().generation());

ResumeCandidates(1);

- const cricket::Connection* best_connection = NULL;

- WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 2000);

+ const cricket::Connection* selected_connection = NULL;

+ WAIT((selected_connection = ep2_ch1()->selected_connection()) != NULL, 2000);

// Wait to verify the connection is not culled.

WAIT(ep1_ch1()->writable(), 2000);

- EXPECT_EQ(ep2_ch1()->best_connection(), best_connection);

- EXPECT_EQ("prflx", ep1_ch1()->best_connection()->remote_candidate().type());

+ EXPECT_EQ(ep2_ch1()->selected_connection(), selected_connection);

+ EXPECT_EQ("prflx",

+ ep1_ch1()->selected_connection()->remote_candidate().type());

DestroyChannels();

}

@@ -1206,12 +1215,12 @@ TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) {

ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,

kDefaultPortAllocatorFlags);

CreateChannels(1);

- const cricket::Connection* best_connection = NULL;

+ const cricket::Connection* selected_connection = NULL;

// Wait until the callee's connections are created.

- WAIT((best_connection = ep2_ch1()->best_connection()) != NULL, 1000);

+ WAIT((selected_connection = ep2_ch1()->selected_connection()) != NULL, 1000);

// Wait to see if they get culled; they shouldn't.

- WAIT(ep2_ch1()->best_connection() != best_connection, 1000);

- EXPECT_TRUE(ep2_ch1()->best_connection() == best_connection);

+ WAIT(ep2_ch1()->selected_connection() != selected_connection, 1000);

+ EXPECT_TRUE(ep2_ch1()->selected_connection() == selected_connection);

DestroyChannels();

}

@@ -1287,10 +1296,10 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) {

EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000);

- EXPECT_TRUE(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

- LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

- RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection() &&

+ LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

+ RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

TestSendRecv(1);

DestroyChannels();

@@ -1340,8 +1349,8 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) {

ep2_ch1()->writable(),

1000);

- EXPECT_TRUE(ep1_ch1()->best_connection() &&

- ep2_ch1()->best_connection());

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection());

TestSendRecv(1);

DestroyChannels();

@@ -1395,7 +1404,7 @@ TEST_F(P2PTransportChannelTest, TestIPv6Connections) {

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000);

EXPECT_TRUE(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

+ ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&

LocalCandidate(ep1_ch1())->address().EqualIPs(kIPv6PublicAddrs[0]) &&

RemoteCandidate(ep1_ch1())->address().EqualIPs(kIPv6PublicAddrs[1]));

@@ -1420,8 +1429,8 @@ TEST_F(P2PTransportChannelTest, TestForceTurn) {

ep2_ch1()->receiving() && ep2_ch1()->writable(),

2000);

- EXPECT_TRUE(ep1_ch1()->best_connection() &&

- ep2_ch1()->best_connection());

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection());

EXPECT_EQ("relay", RemoteCandidate(ep1_ch1())->type());

EXPECT_EQ("relay", LocalCandidate(ep1_ch1())->type());

@@ -1497,10 +1506,10 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionBeforeDoneGathering) {

auto pooled_ports_2 = pooled_session_2->ReadyPorts();

EXPECT_NE(pooled_ports_1.end(),

std::find(pooled_ports_1.begin(), pooled_ports_1.end(),

- ep1_ch1()->best_connection()->port()));

+ ep1_ch1()->selected_connection()->port()));

EXPECT_NE(pooled_ports_2.end(),

std::find(pooled_ports_2.begin(), pooled_ports_2.end(),

- ep2_ch1()->best_connection()->port()));

+ ep2_ch1()->selected_connection()->port()));

}

// Test that a connection succeeds when the P2PTransportChannel uses a pooled

@@ -1540,10 +1549,10 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) {

auto pooled_ports_2 = pooled_session_2->ReadyPorts();

EXPECT_NE(pooled_ports_1.end(),

std::find(pooled_ports_1.begin(), pooled_ports_1.end(),

- ep1_ch1()->best_connection()->port()));

+ ep1_ch1()->selected_connection()->port()));

EXPECT_NE(pooled_ports_2.end(),

std::find(pooled_ports_2.begin(), pooled_ports_2.end(),

- ep2_ch1()->best_connection()->port()));

+ ep2_ch1()->selected_connection()->port()));

}

// Test what happens when we have 2 users behind the same NAT. This can lead

@@ -1614,10 +1623,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {

EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000, 1000);

- EXPECT_TRUE(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

- LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

- RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection() &&

+ LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

+ RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

// Make the receiving timeout shorter for testing.

cricket::IceConfig config = CreateIceConfig(1000, false);

@@ -1628,17 +1637,20 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {

LOG(LS_INFO) << "Failing over...";

fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[1]);

// The best connections will switch, so keep references to them.

- const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();

- const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();

+ const cricket::Connection* selected_connection1 =

+ ep1_ch1()->selected_connection();

+ const cricket::Connection* selected_connection2 =

+ ep2_ch1()->selected_connection();

// We should detect loss of receiving within 1 second or so.

EXPECT_TRUE_WAIT(

- !best_connection1->receiving() && !best_connection2->receiving(), 3000);

+ !selected_connection1->receiving() && !selected_connection2->receiving(),

+ 3000);

// We should switch over to use the alternate addr immediately on both sides

// when we are not receiving.

- EXPECT_TRUE_WAIT(

- ep1_ch1()->best_connection()->receiving() &&

- ep2_ch1()->best_connection()->receiving(), 1000);

+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&

+ ep2_ch1()->selected_connection()->receiving(),

+ 1000);

EXPECT_TRUE(LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]));

EXPECT_TRUE(

RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]));

@@ -1666,10 +1678,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {

EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000, 1000);

- EXPECT_TRUE(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

- LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

- RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

+ EXPECT_TRUE(ep1_ch1()->selected_connection() &&

+ ep2_ch1()->selected_connection() &&

+ LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

+ RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

// Make the receiving timeout shorter for testing.

cricket::IceConfig config = CreateIceConfig(1000, false);

@@ -1680,17 +1692,20 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {

LOG(LS_INFO) << "Failing over...";

fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);

// The best connections will switch, so keep references to them.

- const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();

- const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();

+ const cricket::Connection* selected_connection1 =

+ ep1_ch1()->selected_connection();

+ const cricket::Connection* selected_connection2 =

+ ep2_ch1()->selected_connection();

// We should detect loss of receiving within 1 second or so.

EXPECT_TRUE_WAIT(

- !best_connection1->receiving() && !best_connection2->receiving(), 3000);

+ !selected_connection1->receiving() && !selected_connection2->receiving(),

+ 3000);

// We should switch over to use the alternate addr immediately on both sides

// when we are not receiving.

- EXPECT_TRUE_WAIT(

- ep1_ch1()->best_connection()->receiving() &&

- ep2_ch1()->best_connection()->receiving(), 1000);

+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&

+ ep2_ch1()->selected_connection()->receiving(),

+ 1000);

EXPECT_TRUE(

LocalCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]));

EXPECT_TRUE(RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

@@ -1722,11 +1737,11 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiToWifiConnection) {

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000, 1000);

// Need to wait to make sure the connections on both networks are writable.

- EXPECT_TRUE_WAIT(ep1_ch1()->best_connection() &&

+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() &&

LocalCandidate(ep1_ch1())->address().EqualIPs(wifi[0]) &&

RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1]),

1000);

- EXPECT_TRUE_WAIT(ep2_ch1()->best_connection() &&

+ EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() &&

LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]) &&

RemoteCandidate(ep2_ch1())->address().EqualIPs(wifi[0]),

1000);

@@ -1754,10 +1769,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiOverCellularNetwork) {

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000, 1000);

// Need to wait to make sure the connections on both networks are writable.

- EXPECT_TRUE_WAIT(ep1_ch1()->best_connection() &&

+ EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection() &&

RemoteCandidate(ep1_ch1())->address().EqualIPs(wifi[1]),

1000);

- EXPECT_TRUE_WAIT(ep2_ch1()->best_connection() &&

+ EXPECT_TRUE_WAIT(ep2_ch1()->selected_connection() &&

LocalCandidate(ep2_ch1())->address().EqualIPs(wifi[1]),

1000);

}

@@ -1872,7 +1887,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) {

ep2_ch1()->receiving() && ep2_ch1()->writable(),

1000);

EXPECT_TRUE(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

+ ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&

LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));

@@ -1888,7 +1903,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) {

// We should switch over to use the alternate address after

// an exchange of pings.

EXPECT_TRUE_WAIT(

- ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&

+ ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&

LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&

RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]),

3000);

@@ -1977,12 +1992,32 @@ class P2PTransportChannelPingTest : public testing::Test,

return channel.SendPacket(data, len, options, 0);

}

+ cricket::Connection* CreateConnectionWithCandidate(

+ cricket::P2PTransportChannel& channel,

+ const std::string& ip_addr,

+ int port,

+ int priority,

+ bool writable) {

+ channel.AddRemoteCandidate(CreateHostCandidate(ip_addr, port, priority));

+ cricket::Connection* conn = WaitForConnectionTo(&channel, ip_addr, port);

+ if (conn && writable) {

+ conn->ReceivedPingResponse(); // make it writable

+ }

+ return conn;

+ }

+ void NominateConnection(cricket::Connection* conn) {

+ conn->set_nominated(true);

+ conn->SignalNominated(conn);

+ }

void OnSelectedCandidatePairChanged(

cricket::TransportChannel* transport_channel,

cricket::CandidatePairInterface* selected_candidate_pair,

int last_sent_packet_id) {

last_selected_candidate_pair_ = selected_candidate_pair;

last_sent_packet_id_ = last_sent_packet_id;

+ ++selected_candidate_pair_switches_;

}

void ReceivePingOnConnection(cricket::Connection* conn,

@@ -2018,12 +2053,18 @@ class P2PTransportChannelPingTest : public testing::Test,

bool channel_ready_to_send() { return channel_ready_to_send_; }

void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }

cricket::TransportChannelState channel_state() { return channel_state_; }

+ int get_and_reset_selected_candidate_pair_switches() {

+ int switches = selected_candidate_pair_switches_;

+ selected_candidate_pair_switches_ = 0;

+ return switches;

+ }

private:

std::unique_ptr<rtc::PhysicalSocketServer> pss_;

std::unique_ptr<rtc::VirtualSocketServer> vss_;

rtc::SocketServerScope ss_scope_;

cricket::CandidatePairInterface* last_selected_candidate_pair_ = nullptr;

+ int selected_candidate_pair_switches_ = 0;

int last_sent_packet_id_ = -1;

bool channel_ready_to_send_ = false;

cricket::TransportChannelState channel_state_ = cricket::STATE_INIT;

@@ -2246,7 +2287,7 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) {

conn1->ReceivedPing();

conn1->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));

- EXPECT_TRUE_WAIT(ch.best_connection() != nullptr, 1000);

+ EXPECT_TRUE_WAIT(ch.selected_connection() != nullptr, 1000);

EXPECT_TRUE_WAIT(ch.receiving(), 1000);

EXPECT_TRUE_WAIT(!ch.receiving(), 1000);

}

@@ -2267,7 +2308,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {

ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1));

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

ASSERT_TRUE(conn1 != nullptr);

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

EXPECT_EQ(conn1, last_selected_candidate_pair());

EXPECT_EQ(-1, last_sent_packet_id());

// Channel is not ready to send because it is not writable.

@@ -2282,7 +2323,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {

ch.AddRemoteCandidate(CreateHostCandidate("2.2.2.2", 2, 10));

cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);

ASSERT_TRUE(conn2 != nullptr);

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

EXPECT_EQ(conn2, last_selected_candidate_pair());

EXPECT_EQ(last_packet_id, last_sent_packet_id());

EXPECT_FALSE(channel_ready_to_send());

@@ -2295,13 +2336,13 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {

cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);

ASSERT_TRUE(conn3 != nullptr);

// Because it has a lower priority, the best connection is still conn2.

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

conn3->ReceivedPingResponse(); // Become writable.

// But if it is nominated via use_candidate, it is chosen as the best

// connection.

conn3->set_nominated(true);

conn3->SignalNominated(conn3);

- EXPECT_EQ(conn3, ch.best_connection());

+ EXPECT_EQ(conn3, ch.selected_connection());

EXPECT_EQ(conn3, last_selected_candidate_pair());

EXPECT_EQ(last_packet_id, last_sent_packet_id());

EXPECT_TRUE(channel_ready_to_send());

@@ -2313,17 +2354,17 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {

ch.AddRemoteCandidate(CreateHostCandidate("4.4.4.4", 4, 100));

cricket::Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);

ASSERT_TRUE(conn4 != nullptr);

- EXPECT_EQ(conn3, ch.best_connection());

+ EXPECT_EQ(conn3, ch.selected_connection());

// But if it is nominated via use_candidate and writable, it will be set as

// the best connection.

conn4->set_nominated(true);

conn4->SignalNominated(conn4);

// Not switched yet because conn4 is not writable.

- EXPECT_EQ(conn3, ch.best_connection());

+ EXPECT_EQ(conn3, ch.selected_connection());

reset_channel_ready_to_send();

// The best connection switches after conn4 becomes writable.

conn4->ReceivedPingResponse();

- EXPECT_EQ(conn4, ch.best_connection());

+ EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);

EXPECT_EQ(conn4, last_selected_candidate_pair());

EXPECT_EQ(last_packet_id, last_sent_packet_id());

// SignalReadyToSend is fired again because conn4 is writable.

@@ -2356,9 +2397,9 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {

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

ASSERT_TRUE(conn1 != nullptr);

EXPECT_TRUE(port->sent_binding_response());

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

conn1->ReceivedPingResponse();

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

port->set_sent_binding_response(false);

// Another connection is nominated via use_candidate.

@@ -2366,13 +2407,13 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {

cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);

ASSERT_TRUE(conn2 != nullptr);

// Because it has a lower priority, the best connection is still conn1.

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

// When it is nominated via use_candidate and writable, it is chosen as the

// best connection.

conn2->ReceivedPingResponse(); // Become writable.

conn2->set_nominated(true);

conn2->SignalNominated(conn2);

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

// Another request with unknown address, it will not be set as the best

// connection because the best connection was nominated by the controlling

@@ -2383,7 +2424,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {

ASSERT_TRUE(conn3 != nullptr);

EXPECT_TRUE(port->sent_binding_response());

conn3->ReceivedPingResponse(); // Become writable.

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

port->set_sent_binding_response(false);

// However if the request contains use_candidate attribute, it will be

@@ -2396,9 +2437,9 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {

ASSERT_TRUE(conn4 != nullptr);

EXPECT_TRUE(port->sent_binding_response());

// conn4 is not the best connection yet because it is not writable.

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

conn4->ReceivedPingResponse(); // Become writable.

- EXPECT_EQ(conn4, ch.best_connection());

+ EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout);

// Test that the request from an unknown address contains a ufrag from an old

// generation.

@@ -2427,7 +2468,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {

ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 10));

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

ASSERT_TRUE(conn1 != nullptr);

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

// If a data packet is received on conn2, the best connection should

// switch to conn2 because the controlled side must mirror the media path

@@ -2436,14 +2477,9 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {

cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);

ASSERT_TRUE(conn2 != nullptr);

conn2->ReceivedPing(); // Start receiving.

- // Do not switch because it is not writable.

conn2->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));

- EXPECT_EQ(conn1, ch.best_connection());

- conn2->ReceivedPingResponse(); // Become writable.

- // Switch because it is writable.

- conn2->OnReadPacket("DEF", 3, rtc::CreatePacketTime(0));

- EXPECT_EQ(conn2, ch.best_connection());

+ EXPECT_EQ(conn2, ch.selected_connection());

+ conn2->ReceivedPingResponse(); // Make it writable.

// Now another STUN message with an unknown address and use_candidate will

// nominate the best connection.

@@ -2461,16 +2497,79 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {

cricket::PROTO_UDP, &request, kIceUfrag[1], false);

cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);

ASSERT_TRUE(conn3 != nullptr);

- EXPECT_EQ(conn2, ch.best_connection()); // Not writable yet.

+ EXPECT_EQ(conn2, ch.selected_connection()); // Not writable yet.

conn3->ReceivedPingResponse(); // Become writable.

- EXPECT_EQ(conn3, ch.best_connection());

+ EXPECT_EQ_WAIT(conn3, ch.selected_connection(), kDefaultTimeout);

// Now another data packet will not switch the best connection because the

// best connection was nominated by the controlling side.

conn2->ReceivedPing();

conn2->ReceivedPingResponse();

conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));

- EXPECT_EQ(conn3, ch.best_connection());

+ EXPECT_EQ_WAIT(conn3, ch.selected_connection(), kDefaultTimeout);

+TEST_F(P2PTransportChannelPingTest, TestDonotSwitchConnectionTooFrequently) {

Taylor Brandstetter 2016/06/21 18:33:26 Rename test to ControlledAgentDoesntSwitchConnecti

honghaiz3 2016/06/22 08:03:16 Done.

+ rtc::ScopedFakeClock clock;

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

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

+ PrepareChannel(&ch);

+ ch.SetIceRole(cricket::ICEROLE_CONTROLLED);

+ ch.Connect();

+ ch.MaybeStartGathering();

+ // The connections have decreasing priority.

+ cricket::Connection* conn1 =

+ CreateConnectionWithCandidate(ch, "1.1.1.1", 1, 10, true);

+ ASSERT_TRUE(conn1 != nullptr);

+ cricket::Connection* conn2 =

+ CreateConnectionWithCandidate(ch, "2.2.2.2", 2, 9, true);

+ ASSERT_TRUE(conn2 != nullptr);

+ cricket::Connection* conn3 =

+ CreateConnectionWithCandidate(ch, "3.3.3.3", 3, 8, false);

+ ASSERT_TRUE(conn3 != nullptr);

+ // Simulate aggressive nomination on the controlling side.

+ get_and_reset_selected_candidate_pair_switches();

+ NominateConnection(conn2);

+ EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());

+ EXPECT_EQ(conn2, last_selected_candidate_pair());

+ // Selected candidate pair will switch because conn1 has higher priority.

+ NominateConnection(conn1);

+ EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());

+ EXPECT_EQ(conn1, last_selected_candidate_pair());

+ // Advance the clock by 1ms so that the last data receiving timestamp of

+ // conn2 is larger.

+ SIMULATED_WAIT(false, 1, clock);

+ // Selected candidate pair won't switch until conn2 receives data and becomes

+ // writable again because conn2 has lower priority and may have been pruned.

+ NominateConnection(conn2);

+ conn2->ReceivedPingResponse();

+ EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());

+ // Will switch if conn2 receives data.

+ conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));

+ EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());

+ EXPECT_EQ(conn2, last_selected_candidate_pair());

+ // Advance the clock by 1ms so that the last data receiving timestamp of

+ // conn3 is larger.

+ SIMULATED_WAIT(false, 1, clock);

+ // Selected candidate pair won't switch even if it is nominated and

+ // received data because it is not writable.

+ NominateConnection(conn3);

+ conn3->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));

+ EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());

+ // It will switch if it becomes writable

+ conn3->ReceivedPingResponse();

+ EXPECT_EQ_SIMULATED_WAIT(1, get_and_reset_selected_candidate_pair_switches(),

+ kDefaultTimeout, clock);

+ EXPECT_EQ(conn3, last_selected_candidate_pair());

+ // Make sure sorting won't reselect candidate pair.

+ SIMULATED_WAIT(false, 10, clock);

+ EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());

}

Taylor Brandstetter 2016/06/21 18:33:26 This test's expectations are good but it's testing

honghaiz3 2016/06/22 08:03:16 I break this into three, making each of them small

// Test that if a new remote candidate has the same address and port with

@@ -2524,7 +2623,7 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {

ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1));

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

ASSERT_TRUE(conn1 != nullptr);

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

conn1->ReceivedPingResponse(); // Becomes writable and receiving

// When a higher-priority, nominated candidate comes in, the connections with

@@ -2589,7 +2688,7 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {

ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 100));

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

ASSERT_TRUE(conn1 != nullptr);

- EXPECT_EQ(conn1, ch.best_connection());

+ EXPECT_EQ(conn1, ch.selected_connection());

conn1->ReceivedPingResponse(); // Becomes writable and receiving

// Add a low-priority connection |conn2|, which will be pruned, but it will

@@ -2610,12 +2709,12 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {

ASSERT_TRUE(conn2 != nullptr);

EXPECT_EQ_WAIT(cricket::Connection::STATE_INPROGRESS, conn2->state(), 1000);

conn2->ReceivedPingResponse();

- EXPECT_EQ_WAIT(conn2, ch.best_connection(), 1000);

+ EXPECT_EQ_WAIT(conn2, ch.selected_connection(), 1000);

EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState());

// When |conn1| comes back again, |conn2| will be pruned again.

conn1->ReceivedPingResponse();

- EXPECT_EQ_WAIT(conn1, ch.best_connection(), 1000);

+ EXPECT_EQ_WAIT(conn1, ch.selected_connection(), 1000);

EXPECT_TRUE_WAIT(!conn2->active(), 1000);

EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());

}

@@ -2805,7 +2904,7 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest,

// Verify that conn3 will be the "best connection" since it is readable and

// writable. After |MAX_CURRENT_STRONG_INTERVAL|, it should be the next

// pingable connection.

- EXPECT_TRUE_WAIT(conn3 == ch.best_connection(), 5000);

+ EXPECT_TRUE_WAIT(conn3 == ch.selected_connection(), 5000);

WAIT(false, max_strong_interval + 100);

conn3->ReceivedPingResponse();

ASSERT_TRUE(conn3->writable());

« webrtc/p2p/base/p2ptransportchannel.cc ('K') | « webrtc/p2p/base/p2ptransportchannel.cc ('k') | webrtc/p2p/base/port.h » ('j') | webrtc/p2p/base/port.cc » ('J')