Cast Port Allocator Factory for PeerConnection

remoting/jingle_glue/chromium_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 "remoting/jingle_glue/chromium_socket_factory.h"

[Sergey Ulanov, 2014/07/17 18:54:40]

FYI I've moved this file to remoting/protocol - you will need to merge that
change.

[aiguha, 2014/07/17 19:39:59]

Thanks. When would be the right time to do that?

 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "jingle/glue/utils.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/udp/udp_server_socket.h"
 #include "remoting/jingle_glue/socket_util.h"
@@ skipping 348 matching lines @@
     return NULL;
   return result.release();
 }
 
 talk_base::AsyncPacketSocket*
 ChromiumPacketSocketFactory::CreateServerTcpSocket(
     const talk_base::SocketAddress& local_address,
     int min_port, int max_port,
     int opts) {
   // We don't use TCP sockets for remoting connections.
-  NOTREACHED();
+  // NOTREACHED();

[aiguha, 2014/07/17 03:11:36]

There seems to be no way to restrict WebRTC from trying to use TCP. Thus, when
performing its ICE establishment, it still tries to call this method and hits
the NOTREACHED.
TODO(aiguha): Find a better way to deal with this, as this is likely not
acceptable.

[Sergey Ulanov, 2014/07/17 18:54:40]

I think what might be happening is that the peer (i.e. browser) always allocates
TCP candidates and PeerConnection tries to use them even when TCP is disabled,
i.e. flags disabled only allocation of TCP candidates (i.e.
CreateClientTcpSocket() is never called), but incoming TCP candidates are still
used.

The reason we disable TCP for regular Chromoting connections is because
PseudoTCP performs poorly over TCP. But that reason doesn't apply when casting
the screen using WebRTC, so we actually may want to enable TCP at some point.

I suggest you replace NOTREACHED() with NOTIMPLEMENTED().

[aiguha, 2014/07/17 19:39:59]

Done.

[aiguha, 2014/07/17 19:39:59]

You're right, I think that's exactly what's happening. 

Understood. I'll go ahead and replace it with NOTIMPLEMENTED() for now then. 

   return NULL;
 }
 
 talk_base::AsyncPacketSocket*
 ChromiumPacketSocketFactory::CreateClientTcpSocket(
       const talk_base::SocketAddress& local_address,
       const talk_base::SocketAddress& remote_address,
       const talk_base::ProxyInfo& proxy_info,
       const std::string& user_agent,
       int opts) {
   // We don't use TCP sockets for remoting connections.
   NOTREACHED();
   return NULL;
 }
 
 talk_base::AsyncResolverInterface*
 ChromiumPacketSocketFactory::CreateAsyncResolver() {
   return new talk_base::AsyncResolver();
 }
 
 }  // namespace remoting