Fix Net.Socket.IdleSocketTimeSaving histogram

Fix Net.Socket.IdleSocketTimeSaving histogram

IdleSocketTimeSaving is always reported as 0 because
|connect_timing| is null when sockets are reused.

This CL makes the following changes to fix the histogram:
(1) Reused ClientSocketHandle has the |connect_timing|
of the initial connection establishment.
(2) IdleSocket struct has a |connect_timing| field.
(3) Replace IdleSocketTimeSaving with IdleSocketTimeSaving2

BUG=621197

[xunjieli, 2016-11-28 21:05:13]

The CQ bit was checked by xunjieli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 21:06:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xunjieli, 2016-11-28 21:08:50]

Description was changed from

==========
Fix Net.Socket.IdleSocketTimeSaving histogram

BUG=
==========

to

==========
Fix Net.Socket.IdleSocketTimeSaving histogram

IdleSocketTimeSaving is always reported as 0 because
|connect_timing| is null when sockets are reused.

This CL makes the following changes to fix the histogram:
(1) Reused ClientSocketHandle has the |connect_timing|
of the initial connection establishment.
(2) IdleSocket struct has a |connect_timing| field.
(3) Replace IdleSocketTimeSaving with IdleSocketTimeSaving2
==========

[xunjieli, 2016-11-28 21:08:50]

xunjieli@chromium.org changed reviewers:
+ eroman@chromium.org, mmenke@chromium.org

[xunjieli, 2016-11-28 21:09:27]

Patchset #1 (id:1) has been deleted

[xunjieli, 2016-11-28 21:11:14]

I was looking at UMA today and realized that I made a mistake in measuring
IdleSocketTimeSaving. I have added some plumbing and a test. I should have added
testing for that histogram in the first CL. Sorry about that.

PTAL. Thanks!

[eroman, 2016-11-28 21:58:20]

I'll defer this review to Matt

[mmenke, 2016-11-28 22:23:05]

On 2016/11/28 21:58:20, eroman (slow) wrote:
> I'll defer this review to Matt

I'll get to this tomorrow.

[commit-bot: I haz the power, 2016-11-28 23:27:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-28 23:27:12]

Dry run: This issue passed the CQ dry run.

[xunjieli, 2016-11-29 16:12:20]

Description was changed from

==========
Fix Net.Socket.IdleSocketTimeSaving histogram

IdleSocketTimeSaving is always reported as 0 because
|connect_timing| is null when sockets are reused.

This CL makes the following changes to fix the histogram:
(1) Reused ClientSocketHandle has the |connect_timing|
of the initial connection establishment.
(2) IdleSocket struct has a |connect_timing| field.
(3) Replace IdleSocketTimeSaving with IdleSocketTimeSaving2
==========

to

==========
Fix Net.Socket.IdleSocketTimeSaving histogram

IdleSocketTimeSaving is always reported as 0 because
|connect_timing| is null when sockets are reused.

This CL makes the following changes to fix the histogram:
(1) Reused ClientSocketHandle has the |connect_timing|
of the initial connection establishment.
(2) IdleSocket struct has a |connect_timing| field.
(3) Replace IdleSocketTimeSaving with IdleSocketTimeSaving2

BUG=621197
==========

[mmenke, 2016-11-29 16:35:51]

On 2016/11/28 22:23:05, mmenke wrote:
> On 2016/11/28 21:58:20, eroman (slow) wrote:
> > I'll defer this review to Matt
> 
> I'll get to this tomorrow.

Do we really want to add all this stuff just for this histogram?  Is a histogram
that just contains connect times not good enough?  Admittedly, it may
over-weight connections without an entry in the DNS cache.

[xunjieli, 2016-11-29 16:40:15]

On 2016/11/29 16:35:51, mmenke wrote:
> On 2016/11/28 22:23:05, mmenke wrote:
> > On 2016/11/28 21:58:20, eroman (slow) wrote:
> > > I'll defer this review to Matt
> > 
> > I'll get to this tomorrow.
> 
> Do we really want to add all this stuff just for this histogram?  Is a
histogram
> that just contains connect times not good enough?  

What's the name of this histogram? 

> Admittedly, it may
> over-weight connections without an entry in the DNS cache.

I was also debating about fixing or removing the histogram yesterday. I am fine
with the latter. I can abandon this CL and open another one.

[mmenke, 2016-11-29 17:36:42]

On 2016/11/29 16:40:15, xunjieli wrote:
> On 2016/11/29 16:35:51, mmenke wrote:
> > On 2016/11/28 22:23:05, mmenke wrote:
> > > On 2016/11/28 21:58:20, eroman (slow) wrote:
> > > > I'll defer this review to Matt
> > > 
> > > I'll get to this tomorrow.
> > 
> > Do we really want to add all this stuff just for this histogram?  Is a
> histogram
> > that just contains connect times not good enough?  
> 
> What's the name of this histogram? 
> 
> > Admittedly, it may
> > over-weight connections without an entry in the DNS cache.
> 
> I was also debating about fixing or removing the histogram yesterday. I am
fine
> with the latter. I can abandon this CL and open another one.

I'd prefer to just remove the histogram, unless you think it's really important.

We have Net.DNS_Resolution_And_TCP_Connection_Latency2 (Which covers all
connection types, but doesn't cover everything that's part of connecting for SSL
or proxies).  For SSL, we also have Net.SSL_Connection_Latency2, which is
strictly time to negotiate an SSL connection *after* connection establishment
(i.e., excluding Net.DNS_Resolution_And_TCP_Connection_Latency2).

Because of how we're weirdly covering different types of sockets here, there's
going to be some grunginess in any measurement we use.