New issue
Advanced search Search tips

Issue 921369 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 472729



Sign in to add a comment

Merge SocketParams classes

Project Member Reported by mmenke@chromium.org, Jan 13

Issue description

The fields in net's SocketParams subclasses can basically be subdivided into three types:
1) Things that should be global per-SSL server state (SSLConfig).
2) Things that should be per-socket pool state (Proxy info).
3) Information that's actually needed to distinguish a group from other groups (Host, port, privacy mode, and whether SSL negotiation should be used or not - the latter is currently per-socket-pool state, but with merged socket pools, no longer will be.  SSL hostname is always the host of the destination server, except for SSL proxies, but that information is pool-wide proxy state).

Only things in 3) should be in SocketParams, which will makes SocketParams classes simple enough that we no longer need a bunch of different types.  In fact, I believe we can remove most of the complexity in ClientSocketPoolManager, and just have it create a standard SocketParams object, figure out which socket pool to use (Which is based solely on the proxy in use), and send the params object there.  Then a factory function can take that parameter object, the proxy information (Provided by the socket pool itself), and various global objects (Per-host SSL information, host resolver, etc), and create a ConnectJob of the right type.  So the factory function would look a bit like ClientSocketPoolManager, though hopefully a fair bit simpler.

The simplified params object would also make a reasonable key in each socket pool's group map, so we could get rid of group names.

This can't be completed until both the SocketPool refactor, and removing the SSLConfig object from SSLSocketParams (Is there a bug for that?).  Just filing this in the interest of getting feedback, in case I'm missing anything.
 
I guess there's also user agent for HTTP tunnels which needs to be figured out, as that's set on a per-request basis (And also seems kinda broken).

Sign in to add a comment