Ping backup connection at a slower rate

and make it configurable from the app.
Changed the decision on whether a connection is pingable:
1.Check whether a connection is a backup connection. A connection is considered as a backup connection only if the channel is complete, the connection is active and it is not the best connection.
2. Ping a non-backup connection if it is active and for backup connection, ping it at a slower rate.
Note the default behavior is the same as before.

Also cached the channel state since we are accessing it more often.
BUG=webrtc:5034
R=pthatcher@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#10900}

talk/app/webrtc/java/jni/peerconnection_jni.cc

@@ -1580,6 +1580,9 @@ static void JavaRTCConfigurationToJsepRTCConfiguration(
   jfieldID j_ice_connection_receiving_timeout_id =
       GetFieldID(jni, j_rtc_config_class, "iceConnectionReceivingTimeout", "I");
 
+  jfieldID j_ice_backup_candidate_pair_ping_interval_id = GetFieldID(
+      jni, j_rtc_config_class, "iceBackupCandidatePairPingInterval", "I");
+
   jfieldID j_continual_gathering_policy_id =
       GetFieldID(jni, j_rtc_config_class, "continualGatheringPolicy",
                  "Lorg/webrtc/PeerConnection$ContinualGatheringPolicy;");
@@ -1601,6 +1604,8 @@ static void JavaRTCConfigurationToJsepRTCConfiguration(
       jni, j_rtc_config, j_audio_jitter_buffer_fast_accelerate_id);
   rtc_config->ice_connection_receiving_timeout =
       GetIntField(jni, j_rtc_config, j_ice_connection_receiving_timeout_id);
+  rtc_config->ice_backup_candidate_pair_ping_interval = GetIntField(
+      jni, j_rtc_config, j_ice_backup_candidate_pair_ping_interval_id);
   rtc_config->continual_gathering_policy =
       JavaContinualGatheringPolicyToNativeType(
           jni, j_continual_gathering_policy);

talk/app/webrtc/java/src/org/webrtc/PeerConnection.java

@@ -150,6 +150,7 @@ public class PeerConnection {
     public int audioJitterBufferMaxPackets;
     public boolean audioJitterBufferFastAccelerate;
     public int iceConnectionReceivingTimeout;
+    public int iceBackupCandidatePairPingInterval;
     public KeyType keyType;
     public ContinualGatheringPolicy continualGatheringPolicy;
 
@@ -162,6 +163,7 @@ public class PeerConnection {
       audioJitterBufferMaxPackets = 50;
       audioJitterBufferFastAccelerate = false;
       iceConnectionReceivingTimeout = -1;
+      iceBackupCandidatePairPingInterval = -1;
       keyType = KeyType.ECDSA;
       continualGatheringPolicy = ContinualGatheringPolicy.GATHER_ONCE;
     }

talk/app/webrtc/objc/RTCPeerConnectionInterface.mm

@@ -39,6 +39,7 @@
 @synthesize tcpCandidatePolicy = _tcpCandidatePolicy;
 @synthesize audioJitterBufferMaxPackets = _audioJitterBufferMaxPackets;
 @synthesize iceConnectionReceivingTimeout = _iceConnectionReceivingTimeout;
+@synthesize iceBackupCandidatePairPingInterval = _iceBackupCandidatePairPingInterval;
 
 - (instancetype)init {
   if (self = [super init]) {
@@ -51,6 +52,7 @@
         [RTCEnumConverter tcpCandidatePolicyForNativeEnum:config.tcp_candidate_policy];
     _audioJitterBufferMaxPackets = config.audio_jitter_buffer_max_packets;
     _iceConnectionReceivingTimeout = config.ice_connection_receiving_timeout;
+    _iceBackupCandidatePairPingInterval = config.ice_backup_candidate_pair_ping_interval;
   }
   return self;
 }
@@ -60,7 +62,8 @@
                             rtcpMuxPolicy:(RTCRtcpMuxPolicy)rtcpMuxPolicy
                        tcpCandidatePolicy:(RTCTcpCandidatePolicy)tcpCandidatePolicy
               audioJitterBufferMaxPackets:(int)audioJitterBufferMaxPackets
-            iceConnectionReceivingTimeout:(int)iceConnectionReceivingTimeout {
+            iceConnectionReceivingTimeout:(int)iceConnectionReceivingTimeout
+       iceBackupCandidatePairPingInterval:(int)iceBackupCandidatePairPingInterval {
   if (self = [super init]) {
     _iceTransportsType = iceTransportsType;
     _bundlePolicy = bundlePolicy;
@@ -68,6 +71,7 @@
     _tcpCandidatePolicy = tcpCandidatePolicy;
     _audioJitterBufferMaxPackets = audioJitterBufferMaxPackets;
     _iceConnectionReceivingTimeout = iceConnectionReceivingTimeout;
+    _iceBackupCandidatePairPingInterval = iceBackupCandidatePairPingInterval;
   }
   return self;
 }
@@ -85,8 +89,8 @@
   nativeConfig.tcp_candidate_policy =
       [RTCEnumConverter nativeEnumForTcpCandidatePolicy:_tcpCandidatePolicy];
   nativeConfig.audio_jitter_buffer_max_packets = _audioJitterBufferMaxPackets;
-  nativeConfig.ice_connection_receiving_timeout =
-      _iceConnectionReceivingTimeout;
+  nativeConfig.ice_connection_receiving_timeout = _iceConnectionReceivingTimeout;
+  nativeConfig.ice_backup_candidate_pair_ping_interval = _iceBackupCandidatePairPingInterval;
   return nativeConfig;
 }
 

talk/app/webrtc/objc/public/RTCPeerConnectionInterface.h

@@ -64,12 +64,14 @@ typedef NS_ENUM(NSInteger, RTCTcpCandidatePolicy) {
 @property(nonatomic, assign) RTCTcpCandidatePolicy tcpCandidatePolicy;
 @property(nonatomic, assign) int audioJitterBufferMaxPackets;
 @property(nonatomic, assign) int iceConnectionReceivingTimeout;
+@property(nonatomic, assign) int iceBackupCandidatePairPingInterval;
 
 - (instancetype)initWithIceTransportsType:(RTCIceTransportsType)iceTransportsType
                              bundlePolicy:(RTCBundlePolicy)bundlePolicy
                             rtcpMuxPolicy:(RTCRtcpMuxPolicy)rtcpMuxPolicy
                        tcpCandidatePolicy:(RTCTcpCandidatePolicy)tcpCandidatePolicy
               audioJitterBufferMaxPackets:(int)audioJitterBufferMaxPackets
-            iceConnectionReceivingTimeout:(int)iceConnectionReceivingTimeout;
+            iceConnectionReceivingTimeout:(int)iceConnectionReceivingTimeout
+       iceBackupCandidatePairPingInterval:(int)iceBackupCandidatePairPingInterval;
 
 @end

talk/app/webrtc/peerconnectioninterface.h

@@ -253,7 +253,8 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
     TcpCandidatePolicy tcp_candidate_policy;
     int audio_jitter_buffer_max_packets;
     bool audio_jitter_buffer_fast_accelerate;
-    int ice_connection_receiving_timeout;
+    int ice_connection_receiving_timeout;         // ms
+    int ice_backup_candidate_pair_ping_interval;  // ms
     ContinualGatheringPolicy continual_gathering_policy;
     std::vector<rtc::scoped_refptr<rtc::RTCCertificate>> certificates;
     bool disable_prerenderer_smoothing;
@@ -265,6 +266,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
           audio_jitter_buffer_max_packets(kAudioJitterBufferMaxPackets),
           audio_jitter_buffer_fast_accelerate(false),
           ice_connection_receiving_timeout(kUndefined),
+          ice_backup_candidate_pair_ping_interval(kUndefined),
           continual_gathering_policy(GATHER_ONCE),
           disable_prerenderer_smoothing(false) {}
   };

talk/app/webrtc/webrtcsession.cc

@@ -1244,6 +1244,8 @@ cricket::IceConfig WebRtcSession::ParseIceConfig(
     const PeerConnectionInterface::RTCConfiguration& config) const {
   cricket::IceConfig ice_config;
   ice_config.receiving_timeout_ms = config.ice_connection_receiving_timeout;
+  ice_config.backup_connection_ping_interval =
+      config.ice_backup_candidate_pair_ping_interval;
   ice_config.gather_continually = (config.continual_gathering_policy ==
                                    PeerConnectionInterface::GATHER_CONTINUALLY);
   return ice_config;

webrtc/p2p/base/p2ptransportchannel.cc

@@ -187,6 +187,7 @@ namespace cricket {
 // well on a 28.8K modem, which is the slowest connection on which the voice
 // quality is reasonable at all.
 static const uint32_t PING_PACKET_SIZE = 60 * 8;
+// TODO(honghaiz): Change the word DELAY to INTERVAL whenever appropriate.
 // STRONG_PING_DELAY (480ms) is applied when the best connection is both
 // writable and receiving.
 static const uint32_t STRONG_PING_DELAY = 1000 * PING_PACKET_SIZE / 1000;
@@ -201,7 +202,6 @@ static const uint32_t MAX_CURRENT_STRONG_DELAY = 900;
 
 static const int MIN_CHECK_RECEIVING_DELAY = 50;  // ms
 
-
 P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
                                          int component,
                                          P2PTransport* transport,
@@ -221,7 +221,8 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
       remote_candidate_generation_(0),
       gathering_state_(kIceGatheringNew),
       check_receiving_delay_(MIN_CHECK_RECEIVING_DELAY * 5),
-      receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50) {
+      receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50),
+      backup_connection_ping_interval_(0) {
   uint32_t weak_ping_delay = ::strtoul(
       webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
       nullptr, 10);
@@ -296,9 +297,13 @@ void P2PTransportChannel::SetIceTiebreaker(uint64_t tiebreaker) {
   tiebreaker_ = tiebreaker;
 }
 
+TransportChannelState P2PTransportChannel::GetState() const {
+  return state_;
+}
+
 // A channel is considered ICE completed once there is at most one active
 // connection per network and at least one active connection.
-TransportChannelState P2PTransportChannel::GetState() const {
+TransportChannelState P2PTransportChannel::ComputeState() const {
   if (!had_connection_) {
     return TransportChannelState::STATE_INIT;
   }
@@ -372,18 +377,26 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
   gather_continually_ = config.gather_continually;
   LOG(LS_INFO) << "Set gather_continually to " << gather_continually_;
 
-  if (config.receiving_timeout_ms < 0) {
-    return;
+  if (config.backup_connection_ping_interval >= 0 &&
+      backup_connection_ping_interval_ !=
+          config.backup_connection_ping_interval) {
+    backup_connection_ping_interval_ = config.backup_connection_ping_interval;
+    LOG(LS_INFO) << "Set backup connection ping interval to "
+                 << backup_connection_ping_interval_ << " milliseconds.";
   }
-  receiving_timeout_ = config.receiving_timeout_ms;
-  check_receiving_delay_ =
-      std::max(MIN_CHECK_RECEIVING_DELAY, receiving_timeout_ / 10);
 
-  for (Connection* connection : connections_) {
-    connection->set_receiving_timeout(receiving_timeout_);
+  if (config.receiving_timeout_ms >= 0 &&
+      receiving_timeout_ != config.receiving_timeout_ms) {
+    receiving_timeout_ = config.receiving_timeout_ms;
+    check_receiving_delay_ =
+        std::max(MIN_CHECK_RECEIVING_DELAY, receiving_timeout_ / 10);
+
+    for (Connection* connection : connections_) {
+      connection->set_receiving_timeout(receiving_timeout_);
+    }
+    LOG(LS_INFO) << "Set ICE receiving timeout to " << receiving_timeout_
+                 << " milliseconds";
   }
-  LOG(LS_INFO) << "Set ICE receiving timeout to " << receiving_timeout_
-               << " milliseconds";
 }
 
 // Go into the state of processing candidates, and running in general
@@ -990,7 +1003,7 @@ void P2PTransportChannel::SortConnections() {
 
   // Update the state of this channel.  This method is called whenever the
   // state of any connection changes, so this is a good place to do this.
-  UpdateChannelState();
+  UpdateState();
 }
 
 Connection* P2PTransportChannel::best_nominated_connection() const {
@@ -1050,7 +1063,15 @@ void P2PTransportChannel::SwitchBestConnectionTo(Connection* conn) {
   }
 }
 
-void P2PTransportChannel::UpdateChannelState() {
+// Warning: UpdateState should eventually be called whenever a connection
+// is added, deleted, or the write state of any connection changes so that the
+// transport controller will get the up-to-date channel state. However it
+// should not be called too often; in the case that multiple connection states
+// change, it should be called after all the connection states have changed. For
+// example, we call this at the end of SortConnections.
+void P2PTransportChannel::UpdateState() {
+  state_ = ComputeState();
+
   bool writable = best_connection_ && best_connection_->writable();
   set_writable(writable);
 
@@ -1150,10 +1171,17 @@ void P2PTransportChannel::OnCheckAndPing() {
   thread()->PostDelayed(check_delay, this, MSG_CHECK_AND_PING);
 }
 
+// A connection is considered a backup connection if the channel state
+// is completed, the connection is not the best connection and it is active.
+bool P2PTransportChannel::IsBackupConnection(Connection* conn) const {
+  return state_ == STATE_COMPLETED && conn != best_connection_ &&
+         conn->active();
+}
+
 // Is the connection in a state for us to even consider pinging the other side?
 // We consider a connection pingable even if it's not connected because that's
 // how a TCP connection is kicked into reconnecting on the active side.
-bool P2PTransportChannel::IsPingable(Connection* conn) {
+bool P2PTransportChannel::IsPingable(Connection* conn, uint32_t now) {
   const Candidate& remote = conn->remote_candidate();
   // We should never get this far with an empty remote ufrag.
   ASSERT(!remote.username().empty());
@@ -1169,9 +1197,18 @@ bool P2PTransportChannel::IsPingable(Connection* conn) {
     return false;
   }
 
-  // If the channel is weak, ping all candidates. Otherwise, we only
-  // want to ping connections that have not timed out on writing.
-  return weak() || conn->write_state() != Connection::STATE_WRITE_TIMEOUT;
+  // If the channel is weakly connected, ping all connections.
+  if (weak()) {
+    return true;
+  }
+
+  // Always ping active connections regardless whether the channel is completed
+  // or not, but backup connections are pinged at a slower rate.
+  if (IsBackupConnection(conn)) {
+    return (now >= conn->last_ping_response_received() +
+                       backup_connection_ping_interval_);
+  }
+  return conn->active();
 }
 
 // Returns the next pingable connection to ping.  This will be the oldest
@@ -1195,7 +1232,7 @@ Connection* P2PTransportChannel::FindNextPingableConnection() {
   Connection* oldest_needing_triggered_check = nullptr;
   Connection* oldest = nullptr;
   for (Connection* conn : connections_) {
-    if (!IsPingable(conn)) {
+    if (!IsPingable(conn, now)) {
       continue;
     }
     bool needs_triggered_check =
@@ -1307,6 +1344,9 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) {
     RequestSort();
   }
 
+  UpdateState();
+  // SignalConnectionRemoved should be called after the channel state is
+  // updated because the receiver of the event may access the channel state.
   SignalConnectionRemoved(this);
 }
 

webrtc/p2p/base/p2ptransportchannel.h

@@ -177,9 +177,10 @@ class P2PTransportChannel : public TransportChannelImpl,
   void RequestSort();
   void SortConnections();
   void SwitchBestConnectionTo(Connection* conn);
-  void UpdateChannelState();
+  void UpdateState();
   void HandleAllTimedOut();
   void MaybeStopPortAllocatorSessions();
+  TransportChannelState ComputeState() const;
 
   Connection* GetBestConnectionOnNetwork(rtc::Network* network) const;
   bool CreateConnections(const Candidate& remote_candidate,
@@ -193,7 +194,7 @@ class P2PTransportChannel : public TransportChannelImpl,
   bool IsDuplicateRemoteCandidate(const Candidate& candidate);
   void RememberRemoteCandidate(const Candidate& remote_candidate,
                                PortInterface* origin_port);
-  bool IsPingable(Connection* conn);
+  bool IsPingable(Connection* conn, uint32_t now);
   void PingConnection(Connection* conn);
   void AddAllocatorSession(PortAllocatorSession* session);
   void AddConnection(Connection* connection);
@@ -226,6 +227,7 @@ class P2PTransportChannel : public TransportChannelImpl,
 
   void PruneConnections();
   Connection* best_nominated_connection() const;
+  bool IsBackupConnection(Connection* conn) const;
 
   P2PTransport* transport_;
   PortAllocator* allocator_;
@@ -256,9 +258,11 @@ class P2PTransportChannel : public TransportChannelImpl,
 
   int check_receiving_delay_;
   int receiving_timeout_;
+  int backup_connection_ping_interval_;
   uint32_t last_ping_sent_ms_ = 0;
   bool gather_continually_ = false;
   int weak_ping_delay_ = WEAK_PING_DELAY;
+  TransportChannelState state_ = TransportChannelState::STATE_INIT;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel);
 };

webrtc/p2p/base/p2ptransportchannel_unittest.cc

@@ -101,10 +101,12 @@ enum {
 };
 
 static cricket::IceConfig CreateIceConfig(int receiving_timeout_ms,
-                                          bool gather_continually) {
+                                          bool gather_continually,
+                                          int backup_ping_interval = -1) {
   cricket::IceConfig config;
   config.receiving_timeout_ms = receiving_timeout_ms;
   config.gather_continually = gather_continually;
+  config.backup_connection_ping_interval = backup_ping_interval;
   return config;
 }
 
@@ -1648,6 +1650,43 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
   DestroyChannels();
 }
 
+// Test that the backup connection is pinged at a rate no faster than
+// what was configured.
+TEST_F(P2PTransportChannelMultihomedTest, TestPingBackupConnectionRate) {
+  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);
+  int backup_ping_interval = 2000;
+  ep2_ch1()->SetIceConfig(CreateIceConfig(2000, false, backup_ping_interval));
+  // After the state becomes COMPLETED, the backup connection will be pinged
+  // once every |backup_ping_interval| milliseconds.
+  ASSERT_TRUE_WAIT(ep2_ch1()->GetState() == cricket::STATE_COMPLETED, 1000);
+  const std::vector<cricket::Connection*>& connections =
+      ep2_ch1()->connections();
+  ASSERT_EQ(2U, connections.size());
+  cricket::Connection* backup_conn = connections[1];
+  EXPECT_TRUE_WAIT(backup_conn->writable(), 3000);
+  uint32_t last_ping_response_ms = backup_conn->last_ping_response_received();
+  EXPECT_TRUE_WAIT(
+      last_ping_response_ms < backup_conn->last_ping_response_received(), 5000);
+  int time_elapsed =
+      backup_conn->last_ping_response_received() - last_ping_response_ms;
+  LOG(LS_INFO) << "Time elapsed: " << time_elapsed;
+  EXPECT_GE(time_elapsed, backup_ping_interval);
+}
+
 TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
   AddAddress(0, kAlternateAddrs[0]);
   AddAddress(0, kPublicAddrs[0]);
@@ -2141,7 +2180,9 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) {
   EXPECT_TRUE_WAIT(conn2->pruned(), 1000);
   EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
   conn1->Prune();  // All connections are pruned.
-  EXPECT_EQ(cricket::TransportChannelState::STATE_FAILED, ch.GetState());
+  // Need to wait until the channel state is updated.
+  EXPECT_EQ_WAIT(cricket::TransportChannelState::STATE_FAILED, ch.GetState(),
+                 1000);
 }
 
 // Test that when a low-priority connection is pruned, it is not deleted

webrtc/p2p/base/port.h

@@ -446,7 +446,6 @@ class Connection : public rtc::MessageHandler,
   bool connected() const { return connected_; }
   bool weak() const { return !(writable() && receiving() && connected()); }
   bool active() const {
-    // TODO(honghaiz): Move from using |write_state_| to using |pruned_|.
     return write_state_ != STATE_WRITE_TIMEOUT;
   }
   // A connection is dead if it can be safely deleted.
@@ -525,6 +524,9 @@ class Connection : public rtc::MessageHandler,
   uint32_t last_ping_sent() const { return last_ping_sent_; }
   void Ping(uint32_t now);
   void ReceivedPingResponse();
+  uint32_t last_ping_response_received() const {
+    return last_ping_response_received_;
+  }
 
   // Called whenever a valid ping is received on this connection.  This is
   // public because the connection intercepts the first ping for us.

webrtc/p2p/base/transport.h

@@ -140,7 +140,11 @@ struct TransportStats {
 // Information about ICE configuration.
 struct IceConfig {
   // The ICE connection receiving timeout value.
+  // TODO(honghaiz): Remove suffix _ms to be consistent.
   int receiving_timeout_ms = -1;
+  // Time interval in milliseconds to ping a backup connection when the ICE
+  // channel is strongly connected.
+  int backup_connection_ping_interval = -1;
   // If true, the most recent port allocator session will keep on running.
   bool gather_continually = false;
 };