Promote DRP ClientConfig fetching to its own feature and enable

With the pending rollouts of some features that use the client config,
we should enable everybody to fetch it by default, but leave a kill
switch.

Bug: 1048736
Change-Id: I6f88289173425551c10a0fa44dcc144570a2618b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107561Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751078}

chrome/browser/data_reduction_proxy/data_reduction_proxy_browsertest.cc

@@ -110,7 +110,6 @@ ClientConfig CreateEmptyConfig() {
   return CreateEmptyProxyConfig(kSessionKey, 1000, 0, 0.5f, false);
 }
 
-
 class TestSettingsObserver : public DataReductionProxySettingsObserver {
  public:
   TestSettingsObserver() = default;
@@ -533,7 +532,6 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
   EXPECT_EQ(result, kDummyBody);
 }
 
-
 IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
                        ProxyNotUsedWhenDisabled) {
   net::EmbeddedTestServer test_server;
@@ -627,8 +625,6 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
   EXPECT_EQ(GetBody(), kDummyBody);
 }
 
-
-
 // Test that enabling the holdback disables the proxy and that the client config
 // is fetched when lite page redirect preview is enabled.
 class DataReductionProxyWithHoldbackBrowsertest
@@ -636,39 +632,36 @@ class DataReductionProxyWithHoldbackBrowsertest
       public DataReductionProxyBrowsertest {
  public:
   DataReductionProxyWithHoldbackBrowsertest()
-      : DataReductionProxyBrowsertest(),
-        // Consider the holdback as enabled if holdback is enabled or the
-        // |force_enable_config_service_fetches_| is enabled.
+      :  // Consider the holdback as enabled if holdback is enabled or the
+         // |enable_config_service_fetches_| is enabled.
         data_reduction_proxy_holdback_enabled_(std::get<2>(GetParam()) ||
                                                std::get<0>(GetParam())),
         lite_page_redirect_previews_enabled_(std::get<1>(GetParam())),
-        force_enable_config_service_fetches_(std::get<2>(GetParam())) {}
+        enable_config_service_fetches_(std::get<2>(GetParam())) {}
 
   void SetUp() override {
-    if (force_enable_config_service_fetches_) {
-      scoped_feature_list_.InitAndEnableFeatureWithParameters(
-          data_reduction_proxy::features::kDataReductionProxyHoldback,
-          {{"force_enable_config_service_fetches", "true"}});
-    }
-    if (!force_enable_config_service_fetches_ &&
-        data_reduction_proxy_holdback_enabled_) {
-      scoped_feature_list_.InitAndEnableFeature(
-          data_reduction_proxy::features::kDataReductionProxyHoldback);
-    }
-    if (lite_page_redirect_previews_enabled_) {
-      previews_lite_page_redirect_feature_list_.InitAndEnableFeature(
-          previews::features::kLitePageServerPreviews);
-    }
+    data_reduction_proxy_holdback_feature_list_.InitWithFeatureState(
+        data_reduction_proxy::features::kDataReductionProxyHoldback,
+        data_reduction_proxy_holdback_enabled_);
+
+    fetch_client_config_feature_list_.InitWithFeatureState(
+        data_reduction_proxy::features::kFetchClientConfig,
+        enable_config_service_fetches_);
+
+    previews_lite_page_redirect_feature_list_.InitWithFeatureState(
+        previews::features::kLitePageServerPreviews,
+        lite_page_redirect_previews_enabled_);
 
     InProcessBrowserTest::SetUp();
   }
 
   const bool data_reduction_proxy_holdback_enabled_;
   const bool lite_page_redirect_previews_enabled_;
-  const bool force_enable_config_service_fetches_;
+  const bool enable_config_service_fetches_;
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
+  base::test::ScopedFeatureList data_reduction_proxy_holdback_feature_list_;
+  base::test::ScopedFeatureList fetch_client_config_feature_list_;
   base::test::ScopedFeatureList previews_lite_page_redirect_feature_list_;
 };
 
@@ -717,7 +710,6 @@ class DataReductionProxyExpBrowsertest : public DataReductionProxyBrowsertest {
   }
 };
 
-
 class DataReductionProxyExpFeatureBrowsertest
     : public DataReductionProxyBrowsertest {
  public:
@@ -741,7 +733,6 @@ class DataReductionProxyExpFeatureBrowsertest
   const std::string experiment_name = "foo_feature_experiment";
 };
 
