Dampening connection switch.

If the currently selected connection becomes not receiving and if a backup connection
becomes strong first, we will not switch the connection until X milliseconds is passed
but the selected connection is still not receiving and the backup connection is still receiving. This will prevent the connection switching from happening too frequently.

BUG=

Review-Url: https://codereview.webrtc.org/2143653005
Cr-Commit-Position: refs/heads/master@{#13480}

webrtc/p2p/base/p2ptransportchannel.cc

@@ -80,6 +80,8 @@ const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL = 2500;  // ms
 
 static const int MIN_CHECK_RECEIVING_INTERVAL = 50;  // ms
 
+static const int RECEIVING_SWITCHING_DELAY = 1000;  // ms
+
 // We periodically check if any existing networks do not have any connection
 // and regather on those networks.
 static const int DEFAULT_REGATHER_ON_FAILED_NETWORKS_INTERVAL = 5 * 60 * 1000;
@@ -112,7 +114,8 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
               false /* prioritize_most_likely_candidate_pairs */,
               STABLE_WRITABLE_CONNECTION_PING_INTERVAL,
               true /* presume_writable_when_fully_relayed */,
-              DEFAULT_REGATHER_ON_FAILED_NETWORKS_INTERVAL) {
+              DEFAULT_REGATHER_ON_FAILED_NETWORKS_INTERVAL,
+              RECEIVING_SWITCHING_DELAY) {
   uint32_t weak_ping_interval = ::strtoul(
       webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
       nullptr, 10);
@@ -184,7 +187,8 @@ void P2PTransportChannel::AddConnection(Connection* connection) {
 // TODO(honghaiz): Stop the aggressive nomination on the controlling side and
 // implement the ice-renomination option.
 bool P2PTransportChannel::ShouldSwitchSelectedConnection(
-    Connection* new_connection) const {
+    Connection* new_connection,
+    bool* missed_receiving_unchanged_threshold) const {
   if (!new_connection || selected_connection_ == new_connection) {
     return false;
   }
@@ -193,7 +197,11 @@ bool P2PTransportChannel::ShouldSwitchSelectedConnection(
     return true;
   }
 
-  int cmp = CompareConnections(selected_connection_, new_connection);
+  rtc::Optional<int64_t> receiving_unchanged_threshold(
+      rtc::TimeMillis() - config_.receiving_switching_delay.value_or(0));
+  int cmp = CompareConnections(selected_connection_, new_connection,
+                               receiving_unchanged_threshold,
+                               missed_receiving_unchanged_threshold);
   if (cmp != 0) {
     return cmp < 0;
   }
@@ -203,6 +211,28 @@ bool P2PTransportChannel::ShouldSwitchSelectedConnection(
   return new_connection->rtt() <= selected_connection_->rtt() - kMinImprovement;
 }
 
+bool P2PTransportChannel::MaybeSwitchSelectedConnection(
+    Connection* new_connection,
+    const std::string& reason) {
+  bool missed_receiving_unchanged_threshold = false;
+  if (ShouldSwitchSelectedConnection(new_connection,
+                                     &missed_receiving_unchanged_threshold)) {
+    LOG(LS_INFO) << "Switching selected connection due to " << reason;
+    SwitchSelectedConnection(new_connection);
+    return true;
+  }
+  if (missed_receiving_unchanged_threshold &&
+      config_.receiving_switching_delay) {
+    // If we do not switch to the connection because it missed the receiving
+    // threshold, the new connection is in a better receiving state than the
+    // currently selected connection. So we need to re-check whether it needs
+    // to be switched at a later time.
+    thread()->PostDelayed(RTC_FROM_HERE, *config_.receiving_switching_delay,
+                          this, MSG_SORT_AND_UPDATE_STATE);
+  }
+  return false;
+}
+
 void P2PTransportChannel::SetIceRole(IceRole ice_role) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   if (ice_role_ != ice_role) {
@@ -369,6 +399,11 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
     LOG(LS_INFO) << "Set regather_on_failed_networks_interval to "
                  << *config_.regather_on_failed_networks_interval;
   }
+  if (config.receiving_switching_delay) {
+    config_.receiving_switching_delay = config.receiving_switching_delay;
+    LOG(LS_INFO) << "Set receiving_switching_delay to"
+                 << *config_.receiving_switching_delay;
+  }
 }
 
 const IceConfig& P2PTransportChannel::config() const {
@@ -627,20 +662,16 @@ void P2PTransportChannel::OnNominated(Connection* conn) {
     return;
   }
 
-  if (!ShouldSwitchSelectedConnection(conn)) {
+  if (MaybeSwitchSelectedConnection(conn,
+                                    "nomination on the controlled side")) {
+    // Now that we have selected a connection, it is time to prune other
+    // connections and update the read/write state of the channel.
+    RequestSortAndStateUpdate();
+  } else {
     LOG(LS_INFO)
         << "Not switching the selected connection on controlled side yet: "
         << conn->ToString();
-    return;
   }
-
-  LOG(LS_INFO)
-      << "Switching selected connection on controlled side due to nomination: "
-      << conn->ToString();
-  SwitchSelectedConnection(conn);
-  // Now that we have selected a connection, it is time to prune other
-  // connections and update the read/write state of the channel.
-  RequestSortAndStateUpdate();
 }
 
 void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) {
@@ -987,8 +1018,11 @@ void P2PTransportChannel::MaybeStartPinging() {
 
 // Compare two connections based on their writing, receiving, and connected
 // states.
-int P2PTransportChannel::CompareConnectionStates(const Connection* a,
+int P2PTransportChannel::CompareConnectionStates(
-                                                 const Connection* b) const {
+    const Connection* a,
+    const Connection* b,
+    rtc::Optional<int64_t> receiving_unchanged_threshold,
+    bool* missed_receiving_unchanged_threshold) const {
   // First, prefer a connection that's writable or presumed writable over
   // one that's not writable.
   bool a_writable = a->writable() || PresumedWritable(a);
@@ -1015,7 +1049,12 @@ int P2PTransportChannel::CompareConnectionStates(const Connection* a,
     return a_is_better;
   }
   if (!a->receiving() && b->receiving()) {
-    return b_is_better;
+    if (!receiving_unchanged_threshold ||
+        (a->receiving_unchanged_since() <= *receiving_unchanged_threshold &&
+         b->receiving_unchanged_since() <= *receiving_unchanged_threshold)) {
+      return b_is_better;
+    }
+    *missed_receiving_unchanged_threshold = true;
   }
 
   // WARNING: Some complexity here about TCP reconnecting.
@@ -1082,15 +1121,19 @@ int P2PTransportChannel::CompareConnectionCandidates(
          (b->remote_candidate().generation() + b->port()->generation());
 }
 
-int P2PTransportChannel::CompareConnections(const Connection* a,
+int P2PTransportChannel::CompareConnections(
-                                            const Connection* b) const {
+    const Connection* a,
+    const Connection* b,
+    rtc::Optional<int64_t> receiving_unchanged_threshold,
+    bool* missed_receiving_unchanged_threshold) const {
   RTC_CHECK(a != nullptr);
   RTC_CHECK(b != nullptr);
 
   // We prefer to switch to a writable and receiving connection over a
   // non-writable or non-receiving connection, even if the latter has
   // been nominated by the controlling side.
-  int state_cmp = CompareConnectionStates(a, b);
+  int state_cmp = CompareConnectionStates(a, b, receiving_unchanged_threshold,
+                                          missed_receiving_unchanged_threshold);
   if (state_cmp != 0) {
     return state_cmp;
   }
@@ -1143,11 +1186,11 @@ void P2PTransportChannel::SortConnectionsAndUpdateState() {
   // need to consider switching to.
   std::stable_sort(connections_.begin(), connections_.end(),
                    [this](const Connection* a, const Connection* b) {
-                     int cmp = CompareConnections(a, b);
+                     int cmp = CompareConnections(
+                         a, b, rtc::Optional<int64_t>(), nullptr);
                      if (cmp != 0) {
                        return cmp > 0;
                      }
-
                      // Otherwise, sort based on latency estimate.
                      return a->rtt() < b->rtt();
                    });
@@ -1164,11 +1207,7 @@ void P2PTransportChannel::SortConnectionsAndUpdateState() {
   // If necessary, switch to the new choice. Note that |top_connection| doesn't
   // have to be writable to become the selected connection although it will
   // have higher priority if it is writable.
-  if (ShouldSwitchSelectedConnection(top_connection)) {
+  MaybeSwitchSelectedConnection(top_connection, "sorting");
-    LOG(LS_INFO) << "Switching selected connection after sorting: "
-                 << top_connection->ToString();
-    SwitchSelectedConnection(top_connection);
-  }
 
   // The controlled side can prune only if the selected connection has been
   // nominated because otherwise it may prune the connection that will be
@@ -1622,7 +1661,7 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) {
   // we can just set selected to nullptr and re-choose a best assuming that
   // there was no selected connection.
   if (selected_connection_ == connection) {
-    LOG(LS_INFO) << "selected connection destroyed. Will choose a new one.";
+    LOG(LS_INFO) << "Selected connection destroyed. Will choose a new one.";
     SwitchSelectedConnection(nullptr);
     RequestSortAndStateUpdate();
   } else {
@@ -1726,11 +1765,8 @@ void P2PTransportChannel::OnReadPacket(Connection* connection,
 
   // May need to switch the sending connection based on the receiving media path
   // if this is the controlled side.
-  if (ice_role_ == ICEROLE_CONTROLLED &&
+  if (ice_role_ == ICEROLE_CONTROLLED) {
-      ShouldSwitchSelectedConnection(connection)) {
+    MaybeSwitchSelectedConnection(connection, "data received");
-    LOG(LS_INFO) << "Switching selected connection on controlled side due to "
-                 << "data received: " << connection->ToString();
-    SwitchSelectedConnection(connection);
   }
 }
 

webrtc/p2p/base/p2ptransportchannel.h

@@ -208,7 +208,7 @@ class P2PTransportChannel : public TransportChannelImpl,
   }
 
  private:
-  rtc::Thread* thread() { return worker_thread_; }
+  rtc::Thread* thread() const { return worker_thread_; }
   bool IsGettingPorts() { return allocator_session()->IsGettingPorts(); }
 
   // A transport channel is weak if the current best connection is either
@@ -222,10 +222,18 @@ class P2PTransportChannel : public TransportChannelImpl,
   // that's pingable.
   void MaybeStartPinging();
 
-  // The methods below return a positive value if a is preferable to b,
+  // The methods below return a positive value if |a| is preferable to |b|,
-  // a negative value if b is preferable, and 0 if they're equally preferable.
+  // a negative value if |b| is preferable, and 0 if they're equally preferable.
-  int CompareConnectionStates(const cricket::Connection* a,
+  // If |receiving_unchanged_threshold| is set, then when |b| is receiving and
-                              const cricket::Connection* b) const;
+  // |a| is not, returns a negative value only if |b| has been in receiving
+  // state and |a| has been in not receiving state since
+  // |receiving_unchanged_threshold| and sets
+  // |missed_receiving_unchanged_threshold| to true otherwise.
+  int CompareConnectionStates(
+      const cricket::Connection* a,
+      const cricket::Connection* b,
+      rtc::Optional<int64_t> receiving_unchanged_threshold,
+      bool* missed_receiving_unchanged_threshold) const;
   int CompareConnectionCandidates(const cricket::Connection* a,
                                   const cricket::Connection* b) const;
   // Compares two connections based on the connection states
@@ -234,12 +242,12 @@ class P2PTransportChannel : public TransportChannelImpl,
   // and ShouldSwitchSelectedConnection().
   // Returns a positive value if |a| is better than |b|.
   int CompareConnections(const cricket::Connection* a,
-                         const cricket::Connection* b) const;
+                         const cricket::Connection* b,
+                         rtc::Optional<int64_t> receiving_unchanged_threshold,
+                         bool* missed_receiving_unchanged_threshold) const;
 
   bool PresumedWritable(const cricket::Connection* conn) const;
 
-  bool ShouldSwitchSelectedConnection(const cricket::Connection* selected,
-                                      const cricket::Connection* conn) const;
   void SortConnectionsAndUpdateState();
   void SwitchSelectedConnection(Connection* conn);
   void UpdateState();
@@ -306,8 +314,16 @@ class P2PTransportChannel : public TransportChannelImpl,
   void OnCheckAndPing();
   void OnRegatherOnFailedNetworks();
 
-  // Returns true if the new_connection should be selected for transmission.
+  // Returns true if we should switch to the new connection.
-  bool ShouldSwitchSelectedConnection(Connection* new_connection) const;
+  // sets |missed_receiving_unchanged_threshold| to true if either
+  // the selected connection or the new connection missed its
+  // receiving-unchanged-threshold.
+  bool ShouldSwitchSelectedConnection(
+      Connection* new_connection,
+      bool* missed_receiving_unchanged_threshold) const;
+  // Returns true if the new_connection is selected for transmission.
+  bool MaybeSwitchSelectedConnection(Connection* new_connection,
+                                     const std::string& reason);
 
   void PruneConnections();
   bool IsBackupConnection(const Connection* conn) const;

webrtc/p2p/base/p2ptransportchannel_unittest.cc

@@ -350,6 +350,8 @@ class P2PTransportChannelTestBase : public testing::Test,
         this, &P2PTransportChannelTestBase::OnReadPacket);
     channel->SignalRoleConflict.connect(
         this, &P2PTransportChannelTestBase::OnRoleConflict);
+    channel->SignalSelectedCandidatePairChanged.connect(
+        this, &P2PTransportChannelTestBase::OnSelectedCandidatePairChanged);
     channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd);
     if (remote_ice_credential_source_ == FROM_SETICECREDENTIALS) {
       channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd);
@@ -688,6 +690,19 @@ class P2PTransportChannelTestBase : public testing::Test,
                   new CandidatesData(ch, c));
     }
   }
+  void OnSelectedCandidatePairChanged(
+      TransportChannel* transport_channel,
+      CandidatePairInterface* selected_candidate_pair,
+      int last_sent_packet_id,
+      bool ready_to_send) {
+    ++selected_candidate_pair_switches_;
+  }
+
+  int reset_selected_candidate_pair_switches() {
+    int switches = selected_candidate_pair_switches_;
+    selected_candidate_pair_switches_ = 0;
+    return switches;
+  }
 
   void PauseCandidates(int endpoint) {
     GetEndpoint(endpoint)->save_candidates_ = true;
@@ -850,6 +865,7 @@ class P2PTransportChannelTestBase : public testing::Test,
   Endpoint ep2_;
   RemoteIceCredentialSource remote_ice_credential_source_ = FROM_CANDIDATE;
   bool force_relay_;
+  int selected_candidate_pair_switches_ = 0;
 };
 
 // The tests have only a few outcomes, which we predefine.
@@ -1867,6 +1883,7 @@ TEST_F(P2PTransportChannelMultihomedTest, DISABLED_TestBasic) {
 // 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) {
+  rtc::ScopedFakeClock clock;
   AddAddress(0, kPublicAddrs[0]);
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
@@ -1880,9 +1897,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
 
-  EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
-                              ep2_ch1()->receiving() && ep2_ch1()->writable(),
+                                 ep2_ch1()->receiving() &&
-                          1000, 1000);
+                                 ep2_ch1()->writable(),
+                             3000, clock);
   EXPECT_TRUE(ep1_ch1()->selected_connection() &&
               ep2_ch1()->selected_connection() &&
               LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
@@ -1896,19 +1914,19 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
   // 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 selected connections will switch, so keep references to them.
+  // The selected connections may switch, so keep references to them.
   const Connection* selected_connection1 = ep1_ch1()->selected_connection();
   const Connection* selected_connection2 = ep2_ch1()->selected_connection();
   // We should detect loss of receiving within 1 second or so.
-  EXPECT_TRUE_WAIT(
+  EXPECT_TRUE_SIMULATED_WAIT(
       !selected_connection1->receiving() && !selected_connection2->receiving(),
-      3000);
+      3000, clock);
 
-  // We should switch over to use the alternate addr immediately on both sides
+  // We should switch over to use the alternate addr on both sides
   // when we are not receiving.
-  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection()->receiving() &&
-                       ep2_ch1()->selected_connection()->receiving(),
+                                 ep2_ch1()->selected_connection()->receiving(),
-                   1000);
+                             3000, clock);
   EXPECT_TRUE(LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]));
   EXPECT_TRUE(
       RemoteCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[1]));
