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

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 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 "chrome/browser/chrome_browser_main.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 712 matching lines @@
 
     // Create field trials without activating them, so that this behaves in a
     // consistent manner with field trials created from the server.
     bool result = base::FieldTrialList::CreateTrialsFromString(
         command_line->GetSwitchValueASCII(switches::kForceFieldTrials),
         unforceable_field_trials);
     CHECK(result) << "Invalid --" << switches::kForceFieldTrials
                   << " list specified.";
   }
 
-  if (command_line->HasSwitch(switches::kForceVariationIds)) {
-    // Create default variation ids which will always be included in the
-    // X-Client-Data request header.
-    variations::VariationsHttpHeaderProvider* provider =
-        variations::VariationsHttpHeaderProvider::GetInstance();
-    bool result = provider->SetDefaultVariationIds(
-        command_line->GetSwitchValueASCII(switches::kForceVariationIds));
-    CHECK(result) << "Invalid --" << switches::kForceVariationIds
-                  << " list specified.";
-    metrics->AddSyntheticTrialObserver(provider);
-  }
-
   std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
 
   // Associate parameters chosen in about:flags and create trial/group for them.
   flags_ui::PrefServiceFlagsStorage flags_storage(
       g_browser_process->local_state());
-  about_flags::RegisterAllFeatureVariationParameters(&flags_storage,
-    feature_list.get());
+  std::vector<std::string> variation_ids =
+      about_flags::RegisterAllFeatureVariationParameters(
+          &flags_storage, feature_list.get());
+
+  if (command_line->HasSwitch(switches::kForceVariationIds)) {
+    // Combine the variation ids from chrome://flags with the ones from the
+    // command-line flag.
+    std::vector<std::string> variation_ids_flags = base::SplitString(
+        command_line->GetSwitchValueASCII(switches::kForceVariationIds), ",",
+        base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
+    variation_ids.insert(variation_ids.end(), variation_ids_flags.begin(),
+                         variation_ids_flags.end());
+  }

[Alexei Svitkine (slow), 2016/07/26 14:44:07]

I still think all this work should be being done in the variations code rather
than in chrome_browser_main.cc.

It's nice to make SetDefaultVariationIds(), but we should avoid bloating
chrome_browser_main.cc with extra variations code.

So, I still suggest putting this logic in the variations code. To keep it
simple, however, you can make a new function that does this parsing and
combining and then calls SetDefaultVariationIds(). This way,
SetDefaultVariationIds() can stay simple and the extra logic will be in the
wrapper function.

[jkrcal, 2016/07/26 15:50:39]

Done. It was not super clear to me where to cut the logic. Please let me know if
even more should be in variations (which would require moving the command-line
flag into variations::flags and/or adding metrics dependency into variations).

+
+  if (!variation_ids.empty()) {
+    // Create default variation ids which will always be included in the
+    // X-Client-Data request header.
+    variations::VariationsHttpHeaderProvider* provider =
+        variations::VariationsHttpHeaderProvider::GetInstance();
+    bool result = provider->SetDefaultVariationIds(variation_ids);
+    CHECK(result) << "Invalid list of variation ids specified (either in --"
+                  << switches::kForceVariationIds << " or in chrome://flags)";
+    metrics->AddSyntheticTrialObserver(provider);
+  }
 
   feature_list->InitializeFromCommandLine(
       command_line->GetSwitchValueASCII(switches::kEnableFeatures),
       command_line->GetSwitchValueASCII(switches::kDisableFeatures));
 
 #if defined(FIELDTRIAL_TESTING_ENABLED)
   if (!command_line->HasSwitch(switches::kDisableFieldTrialTestingConfig) &&
       !command_line->HasSwitch(switches::kForceFieldTrials) &&
       !command_line->HasSwitch(variations::switches::kVariationsServerURL)) {
     chrome_variations::AssociateDefaultFieldTrialConfig(feature_list.get());
@@ skipping 1293 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif  // defined(OS_CHROMEOS)
 #endif  // defined(OS_ANDROID)
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }