Record the time duration for establishing proxy connection

Record the time duration for establishing proxy connection

Different UMA histograms are used for secure and insecure proxies. 
Similarly, different histograms are used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339
Review-Url: https://codereview.chromium.org/2768173002
Cr-Commit-Position: refs/heads/master@{#460216}
Committed: https://chromium.googlesource.com/chromium/src/+/4cbf5c1bc96f24e9e660e538cdce3d00562626f4

[tbansal1, 2017-03-22 23:00:01]

Description was changed from

==========
Record UMA

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

to

==========
Record the time duration for establishing proxy connection

Different UMA histogram is used for secure and insecure proxies. 
Similarly, different histogram is used when the proxy connection was
successful vs. when the connection attempt timed out.

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

[tbansal1, 2017-03-22 23:03:01]

tbansal@chromium.org changed reviewers:
+ ryansturm@chromium.org

[tbansal1, 2017-03-22 23:03:02]

ryansturm: ptal. Thanks.

[tbansal1, 2017-03-22 23:07:46]

Description was changed from

==========
Record the time duration for establishing proxy connection

Different UMA histogram is used for secure and insecure proxies. 
Similarly, different histogram is used when the proxy connection was
successful vs. when the connection attempt timed out.

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

to

==========
Record the time duration for establishing proxy connection

Different UMA histogram is used for secure and insecure proxies. 
Similarly, different histogram is used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339
==========

[tbansal1, 2017-03-22 23:24:31]

tbansal@chromium.org changed reviewers:
+ mmenke@chromium.org
- ryansturm@chromium.org

[tbansal1, 2017-03-22 23:24:32]

mmenke: ptal. Thanks.

[tbansal1, 2017-03-23 19:27:25]

Description was changed from

==========
Record the time duration for establishing proxy connection

Different UMA histogram is used for secure and insecure proxies. 
Similarly, different histogram is used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339
==========

to

==========
Record the time duration for establishing proxy connection

Different UMA histograms are used for secure and insecure proxies. 
Similarly, different histograms are used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339
==========

[mmenke, 2017-03-23 21:58:01]

On 2017/03/22 23:24:32, tbansal1 wrote:
> mmenke: ptal. Thanks.

I'll take a look tomorrow (Main thing I want to do is verify that this covers
the right set of proxies).

[tbansal1, 2017-03-23 22:41:55]

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

[commit-bot: I haz the power, 2017-03-23 22:42:54]

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

[commit-bot: I haz the power, 2017-03-23 23:33:55]

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

[commit-bot: I haz the power, 2017-03-23 23:33:55]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[mmenke, 2017-03-27 16:36:33]

LGTM, modulo comment.  Didn't look at the test.

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
File net/http/http_proxy_client_socket_wrapper.cc (right):

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
net/http/http_proxy_client_socket_wrapper.cc:511: base::TimeTicks::Now() -
connect_start_time_);
These names should make it clear that they're only for Http proxies.

[mmenke, 2017-03-27 16:39:11]

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
File net/http/http_proxy_client_socket_wrapper.cc (right):

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
net/http/http_proxy_client_socket_wrapper.cc:632: next_state_ ==
STATE_SSL_CONNECT_COMPLETE) {
One other question:  What about other errors?  Looks like we're currently
ignoring them.  HTTPS auth case looks kinda funky, too (Not sure about HTTP
auth, didn't think about it).

[tbansal1, 2017-03-27 18:28:47]

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

[commit-bot: I haz the power, 2017-03-27 18:29:28]

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

[tbansal1, 2017-03-27 18:29:30]

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
File net/http/http_proxy_client_socket_wrapper.cc (right):

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
net/http/http_proxy_client_socket_wrapper.cc:511: base::TimeTicks::Now() -
connect_start_time_);
On 2017/03/27 16:36:33, mmenke wrote:
> These names should make it clear that they're only for Http proxies.

Done.

https://codereview.chromium.org/2768173002/diff/1/net/http/http_proxy_client_...
net/http/http_proxy_client_socket_wrapper.cc:632: next_state_ ==
STATE_SSL_CONNECT_COMPLETE) {
On 2017/03/27 16:39:11, mmenke wrote:
> One other question:  What about other errors?  Looks like we're currently
> ignoring them.  HTTPS auth case looks kinda funky, too (Not sure about HTTP
> auth, didn't think about it).
Yes, this histogram records time until only the timeout error. Right now, it is
not probably very meaningful since all values are going to be 10 seconds (or 30
seconds). Once I start the dynamic timeout experiment, it will be useful.

I have also added histogram for other errors.

[tbansal1, 2017-03-27 18:29:44]

mmenke: ptal for PS#2. Thanks.

[mmenke, 2017-03-27 19:45:36]

On 2017/03/27 18:29:44, tbansal1 wrote:
> mmenke: ptal for PS#2. Thanks.

Still LGTM

[tbansal1, 2017-03-27 19:47:49]

tbansal@chromium.org changed reviewers:
+ rkaplow@chromium.org

[tbansal1, 2017-03-27 19:47:49]

rkaplow: ptal at histograms.xml. Thanks.

[commit-bot: I haz the power, 2017-03-27 20:00:57]

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

[commit-bot: I haz the power, 2017-03-27 20:00:57]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-03-28 03:03:47]

lgtm

[tbansal1, 2017-03-28 05:26:56]

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

[commit-bot: I haz the power, 2017-03-28 05:27:28]

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

[commit-bot: I haz the power, 2017-03-28 05:33:25]

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

[commit-bot: I haz the power, 2017-03-28 05:33:26]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-03-28 19:56:31]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2017-03-28 19:57:07]

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

[commit-bot: I haz the power, 2017-03-28 21:08:04]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490730991763710,
"parent_rev": "3e2545eea28039a76b9c056c84fddac3cbc9d7d8", "commit_rev":
"4cbf5c1bc96f24e9e660e538cdce3d00562626f4"}

[commit-bot: I haz the power, 2017-03-28 21:09:21]

Description was changed from

==========
Record the time duration for establishing proxy connection

Different UMA histograms are used for secure and insecure proxies. 
Similarly, different histograms are used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339
==========

to

==========
Record the time duration for establishing proxy connection

Different UMA histograms are used for secure and insecure proxies. 
Similarly, different histograms are used when the proxy connection was
successful vs. when the connection attempt timed out.

BUG=704339

Review-Url: https://codereview.chromium.org/2768173002
Cr-Commit-Position: refs/heads/master@{#460216}
Committed:
https://chromium.googlesource.com/chromium/src/+/4cbf5c1bc96f24e9e660e538cdce...
==========

[commit-bot: I haz the power, 2017-03-28 21:09:22]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4cbf5c1bc96f24e9e660e538cdce...