@@ -1921,6 +1939,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControlledSide) {
 // 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) {
+  rtc::ScopedFakeClock clock;
   // Adding alternate address will make sure |kPublicAddrs| has the higher
   // priority than others. This is due to FakeNetwork::AddInterface method.
   AddAddress(0, kAlternateAddrs[0]);
@@ -1933,9 +1952,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
 
   // Create channels and let them go writable, as usual.
   CreateChannels(1);
-  EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
-                              ep2_ch1()->receiving() && ep2_ch1()->writable(),
+                                 ep2_ch1()->receiving() &&
-                          1000, 1000);
+                                 ep2_ch1()->writable(),
+                             3000, clock);
   EXPECT_TRUE(ep1_ch1()->selected_connection() &&
               ep2_ch1()->selected_connection() &&
               LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
@@ -1953,15 +1973,15 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
   const Connection* selected_connection1 = ep1_ch1()->selected_connection();
   const Connection* selected_connection2 = ep2_ch1()->selected_connection();
   // We should detect loss of receiving within 1 second or so.
-  EXPECT_TRUE_WAIT(
+  EXPECT_TRUE_SIMULATED_WAIT(
       !selected_connection1->receiving() && !selected_connection2->receiving(),
-      3000);
+      3000, clock);
 
-  // We should switch over to use the alternate addr immediately on both sides
+  // We should switch over to use the alternate addr on both sides
   // when we are not receiving.
-  EXPECT_TRUE_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection()->receiving() &&
-                       ep2_ch1()->selected_connection()->receiving(),
+                                 ep2_ch1()->selected_connection()->receiving(),
-                   1000);
+                             3000, clock);
   EXPECT_TRUE(
     LocalCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]));
   EXPECT_TRUE(RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
@@ -1971,6 +1991,126 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
   DestroyChannels();
 }
 
