New issue
Advanced search Search tips

Issue 898326 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 902063



Sign in to add a comment

Deprecate DataReductionProxyData

Project Member Reported by tbansal@chromium.org, Oct 23

Issue description

DataReductionProxyData is currently a part of net::URLRequest, and the values in DataReductionProxyData are modified by //net. This would no longer work with network servicification.

A quick read of DataReductionProxyData indicates that it's possible to migrate some of the PreviewsUserData which is already keyed by navigation throttle (https://cs.chromium.org/chromium/src/chrome/browser/previews/previews_ui_tab_helper.h?rcl=cf0900736f3a901ac2e8d02e979526b9a2c84d57&l=156), or simply remove them.
 
Cc: rajendrant@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25

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

commit 9c5a72cc522900a43030ea23d93edbdca06b72a1
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Oct 25 01:29:21 2018

Get DataReductionProxyData from NavigationHandle in network service path

DataReductionProxyData is not supported with network service since it
gets most of the data from net::URLRequest. This creates a mostly
complete version using the NavigationHandle, which should allow us to
log most of the important DRP metrics correctly. A few things which are
not yet supported:
- DataReductionProxyData in
  MetricsWebContentsObserver::ResourceLoadComplete (see TODO there)
- Some fields are not filled in
  DataReductionProxyChromeSettings::CreateDataFromNavigationHandle
- Maybe other things I missed

This required adding a couple fields to NavigationHandle and syncing
the configured DRP proxies to DRPSettings.

http://crbug.com/898326 tracks deprecating DataReductionProxyData
completely.

Bug:  721403 , 898326
Change-Id: I1210948f60a03ed7d632f1547311bbd3826323fb
Reviewed-on: https://chromium-review.googlesource.com/c/1297002
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602560}
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/plugins/pdf_iframe_navigation_throttle_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/previews/previews_ui_tab_helper_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/chrome/browser/safe_browsing/chrome_password_protection_service_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/common/data_reduction_proxy_config_values.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/browser/frame_host/navigation_handle_impl_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/browser/frame_host/origin_policy_throttle_unittest.cc
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/content/public/browser/navigation_handle.h
[modify] https://crrev.com/9c5a72cc522900a43030ea23d93edbdca06b72a1/services/network/public/cpp/resource_response.cc

Status: Available (was: Untriaged)
Owner: rajendrant@chromium.org
Status: Assigned (was: Available)
I will work on removing or migrating the remaining DataReductionProxyData fields.
Blockedon: 902063
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 5

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

commit 811e357c75a23d27b42fc5c1e3c7a8097c855939
Author: rajendrant <rajendrant@chromium.org>
Date: Mon Nov 05 23:18:55 2018

Migrate PLM metrics for data reduction proxy for network-servicification

Move data reduction proxy PLM observer to use OnResourceDataUpdate and use
compression ratio to compute the original content length. This CL also plumbs
URL to RenderFrameObserver::DidStartResponse to distinguish if its a secure
(https) scheme.

Bug: 898326
Change-Id: I82ba4fecc7d40b078d49c1b2c722a8ae87917e26
Reviewed-on: https://chromium-review.googlesource.com/c/1313410
Commit-Queue: rajendrant <rajendrant@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605511}
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_unittest.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/common/page_load_metrics/page_load_metrics.mojom
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/metrics_render_frame_observer.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/page_resource_data_use.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/page_resource_data_use.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/page_timing_metrics_sender.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/chrome/renderer/page_load_metrics/page_timing_metrics_sender.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/content/public/renderer/render_frame_observer.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/content/renderer/render_frame_impl.h
[modify] https://crrev.com/811e357c75a23d27b42fc5c1e3c7a8097c855939/tools/metrics/histograms/histograms.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 27

Labels: -Pri-2 -M-72 Pri-3
Owner: ryansturm@chromium.org
Refreshed during triage. Re-assigning to Ryan since rest of the metrics remaining in DataReductionProxyData are pingback/PLM specific.
Labels: -Pri-3 Pri-2
Labels: M-75
I'll deprecate the non network s13n code path after network s13n is on default for Android.

Sign in to add a comment