Introduce --force-disable-variation-ids parameter

This CL introduces a --force-disable-variation-ids parameter to prevent
variation ids and trigger variation ids from being sent via the X-Client-Data
request header for debugging purposes.

Bug: 1027576
Change-Id: Iacaaee31b1a62e50cc9b61a709cca5651038ddf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930958
Commit-Queue: Dominic Battré <battre@chromium.org>
Auto-Submit: Dominic Battré <battre@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718687}

components/variations/service/variations_field_trial_creator.cc

@@ -511,6 +511,13 @@ bool VariationsFieldTrialCreator::SetupFieldTrials(
       break;
   }
 
+  bool success = http_header_provider->ForceDisableVariationIds(
+      command_line->GetSwitchValueASCII(switches::kForceDisableVariationIds));
+  if (!success) {
+    ExitWithMessage(base::StringPrintf("Invalid --%s list specified.",
+                                       switches::kForceDisableVariationIds));
+  }
+
   feature_list->InitializeFromCommandLine(
       command_line->GetSwitchValueASCII(kEnableFeatures),
       command_line->GetSwitchValueASCII(kDisableFeatures));

components/variations/variations_http_header_provider.cc

@@ -110,19 +110,23 @@ VariationsHttpHeaderProvider::ForceVariationIds(
     const std::string& command_line_variation_ids) {
   default_variation_ids_set_.clear();
 
-  if (!AddDefaultVariationIds(variation_ids))
+  if (!AddVariationIdsToSet(variation_ids, &default_variation_ids_set_))
     return ForceIdsResult::INVALID_VECTOR_ENTRY;
 
-  if (!command_line_variation_ids.empty()) {
-    std::vector<std::string> variation_ids_from_command_line =
-        base::SplitString(command_line_variation_ids, ",",
-                          base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-    if (!AddDefaultVariationIds(variation_ids_from_command_line))
-      return ForceIdsResult::INVALID_SWITCH_ENTRY;
+  if (!ParseVariationIdsParameter(command_line_variation_ids,
+                                  &default_variation_ids_set_)) {
+    return ForceIdsResult::INVALID_SWITCH_ENTRY;
   }
   return ForceIdsResult::SUCCESS;
 }
 
+bool VariationsHttpHeaderProvider::ForceDisableVariationIds(
+    const std::string& command_line_variation_ids) {
+  force_disabled_ids_set_.clear();
+  return ParseVariationIdsParameter(command_line_variation_ids,
+                                    &force_disabled_ids_set_);
+}
+
 void VariationsHttpHeaderProvider::AddObserver(Observer* observer) {
   observer_list_.AddObserver(observer);
 }
@@ -141,6 +145,7 @@ void VariationsHttpHeaderProvider::ResetForTesting() {
   variation_ids_set_.clear();
   default_variation_ids_set_.clear();
   synthetic_variation_ids_set_.clear();
+  force_disabled_ids_set_.clear();
   cached_variation_ids_header_.clear();
   cached_variation_ids_header_signed_in_.clear();
 }
@@ -287,11 +292,13 @@ std::string VariationsHttpHeaderProvider::GenerateBase64EncodedProto(
   return hashed;
 }
 
-bool VariationsHttpHeaderProvider::AddDefaultVariationIds(
-    const std::vector<std::string>& variation_ids) {
+// static
+bool VariationsHttpHeaderProvider::AddVariationIdsToSet(
+    const std::vector<std::string>& variation_ids,
+    std::set<VariationIDEntry>* target_set) {
   for (const std::string& entry : variation_ids) {
     if (entry.empty()) {
-      default_variation_ids_set_.clear();
+      target_set->clear();
       return false;
     }
     bool trigger_id =
@@ -301,16 +308,29 @@ bool VariationsHttpHeaderProvider::AddDefaultVariationIds(
 
     int variation_id = 0;
     if (!base::StringToInt(trimmed_entry, &variation_id)) {
-      default_variation_ids_set_.clear();
+      target_set->clear();
       return false;
     }
-    default_variation_ids_set_.insert(VariationIDEntry(
+    target_set->insert(VariationIDEntry(
         variation_id,
         trigger_id ? GOOGLE_WEB_PROPERTIES_TRIGGER : GOOGLE_WEB_PROPERTIES));
   }
   return true;
 }
 
+// static
+bool VariationsHttpHeaderProvider::ParseVariationIdsParameter(
+    const std::string& command_line_variation_ids,
+    std::set<VariationIDEntry>* target_set) {
+  if (command_line_variation_ids.empty())
+    return true;
+
+  std::vector<std::string> variation_ids_from_command_line =
+      base::SplitString(command_line_variation_ids, ",", base::TRIM_WHITESPACE,
+                        base::SPLIT_WANT_ALL);
+  return AddVariationIdsToSet(variation_ids_from_command_line, target_set);
+}
+
 std::set<VariationsHttpHeaderProvider::VariationIDEntry>
 VariationsHttpHeaderProvider::GetAllVariationIds() {
   lock_.AssertAcquired();
@@ -322,6 +342,9 @@ VariationsHttpHeaderProvider::GetAllVariationIds() {
   for (const VariationIDEntry& entry : synthetic_variation_ids_set_) {
     all_variation_ids_set.insert(entry);
   }
+  for (const VariationIDEntry& entry : force_disabled_ids_set_) {
+    all_variation_ids_set.erase(entry);
+  }
   return all_variation_ids_set;
 }
 

components/variations/variations_http_header_provider.h

@@ -77,6 +77,13 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
       const std::vector<std::string>& variation_ids,
       const std::string& command_line_variation_ids);
 
+  // Ensures that the given variation ids and trigger variation ids are not
+  // encoded in the X-Client-Data request header. This is intended for
+  // development use to force that a server side experiment id is not set.
+  // |command_line_variation_ids| are comma-separted experiment ids.
+  // Returns true on success.
+  bool ForceDisableVariationIds(const std::string& command_line_variation_ids);
+
   // Methods to register or remove observers of variation ids header update.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -95,6 +102,10 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
                            ForceVariationIds_ValidCommandLine);
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
                            ForceVariationIds_Invalid);
+  FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
+                           ForceDisableVariationIds_ValidCommandLine);
+  FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
+                           ForceDisableVariationIds_Invalid);
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
                            OnFieldTrialGroupFinalized);
   FRIEND_TEST_ALL_PREFIXES(VariationsHttpHeaderProviderTest,
@@ -135,9 +146,16 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
   // |is_signed_in| state.
   std::string GenerateBase64EncodedProto(bool is_signed_in);
 
-  // Adds *additional* variation ids and trigger variation ids to be encoded in
-  // the X-Client-Data request header.
-  bool AddDefaultVariationIds(const std::vector<std::string>& variation_ids);
+  // Adds variation ids and trigger variation ids to |target_set|.
+  static bool AddVariationIdsToSet(
+      const std::vector<std::string>& variation_ids,
+      std::set<VariationIDEntry>* target_set);
+
+  // Parses a comma-separated string of variation ids and trigger variation ids
+  // and adds them to |target_set|.
+  static bool ParseVariationIdsParameter(
+      const std::string& command_line_variation_ids,
+      std::set<VariationIDEntry>* target_set);
 
   // Returns the currently active set of variation ids, which includes any
   // default values, synthetic variations and actual field trial variations.
@@ -159,6 +177,9 @@ class VariationsHttpHeaderProvider : public base::FieldTrialList::Observer,
   // Variations ids from synthetic field trials.
   std::set<VariationIDEntry> synthetic_variation_ids_set_;
 
+  // Provides the google experiment ids that are force-disabled by command line.
+  std::set<VariationIDEntry> force_disabled_ids_set_;
+
   std::string cached_variation_ids_header_;
   std::string cached_variation_ids_header_signed_in_;
 

components/variations/variations_http_header_provider_unittest.cc

@@ -125,6 +125,42 @@ TEST_F(VariationsHttpHeaderProviderTest, ForceVariationIds_Invalid) {
   EXPECT_TRUE(provider.GetClientDataHeader(false).empty());
 }
 
+TEST_F(VariationsHttpHeaderProviderTest,
+       ForceDisableVariationIds_ValidCommandLine) {
+  base::test::SingleThreadTaskEnvironment task_environment;
+  VariationsHttpHeaderProvider provider;
+
+  // Valid experiment ids.
+  EXPECT_EQ(VariationsHttpHeaderProvider::ForceIdsResult::SUCCESS,
+            provider.ForceVariationIds({"1", "2", "t3", "t4"}, "5,6,t7,t8"));
+  EXPECT_TRUE(provider.ForceDisableVariationIds("2,t4,6,t8"));
+  provider.InitVariationIDsCacheIfNeeded();
+  std::string variations = provider.GetClientDataHeader(false);
+  EXPECT_FALSE(variations.empty());
+  std::set<VariationID> variation_ids;
+  std::set<VariationID> trigger_ids;
+  ASSERT_TRUE(ExtractVariationIds(variations, &variation_ids, &trigger_ids));
+  EXPECT_TRUE(variation_ids.find(1) != variation_ids.end());
+  EXPECT_FALSE(variation_ids.find(2) != variation_ids.end());
+  EXPECT_TRUE(trigger_ids.find(3) != trigger_ids.end());
+  EXPECT_FALSE(trigger_ids.find(4) != trigger_ids.end());
+  EXPECT_TRUE(variation_ids.find(5) != variation_ids.end());
+  EXPECT_FALSE(variation_ids.find(6) != variation_ids.end());
+  EXPECT_TRUE(trigger_ids.find(7) != trigger_ids.end());
+  EXPECT_FALSE(trigger_ids.find(8) != trigger_ids.end());
+}
+
+TEST_F(VariationsHttpHeaderProviderTest, ForceDisableVariationIds_Invalid) {
+  base::test::SingleThreadTaskEnvironment task_environment;
+  VariationsHttpHeaderProvider provider;
+
+  // Invalid command-line ids.
+  EXPECT_FALSE(provider.ForceDisableVariationIds("abc"));
+  EXPECT_FALSE(provider.ForceDisableVariationIds("tabc456"));
+  provider.InitVariationIDsCacheIfNeeded();
+  EXPECT_TRUE(provider.GetClientDataHeader(false).empty());
+}
+
 TEST_F(VariationsHttpHeaderProviderTest, OnFieldTrialGroupFinalized) {
   base::test::SingleThreadTaskEnvironment task_environment;
   VariationsHttpHeaderProvider provider;

components/variations/variations_switches.cc

@@ -34,6 +34,11 @@ const char kForceFieldTrialParams[] = "force-fieldtrial-params";
 // prefixed with the character "t" will be treated as Trigger Variation Ids.
 const char kForceVariationIds[] = "force-variation-ids";
 
+// Forces to remove Chrome Variation Ids from being sent in X-Client-Data
+// header, specified as a 64-bit encoded list of numeric experiment ids. Ids
+// prefixed with the character "t" will be treated as Trigger Variation Ids.
+const char kForceDisableVariationIds[] = "force-disable-variation-ids";
+
 // Allows overriding the country used for evaluating variations. This is similar
 // to the "Override Variations Country" entry on chrome://translate-internals,
 // but is exposed as a command-line flag to allow testing First Run scenarios.

components/variations/variations_switches.h

@@ -16,6 +16,7 @@ extern const char kEnableBenchmarking[];
 extern const char kFakeVariationsChannel[];
 extern const char kForceFieldTrialParams[];
 extern const char kForceVariationIds[];
+extern const char kForceDisableVariationIds[];
 extern const char kVariationsOverrideCountry[];
 extern const char kVariationsServerURL[];
 extern const char kVariationsInsecureServerURL[];