+// Test that if an interface fails temporarily and then recovers quickly,
+// the selected connection will not switch.
+// The case that it will switch over to the backup connection if the selected
+// connection does not recover after enough time is covered in
+// TestFailoverControlledSide and TestFailoverControllingSide.
+TEST_F(P2PTransportChannelMultihomedTest,
+       TestConnectionSwitchDampeningControlledSide) {
+  rtc::ScopedFakeClock clock;
+  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_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection() &&
+              LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
+              RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+
+  // Make the receiving timeout shorter for testing.
+  IceConfig config = CreateIceConfig(1000, GATHER_ONCE);
+  ep1_ch1()->SetIceConfig(config);
+  ep2_ch1()->SetIceConfig(config);
+  reset_selected_candidate_pair_switches();
+
+  // 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 selected connections may switch, so keep references to them.
+  const Connection* selected_connection1 = ep1_ch1()->selected_connection();
+  const Connection* selected_connection2 = ep2_ch1()->selected_connection();
+  // We should detect loss of receiving within 1 second or so.
+  EXPECT_TRUE_SIMULATED_WAIT(
+      !selected_connection1->receiving() && !selected_connection2->receiving(),
+      3000, clock);
+  // After a short while, the link recovers itself.
+  SIMULATED_WAIT(false, 10, clock);
+  fw()->ClearRules();
+
+  // We should remain on the public address on both sides and no connection
+  // switches should have happened.
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+                                 ep2_ch1()->selected_connection()->receiving(),
+                             3000, clock);
+  EXPECT_TRUE(RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+  EXPECT_TRUE(LocalCandidate(ep2_ch1())->address().EqualIPs(kPublicAddrs[1]));
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
+
+  DestroyChannels();
+}
+
+// Test that if an interface fails temporarily and then recovers quickly,
+// the selected connection will not switch.
+TEST_F(P2PTransportChannelMultihomedTest,
+       TestConnectionSwitchDampeningControllingSide) {
+  rtc::ScopedFakeClock clock;
+  // 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_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+                                 ep2_ch1()->receiving() &&
+                                 ep2_ch1()->writable(),
+                             3000, clock);
+  EXPECT_TRUE(ep1_ch1()->selected_connection() &&
+              ep2_ch1()->selected_connection() &&
+              LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]) &&
+              RemoteCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[1]));
+
+  // Make the receiving timeout shorter for testing.
+  IceConfig config = CreateIceConfig(1000, GATHER_ONCE);
+  ep1_ch1()->SetIceConfig(config);
+  ep2_ch1()->SetIceConfig(config);
+  reset_selected_candidate_pair_switches();
+
+  // 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 selected connections may switch, so keep references to them.
+  const Connection* selected_connection1 = ep1_ch1()->selected_connection();
+  const Connection* selected_connection2 = ep2_ch1()->selected_connection();
+  // We should detect loss of receiving within 1 second or so.
+  EXPECT_TRUE_SIMULATED_WAIT(
+      !selected_connection1->receiving() && !selected_connection2->receiving(),
+      3000, clock);
+  // The link recovers after a short while.
+  SIMULATED_WAIT(false, 10, clock);
+  fw()->ClearRules();
+
+  // We should not switch to the alternate addr on both sides because of the
+  // dampening.
+  EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection()->receiving() &&
+                                 ep2_ch1()->selected_connection()->receiving(),
+                             3000, clock);
+  EXPECT_TRUE(LocalCandidate(ep1_ch1())->address().EqualIPs(kPublicAddrs[0]));
+  EXPECT_TRUE(RemoteCandidate(ep2_ch1())->address().EqualIPs(kPublicAddrs[0]));
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
+  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
@@ -2448,7 +2588,7 @@ class P2PTransportChannelPingTest : public testing::Test,
   bool channel_ready_to_send() { return channel_ready_to_send_; }
   void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }
   TransportChannelState channel_state() { return channel_state_; }
