Differentiate between small and large images in caching metrics

Differentiate between small and large images in caching metrics

This CL differentiates between small and large images when reporting
caching pattern metrics. The purpose is to separate tracking pixel
caching behavior from other images.

BUG=603551
Committed: https://crrev.com/3eec862179fbac345c64156e255f7878e015e807
Cr-Commit-Position: refs/heads/master@{#387572}

[jkarlin, 2016-04-14 14:08:05]

jkarlin@chromium.org changed reviewers:
+ isherman@chromium.org, ttuttle@chromium.org

[jkarlin, 2016-04-14 14:08:06]

ttuttle@ PTAL at http_cache_transaction.cc
isherman@ PTAL at histograms.xml

Thanks!

[Deprecated (see juliatuttle), 2016-04-14 14:11:33]

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

https://codereview.chromium.org/1888043002/diff/1/net/http/http_cache_transac...
net/http/http_cache_transaction.cc:2726: if
(response_headers->GetContentLength() < 100) {
GetContentLength returns -1 if there is no Content-Length header. Do you intend
for those to be considered tiny images?

[jkarlin, 2016-04-14 14:28:52]

Thanks!

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

https://codereview.chromium.org/1888043002/diff/1/net/http/http_cache_transac...
net/http/http_cache_transaction.cc:2726: if
(response_headers->GetContentLength() < 100) {
On 2016/04/14 14:11:33, Julia Tuttle (deprecated) wrote:
> GetContentLength returns -1 if there is no Content-Length header. Do you
intend
> for those to be considered tiny images?

Good point, better to skip them. Done.

[Deprecated (see juliatuttle), 2016-04-14 14:30:19]

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

https://codereview.chromium.org/1888043002/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.cc:2727: if (content_length >= 0 &&
content_length < 100)
Looks like you're missing a {?

[jkarlin, 2016-04-14 14:54:58]

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

https://codereview.chromium.org/1888043002/diff/20001/net/http/http_cache_tra...
net/http/http_cache_transaction.cc:2727: if (content_length >= 0 &&
content_length < 100)
On 2016/04/14 14:30:18, Julia Tuttle (deprecated) wrote:
> Looks like you're missing a {?

*head smack* Done.

[Deprecated (see juliatuttle), 2016-04-14 16:44:29]

Thanks, http_cache_transaction.cc lgtm!

[Ilya Sherman, 2016-04-14 21:35:07]

metrics LGTM

[jkarlin, 2016-04-15 10:59:56]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2016-04-15 11:00:10]

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

[commit-bot: I haz the power, 2016-04-15 11:05:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-15 11:06:14]

Description was changed from

==========
Differentiate between small and large images in caching metrics

This CL differentiates between small and large images when reporting
caching pattern metrics. The purpose is to separate tracking pixel
caching behavior from other images.

BUG=603551
==========

to

==========
Differentiate between small and large images in caching metrics

This CL differentiates between small and large images when reporting
caching pattern metrics. The purpose is to separate tracking pixel
caching behavior from other images.

BUG=603551

Committed: https://crrev.com/3eec862179fbac345c64156e255f7878e015e807
Cr-Commit-Position: refs/heads/master@{#387572}
==========

[commit-bot: I haz the power, 2016-04-15 11:06:15]

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