Add HttpCache.ExternallyConditionalized histogram.

Add HttpCache.ExternallyConditionalized histogram.

When the Blink cache produces conditional requests, the Chrome cache
always sends them to the origin server without attempting to serve them
from the cache. Add a histogram to find out how common conditional
requests from Blink are, and how many are hitting the network
even though a usable Chrome cache entry exists.

BUG=407968
TEST=manual inspection of about:histograms

Committed: https://crrev.com/48391cd8d716af87d70c2a4cf23516892d6748c0
Cr-Commit-Position: refs/heads/master@{#293934}

[Adam Rice, 2014-09-02 08:54:17]

ricea@chromium.org changed reviewers:
+ yhirano@chromium.org

[yhirano, 2014-09-03 06:19:19]

On 2014/09/02 08:54:17, Adam Rice wrote:

I'm not sure what you mean by "unnecessarily". RequiresValidation doesn't return
whether a request has to be validated in terms of conditional requests, does it?

[Adam Rice, 2014-09-03 07:32:02]

On 2014/09/03 06:19:19, yhirano wrote:
> On 2014/09/02 08:54:17, Adam Rice wrote:
> 
> I'm not sure what you mean by "unnecessarily". RequiresValidation doesn't
return
> whether a request has to be validated in terms of conditional requests, does
it?

I have clarified the wording. If RequiresValidation() returns false, Chrome's
cache will normally use the cached entry without validating it with the origin
server. There are other reasons why the cached entry might not be usable to
serve
the request (such as mismatched validators), but I don't think it is necessary
to add more complexity as we only need a rough idea of the proportion of
requests
involved.

[yhirano, 2014-09-03 07:42:36]

lgtm

[Adam Rice, 2014-09-03 07:44:25]

ricea@chromium.org changed reviewers:
+ rvargas@chromium.org

[Adam Rice, 2014-09-03 07:44:26]

+rvargas for net/

[rvargas (doing something else), 2014-09-04 01:45:13]

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

https://codereview.chromium.org/531693003/diff/1/net/http/http_cache_transact...
net/http/http_cache_transaction.cc:2240:
UpdateTransactionPattern(PATTERN_NOT_COVERED);
The description says that it tells whether the response could come from the
cache or not. This case cannot be served from the cache so it would be in the
wrong bucket.

[rvargas (doing something else), 2014-09-04 01:47:57]

On 2014/09/04 01:45:13, rvargas wrote:
>
https://codereview.chromium.org/531693003/diff/1/net/http/http_cache_transact...
> File net/http/http_cache_transaction.cc (right):
> 
>
https://codereview.chromium.org/531693003/diff/1/net/http/http_cache_transact...
> net/http/http_cache_transaction.cc:2240:
> UpdateTransactionPattern(PATTERN_NOT_COVERED);
> The description says that it tells whether the response could come from the
> cache or not. This case cannot be served from the cache so it would be in the
> wrong bucket.

... if the objective is to remove the conditional headers added by blink, the
histogram would be close to reality, but ignores XHR (a reason this code cannot
go away)

[Adam Rice, 2014-09-04 15:19:41]

The primary objective is just to determine how much Blink revalidations will
interfere with our stale-while-revalidate experiment. I'm hoping they will be in
<1% territory.

As a longer-term goal I am interested in serving Blink revalidations from
Chrome's cache when fresh (or acceptably stale) resources are available, but I'm
aware that would involve significant changes to HttpCache::Transaction. This is
less about improving performance and more about making Chrome's behaviour less
weird and more consistent from the point of view of server operators.

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

https://codereview.chromium.org/531693003/diff/1/net/http/http_cache_transact...
net/http/http_cache_transaction.cc:2240:
UpdateTransactionPattern(PATTERN_NOT_COVERED);
On 2014/09/04 01:45:12, rvargas wrote:
> The description says that it tells whether the response could come from the
> cache or not. This case cannot be served from the cache so it would be in the
> wrong bucket.

Sorry, I had some kind of tunnel vision and was convinced I couldn't get that
information for the histogram even though it is calculated directly above.

I have added it to the enum.

[rvargas (doing something else), 2014-09-04 19:33:21]

lgtm

https://codereview.chromium.org/531693003/diff/20001/net/http/http_cache_tran...
File net/http/http_cache_transaction.cc (right):

https://codereview.chromium.org/531693003/diff/20001/net/http/http_cache_tran...
net/http/http_cache_transaction.cc:2245: // TODO(ricea): Add CACHE_USABLE_STALE
once stale-while-revalidate CL landed.
tiny nit: Please add an empty line above this line.

[Adam Rice, 2014-09-05 05:49:29]

ricea@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Adam Rice, 2014-09-05 05:49:30]

+asvitkine for histograms.xml.

https://codereview.chromium.org/531693003/diff/20001/net/http/http_cache_tran...
File net/http/http_cache_transaction.cc (right):

https://codereview.chromium.org/531693003/diff/20001/net/http/http_cache_tran...
net/http/http_cache_transaction.cc:2245: // TODO(ricea): Add CACHE_USABLE_STALE
once stale-while-revalidate CL landed.
On 2014/09/04 19:33:20, rvargas wrote:
> tiny nit: Please add an empty line above this line.

Done.

[Alexei Svitkine (slow), 2014-09-05 15:25:58]

lgtm

[Adam Rice, 2014-09-09 12:51:14]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2014-09-09 12:52:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ricea@chromium.org/531693003/40001

[commit-bot: I haz the power, 2014-09-09 13:48:42]

Committed patchset #3 (id:40001) as 3b86857d23506bd46410025199c8e469a5e358ff

[commit-bot: I haz the power, 2014-09-10 03:53:13]

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