-  int get_and_reset_selected_candidate_pair_switches() {
+  int reset_selected_candidate_pair_switches() {
     int switches = selected_candidate_pair_switches_;
     selected_candidate_pair_switches_ = 0;
     return switches;
@@ -3055,7 +3195,7 @@ TEST_F(P2PTransportChannelPingTest,
   ASSERT_TRUE(conn2 != nullptr);
 
   // Initially, connections are selected based on priority.
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
 
   // conn2 receives data; it becomes selected.
@@ -3063,17 +3203,17 @@ TEST_F(P2PTransportChannelPingTest,
   // conn2 is larger.
   SIMULATED_WAIT(false, 1, clock);
   conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn2, last_selected_candidate_pair());
 
   // conn1 also receives data; it becomes selected due to priority again.
   conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
 
   // Make sure sorting won't reselect candidate pair.
   SIMULATED_WAIT(false, 10, clock);
-  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
 }
 
 TEST_F(P2PTransportChannelPingTest,
@@ -3097,28 +3237,28 @@ TEST_F(P2PTransportChannelPingTest,
   // Advance the clock to have a non-zero last-data-receiving time.
   SIMULATED_WAIT(false, 1, clock);
   conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
 
   // conn2 is nominated; it becomes the selected connection.
   NominateConnection(conn2);
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn2, last_selected_candidate_pair());
 
   NominateConnection(conn1);
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn1, last_selected_candidate_pair());
 
   // conn2 received data more recently; it is selected now because it
   // received data more recently.
   SIMULATED_WAIT(false, 1, clock);
   conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0));
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
   EXPECT_EQ(conn2, last_selected_candidate_pair());
 
   // Make sure sorting won't reselect candidate pair.
   SIMULATED_WAIT(false, 10, clock);
