Fix miss counting on http cache transactions.

Fix miss counting on http cache transactions.

Loads bypassing the cache were counting as misses; this was bad for two reasons. Firstly, we were overcounting misses. Secondly, this was making misses look too performant.

This fix was actually uploaded in https://codereview.chromium.org/11637007/ , but in all the debate about the histograms in that patch, this fix got missed. Here it is again, all by itself.

R=rvargas@chromium.org
BUG=None


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=175203

[gavinp, 2013-01-04 16:37:55]

https://codereview.chromium.org/11741034/diff/1/net/http/http_cache_transacti...
File net/http/http_cache_transaction.cc (right):

https://codereview.chromium.org/11741034/diff/1/net/http/http_cache_transacti...
net/http/http_cache_transaction.cc:735:
UpdateTransactionPattern(PATTERN_NOT_COVERED);
Added bonus: this if statement now meets our coding standards.

[rvargas (doing something else), 2013-01-04 19:50:53]

I'm not sure I agree about the characterization of forced misses not being
misses at all, but I really don't care that much.

lgtm.

[gavinp, 2013-01-04 19:55:42]

On 2013/01/04 19:50:53, rvargas wrote:
> I'm not sure I agree about the characterization of forced misses not being
> misses at all, but I really don't care that much.

I'm sorry that you don't care very much. I think these pattern histograms are
important, and if you don't, let's make sure to talk about that next week.

On the patch in question here, perhaps the problem was that I was using the word
"miss" in the description, when really it's PATTERN_NOT_CACHED. When the user
bypasses the cache, is it the case that the resource was "not cached?"

Since the HttpCache.* histograms are intended to help speed up the cache when
it's being used, would counting these help or hinder that goal?

I think those two questions guide me to say that a LOAD_BYPASSing_CACHE does not
refer to a PATTERN_ENTRY_NOT_CACHED. If I'm wrong, I'd love to hear more about
why.

> 
> lgtm.

[gavinp, 2013-01-04 19:56:09]

Thank you for the LGTM, I'm cq+ ing this despite the red win_rel test, since
that looked very much like a flake.

[commit-bot: I haz the power, 2013-01-04 19:56:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gavinp@chromium.org/11741034/1

[commit-bot: I haz the power, 2013-01-04 21:58:26]

Change committed as 175203

[rvargas (doing something else), 2013-01-04 22:58:52]

On 2013/01/04 19:55:42, gavinp wrote:
> On 2013/01/04 19:50:53, rvargas wrote:
> > I'm not sure I agree about the characterization of forced misses not being
> > misses at all, but I really don't care that much.
> 
> I'm sorry that you don't care very much. I think these pattern histograms are
> important, and if you don't, let's make sure to talk about that next week.
> 
> On the patch in question here, perhaps the problem was that I was using the
word
> "miss" in the description, when really it's PATTERN_NOT_CACHED. When the user
> bypasses the cache, is it the case that the resource was "not cached?"

Correct, bypassing the cache is not the same as not_cached. But that
interpretation (not_cached == miss) is not exclusive to this CL description...
it's been used multiple times. That's one of the issues of this set of
histograms, but it is more productive to talk about it next week in person.

> 
> Since the HttpCache.* histograms are intended to help speed up the cache when
> it's being used, would counting these help or hinder that goal?
> 
> I think those two questions guide me to say that a LOAD_BYPASSing_CACHE does
not
> refer to a PATTERN_ENTRY_NOT_CACHED. If I'm wrong, I'd love to hear more about
> why.
> 
> > 
> > lgtm.