Add other variations (with ids) for NTPSnippets to chrome://flags

components/variations/variations_associated_data.cc

Index: components/variations/variations_associated_data.cc
diff --git a/components/variations/variations_associated_data.cc b/components/variations/variations_associated_data.cc
index d7dda4b97d188a4c208e11ab5f35ba86551cdb94..33c49a9b9bd5ec5d7ad5378e9d031c237cf07b6c 100644
--- a/components/variations/variations_associated_data.cc
+++ b/components/variations/variations_associated_data.cc
@@ -11,6 +11,8 @@
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
+#include "base/strings/string_split.h"
+#include "components/variations/variations_http_header_provider.h"
 
 namespace variations {
 
@@ -166,6 +168,31 @@ class VariationsParamAssociator {
 
 }  // namespace
 
+VariationsHttpHeaderProvider* ForceVariationIDsForHttpHeaders(
+    std::vector<std::string> variation_ids,
+    std::string command_line_variation_ids) {

[Alexei Svitkine (slow), 2016/07/26 16:43:35]

Pass the string by const& and the |variations_ids| by pointer, to avoid the
extra copy. (i.e. just modify the vector that's on the stack of the caller.
Switch the param order so the pointer is last.

[jkrcal, 2016/07/27 10:09:03]

Huh, a silly mistake. Done.

+  if (!variation_ids.empty()) {
+    // Combine the variation ids from chrome://flags with the ones from the

[Alexei Svitkine (slow), 2016/07/26 16:43:35]

Seems strange to mention chrome://flags here, since that's not obvious in this
context. I would just mention the variable names instead.

[jkrcal, 2016/07/27 10:09:04]

Done. (Actually, had a bug there, wanted to check non-emptiness of
|command_line_variation_ids|).

+    // command-line flag.
+    std::vector<std::string> variation_ids_flags =
+        base::SplitString(command_line_variation_ids, ",",
+                          base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+    variation_ids.insert(variation_ids.end(), variation_ids_flags.begin(),
+                         variation_ids_flags.end());
+  }
+
+  if (!variation_ids.empty()) {
+    // Create default variation ids which will always be included in the
+    // X-Client-Data request header.
+    VariationsHttpHeaderProvider* provider =
+        VariationsHttpHeaderProvider::GetInstance();
+    CHECK(provider->SetDefaultVariationIds(variation_ids))
+        << "Invalid list of variation ids specified";
+    return provider;
+  }
+  return nullptr;

[Alexei Svitkine (slow), 2016/07/26 16:43:35]

Seems like the previous code actually had a bug, since it was only calling
AddSyntheticTrialObserver() when the command-line was specified, but that's
actually not the intention (it should always be called). Can you fix this while
you're here?

I actually suggest instead of making this function return the provider, this can
be a member function of the provider and the calling code could just call it on
the provider instance.

[jkrcal, 2016/07/27 10:09:04]

Done.

+}
+
 void AssociateGoogleVariationID(IDCollectionKey key,
                                 const std::string& trial_name,
                                 const std::string& group_name,