-  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
 }
 
 TEST_F(P2PTransportChannelPingTest,
@@ -3139,26 +3279,26 @@ TEST_F(P2PTransportChannelPingTest,
   ASSERT_TRUE(conn2 != nullptr);
 
   NominateConnection(conn1);
-  EXPECT_EQ(1, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(1, reset_selected_candidate_pair_switches());
 
   // conn2 becomes writable; it is selected even though it is not nominated.
   conn2->ReceivedPingResponse(LOW_RTT);
 
-  EXPECT_EQ_SIMULATED_WAIT(1, get_and_reset_selected_candidate_pair_switches(),
+  EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(),
                            kDefaultTimeout, clock);
   EXPECT_EQ_SIMULATED_WAIT(conn2, last_selected_candidate_pair(),
                            kDefaultTimeout, clock);
 
   // If conn1 is also writable, it will become selected.
   conn1->ReceivedPingResponse(LOW_RTT);
-  EXPECT_EQ_SIMULATED_WAIT(1, get_and_reset_selected_candidate_pair_switches(),
+  EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(),
                            kDefaultTimeout, clock);
   EXPECT_EQ_SIMULATED_WAIT(conn1, last_selected_candidate_pair(),
                            kDefaultTimeout, clock);
 
   // Make sure sorting won't reselect candidate pair.
   SIMULATED_WAIT(false, 10, clock);
-  EXPECT_EQ(0, get_and_reset_selected_candidate_pair_switches());
+  EXPECT_EQ(0, reset_selected_candidate_pair_switches());
 }
 
 // Test that if a new remote candidate has the same address and port with
@@ -3289,7 +3429,9 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
   FakePortAllocator pa(rtc::Thread::Current(), nullptr);
   P2PTransportChannel ch("test channel", 1, &pa);
   PrepareChannel(&ch);
-  ch.SetIceConfig(CreateIceConfig(1000, GATHER_ONCE));
+  IceConfig config = CreateIceConfig(1000, GATHER_ONCE);
+  config.receiving_switching_delay = rtc::Optional<int>(800);
+  ch.SetIceConfig(config);
   ch.MaybeStartGathering();
   ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 100));
   Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);

