Do not prune if the current best connection is weak.

Otherwise, we may delete a useful connection because the current best connection may be failing.

BUG=

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

Cr-Commit-Position: refs/heads/master@{#10063}
This commit is contained in:
honghaiz 2015-09-24 13:14:47 -07:00 committed by Commit bot
parent ea70d77fd5
commit 8937437872
3 changed files with 50 additions and 8 deletions

View File

@ -1000,7 +1000,7 @@ void P2PTransportChannel::PruneConnections() {
// which point, we would prune out the current best connection). We leave
// connections on other networks because they may not be using the same
// resources and they may represent very distinct paths over which we can
// switch. If the |primier| connection is not connected, we may be
// switch. If the |premier| connection is not connected, we may be
// reconnecting a TCP connection and temporarily do not prune connections in
// this network. See the big comment in CompareConnections.
@ -1010,14 +1010,16 @@ void P2PTransportChannel::PruneConnections() {
networks.insert(conn->port()->Network());
}
for (rtc::Network* network : networks) {
Connection* primier = GetBestConnectionOnNetwork(network);
if (!(primier && primier->writable() && primier->connected())) {
Connection* premier = GetBestConnectionOnNetwork(network);
// Do not prune connections if the current best connection is weak on this
// network. Otherwise, it may delete connections prematurely.
if (!premier || premier->Weak()) {
continue;
}
for (Connection* conn : connections_) {
if ((conn != primier) && (conn->port()->Network() == network) &&
(CompareConnectionCandidates(primier, conn) >= 0)) {
if ((conn != premier) && (conn->port()->Network() == network) &&
(CompareConnectionCandidates(premier, conn) >= 0)) {
conn->Prune();
}
}
@ -1092,8 +1094,7 @@ void P2PTransportChannel::HandleAllTimedOut() {
}
bool P2PTransportChannel::Weak() const {
return !(best_connection_ && best_connection_->receiving() &&
best_connection_->writable());
return !best_connection_ || best_connection_->Weak();
}
// If we have a best connection, return it, otherwise return top one in the

View File

@ -1832,7 +1832,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
// as the best connection.
ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 10));
cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn1 != nullptr);
ASSERT_TRUE(conn2 != nullptr);
EXPECT_EQ(conn2, ch.best_connection());
// If a stun request with use-candidate attribute arrives, the receiving
@ -1992,3 +1992,42 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
EXPECT_EQ(conn3, ch.best_connection());
}
// When the current best connection is strong, lower-priority connections will
// be pruned. Otherwise, lower-priority connections are kept.
TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
ch.Connect();
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateCandidate("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());
conn1->ReceivedPingResponse(); // Becomes writable and receiving
// When a higher-priority, nominated candidate comes in, the connections with
// lower-priority are pruned.
ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 10));
cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn2 != nullptr);
conn2->ReceivedPingResponse(); // Becomes writable and receiving
conn2->set_nominated(true);
conn2->SignalNominated(conn2);
EXPECT_TRUE_WAIT(conn1->pruned(), 3000);
ch.SetReceivingTimeout(500);
// Wait until conn2 becomes not receiving.
EXPECT_TRUE_WAIT(!conn2->receiving(), 3000);
ch.AddRemoteCandidate(CreateCandidate("3.3.3.3", 3, 1));
cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
ASSERT_TRUE(conn3 != nullptr);
// The best connection should still be conn2. Even through conn3 has lower
// priority and is not receiving/writable, it is not pruned because the best
// connection is not receiving.
WAIT(conn3->pruned(), 1000);
EXPECT_FALSE(conn3->pruned());
}

View File

@ -436,6 +436,8 @@ class Connection : public rtc::MessageHandler,
// be false for TCP connections.
bool connected() const { return connected_; }
bool Weak() const { return !(writable() && receiving() && connected()); }
// Estimate of the round-trip time over this connection.
uint32 rtt() const { return rtt_; }