Support Using ScopedFeatureList in BrowserTest

base/test/test_suite.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 "base/test/test_suite.h"
 
 #include <memory>
 
 #include "base/at_exit.h"
 #include "base/base_paths.h"
@@ skipping 324 matching lines @@
 #endif  // defined(_DEBUG) && defined(_HAS_EXCEPTIONS) && (_HAS_EXCEPTIONS == 1)
 #endif  // defined(OS_WIN)
 }
 
 void TestSuite::Initialize() {
 #if !defined(OS_IOS)
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kWaitForDebugger)) {
     debug::WaitForDebugger(60, true);
   }
 #endif
-
   // Set up a FeatureList instance, so that code using that API will not hit a
   // an error that it's not set. If a FeatureList was created in this way (i.e.
   // one didn't exist previously), it will be cleared in Shutdown() via
   // ClearInstanceForTesting().
-  created_feature_list_ =
-      FeatureList::InitializeInstance(std::string(), std::string());
+  created_feature_list_ = FeatureList::InitializeInstance(

[Ilya Sherman, 2017/05/25 22:48:12]

Could you use a ScopedFeatureList here?

+      CommandLine::ForCurrentProcess()->GetSwitchValueASCII("enable-features"),
+      CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          "disable-features"));

[Ilya Sherman, 2017/05/25 22:48:12]

It looks like the code expects that sometimes this InitializeInstance call might
fail.  That seems wrong.  Are there currently cases where it fails?  Can we
change the code to avoid the possibility of failure, and add an assertion to
that effect?

[Ilya Sherman, 2017/05/25 22:48:12]

nit: Please move the kEnabledFeatures and kDisabledFeatures constants into
//base, and use them here.

 
 #if defined(OS_IOS)
   InitIOSTestMessageLoop();
 #endif  // OS_IOS
 
 #if defined(OS_ANDROID)
   InitAndroidTestMessageLoop();
 #endif  // else defined(OS_ANDROID)
 
   CHECK(debug::EnableInProcessStackDumping());
@@ skipping 50 matching lines @@
 
 void TestSuite::Shutdown() {
   base::debug::StopProfiling();
 
   // Clear the FeatureList that was created by Initialize().
   if (created_feature_list_)
     FeatureList::ClearInstanceForTesting();
 }
 
 }  // namespace base