webrtc/p2p/base/port.cc

@@ -898,12 +898,15 @@ void Connection::set_write_state(WriteState value) {
   }
 }
 
-void Connection::set_receiving(bool value) {
+void Connection::UpdateReceiving(int64_t now) {
-  if (value != receiving_) {
+  bool receiving = now <= last_received() + receiving_timeout_;
-    LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
+  if (receiving_ == receiving) {
-    receiving_ = value;
+    return;
-    SignalStateChange(this);
   }
+  LOG_J(LS_VERBOSE, this) << "set_receiving to " << receiving;
+  receiving_ = receiving;
+  receiving_unchanged_since_ = now;
+  SignalStateChange(this);
 }
 
 void Connection::set_state(State state) {
@@ -948,8 +951,8 @@ void Connection::OnReadPacket(
   if (!port_->GetStunMessage(data, size, addr, &msg, &remote_ufrag)) {
     // The packet did not parse as a valid STUN message
     // This is a data packet, pass it along.
-    set_receiving(true);
     last_data_received_ = rtc::TimeMillis();
+    UpdateReceiving(last_data_received_);
     recv_rate_tracker_.AddSamples(size);
     SignalReadPacket(this, data, size, packet_time);
 
@@ -1165,10 +1168,8 @@ void Connection::UpdateState(int64_t now) {
     set_write_state(STATE_WRITE_TIMEOUT);
   }
 
-  // Check the receiving state.
+  // Update the receiving state.
-  int64_t last_recv_time = last_received();
+  UpdateReceiving(now);
-  bool receiving = now <= last_recv_time + receiving_timeout_;
-  set_receiving(receiving);
   if (dead(now)) {
     Destroy();
   }
@@ -1186,8 +1187,8 @@ void Connection::Ping(int64_t now) {
 }
 
 void Connection::ReceivedPing() {
-  set_receiving(true);
   last_ping_received_ = rtc::TimeMillis();
+  UpdateReceiving(last_ping_received_);
 }
 
 void Connection::ReceivedPingResponse(int rtt) {
@@ -1196,11 +1197,11 @@ void Connection::ReceivedPingResponse(int rtt) {
   // So if we're not already, become writable. We may be bringing a pruned
   // connection back to life, but if we don't really want it, we can always
   // prune it again.
-  set_receiving(true);
+  last_ping_response_received_ = rtc::TimeMillis();
+  UpdateReceiving(last_ping_response_received_);
   set_write_state(STATE_WRITABLE);
   set_state(STATE_SUCCEEDED);
   pings_since_last_response_.clear();
-  last_ping_response_received_ = rtc::TimeMillis();
   rtt_samples_++;
   rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1);
 }

webrtc/p2p/base/port.h

@@ -590,6 +590,10 @@ class Connection : public CandidatePairInterface,
   // Returns the last received time of any data, stun request, or stun
   // response in milliseconds
   int64_t last_received() const;
+  // Returns the last time when the connection changed its receiving state.
+  int64_t receiving_unchanged_since() const {
+    return receiving_unchanged_since_;
+  }
 
   bool stable(int64_t now) const;
 
@@ -618,7 +622,7 @@ class Connection : public CandidatePairInterface,
 
   // Changes the state and signals if necessary.
   void set_write_state(WriteState value);
-  void set_receiving(bool value);
+  void UpdateReceiving(int64_t now);
   void set_state(State state);
   void set_connected(bool value);
 
@@ -648,6 +652,7 @@ class Connection : public CandidatePairInterface,
                                 // side
   int64_t last_data_received_;
   int64_t last_ping_response_received_;
+  int64_t receiving_unchanged_since_ = 0;
   std::vector<SentPing> pings_since_last_response_;
 
   rtc::RateTracker recv_rate_tracker_;

webrtc/p2p/base/transport.h

@@ -194,6 +194,11 @@ struct IceConfig {
   // active network having no connection on it.
   rtc::Optional<int> regather_on_failed_networks_interval;
 
+  // The time period in which we will not switch the selected connection
+  // when a new connection becomes receiving but the selected connection is not
+  // in case that the selected connection may become receiving soon.
+  rtc::Optional<int> receiving_switching_delay;
+
   IceConfig() {}
   IceConfig(int receiving_timeout_ms,
             int backup_connection_ping_interval,
@@ -201,7 +206,8 @@ struct IceConfig {
             bool prioritize_most_likely_candidate_pairs,
             int stable_writable_connection_ping_interval_ms,
             bool presume_writable_when_fully_relayed,
-            int regather_on_failed_networks_interval_ms)
+            int regather_on_failed_networks_interval_ms,
+            int receiving_switching_delay_ms)
       : receiving_timeout(receiving_timeout_ms),
         backup_connection_ping_interval(backup_connection_ping_interval),
         continual_gathering_policy(gathering_policy),
@@ -212,7 +218,8 @@ struct IceConfig {
         presume_writable_when_fully_relayed(
             presume_writable_when_fully_relayed),
         regather_on_failed_networks_interval(
-            regather_on_failed_networks_interval_ms) {}
+            regather_on_failed_networks_interval_ms),
+        receiving_switching_delay(receiving_switching_delay_ms) {}
 };
 
 bool BadTransportDescription(const std::string& desc, std::string* err_desc);