[HttpCache] Measure causes for cache validation requests

[HttpCache] Measure causes for cache validation requests

This CL adds metrics for the causes of validation requests, and breaks them down by resource type.

BUG=611068
Committed: https://crrev.com/9e50a0f7ff51166a36985f4fb55a0e9b352b732e
Cr-Commit-Position: refs/heads/master@{#393577}

[jkarlin, 2016-05-11 16:36:29]

jkarlin@chromium.org changed reviewers:
+ gavinp@chromium.org, rkaplow@chromium.org

[jkarlin, 2016-05-11 16:36:29]

gavinp: PTAL at everything
rkaplow: PTAL at histograms.xml

Thanks!

[rkaplow, 2016-05-11 17:07:43]

lgtm

[gavinp, 2016-05-12 15:02:02]

lgtm.

I'm on the fence about including unconditionalisable resources. They can't be
validated, but they may well have had a freshness period.

I suggest you exclude them since they are such an interesting oddball case that
we may wish to deal with separately, and we can chase them later by expanding
UMA on pattern just for them. Since removing things from UMA later is bad
practice, that's why I preach for not including now.

Their inclusion will definitely skew the numbers. Unconditionalisable resources
are about 4.8% of cache transactions; compare to validations at 4.6% and updates
at 1%.

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
File net/http/http_cache_transaction.cc (left):

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.cc:2715:
UMA_HISTOGRAM_ENUMERATION(std::string("HttpCache.Pattern.MainFrameHTML"),
What was this stuff?

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
File net/http/http_cache_transaction.h (right):

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.h:252: enum ValidationCause {
Could we use an enum class here?

[jkarlin, 2016-05-13 15:06:51]

gavinp and rkaplow, PTAL as I've modified the logic and the histograms.

I think you're right about the unconditionalizable requests. Thanks! I've
updated the CL to monitor them separately for now.

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
File net/http/http_cache_transaction.cc (left):

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.cc:2715:
UMA_HISTOGRAM_ENUMERATION(std::string("HttpCache.Pattern.MainFrameHTML"),
On 2016/05/12 15:02:01, gavinp wrote:
> What was this stuff?

Why was std::string there? It's leftover cruft from a local experiment I was
doing that I failed to edit out in the previous CL.

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
File net/http/http_cache_transaction.h (right):

https://codereview.chromium.org/1969663003/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.h:252: enum ValidationCause {
On 2016/05/12 15:02:02, gavinp wrote:
> Could we use an enum class here?

We could, but we'd still have to static_cast the histogram calls.

[gavinp, 2016-05-13 17:40:23]

lgtm.

[gavinp, 2016-05-13 17:40:25]

lgtm.

[jkarlin, 2016-05-13 17:41:38]

Thanks! rkaplow@ PTAL again as histograms.xml has changed.

[rkaplow, 2016-05-13 17:55:34]

lgtm

[jkarlin, 2016-05-13 17:59:58]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2016-05-13 18:00:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1969663003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1969663003/40001

[commit-bot: I haz the power, 2016-05-13 18:05:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-13 18:06:41]

Description was changed from

==========
[HttpCache] Measure causes for cache validation requests

This CL adds metrics for the causes of validation requests, and breaks them down
by resource type.

BUG=611068
==========

to

==========
[HttpCache] Measure causes for cache validation requests

This CL adds metrics for the causes of validation requests, and breaks them down
by resource type.

BUG=611068

Committed: https://crrev.com/9e50a0f7ff51166a36985f4fb55a0e9b352b732e
Cr-Commit-Position: refs/heads/master@{#393577}
==========

[commit-bot: I haz the power, 2016-05-13 18:06:42]

Patchset 3 (id:??) landed as
https://crrev.com/9e50a0f7ff51166a36985f4fb55a0e9b352b732e
Cr-Commit-Position: refs/heads/master@{#393577}