Fix connection type reporting

A cricket::Connection has both a remote and a local candidate, and the
connection type should be determined by inspecting both. Up till now, only the
remote candidate was being considered. When the connection uses a relay, for
example, one candidate is always "relay" while the other is always "local".
Which is which depends on timing and the order that candidates are generated and
exchanged. In fact, in one of my tests I ended up with the mux channel as
(local,relay) and the video channel as (relay,local) in the same session.

The fix in this CL is to ignore the candidate reported in the event and
to use the most "indirect" connection type of the Connection's local and
remote candidates.

N.B. I have verified that the OnRouteChange event is triggered
correctly. It's only that the |candidate| parameter turned out not to be
very useful.

BUG=380518

Review URL: https://codereview.chromium.org/762523003

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

1 files changed, 32 insertions, 10 deletions

diff --git a/remoting/protocol/libjingle_transport_factory.cc b/remoting/protocol/libjingle_transport_factory.cc
index b7b8755..310b40e 100644
--- a/remoting/protocol/libjingle_transport_factory.cc
+++ b/remoting/protocol/libjingle_transport_factory.cc
@@ -4,6 +4,8 @@
 
 #include "remoting/protocol/libjingle_transport_factory.h"
 
+#include <algorithm>
+
 #include "base/callback.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
@@ -32,6 +34,22 @@ const int kReconnectDelaySeconds = 15;
 // Get fresh STUN/Relay configuration every hour.
 const int kJingleInfoUpdatePeriodSeconds = 3600;
 
+// Utility function to map a cricket::Candidate string type to a
+// TransportRoute::RouteType enum value.
+TransportRoute::RouteType CandidateTypeToTransportRouteType(
+    const std::string& candidate_type) {
+  if (candidate_type == "local") {
+    return TransportRoute::DIRECT;
+  } else if (candidate_type == "stun") {
+    return TransportRoute::STUN;
+  } else if (candidate_type == "relay") {
+    return TransportRoute::RELAY;
+  } else {
+    LOG(FATAL) << "Unknown candidate type: " << candidate_type;
+    return TransportRoute::DIRECT;
+  }
+}
+
 class LibjingleTransport
     : public Transport,
       public base::SupportsWeakPtr<LibjingleTransport>,
@@ -242,22 +260,26 @@ void LibjingleTransport::OnRouteChange(
     const cricket::Candidate& candidate) {
   TransportRoute route;
 
-  if (candidate.type() == "local") {
-    route.type = TransportRoute::DIRECT;
-  } else if (candidate.type() == "stun") {
-    route.type = TransportRoute::STUN;
-  } else if (candidate.type() == "relay") {
-    route.type = TransportRoute::RELAY;
-  } else {
-    LOG(FATAL) << "Unknown candidate type: " << candidate.type();
-  }
+  DCHECK(channel_->best_connection());
+  const cricket::Connection* connection = channel_->best_connection();
+
+  // A connection has both a local and a remote candidate. For our purposes, the
+  // route type is determined by the most indirect candidate type. For example:
+  // it's possible for the local candidate be a "relay" type, while the remote
+  // candidate is "local". In this case, we still want to report a RELAY route
+  // type.
+  static_assert(TransportRoute::DIRECT < TransportRoute::STUN &&
+                TransportRoute::STUN < TransportRoute::RELAY,
+                "Route type enum values are ordered by 'indirectness'");
+  route.type = std::max(
+      CandidateTypeToTransportRouteType(connection->local_candidate().type()),
+      CandidateTypeToTransportRouteType(connection->remote_candidate().type()));
 
   if (!jingle_glue::SocketAddressToIPEndPoint(
           candidate.address(), &route.remote_address)) {
     LOG(FATAL) << "Failed to convert peer IP address.";
   }
 
-  DCHECK(channel_->best_connection());
   const cricket::Candidate& local_candidate =
       channel_->best_connection()->local_candidate();
   if (!jingle_glue::SocketAddressToIPEndPoint(