Prepare for ICE-renomination

webrtc/p2p/base/p2ptransportchannel.cc

Index: webrtc/p2p/base/p2ptransportchannel.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 5238da83ca9302954e00b4a5204fc5121eda6851..2d75585b8ac35e59c96db0fef661a38e90ed4038 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -1141,11 +1141,10 @@ int P2PTransportChannel::CompareConnections(
   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()) {
@@ -1216,7 +1215,7 @@ void P2PTransportChannel::SortConnectionsAndUpdateState() {
   // 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();
   }
 
@@ -1286,6 +1285,9 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) {
   Connection* old_selected_connection = selected_connection_;
   selected_connection_ = conn;
   if (selected_connection_) {
+    if (peer_supports_renomination_) {
+      ++nominating_value_;
+    }
     if (old_selected_connection) {
       LOG_J(LS_INFO, this) << "Previous selected connection: "
                            << old_selected_connection->ToString();
@@ -1586,9 +1588,8 @@ void P2PTransportChannel::MarkConnectionPinged(Connection* 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
+// |use_candidate_attr| and |nominating_value| flags. One of the flags is set to
+// nominate |conn| if 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
@@ -1598,16 +1599,31 @@ void P2PTransportChannel::MarkConnectionPinged(Connection* conn) {
 //      b.1) |conn| is the selected_connection AND
 //      b.2) |conn| is writable.
 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();
-  }
-  conn->set_use_candidate_attr(use_candidate);
+  bool use_candidate_attr = false;
+  int nominating_value = 0;
+  if (ice_role_ == ICEROLE_CONTROLLING) {
+    bool should_nominate = false;
+    if (remote_ice_mode_ == ICEMODE_FULL) {
+      should_nominate =
+          (conn == selected_connection_) || (selected_connection_ == NULL) ||
+          (!selected_connection_->writable()) ||
+          CompareConnectionCandidates(selected_connection_, conn) < 0;
+    } else if (remote_ice_mode_ == ICEMODE_LITE) {
+      should_nominate =
+          (conn == selected_connection_) && selected_connection_->writable();
+    }
+    if (should_nominate) {
+      if (peer_supports_renomination_) {
+        nominating_value = nominating_value_;

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

If using renomination, shouldn't we only nominate the selected connection?

If the selected connection becomes unwritable, then this code would nominate
everything with the same value, basically falling back to aggressive nomination.

If the selected connection then becomes writable again, but others do too, how
would the controlled side know which to use? They all have the same nomination
value.

[honghaiz3, 2016/07/28 04:19:14]

Done. 
Although the previous implementation should also work, the renomination draft
says to disable aggressive nomination when using renomination. 

+      } else {
+        use_candidate_attr = true;
+      }
+    }
+  }
+  // A connection may be the selected one earlier, but be switched later,
+  // so its nominating_value may be set to 0.
+  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_);
 }