Add the last_sent_packet_id to the candidate pair change signal

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 299 matching lines @@
     cricket::P2PTransportChannel* channel = new cricket::P2PTransportChannel(
         "test content name", component, GetAllocator(endpoint));
     channel->SignalCandidateGathered.connect(
         this, &P2PTransportChannelTestBase::OnCandidateGathered);
     channel->SignalCandidatesRemoved.connect(
         this, &P2PTransportChannelTestBase::OnCandidatesRemoved);
     channel->SignalReadPacket.connect(
         this, &P2PTransportChannelTestBase::OnReadPacket);
     channel->SignalRoleConflict.connect(
         this, &P2PTransportChannelTestBase::OnRoleConflict);
-    channel->SignalSelectedCandidatePairChanged.connect(
-        this, &P2PTransportChannelTestBase::OnSelectedCandidatePairChanged);
     channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd);
     if (clear_remote_candidates_ufrag_pwd_) {
       // This only needs to be set if we're clearing them from the
       // candidates.  Some unit tests rely on this not being set.
       channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd);
     }
     channel->SetIceRole(GetEndpoint(endpoint)->ice_role());
     channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker());
     channel->Connect();
     channel->MaybeStartGathering();
@@ skipping 405 matching lines @@
     packets.push_front(std::string(data, len));
   }
   void OnRoleConflict(cricket::TransportChannelImpl* channel) {
     GetEndpoint(channel)->OnRoleConflict(true);
     cricket::IceRole new_role =
         GetEndpoint(channel)->ice_role() == cricket::ICEROLE_CONTROLLING ?
             cricket::ICEROLE_CONTROLLED : cricket::ICEROLE_CONTROLLING;
     channel->SetIceRole(new_role);
   }
 
-  void OnSelectedCandidatePairChanged(
-      cricket::TransportChannel* channel,
-      cricket::CandidatePairInterface* candidate_pair) {
-    ++num_selected_candidate_pair_changes_;
-  }
-
   int SendData(cricket::TransportChannel* channel,
                const char* data, size_t len) {
     rtc::PacketOptions options;
     return channel->SendPacket(data, len, options, 0);
   }
   bool CheckDataOnChannel(cricket::TransportChannel* channel,
                           const char* data, int len) {
     return GetChannelData(channel)->CheckData(data, len);
   }
   static const cricket::Candidate* LocalCandidate(
@@ skipping 33 matching lines @@
   }
 
   void set_clear_remote_candidates_ufrag_pwd(bool clear) {
     clear_remote_candidates_ufrag_pwd_ = clear;
   }
 
   void set_force_relay(bool relay) {
     force_relay_ = relay;
   }
 
-  int num_selected_candidate_pair_changes() {
-    return num_selected_candidate_pair_changes_;
-  }
-  void set_num_selected_candidate_pair_changes(int num) {
-    num_selected_candidate_pair_changes_ = num;
-  }
-
  private:
   rtc::Thread* main_;
   rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
   rtc::scoped_ptr<rtc::VirtualSocketServer> vss_;
   rtc::scoped_ptr<rtc::NATSocketServer> nss_;
   rtc::scoped_ptr<rtc::FirewallSocketServer> ss_;
   rtc::SocketServerScope ss_scope_;
   rtc::scoped_ptr<cricket::TestStunServer> stun_server_;
   cricket::TestTurnServer turn_server_;
   cricket::TestRelayServer relay_server_;
   rtc::SocksProxyServer socks_server1_;
   rtc::SocksProxyServer socks_server2_;
   Endpoint ep1_;
   Endpoint ep2_;
-  int num_selected_candidate_pair_changes_ = 0;
   bool clear_remote_candidates_ufrag_pwd_;
   bool force_relay_;
 };
 
 // The tests have only a few outcomes, which we predefine.
 const P2PTransportChannelTestBase::Result P2PTransportChannelTestBase::
     kLocalUdpToLocalUdp("local", "udp", "local", "udp",
                         "local", "udp", "local", "udp", 1000);
 const P2PTransportChannelTestBase::Result P2PTransportChannelTestBase::
     kLocalUdpToStunUdp("local", "udp", "stun", "udp",
@@ skipping 750 matching lines @@
 
 // Test that we can establish connectivity when both peers are multihomed.
 TEST_F(P2PTransportChannelMultihomedTest, DISABLED_TestBasic) {
   AddAddress(0, kPublicAddrs[0]);
   AddAddress(0, kAlternateAddrs[0]);
   AddAddress(1, kPublicAddrs[1]);
   AddAddress(1, kAlternateAddrs[1]);
   Test(kLocalUdpToLocalUdp);
 }
 
-// Test that we can quickly switch links if an interface goes down. This also
-// tests that when the link switches, |SignalSelectedCandidatePairChanged| will
-// be fired. The controlled side has two interfaces and one will die.
+// Test that we can quickly switch links if an interface goes down.
+// The controlled side has two interfaces and one will die.
 TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
   AddAddress(0, kPublicAddrs[0]);
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
   AddAddress(1, kAlternateAddrs[1]);
   AddAddress(1, kPublicAddrs[1]);
 
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
 
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   EXPECT_TRUE(
       ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
   // Make the receiving timeout shorter for testing.
   cricket::IceConfig config = CreateIceConfig(1000, false);
   ep1_ch1()->SetIceConfig(config);
   ep2_ch1()->SetIceConfig(config);
 
-  set_num_selected_candidate_pair_changes(0);
   // Blackhole any traffic to or from the public addrs.
   LOG(LS_INFO) << "Failing over...";
   fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[1]);
   // The best connections will switch, so keep references to them.
   const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();
   const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();
   // We should detect loss of receiving within 1 second or so.
   EXPECT_TRUE_WAIT(
       !best_connection1->receiving() && !best_connection2->receiving(), 3000);
 
   // We should switch over to use the alternate addr immediately on both sides
   // when we are not receiving.
   EXPECT_TRUE_WAIT(
       ep1_ch1()->best_connection()->receiving() &&
       ep2_ch1()->best_connection()->receiving(), 1000);
   EXPECT_TRUE(LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]));
   EXPECT_TRUE(
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]));
   EXPECT_TRUE(
       LocalCandidate(ep2_ch1())->address().EqualIPs(kAlternateAddrs[1]));
-  // It should have changed twice, one on each side.
-  EXPECT_EQ(2, num_selected_candidate_pair_changes());

[Taylor Brandstetter, 2016/03/30 21:17:14]

Why is this being removed?

[honghaiz3, 2016/03/30 21:48:09]

I moved the tests  for selected candidate pair to
TestSelectConnectionBeforeNomination. 

 
   DestroyChannels();
 }
 
-// Test that we can quickly switch links if an interface goes down. This also
-// tests that when the link switches, |SignalSelectedCandidatePairChanged| will
-// be fired. The controlling side has two interfaces and one will die.
+// Test that we can quickly switch links if an interface goes down.
+// The controlling side has two interfaces and one will die.
 TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
   AddAddress(0, kAlternateAddrs[0]);
   AddAddress(0, kPublicAddrs[0]);
   AddAddress(1, kPublicAddrs[1]);
 
   // Use only local ports for simplicity.
   SetAllocatorFlags(0, kOnlyLocalPorts);
   SetAllocatorFlags(1, kOnlyLocalPorts);
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
   EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
                               ep2_ch1()->receiving() && ep2_ch1()->writable(),
                           1000, 1000);
   EXPECT_TRUE(
       ep1_ch1()->best_connection() && ep2_ch1()->best_connection() &&
       LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
 
   // Make the receiving timeout shorter for testing.
   cricket::IceConfig config = CreateIceConfig(1000, false);
   ep1_ch1()->SetIceConfig(config);
   ep2_ch1()->SetIceConfig(config);
-  set_num_selected_candidate_pair_changes(0);
 
   // Blackhole any traffic to or from the public addrs.
   LOG(LS_INFO) << "Failing over...";
   fw()->AddRule(false, rtc::FP_ANY, rtc::FD_ANY, kPublicAddrs[0]);
   // The best connections will switch, so keep references to them.
   const cricket::Connection* best_connection1 = ep1_ch1()->best_connection();
   const cricket::Connection* best_connection2 = ep2_ch1()->best_connection();
   // We should detect loss of receiving within 1 second or so.
   EXPECT_TRUE_WAIT(
       !best_connection1->receiving() && !best_connection2->receiving(), 3000);
 
   // We should switch over to use the alternate addr immediately on both sides
   // when we are not receiving.
   EXPECT_TRUE_WAIT(
       ep1_ch1()->best_connection()->receiving() &&
       ep2_ch1()->best_connection()->receiving(), 1000);
   EXPECT_TRUE(
     LocalCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]));
   EXPECT_TRUE(RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
   EXPECT_TRUE(
       RemoteCandidate(ep2_ch1())->address().EqualIPs(kAlternateAddrs[0]));
-  EXPECT_EQ(2, num_selected_candidate_pair_changes());
 
   DestroyChannels();
 }
 
 // Tests that a Wifi-Wifi connection has the highest precedence.
 TEST_F(P2PTransportChannelMultihomedTest, TestPreferWifiToWifiConnection) {
   // The interface names are chosen so that |cellular| would have higher
   // candidate priority if it is not for the network type.
   auto& wifi = kAlternateAddrs;
   auto& cellular = kPublicAddrs;
@@ skipping 197 matching lines @@
   P2PTransportChannelPingTest()
       : pss_(new rtc::PhysicalSocketServer),
         vss_(new rtc::VirtualSocketServer(pss_.get())),
         ss_scope_(vss_.get()) {}
 
  protected:
   void PrepareChannel(cricket::P2PTransportChannel* ch) {
     ch->SetIceRole(cricket::ICEROLE_CONTROLLING);
     ch->SetIceCredentials(kIceUfrag[0], kIcePwd[0]);
     ch->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]);
