New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 844187 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug
Proj-Servicification

Blocking:
issue 844146
issue 848075



Sign in to add a comment

Mojoify network::ProxyResolvingClientSocket

Project Member Reported by xunji...@chromium.org, May 17 2018

Issue description

ProxyingResolvingClientSocket connects a TCP or a TLS socket which respects system's proxy settings. The class is previously moved from jingle_glue to services/network, as a part of servicifying network stack's sockets. 
The old design doc is at: https://docs.google.com/document/d/1iQl_Y2o7vykiPXpZiNbKov-WZbUb4RprmXIk311tTso/edit


ProxyingResolvingClientSocket is still a C++ class. I've been doing a couple of cleanups to get it in the right shape for servicifying. 

Filing a bug to make an equivalent mojo interface of network::ProxyResolvingClientSocket, so we can move consumers (P2P sockets by WebRTC in particular) to network service.
 

Comment 1 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Windows OS-Linux OS-Mac OS-Chrome OS-Android
Blocking: 848075
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc515750b2b498992b385cbe2f21e76fc3c88d23

commit cc515750b2b498992b385cbe2f21e76fc3c88d23
Author: Helen Li <xunjieli@chromium.org>
Date: Thu Jun 14 19:47:54 2018

Add proxy_resolving_socket.mojom to network service

GCM and WebRTC depend on the ability to create proxied TCP and TLS
sockets. This CL adds a mojo socket interface for this type of sockets.

This CL adds:
- network::mojom::ProxyResolvingSocketFactory
  This represents an interface that can create proxied sockets. Each instance
  has separate socket pools hanging off an HttpNetworkSession that is built
  using NetworkContext's params.
- network::mojom::ProxyResolvingSocket
  This represents a socket interface that establish socket connection that
  respects system's proxy settings.

There are functionalities missing. I am planning to add them in follow-up CLs.

Bug:  844187 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I5698e02d79708c0e07e57b1f4ac0f82cf484916d
Reviewed-on: https://chromium-review.googlesource.com/1080948
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567374}
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/BUILD.gn
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/network_context.cc
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/network_context.h
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_client_socket.cc
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_client_socket.h
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_socket_factory_mojo.cc
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_socket_factory_mojo.h
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_socket_mojo.cc
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_socket_mojo.h
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/proxy_resolving_socket_mojo_unittest.cc
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/public/mojom/BUILD.gn
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/public/mojom/network_context.mojom
[add] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/public/mojom/proxy_resolving_socket.mojom
[modify] https://crrev.com/cc515750b2b498992b385cbe2f21e76fc3c88d23/services/network/test/test_network_context.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29c74040d247a982ae51c0b8fb1eb5860f98c6f5

commit 29c74040d247a982ae51c0b8fb1eb5860f98c6f5
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Jun 15 01:45:56 2018

Use SSLConfig from SSLConfigService for ProxyResolvingClientSocketFactory

ProxyResolvingClientSocketFactory's consumers (p2p sockets used by WebRTC)
currently use a default-initialized net::SSLConfig struct. This net::SSLConfig
doesn't respect admin policies and system settings.

This CL uses the SSLConfig from SSLConfigService which has the right SSLConfig.

This CL additionally removes one unneeded param (net::ClientSocketFactory*)
from the factory's constructor.

This CL is to prepare for servicifying ProxyResolvingClientSocket.

Bug:  844187 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Idf18c9f27a0080a55b3dca16ba593302be3486bb
Reviewed-on: https://chromium-review.googlesource.com/1100340
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567508}
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/content/browser/renderer_host/p2p/socket_dispatcher_host.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/content/browser/renderer_host/p2p/socket_host_tcp.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/content/browser/renderer_host/p2p/socket_host_tcp_unittest.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/jingle/glue/xmpp_client_socket_factory.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/remoting/signaling/xmpp_signal_strategy.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/remoting/signaling/xmpp_signal_strategy_unittest.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/services/network/proxy_resolving_client_socket_factory.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/services/network/proxy_resolving_client_socket_factory.h
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/services/network/proxy_resolving_client_socket_unittest.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/services/network/proxy_resolving_socket_factory_mojo.cc
[modify] https://crrev.com/29c74040d247a982ae51c0b8fb1eb5860f98c6f5/services/network/proxy_resolving_socket_factory_mojo.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5b864d75f134fe83e4ebdd9e7acc94ccca56507

commit b5b864d75f134fe83e4ebdd9e7acc94ccca56507
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Jun 15 16:10:26 2018

Make GCM's ConnectionFactoryImpl uses ProxyResolvingClientSocketFactory

ProxyResolvingClientSocketFactory takes care of creating a new
HttpNetworkSession that is based on a given net::URLRequestContext.

This CL switches GCM's ConnectionFactoryImpl to using
ProxyResolvingClientSocketFactory, so there is less code duplication. This
will makes it easier to standardize usage of ProxyResolvingClientSocket to
prepare for servicifying it.

Bug:  844187 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I06b1f6b7c6fc7e8c458b24f2ee3dcc5664bd4c21
Reviewed-on: https://chromium-review.googlesource.com/1101058
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567674}
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/components/gcm_driver/gcm_client_impl.cc
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/components/gcm_driver/gcm_client_impl.h
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/components/gcm_driver/gcm_client_impl_unittest.cc
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/google_apis/gcm/engine/connection_factory_impl.cc
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/google_apis/gcm/engine/connection_factory_impl.h
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/google_apis/gcm/engine/connection_factory_impl_unittest.cc
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/google_apis/gcm/tools/mcs_probe.cc
[modify] https://crrev.com/b5b864d75f134fe83e4ebdd9e7acc94ccca56507/services/network/proxy_resolving_client_socket_factory.cc

Status: Fixed (was: Started)
We have a mojo interface for P2P's proxied sockets. I am working on converting P2P over to use this new mojo interface. That's tracked in the master bug, so I am closing this one.

Sign in to add a comment