Fix the hostname used to setup a TURN/TSL connection.

content/renderer/p2p/ipc_socket_factory.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/p2p/ipc_socket_factory.h"
 
 #include <algorithm>
 #include <deque>
 
 #include "base/compiler_specific.h"
@@ skipping 217 matching lines @@
   remote_address_ = remote_address;
   state_ = IS_OPENING;
 
   net::IPEndPoint local_endpoint;
   if (!jingle_glue::SocketAddressToIPEndPoint(
           local_address, &local_endpoint)) {
     return false;
   }
 
   net::IPEndPoint remote_endpoint;
-  if (!remote_address.IsNil() && !jingle_glue::SocketAddressToIPEndPoint(
-      remote_address, &remote_endpoint) && !IsTcpClientSocket(type_)) {
+  if (!remote_address.IsNil() &&

[Sergey Ulanov, 2014/08/14 20:47:24]

I suggest formatting this code as follows to make it more readable:

 if (!remote_address.IsNil()) {
   DCHECK(IsTcpClientSocket(type_));

   if (remote_address.IsUnresolvedIP()) {
      remote_endpoint =
          net::IPEndPoint(net::IPAddressNumber(), remote_address.port());
   } else {
      if (!jingle_glue::SocketAddressToIPEndPoint(
              remote_address, &remote_endpoint)) {
        return false;
      }
   }
 }

[jiayl, 2014/08/14 22:31:47]

That isn't the same logic. 
1. The DCHECK is not true if the IP is resolved.
2. If the IP is unresolved, is it safe to assert TCP? Could it cause problem in
release build?

On 2014/08/14 20:47:24, Sergey Ulanov wrote:

[jiayl, 2014/08/15 00:05:33]

I would rather just fix the hostname issue instead of changing other logic at
the last minute before a branch cut.

PTAL.

[Sergey Ulanov, 2014/08/15 00:37:06]

What are the cases when it's not true? remote_address is used only for client
TCP sockets and AFAICT it's Nil in all other cases. 
browser process just ignores remote_address for UPD sockets and server TCP
sockets.

[Sergey Ulanov, 2014/08/15 00:37:06]

I don't think it changes logic if you take into account that remote_address is
specified only for tcp client sockets. But feel free to file a bug to cleanup it
later if you disagree.

[jiayl, 2014/08/15 16:23:11]

OK. That makes sense. done.

On 2014/08/15 00:37:06, Sergey Ulanov wrote:

+      (remote_address.IsUnresolvedIP() ||
+       !jingle_glue::SocketAddressToIPEndPoint(remote_address,
+                                               &remote_endpoint)) &&
+      !IsTcpClientSocket(type_)) {
     // Non TCP sockets must have a resolved remote address.
     return false;
   }
 
+  if (remote_address.IsUnresolvedIP()) {
+    remote_endpoint =
+        net::IPEndPoint(net::IPAddressNumber(), remote_address.port());
+  }
+
   // We need to send both resolved and unresolved address in Init. Unresolved
   // address will be used in case of TLS for certificate hostname matching.
   // Certificate will be tied to domain name not to IP address.
-  std::string remote_hostname = remote_address.hostname() + ":" +
-      remote_address.PortAsString();
-  P2PHostAndIPEndPoint remote_info(remote_hostname, remote_endpoint);
+  P2PHostAndIPEndPoint remote_info(remote_address.hostname(), remote_endpoint);
 
   client->Init(type, local_endpoint, remote_info, this);
 
   return true;
 }
 
 void IpcPacketSocket::InitAcceptedTcp(
     P2PSocketClient* client,
     const rtc::SocketAddress& local_address,
     const rtc::SocketAddress& remote_address) {
@@ skipping 404 matching lines @@
 }
 
 rtc::AsyncResolverInterface*
 IpcPacketSocketFactory::CreateAsyncResolver() {
   scoped_ptr<AsyncAddressResolverImpl> resolver(
     new AsyncAddressResolverImpl(socket_dispatcher_));
   return resolver.release();
 }
 
 }  // namespace content