New issue
Advanced search Search tips

Issue 808498 link

Starred by 6 users
Status: Fixed
Owner:
rajendrant@chromium.org
Closed: Oct 30
Cc:
cduvall@chromium.org
dxie@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug
Proj-Servicification

Blocking:
issue 840378



Sign in to add a comment

Track data usage by NetworkTrafficAnnotation in the network service

Project Member Reported by ryansturm@chromium.org, Feb 2 2018 
A wrapper class, ServiceURLLoader, should be consumed by all Chrome services to track data usage related to Chrome services. This is a replacement for the URLFetcher service code, and URLFetchers will be transistioned one by one from URLFetcher to ServiceURLLoader.

Comment 1 by tbansal@chromium.org, Mar 7 2018

Labels: -M-66 M-67
Refreshed during triage.

Comment 2 by ryansturm@chromium.org, Mar 19 2018

Labels: -Pri-1 Pri-2

Comment 3 by rajendrant@chromium.org, Mar 19 2018 

Refreshed during triage.
Lowered priority.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020

commit 1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020
Author: Ryan Sturm <ryansturm@chromium.org>
Date: Thu Apr 19 01:13:28 2018

Adding a MeasuredURLLoader which will wrap SimpleURLLoader

This class will be consuemd by all Chrome services/features for the purposes of
tracking data usage and other statistics about loading behavior.

This CL introduces the class, but does not add any consumers, and the
functionality to track loads is not yet added.

The purpose of MeasuredURLLoader is to provide a loading API for Chrome
services that will track metrics around their data use to monitor if any
services have excessive data use. The expectation is that most
instances of URLFetcher will be transitioned to ServiceURLLoader with
any loads that should be untracked being moved to SimpleURLLoader.

Bug:  808498 
Change-Id: I6fc75e37627272e23e96c3341d3593a185c0bf36
Reviewed-on: https://chromium-review.googlesource.com/899822
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551905}
[modify] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/components/data_use_measurement/core/DEPS
[add] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/components/data_use_measurement/core/measured_url_loader.cc
[add] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/components/data_use_measurement/core/measured_url_loader.h
[add] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/components/data_use_measurement/core/measured_url_loader_unittest.cc
[modify] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020/tools/metrics/histograms/histograms.xml
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 19 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78e88337d829a274eb2b433159afabf1af41af92

commit 78e88337d829a274eb2b433159afabf1af41af92
Author: Ryan Sturm <ryansturm@chromium.org>
Date: Thu Apr 19 15:21:00 2018

Revert "Adding a MeasuredURLLoader which will wrap SimpleURLLoader"

This reverts commit 1de0a8de73b4c1acea7ee5dd72fbbd1b782e6020.

Reason for revert: Accidentally submitted.

Original change's description:
> Adding a MeasuredURLLoader which will wrap SimpleURLLoader
> 
> This class will be consuemd by all Chrome services/features for the purposes of
> tracking data usage and other statistics about loading behavior.
> 
> This CL introduces the class, but does not add any consumers, and the
> functionality to track loads is not yet added.
> 
> The purpose of MeasuredURLLoader is to provide a loading API for Chrome
> services that will track metrics around their data use to monitor if any
> services have excessive data use. The expectation is that most
> instances of URLFetcher will be transitioned to ServiceURLLoader with
> any loads that should be untracked being moved to SimpleURLLoader.
> 
> Bug:  808498 
> Change-Id: I6fc75e37627272e23e96c3341d3593a185c0bf36
> Reviewed-on: https://chromium-review.googlesource.com/899822
> Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551905}

TBR=rkaplow@chromium.org,blundell@chromium.org,mmenke@chromium.org,pilgrim@chromium.org,tbansal@chromium.org,ryansturm@chromium.org

Change-Id: I6331bd049eb0e0e79bb6c321aa41d1037e03724d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808498 
Reviewed-on: https://chromium-review.googlesource.com/1019398
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552023}
[modify] https://crrev.com/78e88337d829a274eb2b433159afabf1af41af92/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/78e88337d829a274eb2b433159afabf1af41af92/components/data_use_measurement/core/DEPS
[delete] https://crrev.com/225f8a53bf7c21ab23f274e1741870609b6c01aa/components/data_use_measurement/core/measured_url_loader.cc
[delete] https://crrev.com/225f8a53bf7c21ab23f274e1741870609b6c01aa/components/data_use_measurement/core/measured_url_loader.h
[delete] https://crrev.com/225f8a53bf7c21ab23f274e1741870609b6c01aa/components/data_use_measurement/core/measured_url_loader_unittest.cc
[modify] https://crrev.com/78e88337d829a274eb2b433159afabf1af41af92/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/78e88337d829a274eb2b433159afabf1af41af92/tools/metrics/histograms/histograms.xml

