Proxy support for P2P TCP Socket

Proxy detection is important feature for the P2P sockets, as
it provides additional security for enterprise use cases.

Communication between webrtc endpoints happens through UDP/TCP/STUN
with or without the help of TURN server.

In case of firewall restrictions like outgoing UDP block and allowing
only secure ports, direct communication between two endbecomes impossible 
without the help of TURN.
In the presence of proxies, direct communication even to TURN server also 
becomes impossible without first esablishing connection with the proxy.

This CL is trying to address the last problem. 
If proxies are configured(HTTP/S/SOCKS) send data(stun+media) through it.

jingle_glue::ProxyResolvingClientSocket solves the problem of proxy detection and establishing the connection and also it's a type of net::StreamSocket, this class can be directly used in P2PSocketHostTcpBase.

R=sergeyu@chromium.org,juberti@chromium.org,thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207919

[Mallinath (Gone from Chromium), 2013-06-19 00:00:56]

Justin/Sergey,

Please review this CL. Unit tests are not complete yet, i am working on
addressing it.

[Mallinath (Gone from Chromium), 2013-06-20 00:58:14]

On 2013/06/19 00:00:56, mallinath2 wrote:
> Justin/Sergey,
> 
> Please review this CL. Unit tests are not complete yet, i am working on
> addressing it.

Ping...

[Sergey Ulanov, 2013-06-20 01:50:02]

looks mostly good.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_dispatcher_host.h (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_dispatcher_host.h:30:
net::URLRequestContextGetter* getter);
better to call it url_context.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_dispatcher_host.h:80:
scoped_refptr<net::URLRequestContextGetter> context_getter_;
url_context_

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host.cc:76:
net::URLRequestContextGetter* context_getter) {
use the same name here and in the header. I would prefer calling it
|url_context|

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host.h (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host.h:30:
net::URLRequestContextGetter* getter);
nit: call it url_context? |getter| is too ambiguous.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp.cc:30: net::StreamSocket*
CreateSocket(
This function is called in only one place and it just creates new
ProxyResolvingClientSocket(), so I think you don't really need it.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp_server.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_server.cc:27:
net::URLRequestContextGetter* context_getter)
I think you don't really need context_getter here. Proxy is useless for incoming
connections anyway, and P2PSocketHostTcp() will never use it.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:34: if
(socket_type_ == P2P_SOCKET_TCP_CLIENT)
add {} for multi-line if.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:36: &sender_, 0,
NULL));
this can fit on the previous line.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:82: :
P2PSocketHostTcpTestBase(P2P_SOCKET_STUN_TCP_CLIENT) {}
nit: move } to the next line.

[Mallinath (Gone from Chromium), 2013-06-20 05:24:14]

PTAL

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_dispatcher_host.h (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_dispatcher_host.h:30:
net::URLRequestContextGetter* getter);
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> better to call it url_context.

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_dispatcher_host.h:80:
scoped_refptr<net::URLRequestContextGetter> context_getter_;
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> url_context_

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host.cc:76:
net::URLRequestContextGetter* context_getter) {
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> use the same name here and in the header. I would prefer calling it
> |url_context|

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host.h (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host.h:30:
net::URLRequestContextGetter* getter);
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> nit: call it url_context? |getter| is too ambiguous.

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp.cc:30: net::StreamSocket*
CreateSocket(
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> This function is called in only one place and it just creates new
> ProxyResolvingClientSocket(), so I think you don't really need it.

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp_server.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_server.cc:27:
net::URLRequestContextGetter* context_getter)
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> I think you don't really need context_getter here. Proxy is useless for
incoming
> connections anyway, and P2PSocketHostTcp() will never use it.

Agree. Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc (right):

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:34: if
(socket_type_ == P2P_SOCKET_TCP_CLIENT)
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> add {} for multi-line if.

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:36: &sender_, 0,
NULL));
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> this can fit on the previous line.

Done.

https://codereview.chromium.org/17132012/diff/10001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc:82: :
P2PSocketHostTcpTestBase(P2P_SOCKET_STUN_TCP_CLIENT) {}
On 2013/06/20 01:50:02, Sergey Ulanov wrote:
> nit: move } to the next line.

Done.

[Sergey Ulanov, 2013-06-20 19:56:33]

lgtm

[Mallinath (Gone from Chromium), 2013-06-20 20:23:58]

+thakis@chromium.org - for content/browser/render_host

[Mallinath (Gone from Chromium), 2013-06-21 00:05:08]

On 2013/06/20 20:23:58, mallinath2 wrote:
> mailto:+thakis@chromium.org - for content/browser/render_host

Ping @thakis

[juberti, 2013-06-21 04:30:34]

Code lgtm

Can this be unit tested? Have we done any tests with real proxies?

https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp.cc (right):

https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp.cc:33:
//weak_ptr_factory_(this),
Remove?

[Nico, 2013-06-21 04:37:14]

renderer_host lgtm

[Mallinath (Gone from Chromium), 2013-06-21 05:46:00]

On 2013/06/21 04:30:34, juberti wrote:
> Code lgtm
> 
> Can this be unit tested? Have we done any tests with real proxies?
> 
>
https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
> File content/browser/renderer_host/p2p/socket_host_tcp.cc (right):
> 
>
https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
> content/browser/renderer_host/p2p/socket_host_tcp.cc:33:
> //weak_ptr_factory_(this),
> Remove?

Done some testing by installing squid proxy server on my local machine. But not
tested with real proxy server. 
For unit testing, i am exploring how we can do that and how existing chrome net
does.

[Mallinath (Gone from Chromium), 2013-06-21 05:46:10]

https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
File content/browser/renderer_host/p2p/socket_host_tcp.cc (right):

https://codereview.chromium.org/17132012/diff/65001/content/browser/renderer_...
content/browser/renderer_host/p2p/socket_host_tcp.cc:33:
//weak_ptr_factory_(this),
On 2013/06/21 04:30:35, juberti wrote:
> Remove?

Done.

[commit-bot: I haz the power, 2013-06-21 19:01:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mallinath@chromium.org/17132012/85002

[commit-bot: I haz the power, 2013-06-21 21:07:57]

Change committed as 207919