Connection resurrected with incorrect candidate type.

webrtc/p2p/base/p2ptransportchannel_unittest.cc

 /*
  *  Copyright 2009 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1703 matching lines @@
   // an exchange of pings.
   EXPECT_TRUE_WAIT(
       ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]),
       3000);
 
   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]);
     ch->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
   }
 
   void OnChannelRequestSignaling(cricket::TransportChannelImpl* channel) {
     channel->OnSignalingReady();
   }
 
@@ skipping 32 matching lines @@
     EXPECT_TRUE_WAIT(conn->last_ping_sent() > 0, 3000);
     return conn->last_ping_sent();
   }
 
  private:
   rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
   rtc::scoped_ptr<rtc::VirtualSocketServer> vss_;
   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);
   ch.Connect();
   ch.OnCandidate(CreateCandidate("1.1.1.1", 1, 1));
   ch.OnCandidate(CreateCandidate("2.2.2.2", 2, 2));
 
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   // Receiving a ping causes a "triggered check" which should make conn1
   // be pinged first instead of conn2, even though conn2 has a higher priority.
   conn1->ReceivedPing();
   uint32 ping1 = WaitForFirstPing(conn1);
   uint32 ping2 = WaitForFirstPing(conn2);
   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);
   PrepareChannel(&ch);
   ch.Connect();
   ch.OnCandidate(CreateCandidate("1.1.1.1", 1, 1));
   ch.OnCandidate(CreateCandidate("2.2.2.2", 2, 2));
 
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
   ASSERT_TRUE(conn2 != nullptr);
 
   // No ping received, so no "triggered check" so conn2 is pinged
   // before conn1 because it has a higher priority.
   uint32 ping2 = WaitForFirstPing(conn2);
   uint32 ping1 = WaitForFirstPing(conn1);
   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);
   ch.Connect();
   ch.OnCandidate(CreateCandidate("1.1.1.1", 1, 1));
   ch.OnCandidate(CreateCandidate("2.2.2.2", 2, 2));
 
   cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn1 != nullptr);
@@ skipping 12 matching lines @@
   if ((ping1 - before_pings) < 900) {
     // If ping2 takes longer than 900ms to send, then there's a chance
     // that ping1 will go out before ping2 because of the pings that
     // keep a connection writable, which come before triggered checks.
     // This would only happen if the tests are running very, very
     // slowly.  But to avoid the tests being flaky on very slow bots,
     // we add the check.
     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);
+}