Record data use of favicon

Record data use of favicon

This CL records the network data use for downloading favicons. Also the
size of non 200 responses for favicon requests are recorded.

BUG=698347
Review-Url: https://codereview.chromium.org/2764893003
Cr-Commit-Position: refs/heads/master@{#462975}
Committed: https://chromium.googlesource.com/chromium/src/+/6ec568654e74b51c244d67ebb9f326fae24ec28f

[rajendrant, 2017-03-21 15:54:09]

rajendrant@google.com changed reviewers:
+ rajendrant@google.com, ryansturm@chromium.org

[rajendrant, 2017-03-21 15:54:10]

ptal

[pkotwicz, 2017-03-22 00:44:13]

Description was changed from

==========
Record data use of favicon

Thsi CL records the network data use for downloading favicons. Also the
size of non 200 responses for favicon requests are recorded.

BUG=698347
==========

to

==========
Record data use of favicon

This CL records the network data use for downloading favicons. Also the
size of non 200 responses for favicon requests are recorded.

BUG=698347
==========

[RyanSturm, 2017-03-22 22:00:26]

lgtm % nits

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
File components/data_use_measurement/content/content_url_request_classifier.h
(right):

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/content/content_url_request_classifier.h:1: //
Copyright 2016 The Chromium Authors. All rights reserved.
You have a typo in the CL description s/Thsi CL records/This CL records/

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/content/content_url_request_classifier.h:21:
bool IsUserRequest(const net::URLRequest& request) const override;
nit: Can you add a comment to the top of this group of overrides like "//
URLRequestClassifier:" and remove the vertical spaces between the methods.

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
File components/data_use_measurement/core/url_request_classifier.h (right):

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/core/url_request_classifier.h:32: // Returns
true if the URLRequest |request| belongs to favicon.
"Returns true if |request| is fetching a favicon."

[Raj, 2017-03-23 17:06:45]

rajendrant@chromium.org changed reviewers:
+ holte@chromium.org
- rajendrant@google.com

[Raj, 2017-03-23 17:06:47]

holte: ptal histograms.xml

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
File components/data_use_measurement/content/content_url_request_classifier.h
(right):

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/content/content_url_request_classifier.h:1: //
Copyright 2016 The Chromium Authors. All rights reserved.
On 2017/03/22 22:00:26, Ryan Sturm wrote:
> You have a typo in the CL description s/Thsi CL records/This CL records/

I did not find that. May be you edited and fixed it?

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/content/content_url_request_classifier.h:21:
bool IsUserRequest(const net::URLRequest& request) const override;
On 2017/03/22 22:00:26, Ryan Sturm wrote:
> nit: Can you add a comment to the top of this group of overrides like "//
> URLRequestClassifier:" and remove the vertical spaces between the methods.

Done.

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
File components/data_use_measurement/core/url_request_classifier.h (right):

https://codereview.chromium.org/2764893003/diff/1/components/data_use_measure...
components/data_use_measurement/core/url_request_classifier.h:32: // Returns
true if the URLRequest |request| belongs to favicon.
On 2017/03/22 22:00:26, Ryan Sturm wrote:
> "Returns true if |request| is fetching a favicon."

Done.

[Steven Holte, 2017-03-24 01:50:58]

lgtm

[Raj, 2017-03-24 06:45:18]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2017-03-24 06:45:18]

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

[commit-bot: I haz the power, 2017-03-24 06:45:27]

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, 2017-03-24 06:47:24]

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

[commit-bot: I haz the power, 2017-03-24 06:47:25]

Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Raj, 2017-03-24 17:05:28]

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

[commit-bot: I haz the power, 2017-03-24 17:06:18]

Dry run: 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, 2017-03-24 17:16:49]

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

[commit-bot: I haz the power, 2017-03-24 17:16:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Raj, 2017-03-24 19:36:02]

Patchset #3 (id:40001) has been deleted

[Raj, 2017-04-07 18:04:03]

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

[commit-bot: I haz the power, 2017-04-07 18:04:47]

Dry run: 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, 2017-04-07 19:38:56]

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

[commit-bot: I haz the power, 2017-04-07 19:38:57]

Dry run: This issue passed the CQ dry run.

[Raj, 2017-04-07 19:57:57]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2017-04-07 19:57:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryansturm@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2764893003/#ps80001
(title: "fix tryjob")

[commit-bot: I haz the power, 2017-04-07 19:58:35]

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, 2017-04-07 20:05:30]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491595077175650,
"parent_rev": "8dcf8d7130873385e61338446a377e76966545a2", "commit_rev":
"6ec568654e74b51c244d67ebb9f326fae24ec28f"}

[commit-bot: I haz the power, 2017-04-07 20:06:55]

Description was changed from

==========
Record data use of favicon

This CL records the network data use for downloading favicons. Also the
size of non 200 responses for favicon requests are recorded.

BUG=698347
==========

to

==========
Record data use of favicon

This CL records the network data use for downloading favicons. Also the
size of non 200 responses for favicon requests are recorded.

BUG=698347

Review-Url: https://codereview.chromium.org/2764893003
Cr-Commit-Position: refs/heads/master@{#462975}
Committed:
https://chromium.googlesource.com/chromium/src/+/6ec568654e74b51c244d67ebb9f3...
==========

[commit-bot: I haz the power, 2017-04-07 20:06:56]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6ec568654e74b51c244d67ebb9f3...