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

components/variations/variations_associated_data.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/variations/variations_associated_data.h"
 
 #include <map>
 #include <utility>
 #include <vector>
 
 #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 {
 
 namespace {
 
 const char kGroupTesting[] = "Testing";
 
 // The internal singleton accessor for the map, used to keep it thread-safe.
 class GroupMapAccessor {
  public:
@@ skipping 135 matching lines @@
   ~VariationsParamAssociator() {}
 
   base::Lock lock_;
   std::map<VariationKey, VariationParams> variation_params_;
 
   DISALLOW_COPY_AND_ASSIGN(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,
                                 VariationID id) {
   GroupMapAccessor::GetInstance()->AssociateID(
       key, MakeActiveGroupId(trial_name, group_name), id, false);
 }
 
 void AssociateGoogleVariationIDForce(IDCollectionKey key,
                                      const std::string& trial_name,
@@ skipping 99 matching lines @@
   GroupMapAccessor::GetInstance()->ClearAllMapsForTesting();
 }
 
 void ClearAllVariationParams() {
   VariationsParamAssociator::GetInstance()->ClearAllParamsForTesting();
 }
 
 }  // namespace testing
 
 }  // namespace variations