Comment 6 by xunji...@chromium.org, May 7 2018

Blocking: 840378

Comment 7 by ryansturm@chromium.org, May 22 2018

Labels: -M-67 M-69
I will work on this soon.

Comment 8 by ryansturm@chromium.org, Jun 22 2018 

I am working with raj to make sure we have a consistent approach to this using NetworkTrafficAnnotations.

Comment 9 by rajendrant@chromium.org, Aug 8 

As per the latest discussion, the plan was to report service data usage from within the network process by hooking onto NetworkContext::ContextNetworkDelegate.
This will make services/network depend on data_use_measurement.

Comment 10 by mmenke@chromium.org, Aug 16

Components: Internals>Services>Network

Comment 11 by jam@chromium.org, Aug 21

Labels: Hotlist-KnownIssue

Comment 12 by jam@chromium.org, Sep 4

Labels: -Hotlist-KnownIssue

Comment 13 by dxie@chromium.org, Sep 6

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows

Comment 14 by dxie@google.com, Sep 6

Labels: -M-69 ReleaseBlock-Stable M-71

Comment 15 by dxie@chromium.org, Sep 7

Labels: -Pri-2 -ReleaseBlock-Stable Proj-Servicification-Stable Hotlist-KnownIssue Pri-1

Comment 16 by ryansturm@chromium.org, Sep 7

Owner: rajendrant@chromium.org
Summary: Track data usage by NetworkTrafficAnnotation in the network service (was: Add a ServiceURLLoader to track service data usage for services using SimpleURLLoader)
Update: this approach is changing, so I renamed this bug and assigned it rajendrant.

Comment 17 by sclit...@chromium.org, Sep 19 

Refreshed during triage.

Comment 18 by tbansal@chromium.org, Sep 24 

Looks like this is fixed? rajendrant@, any updates?

Comment 19 by dxie@chromium.org, Oct 2 

ping ... is this done? thx! We would like to close this in M71.

Comment 20 by rajendrant@google.com, Oct 3 

I am starting to work on this in a couple of days.

Comment 21 by dxie@chromium.org, Oct 4 

we would like to have this in M71 timeframe. Thanks!
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 12 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dce70a031f697342a52b900211aa0ba7eceab082

commit dce70a031f697342a52b900211aa0ba7eceab082
Author: rajendrant <rajendrant@chromium.org>
Date: Fri Oct 12 06:24:24 2018

Remove service name and refactor data use update to metrics service

This CL does a bunch of steps to make the servicification easy to do.
1. Removes the usage of ServiceName in data_use_measurement
2. Refactors the data use update to metrics service to not take service
name and instead send if the data use is from metrics component.
3. To remove the UpdateUsagePrefCallback from DataUseMeasurement(),
ChromeDataUseMeasurement is introduced that does the posting to UI thread
(from io_thread.cc). This helps in servicification since there won't be
thread hop. This also helps when CDUA classes are removed, and
ChromeDataUseMeasurement can be there without dependency.

Bug:  808498 
Change-Id: I7a4421c9798447c1ca4d39698f69b227cc773d48
Reviewed-on: https://chromium-review.googlesource.com/c/1272256
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599113}
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/BUILD.gn
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
[add] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[add] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/data_use_measurement/chrome_data_use_measurement.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/io_thread.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/io_thread.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/net/system_network_context_manager.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/DEPS
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_ascriber.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_ascriber.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_measurement.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_measurement_unittest.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_network_delegate.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_network_delegate.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_network_delegate_unittest.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_user_data.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/data_use_measurement/core/data_use_user_data.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/domain_reliability/uploader.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/domain_reliability/uploader.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/data_use_tracker.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/data_use_tracker.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/data_use_tracker_unittest.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/metrics_service.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/metrics_service.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/reporting_service.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/components/metrics/reporting_service.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/google_apis/gaia/gaia_auth_util.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/google_apis/gaia/gaia_auth_util.h
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/google_apis/gaia/gaia_oauth_client.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/google_apis/gaia/oauth2_api_call_flow.cc
[modify] https://crrev.com/dce70a031f697342a52b900211aa0ba7eceab082/tools/metrics/histograms/histograms.xml

