Connection resurrected with incorrect candidate type.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 4217d0254892960be8f94c29b06fc26b6f5fd9e3..96b2af9cce1c38ce98c8de58b83de9702a599a71 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -1721,19 +1721,19 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) {
   DestroyChannels();
 }
 
-class P2PTransportChannelPingOrderTest : public testing::Test,
-                                         public sigslot::has_slots<> {
+class P2PTransportChannelSingleEndpointTest : public testing::Test,

[pthatcher1, 2015/06/09 23:58:03]

Let's just call it P2PTransportChannelTest

[guoweis_webrtc, 2015/06/10 18:12:41]

P2PTransportChannelTest exists already.

+                                              public sigslot::has_slots<> {
  public:
-  P2PTransportChannelPingOrderTest() :
-      pss_(new rtc::PhysicalSocketServer),
-      vss_(new rtc::VirtualSocketServer(pss_.get())),
-      ss_scope_(vss_.get()) {
-  }
+  P2PTransportChannelSingleEndpointTest()
+      : pss_(new rtc::PhysicalSocketServer),
+        vss_(new rtc::VirtualSocketServer(pss_.get())),
+        ss_scope_(vss_.get()) {}
 
  protected:
   void PrepareChannel(cricket::P2PTransportChannel* ch) {
     ch->SignalRequestSignaling.connect(
-        this, &P2PTransportChannelPingOrderTest::OnChannelRequestSignaling);
+        this,
+        &P2PTransportChannelSingleEndpointTest::OnChannelRequestSignaling);
     ch->SetIceProtocolType(cricket::ICEPROTO_RFC5245);
     ch->SetIceRole(cricket::ICEROLE_CONTROLLING);
     ch->SetIceCredentials(kIceUfrag[0], kIcePwd[0]);
@@ -1786,7 +1786,7 @@ class P2PTransportChannelPingOrderTest : public testing::Test,
   rtc::SocketServerScope ss_scope_;
 };
 
-TEST_F(P2PTransportChannelPingOrderTest, TestTriggeredChecks) {
+TEST_F(P2PTransportChannelSingleEndpointTest, TestTriggeredChecks) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
   PrepareChannel(&ch);
@@ -1807,7 +1807,7 @@ TEST_F(P2PTransportChannelPingOrderTest, TestTriggeredChecks) {
   EXPECT_LT(ping1, ping2);
 }
 
-TEST_F(P2PTransportChannelPingOrderTest,
+TEST_F(P2PTransportChannelSingleEndpointTest,
        TestNoTriggeredChecksWithoutIncomingPing) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
@@ -1828,7 +1828,8 @@ TEST_F(P2PTransportChannelPingOrderTest,
   EXPECT_LT(ping2, ping1);
 }
 
-TEST_F(P2PTransportChannelPingOrderTest, TestNoTriggeredChecksWhenWritable) {
+TEST_F(P2PTransportChannelSingleEndpointTest,
+       TestNoTriggeredChecksWhenWritable) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, nullptr, &pa);
   PrepareChannel(&ch);
@@ -1861,3 +1862,53 @@ TEST_F(P2PTransportChannelPingOrderTest, TestNoTriggeredChecksWhenWritable) {
     EXPECT_LT(ping2, ping1);
   }
 }
+
+TEST_F(P2PTransportChannelSingleEndpointTest, ConnectionResurrection) {
+  cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+  cricket::P2PTransportChannel ch("connection resurrection", 1, nullptr, &pa);
+  PrepareChannel(&ch);
+  ch.Connect();
+
+  // Create conn1 and keep track of original candidate priority.
+  ch.OnCandidate(CreateCandidate("1.1.1.1", 1, 1));
+  cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
+  ASSERT_TRUE(conn1 != nullptr);
+  uint32 remote_priority = conn1->remote_candidate().priority();
+
+  // Create a higher priority candidate and make the connection
+  // readable/writable. This will prune conn1.
+  ch.OnCandidate(CreateCandidate("2.2.2.2", 2, 2));
+  cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+  ASSERT_TRUE(conn2 != nullptr);
+  conn2->ReceivedPing();
+  conn2->ReceivedPingResponse();
+
+  // Wait for conn1 being destroyed.
+  EXPECT_TRUE_WAIT(GetConnectionTo(&ch, "1.1.1.1", 1) == nullptr, 3000);
+  cricket::Port* port = static_cast<cricket::Port*>(ch.ports()[0]);

[pthatcher1, 2015/06/09 23:58:03]

Can you make this its own function?

[guoweis_webrtc, 2015/06/10 18:12:41]

Done.

+
+  // Create a minimal STUN message with prflx priority.
+  cricket::IceMessage request;
+  request.SetType(cricket::STUN_BINDING_REQUEST);
+  request.AddAttribute(new cricket::StunByteStringAttribute(
+      cricket::STUN_ATTR_USERNAME, kIceUfrag[1]));
+  uint32 prflx_priority = cricket::ICE_TYPE_PREFERENCE_PRFLX << 24 |
+                          (conn1->remote_candidate().priority() & 0x00FFFFFF);

[pthatcher1, 2015/06/09 23:58:03]

Can you make this its own method?

[guoweis_webrtc, 2015/06/10 18:12:41]

Done.

+  request.AddAttribute(new cricket::StunUInt32Attribute(
+      cricket::STUN_ATTR_PRIORITY, prflx_priority));
+  EXPECT_NE(prflx_priority, remote_priority);
+
+  // conn1 should be resurrected with original priority.
+  port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1),
+                             cricket::PROTO_UDP, &request, kIceUfrag[1], false);
+  conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
+  ASSERT_TRUE(conn1 != nullptr);
+  EXPECT_EQ(conn1->remote_candidate().priority(), remote_priority);
+
+  // conn3, a real prflx connection, should have prflx priority.
+  port->SignalUnknownAddress(port, rtc::SocketAddress("3.3.3.3", 1),
+                             cricket::PROTO_UDP, &request, kIceUfrag[1], false);
+  cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 1);
+  ASSERT_TRUE(conn3 != nullptr);
+  EXPECT_EQ(conn3->remote_candidate().priority(), prflx_priority);
+}