Clean up the DRP experiments code

Clean up the Data Reduction Proxy (DRP) server experiments code.
1. Compute the set of server experiments at a single place.
The logic for checking the field trial and switches lives
in params.cc.

2. Treat the set of server experiment finch param as an opaque string
that Chrome passes to the server in the request headers.

3. Add browsertests

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

chrome/browser/data_reduction_proxy/data_reduction_proxy_browsertest.cc

@@ -5,6 +5,7 @@
 #include <tuple>
 
 #include "base/bind.h"
+#include "base/metrics/field_trial_param_associator.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ -37,6 +38,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/proxy_config/proxy_config_dictionary.h"
 #include "components/proxy_config/proxy_config_pref_names.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/storage_partition.h"
@@ -210,10 +212,15 @@ class DataReductionProxyBrowsertestBase : public InProcessBrowserTest {
   // headers.
   void MonitorAndVerifyRequestsToProxyServer(
       const net::test_server::HttpRequest& request) {
+    ++count_proxy_server_requests_received_;
     // All requests to proxy server should have at least these headers.
     EXPECT_NE(request.headers.end(),
               request.headers.find(data_reduction_proxy::chrome_proxy_header()))
         << " url=" << request.GetURL() << " path=" << request.GetURL().path();
+
+    VerifyChromeProxyRequestHeader(
+        request.headers.at(data_reduction_proxy::chrome_proxy_header()));
+
     EXPECT_NE(
         request.headers.end(),
         request.headers.find(data_reduction_proxy::chrome_proxy_ect_header()))
@@ -223,6 +230,26 @@ class DataReductionProxyBrowsertestBase : public InProcessBrowserTest {
     monitored_urls_.push_back(request.GetURL());
   }
 
+  void VerifyChromeProxyRequestHeader(
+      const std::string& chrome_proxy_header_value) const {
+    bool exp_found = false;
+    for (const auto& attributes : base::SplitStringPiece(
+             chrome_proxy_header_value, ",", base::TRIM_WHITESPACE,
+             base::SPLIT_WANT_NONEMPTY)) {
+      if (base::StartsWith(attributes,
+                           "exp=", base::CompareCase::INSENSITIVE_ASCII)) {
+        const auto attribute_split = base::SplitStringPiece(
+            attributes, "=", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+        EXPECT_EQ(2u, attribute_split.size());
+        EXPECT_EQ(expect_exp_value_in_request_header_, attribute_split[1]);
+        exp_found = true;
+      }
+    }
+    EXPECT_EQ(!expect_exp_value_in_request_header_.empty(), exp_found)
+        << " expect_exp_value_in_request_header_="
+        << expect_exp_value_in_request_header_;
+  }
+
   // Returns true if a request for URL with path |url_path| was observed by
   // |this|. The method only compares the path instead of the full URL since the
   // hostname may be different due to the use of mock host in the browsertests
@@ -276,6 +303,10 @@ class DataReductionProxyBrowsertestBase : public InProcessBrowserTest {
 
   void WaitForConfig() { config_run_loop_->Run(); }
 
+  std::string expect_exp_value_in_request_header_;
+
+  size_t count_proxy_server_requests_received_ = 0u;
+
  private:
   std::unique_ptr<net::test_server::HttpResponse> GetConfigResponse(
       const net::test_server::HttpRequest& request) {
@@ -633,6 +664,37 @@ INSTANTIATE_TEST_SUITE_P(,
                          DataReductionProxyWithHoldbackBrowsertest,
                          ::testing::Values(false, true));
 
+class DataReductionProxyExpBrowsertest : public DataReductionProxyBrowsertest {
+ public:
+  void SetUp() override {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        data_reduction_proxy::switches::kDataReductionProxyExperiment,
+        "foo_experiment");
+
+    DataReductionProxyBrowsertest::SetUp();
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(DataReductionProxyExpBrowsertest,
+                       ChromeProxyExpHeaderSet) {
+  expect_exp_value_in_request_header_ = "foo_experiment";
+
+  net::EmbeddedTestServer proxy_server;
+  proxy_server.RegisterRequestMonitor(base::BindRepeating(
+      &DataReductionProxyBrowsertest::MonitorAndVerifyRequestsToProxyServer,
+      base::Unretained(this)));
+  proxy_server.RegisterRequestHandler(
+      base::BindRepeating(&BasicResponse, kPrimaryResponse));
+  ASSERT_TRUE(proxy_server.Start());
+  SetConfig(CreateConfigForServer(proxy_server));
+  // A network change forces the config to be fetched.
+  SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_3G);
+  WaitForConfig();
+
+  ui_test_utils::NavigateToURL(browser(), GURL("http://does.not.resolve/foo"));
+  EXPECT_LE(1u, count_proxy_server_requests_received_);
+}
+
 class DataReductionProxyBrowsertestWithNetworkService
     : public DataReductionProxyBrowsertest {
  public:
@@ -731,6 +793,12 @@ class DataReductionProxyFallbackBrowsertest
     location_header_ = header;
   }
 
+  void TearDown() override {
+    if (IsNetworkServiceEnabled()) {
+      EXPECT_LE(1u, count_proxy_server_requests_received_);
+    }
+  }
+
  private:
   std::unique_ptr<net::test_server::HttpResponse> AddChromeProxyHeader(
       const net::test_server::HttpRequest& request) {

chrome/browser/previews/previews_lite_page_navigation_throttle.cc

@@ -338,16 +338,8 @@ bool PreviewsLitePageNavigationThrottle::IsEligibleForPreview() const {
 GURL PreviewsLitePageNavigationThrottle::GetPreviewsURLForURL(
     const GURL& original_url) {
   DCHECK(original_url.is_valid());
-  std::string experiment_id = variations::GetVariationParamValue(
-      data_reduction_proxy::params::GetServerExperimentsFieldTrialName(),
-      data_reduction_proxy::kExperimentsOption);
-
-  // Allow the command line to override any variations-provided experiment.
-  std::string cmd_line_experiment =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          data_reduction_proxy::switches::kDataReductionProxyExperiment);
-  if (!cmd_line_experiment.empty())
-    experiment_id = cmd_line_experiment;
+  const std::string experiment_id =
+      data_reduction_proxy::params::GetDataSaverServerExperiments();
 
   std::string experiment_query;
   if (!experiment_id.empty()) {

chrome/browser/previews/previews_lite_page_navigation_throttle_unittest.cc

@@ -134,12 +134,16 @@ TEST(PreviewsLitePageNavigationThrottleTest, TestGetPreviewsURL) {
         {{"previews_host", test_case.previews_host}});
 
     base::FieldTrialList::CreateFieldTrial(
-        data_reduction_proxy::params::GetServerExperimentsFieldTrialName(),
+        data_reduction_proxy::params::
+            GetDataSaverServerExperimentsFieldTrialNameForTesting(),
         "enabled");
     std::map<std::string, std::string> server_experiment;
-    server_experiment["exp"] = test_case.experiment_variation;
+    server_experiment[data_reduction_proxy::params::
+                          GetDataSaverServerExperimentsOptionName()] =
+        test_case.experiment_variation;
     variations::AssociateVariationParams(
-        data_reduction_proxy::params::GetServerExperimentsFieldTrialName(),
+        data_reduction_proxy::params::
+            GetDataSaverServerExperimentsFieldTrialNameForTesting(),
         "enabled", server_experiment);
 
     EXPECT_EQ(PreviewsLitePageNavigationThrottle::GetPreviewsURLForURL(

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc

@@ -56,7 +56,6 @@ const char kSecureSessionHeaderOption[] = "s";
 const char kBuildNumberHeaderOption[] = "b";
 const char kPatchNumberHeaderOption[] = "p";
 const char kClientHeaderOption[] = "c";
-const char kExperimentsOption[] = "exp";
 const char kPageIdOption[] = "pid";
 
 // The empty version for the authentication protocol. Currently used by
@@ -85,6 +84,7 @@ DataReductionProxyRequestOptions::DataReductionProxyRequestOptions(
     const std::string& version,
     DataReductionProxyConfig* config)
     : client_(util::GetStringForClient(client)),
+      server_experiments_(params::GetDataSaverServerExperiments()),
       data_reduction_proxy_config_(config),
       current_page_id_(base::RandUint64()) {
   DCHECK(data_reduction_proxy_config_);
@@ -98,7 +98,7 @@ void DataReductionProxyRequestOptions::Init() {
   DCHECK(thread_checker_.CalledOnValidThread());
   key_ = GetDefaultKey(),
   UpdateCredentials();
-  UpdateExperiments();
+  RegenerateRequestHeaderValue();
   // Called on the UI thread, but should be checked on the IO thread.
   thread_checker_.DetachFromThread();
 }
@@ -108,38 +108,6 @@ std::string DataReductionProxyRequestOptions::GetHeaderValueForTesting() const {
   return header_value_;
 }
 
-void DataReductionProxyRequestOptions::UpdateExperiments() {
-  experiments_.clear();
-  std::string experiments =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          data_reduction_proxy::switches::kDataReductionProxyExperiment);
-
-  // The command line override takes precedence over field trial "exp"
-  // directives.
-  if (!experiments.empty()) {
-    base::StringTokenizer experiment_tokenizer(experiments, ", ");
-    experiment_tokenizer.set_quote_chars("\"");
-    while (experiment_tokenizer.GetNext()) {
-      if (!experiment_tokenizer.token().empty())
-        experiments_.push_back(experiment_tokenizer.token());
-    }
-  } else {
-    // If no other "exp" directive is forced by flags, add the field trial
-    // value.
-    AddServerExperimentFromFieldTrial();
-  }
-
-  RegenerateRequestHeaderValue();
-}
-
-void DataReductionProxyRequestOptions::AddServerExperimentFromFieldTrial() {
-  if (!params::IsIncludedInServerExperimentsFieldTrial())
-    return;
-  const std::string server_experiment = variations::GetVariationParamValue(
-      params::GetServerExperimentsFieldTrialName(), kExperimentsOption);
-  if (!server_experiment.empty())
-    experiments_.push_back(server_experiment);
-}
 
 void DataReductionProxyRequestOptions::AddPageIDRequestHeader(
     net::HttpRequestHeaders* request_headers,
@@ -248,8 +216,11 @@ void DataReductionProxyRequestOptions::RegenerateRequestHeaderValue() {
   DCHECK(!patch_.empty());
   headers.push_back(FormatOption(kPatchNumberHeaderOption, patch_));
 
-  for (const auto& experiment : experiments_)
-    headers.push_back(FormatOption(kExperimentsOption, experiment));
+  if (!server_experiments_.empty()) {
+    headers.push_back(
+        FormatOption(params::GetDataSaverServerExperimentsOptionName(),
+                     server_experiments_));
+  }
 
   header_value_ = base::JoinString(headers, ", ");
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.h

@@ -32,7 +32,6 @@ extern const char kSecureSessionHeaderOption[];
 extern const char kBuildNumberHeaderOption[];
 extern const char kPatchNumberHeaderOption[];
 extern const char kClientHeaderOption[];
-extern const char kExperimentsOption[];
 
 #if defined(OS_ANDROID)
 extern const char kAndroidWebViewProtocolVersion[];
@@ -126,13 +125,6 @@ class DataReductionProxyRequestOptions {
   // session info. crbug.com/709624
   void ResetPageId();
 
-  // Updates the value of the experiments to be run and regenerate the header if
-  // necessary.
-  void UpdateExperiments();
-
-  // Adds the server-side experiment from the field trial.
-  void AddServerExperimentFromFieldTrial();
-
   // Generates and updates the session ID and credentials.
   void UpdateCredentials();
 
@@ -147,11 +139,11 @@ class DataReductionProxyRequestOptions {
   std::string key_;
 
   // Name of the client and version of the data reduction proxy protocol to use.
-  std::string client_;
+  const std::string client_;
   std::string secure_session_;
   std::string build_;
   std::string patch_;
-  std::vector<std::string> experiments_;
+  const std::string server_experiments_;
 
   // Must outlive |this|.
   DataReductionProxyConfig* data_reduction_proxy_config_;

components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options_unittest.cc

@@ -90,7 +90,7 @@ void SetHeaderExpectations(const std::string& secure_session,
                            const std::string& build,
                            const std::string& patch,
                            const std::string& page_id,
-                           const std::vector<std::string> experiments,
+                           const std::string& server_experiments,
                            std::string* expected_header) {
   std::vector<std::string> expected_options;
   if (!secure_session.empty()) {
@@ -108,9 +108,10 @@ void SetHeaderExpectations(const std::string& secure_session,
   expected_options.push_back(std::string(kPatchNumberHeaderOption) + "=" +
                              patch);
 
-  for (const auto& experiment : experiments) {
+  if (!server_experiments.empty()) {
     expected_options.push_back(
-        std::string(kExperimentsOption) + "=" + experiment);
+        std::string(params::GetDataSaverServerExperimentsOptionName()) + "=" +
+        server_experiments);
   }
 
   EXPECT_FALSE(page_id.empty());
@@ -182,12 +183,12 @@ class DataReductionProxyRequestOptionsTest : public testing::Test {
 TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationOnIOThread) {
   std::string expected_header;
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId, std::vector<std::string>(),
+                        kExpectedPatch, kPageId, std::string(),
                         &expected_header);
 
   std::string expected_header2;
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId2, std::vector<std::string>(),
+                        kExpectedPatch, kPageId2, std::string(),
                         &expected_header2);
 
   CreateRequestOptions(kVersion);
@@ -203,7 +204,7 @@ TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationOnIOThread) {
 TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationIgnoresEmptyKey) {
   std::string expected_header;
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId, std::vector<std::string>(),
+                        kExpectedPatch, kPageId, std::string(),
                         &expected_header);
   CreateRequestOptions(kVersion);
   VerifyExpectedHeader(expected_header, kPageIdValue);
@@ -217,7 +218,7 @@ TEST_F(DataReductionProxyRequestOptionsTest, AuthorizationIgnoresEmptyKey) {
 TEST_F(DataReductionProxyRequestOptionsTest, SecureSession) {
   std::string expected_header;
   SetHeaderExpectations(kSecureSession, kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId, std::vector<std::string>(),
+                        kExpectedPatch, kPageId, std::string(),
                         &expected_header);
 
   CreateRequestOptions(kVersion);
@@ -228,7 +229,7 @@ TEST_F(DataReductionProxyRequestOptionsTest, SecureSession) {
 TEST_F(DataReductionProxyRequestOptionsTest, CallsHeaderCallback) {
   std::string expected_header;
   SetHeaderExpectations(kSecureSession, kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId, std::vector<std::string>(),
+                        kExpectedPatch, kPageId, std::string(),
                         &expected_header);
 
   CreateRequestOptionsWithCallback(kVersion);
@@ -247,13 +248,10 @@ TEST_F(DataReductionProxyRequestOptionsTest, CallsHeaderCallback) {
 TEST_F(DataReductionProxyRequestOptionsTest, ParseExperiments) {
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       data_reduction_proxy::switches::kDataReductionProxyExperiment,
-      "staging,\"foo,bar\"");
-  std::vector<std::string> expected_experiments;
-  expected_experiments.push_back("staging");
-  expected_experiments.push_back("\"foo,bar\"");
+      "staging,foo,bar");
   std::string expected_header;
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
-                        kExpectedPatch, kPageId, expected_experiments,
+                        kExpectedPatch, kPageId, "staging,foo,bar",
                         &expected_header);
 
   CreateRequestOptions(kVersion);
@@ -282,42 +280,50 @@ TEST_F(DataReductionProxyRequestOptionsTest, ParseExperimentsFromFieldTrial) {
       // Experiments from command line switch should override.
       {kFieldTrialGroupFoo, kExperimentBar, false, kExperimentBar},
       {kFieldTrialGroupBar, kExperimentFoo, false, kExperimentFoo},
-      {kFieldTrialGroupFoo, kExperimentBar, true, kExperimentBar},
-      {kFieldTrialGroupBar, kExperimentFoo, true, kExperimentFoo},
+      {kFieldTrialGroupFoo, kExperimentBar, false, kExperimentBar},
+      {kFieldTrialGroupBar, kExperimentFoo, false, kExperimentFoo},
+      {kFieldTrialGroupFoo, kExperimentBar, true, std::string()},
+      {kFieldTrialGroupBar, kExperimentFoo, true, std::string()},
   };
 
   std::map<std::string, std::string> server_experiment_foo,
       server_experiment_bar;
 
-  server_experiment_foo["exp"] = kExperimentFoo;
-  server_experiment_bar["exp"] = kExperimentBar;
+  server_experiment_foo[params::GetDataSaverServerExperimentsOptionName()] =
+      kExperimentFoo;
+  server_experiment_bar[params::GetDataSaverServerExperimentsOptionName()] =
+      kExperimentBar;
   ASSERT_TRUE(variations::AssociateVariationParams(
-      params::GetServerExperimentsFieldTrialName(), kFieldTrialGroupFoo,
-      server_experiment_foo));
+      params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+      kFieldTrialGroupFoo, server_experiment_foo));
   ASSERT_TRUE(variations::AssociateVariationParams(
-      params::GetServerExperimentsFieldTrialName(), kFieldTrialGroupBar,
-      server_experiment_bar));
+      params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+      kFieldTrialGroupBar, server_experiment_bar));
 
   for (const auto& test : tests) {
-    std::vector<std::string> expected_experiments;
+    std::string expected_experiments;
 
     base::CommandLine::ForCurrentProcess()->InitFromArgv(0, nullptr);
 
-    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-        data_reduction_proxy::switches::kDataReductionProxyExperiment,
-        test.command_line_experiment);
     if (test.disable_server_experiments_via_flag) {
       base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
           switches::kDataReductionProxyServerExperimentsDisabled, "");
+    } else {
+      base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+          data_reduction_proxy::switches::kDataReductionProxyExperiment,
+          test.command_line_experiment);
+      base::CommandLine::ForCurrentProcess()->RemoveSwitch(
+          switches::kDataReductionProxyServerExperimentsDisabled);
     }
 
     std::string expected_header;
     base::FieldTrialList field_trial_list(nullptr);
     base::FieldTrialList::CreateFieldTrial(
-        params::GetServerExperimentsFieldTrialName(), test.field_trial_group);
+        params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+        test.field_trial_group);
 
     if (!test.expected_experiment.empty())
-      expected_experiments.push_back(test.expected_experiment);
+      expected_experiments = test.expected_experiment;
 
     SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
                           kExpectedPatch, kPageId, expected_experiments,
@@ -335,16 +341,16 @@ TEST_F(DataReductionProxyRequestOptionsTest, TestExperimentPrecedence) {
 
   // Field trial has the lowest priority.
   std::map<std::string, std::string> server_experiment;
-  server_experiment["exp"] = "foo";
+  server_experiment[params::GetDataSaverServerExperimentsOptionName()] = "foo";
   ASSERT_TRUE(variations::AssociateVariationParams(
-      params::GetServerExperimentsFieldTrialName(), "enabled",
-      server_experiment));
+      params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+      "enabled", server_experiment));
 
   base::FieldTrialList field_trial_list(nullptr);
   base::FieldTrialList::CreateFieldTrial(
-      params::GetServerExperimentsFieldTrialName(), "enabled");
-  std::vector<std::string> expected_experiments;
-  expected_experiments.push_back("foo");
+      params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+      "enabled");
+  std::string expected_experiments = "foo";
   std::string expected_header;
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
                         kExpectedPatch, kPageId, expected_experiments,
@@ -355,8 +361,7 @@ TEST_F(DataReductionProxyRequestOptionsTest, TestExperimentPrecedence) {
   // Setting the experiment explicitly has the highest priority.
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       data_reduction_proxy::switches::kDataReductionProxyExperiment, "bar");
-  expected_experiments.clear();
-  expected_experiments.push_back("bar");
+  expected_experiments = "bar";
   SetHeaderExpectations(std::string(), kClientStr, kExpectedBuild,
                         kExpectedPatch, kPageId, expected_experiments,
                         &expected_header);

components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc

@@ -57,6 +57,8 @@ const char kServerExperimentsFieldTrial[] =
 // LitePage black list version.
 const char kLitePageBlackListVersion[] = "lite-page-blacklist-version";
 
+const char kExperimentsOption[] = "exp";
+
 bool IsIncludedInFieldTrial(const std::string& name) {
   return base::StartsWith(base::FieldTrialList::FindFullName(name), kEnabled,
                           base::CompareCase::SENSITIVE);
@@ -84,6 +86,13 @@ bool CanShowAndroidLowMemoryDevicePromo() {
   return false;
 }
 
+// Returns true if this client is part of the field trial that should enable
+// server experiments for the data reduction proxy.
+bool IsIncludedInServerExperimentsFieldTrial() {
+  return base::FieldTrialList::FindFullName(kServerExperimentsFieldTrial)
+             .find(kDisabled) != 0;
+}
+
 }  // namespace
 
 namespace data_reduction_proxy {
@@ -124,13 +133,6 @@ const char* GetLoFiFlagFieldTrialName() {
   return kLoFiFlagFieldTrial;
 }
 
-bool IsIncludedInServerExperimentsFieldTrial() {
-  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
-             data_reduction_proxy::switches::
-                 kDataReductionProxyServerExperimentsDisabled) &&
-         base::FieldTrialList::FindFullName(kServerExperimentsFieldTrial)
-                 .find(kDisabled) != 0;
-}
 
 bool FetchWarmupProbeURLEnabled() {
   return !base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -339,7 +341,37 @@ bool GetOverrideProxiesForHttpFromCommandLine(
   return true;
 }
 
-const char* GetServerExperimentsFieldTrialName() {
+std::string GetDataSaverServerExperimentsOptionName() {
+  return kExperimentsOption;
+}
+
+std::string GetDataSaverServerExperiments() {
+  const std::string cmd_line_experiment =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          data_reduction_proxy::switches::kDataReductionProxyExperiment);
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          data_reduction_proxy::switches::
+              kDataReductionProxyServerExperimentsDisabled)) {
+    // Both kDataReductionProxyExperiment and
+    // kDataReductionProxyServerExperimentsDisabled switches can't be set at the
+    // same time.
+    DCHECK(cmd_line_experiment.empty());
+    return std::string();
+  }
+
+  // Experiment set using command line overrides field trial.
+  if (!cmd_line_experiment.empty())
+    return cmd_line_experiment;
+
+  // Next, check if the experiment is set using the field trial.
+  if (!IsIncludedInServerExperimentsFieldTrial())
+    return std::string();
+  return variations::GetVariationParamValue(kServerExperimentsFieldTrial,
+                                            kExperimentsOption);
+}
+
+const char* GetDataSaverServerExperimentsFieldTrialNameForTesting() {
   return kServerExperimentsFieldTrial;
 }
 

components/data_reduction_proxy/core/common/data_reduction_proxy_params.h

@@ -57,10 +57,6 @@ const char* GetLoFiFieldTrialName();
 // TODO(ryansturm): crbug.com/759052 Cleanup once fully cutover to new blacklist
 const char* GetLoFiFlagFieldTrialName();
 
-// Returns true if this client is part of the field trial that should enable
-// server experiments for the data reduction proxy.
-bool IsIncludedInServerExperimentsFieldTrial();
-
 // Returns true if this client has the command line switch to enable forced
 // pageload metrics pingbacks on every page load.
 bool IsForcePingbackEnabledViaFlags();
@@ -104,8 +100,18 @@ int GetFieldTrialParameterAsInteger(const std::string& group,
 bool GetOverrideProxiesForHttpFromCommandLine(
     std::vector<DataReductionProxyServer>* override_proxies_for_http);
 
+// Returns the server experiments option name. This name is used in the request
+// headers to the data saver proxy. This name is also used to set the experiment
+// name using finch trial.
+std::string GetDataSaverServerExperimentsOptionName();
+
+// Returns the server experiment. This name is used in the request
+// headers to the data saver proxy. Returned value may be empty indicating no
+// experiment is enabled.
+std::string GetDataSaverServerExperiments();
+
 // Returns the name of the server side experiment field trial.
-const char* GetServerExperimentsFieldTrialName();
+const char* GetDataSaverServerExperimentsFieldTrialNameForTesting();
 
 // Returns the URL to check to decide if the secure proxy origin should be
 // used.

components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

@@ -13,6 +13,8 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_param_associator.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/optional.h"
 #include "base/test/scoped_feature_list.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_features.h"
@@ -112,36 +114,66 @@ TEST_F(DataReductionProxyParamsTest, AreServerExperimentsEnabled) {
     bool expected;
   } tests[] = {
       {
-          "Field trial not set", "", false, true,
+          "Field trial not set",
+          "Enabled_42",
+          false,
+          true,
       },
       {
-          "Field trial not set, flag set", "", true, false,
+          "Field trial not set, flag set",
+          "",
+          true,
+          false,
       },
       {
-          "Enabled", "Enabled", false, true,
+          "Enabled",
+          "Enabled",
+          false,
+          true,
       },
       {
-          "Enabled via field trial but disabled via flag", "Enabled", true,
+          "Enabled via field trial but disabled via flag",
+          "Enabled",
+          true,
           false,
       },
       {
-          "Disabled via field trial", "Disabled", false, false,
+          "Disabled via field trial",
+          "Disabled",
+          false,
+          false,
       },
   };
 
   for (const auto& test : tests) {
+    base::FieldTrialParamAssociator::GetInstance()->ClearAllParamsForTesting();
+
     base::FieldTrialList field_trial_list(nullptr);
-    if (!test.trial_group_value.empty()) {
-      ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial(
-          "DataReductionProxyServerExperiments", test.trial_group_value));
+    std::map<std::string, std::string> variation_params;
+    std::string exp_name;
+
+    if (test.trial_group_value != "Disabled") {
+      exp_name = "foobar";
+      variation_params[params::GetDataSaverServerExperimentsOptionName()] =
+          exp_name;
+      ASSERT_TRUE(base::AssociateFieldTrialParams(
+          params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+          test.trial_group_value, variation_params));
+      base::FieldTrialList::CreateFieldTrial(
+          params::GetDataSaverServerExperimentsFieldTrialNameForTesting(),
+          test.trial_group_value);
     }
 
     base::CommandLine::ForCurrentProcess()->InitFromArgv(0, nullptr);
     if (test.disable_flag_set) {
+      exp_name = "";
       base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
           switches::kDataReductionProxyServerExperimentsDisabled, "");
+    } else {
+      base::CommandLine::ForCurrentProcess()->RemoveSwitch(
+          switches::kDataReductionProxyServerExperimentsDisabled);
     }
-    EXPECT_EQ(test.expected, params::IsIncludedInServerExperimentsFieldTrial())
+    EXPECT_EQ(exp_name, params::GetDataSaverServerExperiments())
         << test.test_case;
   }
 }