Comment 23 by dxie@chromium.org, Oct 15

Labels: Merge-Request-71
Project Member

Comment 24 by sheriffbot@chromium.org, Oct 15

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 25 by gov...@chromium.org, Oct 15

Cc: dxie@chromium.org
+dxie@, pls confirm which CL you're requesting a merge and provide merge justification. Thank you.

Comment 26 by rajendrant@chromium.org, Oct 16 

govind@
I am requesting for merge of CL at #c22
The change records data use of Chrome services when network service feature is enabled. It will be useful to keep chrome-services data use under check.

Comment 27 by gov...@chromium.org, Oct 16

Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #26.

Comment 28 by gov...@chromium.org, Oct 17 

Pls merge your change to M71 branch 3578 ASAP so we can pick it up for next M71 Dev/Beta release. Thank you.
Project Member

Comment 29 by bugdroid1@chromium.org, Oct 17

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4008c4636794cf8c42cd0c99849914740129d38d

commit 4008c4636794cf8c42cd0c99849914740129d38d
Author: rajendrant <rajendrant@chromium.org>
Date: Wed Oct 17 21:40:50 2018

Remove service name and refactor data use update to metrics service

This CL does a bunch of steps to make the servicification easy to do.
1. Removes the usage of ServiceName in data_use_measurement
2. Refactors the data use update to metrics service to not take service
name and instead send if the data use is from metrics component.
3. To remove the UpdateUsagePrefCallback from DataUseMeasurement(),
ChromeDataUseMeasurement is introduced that does the posting to UI thread
(from io_thread.cc). This helps in servicification since there won't be
thread hop. This also helps when CDUA classes are removed, and
ChromeDataUseMeasurement can be there without dependency.

Bug:  808498 
Change-Id: I7a4421c9798447c1ca4d39698f69b227cc773d48
Reviewed-on: https://chromium-review.googlesource.com/c/1272256
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599113}(cherry picked from commit dce70a031f697342a52b900211aa0ba7eceab082)
Reviewed-on: https://chromium-review.googlesource.com/c/1286861
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#104}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/BUILD.gn
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
[add] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[add] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/data_use_measurement/chrome_data_use_measurement.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/io_thread.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/io_thread.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/net/system_network_context_manager.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/net/system_network_context_manager.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/DEPS
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_ascriber.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_ascriber.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_measurement.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_measurement_unittest.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_network_delegate.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_network_delegate.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_network_delegate_unittest.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_user_data.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/data_use_measurement/core/data_use_user_data.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/domain_reliability/uploader.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/domain_reliability/uploader.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/data_use_tracker.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/data_use_tracker.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/data_use_tracker_unittest.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/metrics_service.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/metrics_service.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/reporting_service.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/components/metrics/reporting_service.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/google_apis/gaia/gaia_auth_util.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/google_apis/gaia/gaia_auth_util.h
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/google_apis/gaia/gaia_oauth_client.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/google_apis/gaia/oauth2_api_call_flow.cc
[modify] https://crrev.com/4008c4636794cf8c42cd0c99849914740129d38d/tools/metrics/histograms/histograms.xml
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adb22130c129125461857eafe66342209939b71b

commit adb22130c129125461857eafe66342209939b71b
Author: rajendrant <rajendrant@chromium.org>
Date: Thu Oct 18 21:55:48 2018

NetworkService: Report data use of URLLoader to browser process

When the requests complete, the total received and sent bytes are sent
to browser process. ChromeDataUseMeasurement records metrics on Chrome-services
data usage and updates metrics service.

Subsequent CLs will record more metrics and also report this to data
reduction proxy settings page.

When NetworkService is disabled, DataUseMeasurement will get network
delegate callbacks and record metrics.

Bug:  808498 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I113e480e4e1c67a65ff7461eb5ae2166a6b038d2
Reviewed-on: https://chromium-review.googlesource.com/c/1274202
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600928}
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/browser_process.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/browser_process_impl.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/data_use_measurement/chrome_data_use_measurement.h
[add] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/browser/data_use_measurement/data_use_measurement_browsertest.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/test/BUILD.gn
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/DEPS
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/data_use_measurement.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/data_use_measurement_unittest.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/data_use_network_delegate.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/components/data_use_measurement/core/data_use_network_delegate_unittest.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/content/browser/network_service_client.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/content/browser/network_service_client.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/content/public/browser/content_browser_client.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/services/network/test/test_network_service_client.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/services/network/test/test_network_service_client.h
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/services/network/url_loader.cc
[modify] https://crrev.com/adb22130c129125461857eafe66342209939b71b/services/network/url_loader_unittest.cc

