From 5a2463796e3af75eec36b953878f4ce536240af3 Mon Sep 17 00:00:00 2001
From: Honghai Zhang <honghaiz@webrtc.org>
Date: Mon, 2 May 2016 17:28:35 -0700
Subject: [PATCH] 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}
---
 webrtc/p2p/base/p2ptransportchannel.cc        |  8 ++++++--
 .../p2p/base/p2ptransportchannel_unittest.cc  | 19 +++++++++++++------
 webrtc/p2p/client/fakeportallocator.h         |  2 +-
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 9822eb4c49..206113c4eb 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -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();
   }
 
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index c7dbacf881..21564e6bab 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -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);
diff --git a/webrtc/p2p/client/fakeportallocator.h b/webrtc/p2p/client/fakeportallocator.h
index d16c9b6d5c..76357a5ffa 100644
--- a/webrtc/p2p/client/fakeportallocator.h
+++ b/webrtc/p2p/client/fakeportallocator.h
@@ -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();