P2PTransportChannel::PingConnection seems to have issue setting use_candidate

[GoogleCodeExporter]

In linjingle 46778423:

P2PTransportChannel::PingConnection

the code doesn't match the explanation of what should happen and seems to have 
two issues:

1) use_candidate should check best_connection_->writable() == false
2) use_candidate should check ((conn->priority() > 
best_connection_->priority()) && conn->writable()) since by that point there is 
already a best_connection_ and it is writable so we shouldn't switch to another 
connection unless it is writable.

Therefore I believe the code should look like:

void P2PTransportChannel::PingConnection(Connection* conn) {
  bool use_candidate = false;
  if (protocol_type_ == ICEPROTO_RFC5245) {
    if (remote_ice_mode_ == ICEMODE_FULL && role_ == ROLE_CONTROLLING) {
      use_candidate = (conn == best_connection_) ||
                      (best_connection_ == NULL) ||
                      (!best_connection_->writable()) ||
                      ((conn->priority() > best_connection_->priority()) && conn->writable()));
    } else if (remote_ice_mode_ == ICEMODE_LITE && conn == best_connection_) {
      use_candidate = best_connection_->writable();
    }
  }
  conn->set_use_candidate_attr(use_candidate);
  conn->Ping(talk_base::Time());
}

Original issue reported on code.google.com by fineb...@vline.me on 23 May 2013 at 1:57

[GoogleCodeExporter]

Original comment by juberti@google.com on 31 May 2013 at 4:01

• Changed state: Duplicate