Comment 31 by dxie@google.com, Oct 18

Labels: -merge-merged-3578 Merge-Request-72
merge request for C#30

Comment 32 by dxie@google.com, Oct 18

Labels: -Merge-Request-72 Merge-Request-71
Project Member

Comment 33 by sheriffbot@chromium.org, Oct 18

Labels: -Merge-Request-71 Merge-Review-71
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 34 by gov...@chromium.org, Oct 19 

CL list at #30 is not baked in canary yet also it seems like a big cl. Pls update bug with canary result tomorrow.

Also may i please know why multiple merge requests for this bug as we already took large merge at #29?
Project Member

Comment 35 by bugdroid1@chromium.org, Oct 19 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/859b0299534a83a7f6ad8c307fe9c4227c569b53

commit 859b0299534a83a7f6ad8c307fe9c4227c569b53
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Oct 19 00:09:20 2018

Revert "NetworkService: Report data use of URLLoader to browser process"

This reverts commit adb22130c129125461857eafe66342209939b71b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 600928 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vYWRiMjIxMzBjMTI5MTI1NDYxODU3ZWFmZTY2MzQyMjA5OTM5YjcxYgw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/linux-xenial-rel/4268

Sample Failed Step: network_service_browser_tests on Ubuntu-16.04

Sample Flaky Test: DataUseMeasurementBrowserTestWithDataSaverEnabled.CheckServicesDataUseRecorded

Original change's description:
> NetworkService: Report data use of URLLoader to browser process
> 
> When the requests complete, the total received and sent bytes are sent
> to browser process. ChromeDataUseMeasurement records metrics on Chrome-services
> data usage and updates metrics service.
> 
> Subsequent CLs will record more metrics and also report this to data
> reduction proxy settings page.
> 
> When NetworkService is disabled, DataUseMeasurement will get network
> delegate callbacks and record metrics.
> 
> Bug:  808498 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I113e480e4e1c67a65ff7461eb5ae2166a6b038d2
> Reviewed-on: https://chromium-review.googlesource.com/c/1274202
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600928}

Change-Id: I6fdfa077e01fb3ae0693be88eb58361a6bb17a13
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808498 ,  896942 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/c/1290137
Cr-Commit-Position: refs/heads/master@{#600975}
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/browser_process.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/browser_process_impl.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/browser/data_use_measurement/chrome_data_use_measurement.h
[delete] https://crrev.com/ce5c2317dab655c8913ff31cbee0b438138f1b7f/chrome/browser/data_use_measurement/data_use_measurement_browsertest.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/test/BUILD.gn
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/DEPS
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/data_use_measurement.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/data_use_measurement_unittest.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/data_use_network_delegate.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/components/data_use_measurement/core/data_use_network_delegate_unittest.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/content/browser/network_service_client.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/content/browser/network_service_client.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/content/public/browser/content_browser_client.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/services/network/test/test_network_service_client.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/services/network/test/test_network_service_client.h
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/services/network/url_loader.cc
[modify] https://crrev.com/859b0299534a83a7f6ad8c307fe9c4227c569b53/services/network/url_loader_unittest.cc

Comment 36 by gov...@chromium.org, Oct 19

Labels: -Merge-Review-71 Merge-Rejected-71
Rejecting merge to M71 as CL got reverted at #35.
Project Member

Comment 37 by bugdroid1@chromium.org, Oct 19 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/feea859b767aa478ecc8f0ca4f3c070f93defa06

commit feea859b767aa478ecc8f0ca4f3c070f93defa06
Author: rajendrant <rajendrant@chromium.org>
Date: Fri Oct 19 18:26:06 2018

Reland "NetworkService: Report data use of URLLoader to browser process"

This is a reland of adb22130c129125461857eafe66342209939b71b

Original change's description:
> NetworkService: Report data use of URLLoader to browser process
>
> When the requests complete, the total received and sent bytes are sent
> to browser process. ChromeDataUseMeasurement records metrics on Chrome-services
> data usage and updates metrics service.
>
> Subsequent CLs will record more metrics and also report this to data
> reduction proxy settings page.
>
> When NetworkService is disabled, DataUseMeasurement will get network
> delegate callbacks and record metrics.
>
> Bug:  808498 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I113e480e4e1c67a65ff7461eb5ae2166a6b038d2
> Reviewed-on: https://chromium-review.googlesource.com/c/1274202
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600928}