+    ch->SignalSelectedCandidatePairChanged.connect(
+        this, &P2PTransportChannelPingTest::OnSelectedCandidatePairChanged);
     ch->SignalReadyToSend.connect(this,
                                   &P2PTransportChannelPingTest::OnReadyToSend);
   }
 
   cricket::Candidate CreateHostCandidate(const std::string& ip,
                                          int port,
                                          int priority,
                                          const std::string& ufrag = "") {
     cricket::Candidate c;
     c.set_address(rtc::SocketAddress(ip, port));
@@ skipping 31 matching lines @@
 
   cricket::Connection* FindNextPingableConnectionAndPingIt(
       cricket::P2PTransportChannel* ch) {
     cricket::Connection* conn = ch->FindNextPingableConnection();
     if (conn) {
       ch->MarkConnectionPinged(conn);
     }
     return conn;
   }
 
+  void OnSelectedCandidatePairChanged(
+      cricket::TransportChannel* transport_channel,
+      cricket::CandidatePairInterface* selected_candidate_pair,
+      int last_sent_packet_id) {
+    last_selected_candidate_pair_ = selected_candidate_pair;
+    last_sent_packet_id_ = last_sent_packet_id;
+  }
+
   void OnReadyToSend(cricket::TransportChannel* channel) {
     channel_ready_to_send_ = true;
   }
 
+  cricket::CandidatePairInterface* last_selected_candidate_pair() {
+    return last_selected_candidate_pair_;
+  }
+  int last_sent_packet_id() { return last_sent_packet_id_; }
   bool channel_ready_to_send() { return channel_ready_to_send_; }
   void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }
 
  private:
   rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
   rtc::scoped_ptr<rtc::VirtualSocketServer> vss_;
   rtc::SocketServerScope ss_scope_;
+  cricket::CandidatePairInterface* last_selected_candidate_pair_ = nullptr;
+  int last_sent_packet_id_ = -1;
   bool channel_ready_to_send_ = false;
 };
 
 TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("trigger checks", 1, &pa);
   PrepareChannel(&ch);
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1));
@@ skipping 170 matching lines @@
   conn1->ReceivedPing();
   conn1->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0));
   EXPECT_TRUE_WAIT(ch.best_connection() != nullptr, 1000);
   EXPECT_TRUE_WAIT(ch.receiving(), 1000);
   EXPECT_TRUE_WAIT(!ch.receiving(), 1000);
 }
 
 // The controlled side will select a connection as the "best connection" based
 // on priority until the controlling side nominates a connection, at which
 // point the controlled side will select that connection as the
-// "best connection". Plus, the channel will fire SignalReadyToSend if the new
-// best connection is writable.
+// "best connection". Plus, SignalSelectedCandidatePair will be fired if the
+// best connection changes and SignalReadyToSend will be fired if the new best
+// connection is writable.
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) {
   cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   cricket::P2PTransportChannel ch("receiving state change", 1, &pa);
   PrepareChannel(&ch);
   ch.SetIceRole(cricket::ICEROLE_CONTROLLED);
   ch.Connect();
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateHostCandidate("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());
-  // Channel is not ready because it is not writable.
+  EXPECT_EQ(conn1, last_selected_candidate_pair());
+  EXPECT_EQ(-1, last_sent_packet_id());
+  // Channel is not ready to send because it is not writable.
   EXPECT_FALSE(channel_ready_to_send());
 
+  int last_packet_id = 0;
+  ch.set_last_sent_packet_id(++last_packet_id);
   // When a higher priority candidate comes in, the new connection is chosen
   // as the best connection.
   ch.AddRemoteCandidate(CreateHostCandidate("2.2.2.2", 2, 10));
   cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
   ASSERT_TRUE(conn2 != nullptr);
   EXPECT_EQ(conn2, ch.best_connection());
+  EXPECT_EQ(conn2, last_selected_candidate_pair());
+  EXPECT_EQ(last_packet_id, last_sent_packet_id());
   EXPECT_FALSE(channel_ready_to_send());
 
   // If a stun request with use-candidate attribute arrives, the receiving
   // connection will be set as the best connection, even though
   // its priority is lower.
+  ch.set_last_sent_packet_id(++last_packet_id);
   ch.AddRemoteCandidate(CreateHostCandidate("3.3.3.3", 3, 1));
   cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
   ASSERT_TRUE(conn3 != nullptr);
   // Because it has a lower priority, the best connection is still conn2.
   EXPECT_EQ(conn2, ch.best_connection());
   conn3->ReceivedPingResponse();  // Become writable.
   // But if it is nominated via use_candidate, it is chosen as the best
   // connection.
   conn3->set_nominated(true);
   conn3->SignalNominated(conn3);
   EXPECT_EQ(conn3, ch.best_connection());
+  EXPECT_EQ(conn3, last_selected_candidate_pair());
+  EXPECT_EQ(last_packet_id, last_sent_packet_id());
   EXPECT_TRUE(channel_ready_to_send());
 
   // Even if another higher priority candidate arrives,
   // it will not be set as the best connection because the best connection
   // is nominated by the controlling side.
+  ch.set_last_sent_packet_id(++last_packet_id);
   ch.AddRemoteCandidate(CreateHostCandidate("4.4.4.4", 4, 100));
   cricket::Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);
   ASSERT_TRUE(conn4 != nullptr);
   EXPECT_EQ(conn3, ch.best_connection());
   // But if it is nominated via use_candidate and writable, it will be set as
   // the best connection.
   conn4->set_nominated(true);
   conn4->SignalNominated(conn4);
   // Not switched yet because conn4 is not writable.
   EXPECT_EQ(conn3, ch.best_connection());
   reset_channel_ready_to_send();
   // The best connection switches after conn4 becomes writable.
   conn4->ReceivedPingResponse();
   EXPECT_EQ(conn4, ch.best_connection());
+  EXPECT_EQ(conn4, last_selected_candidate_pair());
+  EXPECT_EQ(last_packet_id, last_sent_packet_id());
   // SignalReadyToSend is fired again because conn4 is writable.
   EXPECT_TRUE(channel_ready_to_send());
 }
 
 // The controlled side will select a connection as the "best connection" based
 // on requests from an unknown address before the controlling side nominates
 // a connection, and will nominate a connection from an unknown address if the
 // request contains the use_candidate attribute. Plus, it will also sends back
 // a ping response and set the ICE pwd in the remote candidate appropriately.
 TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
@@ skipping 521 matching lines @@
 
   // TCP Relay/Relay is the next.
   VerifyNextPingableConnection(cricket::RELAY_PORT_TYPE,
                                cricket::RELAY_PORT_TYPE,
                                cricket::TCP_PROTOCOL_NAME);
 
   // Finally, Local/Relay will be pinged.
   VerifyNextPingableConnection(cricket::LOCAL_PORT_TYPE,
                                cricket::RELAY_PORT_TYPE);
 }