Refine NetworkTimeTracker histograms

Refine NetworkTimeTracker histograms

For a large percentage of CERT_DATE_INVALID errors, network time is not
available due to no sync, despite an attempt to query for network time
sync as early as possible in browser startup.

This CL modifies/adds UMA histograms to answer two questions:
1. What is the latency of network time fetches? (i.e. would it be
feasible to wait to show the interstitial until a network time query has
completed?)
2. When network time is unavailable due to no sync, is it because a
query hasn't been attempted yet, or because all prior query attempts
have failed, or perhaps because the first query attempt hasn't completed
yet?

BUG=589700
Committed: https://crrev.com/53c2e7691edb00c873db5c6e76a12c96f0c44aee
Cr-Commit-Position: refs/heads/master@{#425475}

[estark, 2016-10-14 05:40:16]

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

[commit-bot: I haz the power, 2016-10-14 05:40:34]

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

[estark, 2016-10-14 05:43:21]

estark@chromium.org changed reviewers:
+ jwd@chromium.org, mab@google.com, meacer@chromium.org

[estark, 2016-10-14 05:43:22]

jwd: can you please review histograms.xml?

mab + meacer: can you please review components/network_time and
components/ssl_errors? (mab for domain expertise, meacer too because he might
work on this code soon and also I think technically a non-committer is not
supposed to be the only reviewer)

Thanks, all!

[commit-bot: I haz the power, 2016-10-14 06:31:56]

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

[commit-bot: I haz the power, 2016-10-14 06:31:57]

Dry run: This issue passed the CQ dry run.

[mab, 2016-10-14 06:35:02]

lgtm

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker.cc (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker.cc:487: if
(!UpdateTimeFromResponse()) {  // On error, back off.
Could you set time_query_failed_ here?

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker.h (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker.h:190: // True if a time query has
failed (either a network error, a non-200
like true if any time query has failed ever?

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker_unittest.cc (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker_unittest.cc:752: // immediately
while the request is still pending..
Nit: ..

[mab, 2016-10-14 06:35:03]

lgtm

[estark, 2016-10-14 07:10:30]

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

[commit-bot: I haz the power, 2016-10-14 07:10:45]

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

[estark, 2016-10-14 07:23:46]

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker.cc (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker.cc:487: if
(!UpdateTimeFromResponse()) {  // On error, back off.
On 2016/10/14 06:35:01, mab wrote:
> Could you set time_query_failed_ here?

Oh, yes, that's much better, thanks. Done.

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker.h (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker.h:190: // True if a time query has
failed (either a network error, a non-200
On 2016/10/14 06:35:01, mab wrote:
> like true if any time query has failed ever?

In this object's lifetime, yeah. It's just used to distinguish whether we've
made any attempts or not when we have no sync. And come to think of it,
|time_query_completed_| is probably a more natural way to track it. Changed to
that.

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
File components/network_time/network_time_tracker_unittest.cc (right):

https://codereview.chromium.org/2415703005/diff/1/components/network_time/net...
components/network_time/network_time_tracker_unittest.cc:752: // immediately
while the request is still pending..
On 2016/10/14 06:35:02, mab wrote:
> Nit: ..

Done.

[commit-bot: I haz the power, 2016-10-14 08:06:44]

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

[commit-bot: I haz the power, 2016-10-14 08:06:45]

Dry run: This issue passed the CQ dry run.

[jwd, 2016-10-14 15:27:16]

Histograms lgtm

[meacer, 2016-10-14 21:25:39]

Lgtm

[estark, 2016-10-14 21:33:43]

The CQ bit was checked by estark@chromium.org

[estark, 2016-10-14 21:33:44]

The patchset sent to the CQ was uploaded after l-g-t-m from mab@google.com
Link to the patchset: https://codereview.chromium.org/2415703005/#ps20001
(title: "mab comments")

[commit-bot: I haz the power, 2016-10-14 21:34:22]

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, 2016-10-14 21:41:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-14 21:43:32]

Description was changed from

==========
Refine NetworkTimeTracker histograms

For a large percentage of CERT_DATE_INVALID errors, network time is not
available due to no sync, despite an attempt to query for network time
sync as early as possible in browser startup.

This CL modifies/adds UMA histograms to answer two questions:
1. What is the latency of network time fetches? (i.e. would it be
feasible to wait to show the interstitial until a network time query has
completed?)
2. When network time is unavailable due to no sync, is it because a
query hasn't been attempted yet, or because all prior query attempts
have failed, or perhaps because the first query attempt hasn't completed
yet?

BUG=589700
==========

to

==========
Refine NetworkTimeTracker histograms

For a large percentage of CERT_DATE_INVALID errors, network time is not
available due to no sync, despite an attempt to query for network time
sync as early as possible in browser startup.

This CL modifies/adds UMA histograms to answer two questions:
1. What is the latency of network time fetches? (i.e. would it be
feasible to wait to show the interstitial until a network time query has
completed?)
2. When network time is unavailable due to no sync, is it because a
query hasn't been attempted yet, or because all prior query attempts
have failed, or perhaps because the first query attempt hasn't completed
yet?

BUG=589700

Committed: https://crrev.com/53c2e7691edb00c873db5c6e76a12c96f0c44aee
Cr-Commit-Position: refs/heads/master@{#425475}
==========

[commit-bot: I haz the power, 2016-10-14 21:43:33]

Patchset 2 (id:??) landed as
https://crrev.com/53c2e7691edb00c873db5c6e76a12c96f0c44aee
Cr-Commit-Position: refs/heads/master@{#425475}