TBR=kinuko@chromium.org

Bug:  808498 
Change-Id: I341012581527fefeb6fd23ff16af4ed3a8373aa7
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/c/1290522
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601229}
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/browser_process.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/browser_process_impl.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/data_use_measurement/chrome_data_use_measurement.h
[add] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/browser/data_use_measurement/data_use_measurement_browsertest.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/test/BUILD.gn
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/BUILD.gn
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/DEPS
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/data_use_measurement.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/data_use_measurement_unittest.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/data_use_network_delegate.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/components/data_use_measurement/core/data_use_network_delegate_unittest.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/content/browser/network_service_client.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/content/browser/network_service_client.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/content/public/browser/content_browser_client.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/services/network/test/test_network_service_client.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/services/network/test/test_network_service_client.h
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/services/network/url_loader.cc
[modify] https://crrev.com/feea859b767aa478ecc8f0ca4f3c070f93defa06/services/network/url_loader_unittest.cc
Project Member

Comment 38 by cr-audit...@appspot.gserviceaccount.com, Oct 23

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/4008c4636794cf8c42cd0c99849914740129d38d

Commit: 4008c4636794cf8c42cd0c99849914740129d38d
Author: rajendrant@chromium.org
Commiter: rajendrant@chromium.org
Date: 2018-10-17 21:40:50 +0000 UTC

Remove service name and refactor data use update to metrics service

This CL does a bunch of steps to make the servicification easy to do.
1. Removes the usage of ServiceName in data_use_measurement
2. Refactors the data use update to metrics service to not take service
name and instead send if the data use is from metrics component.
3. To remove the UpdateUsagePrefCallback from DataUseMeasurement(),
ChromeDataUseMeasurement is introduced that does the posting to UI thread
(from io_thread.cc). This helps in servicification since there won't be
thread hop. This also helps when CDUA classes are removed, and
ChromeDataUseMeasurement can be there without dependency.

Bug:  808498 
Change-Id: I7a4421c9798447c1ca4d39698f69b227cc773d48
Reviewed-on: https://chromium-review.googlesource.com/c/1272256
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599113}(cherry picked from commit dce70a031f697342a52b900211aa0ba7eceab082)
Reviewed-on: https://chromium-review.googlesource.com/c/1286861
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#104}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Comment 39 by cduvall@chromium.org, Oct 23

Cc: cduvall@chromium.org
Project Member

Comment 40 by bugdroid1@chromium.org, Oct 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f40edda8eaa00675d1fd1cb7c46b5710861bc95b

commit f40edda8eaa00675d1fd1cb7c46b5710861bc95b
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Oct 23 21:17:07 2018

Report services data use to data saver

When network service is enabled, data saver site-breakdown gets the
services and downloads data usage from DataUseMeasurement.

Bug:  808498 

Change-Id: I9ec7c98e67b5e8db531a345de1f5f89824efa807
Reviewed-on: https://chromium-review.googlesource.com/c/1295729
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602098}
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/chrome/browser/data_use_measurement/chrome_data_use_measurement.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_use_measurement/core/data_use_measurement.cc
[modify] https://crrev.com/f40edda8eaa00675d1fd1cb7c46b5710861bc95b/components/data_use_measurement/core/data_use_measurement.h

Comment 41 by rajendrant@chromium.org, Oct 30

Status: Fixed (was: Started)
I am marking this as fixed since some of the most common service metrics landed.
https://crbug.com/827679 will be used for migrating the other data use metrics

Comment 42 Deleted

Comment 43 by asvitk...@chromium.org, Dec 14 

I just want to follow up here. I see a lot of changes landed for this implementation.

Has any verification been done to make sure the new implementation is working as expected? For example, if the metrics should work with and without network service, have we looked at the histograms splits by the trial to make sure there is no difference?

I ask because NetworkService is starting to be enabled on Android and on Android we require the integration with MetricsService for data use tracking to work. And I want to make sure that this indeed does work.
►

Sign in to add a comment