New issue
Advanced search Search tips

Issue 597768 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 599302
issue 675742
issue 724242



Sign in to add a comment

Make Data reduction proxy use client config by default

Project Member Reported by tbansal@chromium.org, Mar 24 2016

Issue description

This includes removing the unused code.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 28 2016

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

commit 849951145321fe4f254e87496a855631c6e777e4
Author: tbansal <tbansal@chromium.org>
Date: Mon Mar 28 18:38:28 2016

Enable DRP config service by default

We have been running this experiment at 50% stable, and are
now ready to move to 100%.

This CL does not remove the code that will now be deprecated
because of DRP config being enabled 100% of time. We should
remove that code at some point in future.

BUG= 597768 

Review URL: https://codereview.chromium.org/1830343002

Cr-Commit-Position: refs/heads/master@{#383527}

[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/chrome/browser/about_flags.cc
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_android.json
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_chromeos.json
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_ios.json
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_linux.json
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_mac.json
[modify] https://crrev.com/849951145321fe4f254e87496a855631c6e777e4/testing/variations/fieldtrial_testing_config_win.json

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 28 2016

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

commit 270c4efdb844a9aaf671c0e618da9d5c6843980c
Author: tbansal <tbansal@chromium.org>
Date: Mon Mar 28 21:28:57 2016

Revert of Enable DRP config service by default (patchset #3 id:80001 of https://codereview.chromium.org/1830343002/ )

Reason for revert:
Reverting because this broke Cronet tests:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Data%20Reduction%20Proxy%20Builder/builds/1601

Original issue's description:
> Enable DRP config service by default
>
> We have been running this experiment at 50% stable, and are
> now ready to move to 100%.
>
> This CL does not remove the code that will now be deprecated
> because of DRP config being enabled 100% of time. We should
> remove that code at some point in future.
>
> BUG= 597768 
>
> Committed: https://crrev.com/849951145321fe4f254e87496a855631c6e777e4
> Cr-Commit-Position: refs/heads/master@{#383527}

TBR=sclittle@chromium.org,holte@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 597768 

Review URL: https://codereview.chromium.org/1839783002

Cr-Commit-Position: refs/heads/master@{#383567}

[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/chrome/browser/about_flags.cc
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_android.json
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_chromeos.json
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_ios.json
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_linux.json
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_mac.json
[modify] https://crrev.com/270c4efdb844a9aaf671c0e618da9d5c6843980c/testing/variations/fieldtrial_testing_config_win.json

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2016

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

commit d6dc2f61bde358880132c898dc99dd995d9b7103
Author: tbansal <tbansal@chromium.org>
Date: Tue Mar 29 01:30:01 2016

Override DRP proxies from command line switches

If "spdy-proxy-dev-auth-origin" or
"spdy-proxy-auth-fallback" switch is specified on the
command line, then DRP proxies are overriden even if
Chromium is using client config service.

This change is required to preserve the functionality of
these switches after Chromium permanently moves to
the client config service.

BUG= 597768 

Review URL: https://codereview.chromium.org/1834373002

Cr-Commit-Position: refs/heads/master@{#383648}

[modify] https://crrev.com/d6dc2f61bde358880132c898dc99dd995d9b7103/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values_unittest.cc
[modify] https://crrev.com/d6dc2f61bde358880132c898dc99dd995d9b7103/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/d6dc2f61bde358880132c898dc99dd995d9b7103/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 31 2016

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

commit 57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a
Author: tbansal <tbansal@chromium.org>
Date: Thu Mar 31 17:33:13 2016

Enable DRP config service by default

We have been running this experiment at 50% stable, and are
now ready to move to 100%.

This CL does not remove the code that will now be deprecated
because of DRP config being enabled 100% of time. We should
remove that code at some point in future.

This is reland of
https://codereview.chromium.org/1830343002/ which had to be
reverted because of failing tests.

BUG= 597768 
TBR=holte@chromium.org

Review URL: https://codereview.chromium.org/1845773002

Cr-Commit-Position: refs/heads/master@{#384319}

[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/chrome/browser/about_flags.cc
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_android.json
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_chromeos.json
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_ios.json
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_linux.json
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_mac.json
[modify] https://crrev.com/57d8bc8d65f8bc73ce8c74e7b092c19002ca5c3a/testing/variations/fieldtrial_testing_config_win.json

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 2 2016

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

commit 50cb614060239a1850c03dd88da4b5f8ae341dc2
Author: tbansal <tbansal@chromium.org>
Date: Sat Apr 02 00:17:16 2016

Add kill switch for DRP config service

Config client is enabled by default. It can be disabled only
if Chromium belongs to a field trial group whose name starts
with "Disabled".

BUG= 597768 

Review URL: https://codereview.chromium.org/1849293002

Cr-Commit-Position: refs/heads/master@{#384751}

[modify] https://crrev.com/50cb614060239a1850c03dd88da4b5f8ae341dc2/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/50cb614060239a1850c03dd88da4b5f8ae341dc2/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

Blockedon: 599302
Owner: ----
Status: (was: Started)
Unassigning the bug so somebody else can take it.
Note that the client config races with the initial page load. If the client config comes back first, we use the DRP for the initial request, otherwise we don't.

For testing, it is useful to have some way to ensure that the first request goes through the DRP. "Testing" includes local tests and also remote tests runs on WebPageTest.

Comment 9 by bengr@chromium.org, Jun 23 2016

Status: Available

Comment 10 by bengr@chromium.org, Oct 28 2016

Status: Started (was: Available)
Is this fixed?
Fixed for Chrome. Not for Cronet.
Blockedon: 675742
Labels: CleanUp
Status: Available (was: Started)
Owner: tbansal@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 10 2017

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

commit aaed3d6688dd2599809a4ddb50e8d2414f5e96ef
Author: tbansal <tbansal@chromium.org>
Date: Mon Jul 10 21:52:48 2017

DRP: Make secure proxy check URL a DRP param

Make secure proxy check URL as a data reduction proxy param instead of
a config value. This further cuts down on the functionality provided
by data_reduction_proxy_config_values class.

Also, remove unused flags for carrier testing, and for setting warmup url.

BUG= 597768 

Review-Url: https://codereview.chromium.org/2967993002
Cr-Commit-Position: refs/heads/master@{#485412}

[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/chrome/browser/about_flags.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_config_values.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
[modify] https://crrev.com/aaed3d6688dd2599809a4ddb50e8d2414f5e96ef/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h

Comment 18 by bengr@chromium.org, Jul 11 2017

Status: Started (was: Assigned)
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 12 2017

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

commit 981b49ad53b91e640ca9e83751e73f9c60dcf782
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Jul 12 01:49:42 2017

DRP: Remove default and fallback origin functions

With client config, the number of data reduction proxies (DRP) could
vary from 1 to N. This CL removes the deprecated code from DRP
params which assumes that there are exactly 2 proxies.

Bug:  597768 
Change-Id: I8e23d681b6c84c22cc3f4e5f54e754353398c6f4
Reviewed-on: https://chromium-review.googlesource.com/566672
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485784}
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/content/browser/content_lofi_decider_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.cc
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h
[modify] https://crrev.com/981b49ad53b91e640ca9e83751e73f9c60dcf782/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 24 2017

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

commit 4669827c31a91df88ca6a2f0377fe544d05bd32f
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Jul 24 22:57:36 2017

DataReductionProxy (DRP): Cleanup DRP params.

Remove a couple more methods from DRP params.

Bug:  597768 
Change-Id: Icd8eec05643373b3a19dab0c76ab481101ac7969
Reviewed-on: https://chromium-review.googlesource.com/580378
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Megan Jablonski <megjablon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489118}
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values.h
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_mutable_config_values_unittest.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/4669827c31a91df88ca6a2f0377fe544d05bd32f/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h

Cc: tbansal@chromium.org
Owner: ----
Status: Available (was: Started)
What's left to do for this issue?
Data reduction proxy already uses client config by default. However, this bug also covers the work for removing the code for configuring DRP using hard-coded proxies through the old params framework.

However, a large percentage of the DRP unittests use the hard-coded proxies. We just need a way to expose some testing framework from the client config class that allows writing unit tests quickly while also removing the old params framework.
Blockedon: 724242
Refreshed during triage.
Cc: -tbansal@chromium.org
Owner: tbansal@chromium.org
Status: Assigned (was: Available)
Tarun, would you please file a bug for the remaining work and close this bug?
The blocking bugs track the work that's still left. This bug tracks the work of removing the deprecated code (non-client config code).
Refreshed during triage.

Comment 28 by bengr@chromium.org, Jun 22 2018

Status: Fixed (was: Assigned)

Sign in to add a comment