Do not stop a session unless the candidate of a writable connection belongs to the

latest generation.

BUG=webrtc:5644
R=deadbeef@webrtc.org, pthatcher@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#12599}
This commit is contained in:
Honghai Zhang 2016-05-02 17:28:35 -07:00
parent 5dd42fd849
commit 5a2463796e
3 changed files with 20 additions and 9 deletions

View File

@ -1405,10 +1405,14 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
}
// May stop the allocator session when at least one connection becomes
// strongly connected after starting to get ports. It is not enough to check
// strongly connected after starting to get ports and the local candidate of
// the connection is at the latest generation. It is not enough to check
// that the connection becomes weakly connected because the connection may be
// changing from (writable, receiving) to (writable, not receiving).
if (!connection->weak()) {
bool strongly_connected = !connection->weak();
bool latest_generation = connection->local_candidate().generation() >=
allocator_session()->generation();
if (strongly_connected && latest_generation) {
MaybeStopPortAllocatorSessions();
}

View File

@ -2549,9 +2549,10 @@ TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {
EXPECT_TRUE_WAIT(ch.connections().empty(), 1000);
}
// Test that after a port allocator session is started, it will be stopped
// when a new connection becomes writable and receiving. Also test that this
// holds even if the transport channel did not lose the writability.
// Tests that after a port allocator session is started, it will be stopped
// when a new connection becomes writable and receiving. Also tests that if a
// connection belonging to an old session becomes writable, it won't stop
// the current port allocator session.
TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, &pa);
@ -2565,11 +2566,17 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) {
conn1->ReceivedPingResponse(); // Becomes writable and receiving
EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts());
// Restart gathering even if the transport channel is still writable.
// It should stop getting ports after a new connection becomes strongly
// connected.
// Start a new session. Even though conn1, which belongs to an older
// session, becomes unwritable and writable again, it should not stop the
// current session.
ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]);
ch.MaybeStartGathering();
conn1->Prune();
conn1->ReceivedPingResponse();
EXPECT_TRUE(ch.allocator_session()->IsGettingPorts());
// But if a new connection created from the new session becomes writable,
// it will stop the current session.
ch.AddRemoteCandidate(CreateHostCandidate("2.2.2.2", 2, 100));
cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn2 != nullptr);

View File

@ -120,7 +120,7 @@ class FakePortAllocatorSession : public PortAllocatorSession {
void AddPort(cricket::Port* port) {
port->set_component(component_);
port->set_generation(0);
port->set_generation(generation());
port->SignalPortComplete.connect(
this, &FakePortAllocatorSession::OnPortComplete);
port->PrepareAddress();