-
 // Threadsafe log for recording a sequence of events as newline separated text.
 class EventLog {
  public:
@@ -859,7 +850,6 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
     EXPECT_FALSE(observer.last_initiator_origin().has_value());
   }
 
-
   // Simulate clicking on a same-site link.
   {
     content::TestNavigationObserver observer(
@@ -883,5 +873,4 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyBrowsertest,
   }
 }
 
-
 }  // namespace data_reduction_proxy

chrome/browser/prerender/isolated/isolated_prerender_browsertest.cc

@@ -131,11 +131,11 @@ class IsolatedPrerenderBrowserTest
   }
 
   void SetUp() override {
-    scoped_feature_list_.InitAndEnableFeature(features::kIsolatePrerenders);
-
-    holdback_scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        data_reduction_proxy::features::kDataReductionProxyHoldback,
-        {{"force_enable_config_service_fetches", "true"}});
+    scoped_feature_list_.InitWithFeatures(
+        {features::kIsolatePrerenders,
+         data_reduction_proxy::features::kDataReductionProxyHoldback,
+         data_reduction_proxy::features::kFetchClientConfig},
+        {});
 
     InProcessBrowserTest::SetUp();
   }
@@ -276,7 +276,6 @@ class IsolatedPrerenderBrowserTest
   }
 
   base::test::ScopedFeatureList scoped_feature_list_;
-  base::test::ScopedFeatureList holdback_scoped_feature_list_;
   std::unique_ptr<net::EmbeddedTestServer> origin_server_;
   std::unique_ptr<net::EmbeddedTestServer> config_server_;
   size_t origin_server_request_with_cookies_ = 0;

components/data_reduction_proxy/core/common/data_reduction_proxy_features.cc

@@ -9,7 +9,6 @@
 namespace data_reduction_proxy {
 namespace features {
 
-
 // Enables the data saver promo for low memory Android devices.
 const base::Feature kDataReductionProxyLowMemoryDevicePromo{
     "DataReductionProxyLowMemoryDevicePromo",
@@ -27,9 +26,8 @@ const base::Feature kDataReductionProxyHoldback{
 
 // Enables data reduction proxy when network service is enabled.
 const base::Feature kDataReductionProxyEnabledWithNetworkService{
-  "DataReductionProxyEnabledWithNetworkService",
-      base::FEATURE_ENABLED_BY_DEFAULT
-};
+    "DataReductionProxyEnabledWithNetworkService",
+    base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Enables block action of all proxies when 502 is received with no
 // Chrome-Proxy header. The block duration is configurable via field trial with
@@ -67,5 +65,9 @@ const base::Feature kDataReductionProxyAggressiveConfigFetch{
 const base::Feature kReportSaveDataSavings{"ReportSaveDataSavings",
                                            base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables fetching the Client Config for server-based Lite Mode features.
+const base::Feature kFetchClientConfig{"DataReductionProxyFetchClientConfig",
+                                       base::FEATURE_ENABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/common/data_reduction_proxy_features.h

@@ -21,6 +21,7 @@ extern const base::Feature kDataReductionProxyDisableProxyFailedWarmup;
 extern const base::Feature kDataReductionProxyServerExperiments;
 extern const base::Feature kDataReductionProxyAggressiveConfigFetch;
 extern const base::Feature kReportSaveDataSavings;
+extern const base::Feature kFetchClientConfig;
 
 }  // namespace features
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc

@@ -91,16 +91,10 @@ bool IsIncludedInHoldbackFieldTrial() {
 }
 
 bool ForceEnableClientConfigServiceForAllDataSaverUsers() {
-  // Client config is enabled for all data users that are not in the
-  // kDataReductionProxyHoldback. Users that have kDataReductionProxyHoldback
-  // enabled have config service client enabled only if
-  // |force_enable_config_service_fetches| is set to true.
-  return GetFieldTrialParamByFeatureAsBool(
-      data_reduction_proxy::features::kDataReductionProxyHoldback,
-      "force_enable_config_service_fetches", false);
+  return base::FeatureList::IsEnabled(
+      data_reduction_proxy::features::kFetchClientConfig);
 }
 
-
 bool IsForcePingbackEnabledViaFlags() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       data_reduction_proxy::switches::kEnableDataReductionProxyForcePingback);
@@ -288,8 +282,6 @@ std::string GetDataSaverServerExperiments() {
       features::kDataReductionProxyServerExperiments, kExperimentsOption);
 }
 
-
-
 bool IsEnabledWithNetworkService() {
   return base::FeatureList::IsEnabled(
       data_reduction_proxy::features::