Prepare for ICE-renomination

webrtc/p2p/base/p2ptransportchannel.cc

 /*
  *  Copyright 2004 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 1123 matching lines @@
   // been nominated by the controlling side.
   int state_cmp = CompareConnectionStates(a, b, receiving_unchanged_threshold,
                                           missed_receiving_unchanged_threshold);
   if (state_cmp != 0) {
     return state_cmp;
   }
 
   if (ice_role_ == ICEROLE_CONTROLLED) {
     // Compare the connections based on the nomination states and the last data
     // received time if this is on the controlled side.
-    if (a->nominated() && !b->nominated()) {
-      return a_is_better;
-    }
-    if (!a->nominated() && b->nominated()) {
-      return b_is_better;
+    int cmp = a->nominated_value() - b->nominated_value();
+    if (cmp != 0) {
+      // A positive value indicates |a| is better.
+      return cmp;
     }
 
     if (a->last_data_received() > b->last_data_received()) {
       return a_is_better;
     }
     if (a->last_data_received() < b->last_data_received()) {
       return b_is_better;
     }
   }
 
@@ skipping 46 matching lines @@
       (connections_.size() > 0) ? connections_[0] : nullptr;
 
   // 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.
   MaybeSwitchSelectedConnection(top_connection, "sorting");
 
   // The controlled side can prune only if the selected connection has been
   // nominated because otherwise it may prune the connection that will be
   // selected by the controlling side.
-  // TODO(honghaiz): This is not enough to prevent a connection from being
-  // pruned too early because with aggressive nomination, the controlling side
-  // will nominate every connection until it becomes writable.
   if (ice_role_ == ICEROLE_CONTROLLING ||
-      (selected_connection_ && selected_connection_->nominated())) {
+      (selected_connection_ && selected_connection_->nominated_value() > 0)) {
     PruneConnections();
   }
 
   // Check if all connections are timedout.
   bool all_connections_timedout = true;
   for (size_t i = 0; i < connections_.size(); ++i) {
     if (connections_[i]->write_state() != Connection::STATE_WRITE_TIMEOUT) {
       all_connections_timedout = false;
       break;
     }
@@ skipping 349 matching lines @@
   return conn_to_ping;
 }
 
 void P2PTransportChannel::MarkConnectionPinged(Connection* conn) {
   if (conn && pinged_connections_.insert(conn).second) {
     unpinged_connections_.erase(conn);
   }
 }
 
 // Apart from sending ping from |conn| this method also updates
-// |use_candidate_attr| flag. The criteria to update this flag is
-// explained below.
-// Set USE-CANDIDATE if doing ICE AND this channel is in CONTROLLING AND
-//    a) Channel is in FULL ICE AND
-//      a.1) |conn| is the selected connection OR
-//      a.2) there is no selected connection OR
-//      a.3) the selected connection is unwritable OR
-//      a.4) |conn| has higher priority than selected_connection.
-//    b) we're doing LITE ICE AND
-//      b.1) |conn| is the selected_connection AND
-//      b.2) |conn| is writable.
+// |use_candidate_attr| and |nominating_value| flags. The criteria to update
+// the flags is explained below.
+// Set the flags to nominate |conn| if this channel is in CONTROLLING AND
+//   1) |conn| is the selected_connection AND
+//   2) |conn| is writable.
+// This is very much like passive-aggressive nomination.
 void P2PTransportChannel::PingConnection(Connection* conn) {
-  bool use_candidate = false;
-  if (remote_ice_mode_ == ICEMODE_FULL && ice_role_ == ICEROLE_CONTROLLING) {
-    use_candidate =
-        (conn == selected_connection_) || (selected_connection_ == NULL) ||
-        (!selected_connection_->writable()) ||
-        (CompareConnectionCandidates(selected_connection_, conn) < 0);
-  } else if (remote_ice_mode_ == ICEMODE_LITE && conn == selected_connection_) {
-    use_candidate = selected_connection_->writable();
+  bool use_candidate_attr = false;
+  int nominating_value = 0;
+  if (ice_role_ == ICEROLE_CONTROLLING) {
+    bool should_nominate =
+        (conn == selected_connection_) && selected_connection_->writable();
+    if (should_nominate) {
+      if (peer_supports_renomination_) {
+        if (last_nominating_connection_ != conn) {
+          ++nominating_value_;
+          last_nominating_connection_ = conn;
+        }
+        nominating_value = nominating_value_;

[Taylor Brandstetter, 2016/07/22 23:48:29]

Instead of using a temporary variable "nominating_value" and setting it outside
the "if" block, you can just set it here.

[honghaiz3, 2016/07/25 22:24:41]

That's not the intended behavior. 
A connection may be the selected one first and become not later.
In that case, we want to set nominating_value to 0 here. 
The same applies to the set_use_candidate_attr. 

[Taylor Brandstetter, 2016/07/27 22:20:55]

I understand. Previously, "set_nominating_value(0)" would have done nothing, but
now it has meaning.

+      } else {
+        use_candidate_attr = true;
+      }
+    }
   }
-  conn->set_use_candidate_attr(use_candidate);
+  conn->set_nominating_value(nominating_value);
+  conn->set_use_candidate_attr(use_candidate_attr);
   last_ping_sent_ms_ = rtc::TimeMillis();
   conn->Ping(last_ping_sent_ms_);
 }
 
 // When a connection's state changes, we need to figure out who to use as
 // the selected connection again.  It could have become usable, or become
 // unusable.
 void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
@@ skipping 272 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket