New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 836029 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 721403
issue 808201
issue 824088
issue 833845



Sign in to add a comment

Migrate DataUseAscriber consumers to PageLoadMetrics

Project Member Reported by rajendrant@chromium.org, Apr 23 2018

Issue description

DataUseAscriber is being deprecated and removed. It’s consumers will move to sourcing information from the page load metrics harness.

Following is the design doc:

https://docs.google.com/document/d/1qJYdvt0USWAHbJ9hvCkfauZ9ItMoyYCY2w0Fodvse0M/edit#
 
Blocking: 808201
Raj, are you active on this refactor? Should it be higher priority?

I was holding off on 808201 for this refactor. Let me know if I should not.
Labels: -Pri-3 Pri-2
Doug,

Yes. I am actively working on the refactor.
Couple of CLs in review on this.
Cc: dougarnett@chromium.org
Blocking: 833845
Blocking: 824088
Status: Started (was: Assigned)
Labels: M-69

Comment 8 by mmenke@chromium.org, Jun 15 2018

Components: Internals>Services>Network
Labels: Proj-Servicification
Adding NetworkService labels, since the old implementation isn't compatible with the network service.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 19 2018

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

commit 2e83a107d6a3224afd01a6e5192da6501db52ce6
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Jun 19 05:58:02 2018

Report transfer size updates, resource starts and completes to chrome

Background:
Currently page load metrics tracks data usage when requests complete.
This misses canceled requests when the tab closes. The current codepath
will also deprecated due to the upcoming network servicification work.

This CL reports the continuous data use to page load metrics in renderer.
Response starts and completions are reported as well so that the observer
can keep track of data per resource. Subsequent CLs will send this to
browser process via the existing PageLoadMetrics mojo.

Bug:  836029 
Change-Id: I023ea08b2b8f1974ea40586afeee3a27b9900782
Reviewed-on: https://chromium-review.googlesource.com/1042795
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568353}
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/content/public/renderer/render_frame_observer.h
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/content/renderer/loader/resource_dispatcher.h
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2e83a107d6a3224afd01a6e5192da6501db52ce6/content/renderer/render_frame_impl.h

Comment 10 by dougt@chromium.org, Jun 26 2018

rajendrant@ Are you still working on this?
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 3

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

commit 0121950000dfb41c650e65b0c046365d57ed6f54
Author: rajendrant <rajendrant@chromium.org>
Date: Tue Jul 03 00:56:48 2018

Throttle transfer size updates and relax when it was reported

Currently transfer size updates are reported only when report_raw_headers is set.
This CL relaxes that requirement and checks if either report_raw_headers is set
(to work with devtools) or the renderer has permission to read the resource.

This CL also throttles the transfer size updates to be reported only once per
500 ms per resource.

Bug:  836029 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I7e34a0b0c5fad7e424772c075041c0609dc1b365
Reviewed-on: https://chromium-review.googlesource.com/1050814
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572064}
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/cross_site_document_resource_handler.cc
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/mojo_async_resource_handler.h
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/mojo_async_resource_handler_unittest.cc
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/resource_request_info_impl.cc
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/resource_request_info_impl.h
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/content/browser/loader/url_loader_factory_impl_unittest.cc
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/services/network/public/mojom/url_loader.mojom
[modify] https://crrev.com/0121950000dfb41c650e65b0c046365d57ed6f54/services/network/test/test_url_loader_client.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 3

Labels: Hotlist-KnownIssue
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 12

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

commit db2671aa51b5f771f494b2911620711dbdddd115
Author: rajendrant <rajendrant@chromium.org>
Date: Thu Jul 12 00:04:15 2018

Report data use from renderer to browser page load metrics

Report the continuous data use to page load metrics in browser process
via the existing renderer -> browser PageLoadMetrics mojo.

Bug:  836029 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1c7b47eb41e13f9b72d087e3ca2c0a7096c11b29
Reviewed-on: https://chromium-review.googlesource.com/1042877
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574421}
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/metrics_web_contents_observer.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/observers/page_load_metrics_observer_tester.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_metrics_observer.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_tracker.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/browser/page_load_metrics/page_load_tracker.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/common/page_load_metrics/page_load_metrics.mojom
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/BUILD.gn
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/fake_page_timing_sender.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/fake_page_timing_sender.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/metrics_render_frame_observer_unittest.cc
[add] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_resource_data_use.cc
[add] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_resource_data_use.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_timing_metrics_sender_unittest.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/chrome/renderer/page_load_metrics/page_timing_sender.h
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/components/data_reduction_proxy/DEPS
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/components/data_reduction_proxy/core/common/BUILD.gn
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
[modify] https://crrev.com/db2671aa51b5f771f494b2911620711dbdddd115/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 12

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

commit 74f3e3d131a093053a5e5f737e162c99e1c69e62
Author: rajendrant <rajendrant@chromium.org>
Date: Thu Jul 12 08:45:55 2018

Use PLM for data saver site-breakdown

This CL adds PLM observer for site-breakdown that records the data use
by host. The new codepath is enabled and the old data use asriber observer
based site-breakdown codepath is disabled, based on a feature.

Bug:  836029 
Change-Id: I667d0dc6e0227c83a46d353b2feffaec9043a219
Reviewed-on: https://chromium-review.googlesource.com/1050868
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574506}
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/browser/BUILD.gn
[add] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/browser/page_load_metrics/observers/data_saver_site_breakdown_metrics_observer.cc
[add] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/browser/page_load_metrics/observers/data_saver_site_breakdown_metrics_observer.h
[add] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/browser/page_load_metrics/observers/data_saver_site_breakdown_metrics_observer_browsertest.cc
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/chrome/test/BUILD.gn
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/components/data_reduction_proxy/core/common/data_reduction_proxy_features.cc
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/components/data_reduction_proxy/core/common/data_reduction_proxy_features.h
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/74f3e3d131a093053a5e5f737e162c99e1c69e62/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 16

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

commit 4224f9470e01e1709d634434b96e47205bf51176
Author: rajendrant <rajendrant@chromium.org>
Date: Mon Jul 16 22:59:16 2018

Report non-content requests to data saver other host

Report the ResourceInfo-less requests to site-breakdown other hostname. These
requests will not be tracked by page load metrics infra which only handles
requests scoped to page loads.

Bug:  836029 
Change-Id: I23180e05f367e79e1c6a6802913293988a558de8
Reviewed-on: https://chromium-review.googlesource.com/1134506
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575463}
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/content/browser/content_resource_type_provider.cc
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/content/browser/content_resource_type_provider.h
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/browser/data_reduction_proxy_util.cc
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/browser/data_reduction_proxy_util.h
[modify] https://crrev.com/4224f9470e01e1709d634434b96e47205bf51176/components/data_reduction_proxy/core/common/resource_type_provider.h

I am going ahead with the beta experimentation since there is no regression seen in the metrics.
DataReductionProxy.StartupSavingsPercent
DataReductionProxy.UserViewedOriginalSize
DataReductionProxy.UserViewedSavingsSize

https://uma.googleplex.com/p/chrome/variations/?sid=736d71c0d4133a9831f58eca0ddbc23d
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 15

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

commit 7c38e19b3b171d1993e3cfb71e33daecfe3d3299
Author: rajendrant <rajendrant@chromium.org>
Date: Wed Aug 15 21:28:14 2018

Add field trial config for the DataSaverSiteBreakdownUsingPageLoadMetrics feature

Bug:  836029 
Change-Id: Ia0f2a3608eb9be51b652b2519365737276e4f2c6
Reviewed-on: https://chromium-review.googlesource.com/1175558
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583402}
[modify] https://crrev.com/7c38e19b3b171d1993e3cfb71e33daecfe3d3299/testing/variations/fieldtrial_testing_config.json

Labels: -Hotlist-KnownIssue
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Labels: -M-69 ReleaseBlock-Stable M-71
Labels: -Pri-2 -ReleaseBlock-Stable Proj-Servicification-Stable Hotlist-KnownIssue Pri-1
I am going ahead with the stable experimentation since there is no regression seen in the metrics in beta/dev/canary.
DataReductionProxy.StartupSavingsPercent
DataReductionProxy.UserViewedOriginalSize
DataReductionProxy.UserViewedSavingsSize

https://uma.googleplex.com/p/chrome/variations/?sid=37fbe42168381eda48b98ce1089ced44
Cc: aposner@chromium.org
Refreshed during triage.
Refreshed during triage. rajendrant@, any updates? Looks like we update the population sizes now.
hi rajendrant@, any updates here? We would like to make sure this bug is fixed in M71. Please let us know if there are any blockers here. thx!
I am bumping up the stable experimentation to 25%.
No regression seen in the metrics.

https://uma.googleplex.com/p/chrome/variations/?sid=f62a6d30aea3cf53044a8a45f930c219
i'd assume there isn't any further code changes? At this moment, we are doing rollout and potential fix?
Blocking: 721403
Cc: cduvall@chromium.org
Raj, can you report what remains to be done on this task?
Status: Fixed (was: Started)
All the work is done for pre-mojo and post-mojo.

Sign in to add a comment