Support Using ScopedFeatureList in BrowserTest

base/test/test_suite.cc

Index: base/test/test_suite.cc
diff --git a/base/test/test_suite.cc b/base/test/test_suite.cc
index c9bfe9e0f0173558f90130aead0fabe5b8370864..bc2745e9751471f07c9297e98ef10bf81ed340ee 100644
--- a/base/test/test_suite.cc
+++ b/base/test/test_suite.cc
@@ -135,8 +135,7 @@ int RunUnitTestsUsingBaseTestSuite(int argc, char **argv) {
                          Bind(&TestSuite::Run, Unretained(&test_suite)));
 }
 
-TestSuite::TestSuite(int argc, char** argv)
-    : initialized_command_line_(false), created_feature_list_(false) {
+TestSuite::TestSuite(int argc, char** argv) : initialized_command_line_(false) {
   PreInitialize();
   InitializeFromCommandLine(argc, argv);
   // Logging must be initialized before any thread has a chance to call logging
@@ -146,7 +145,7 @@ TestSuite::TestSuite(int argc, char** argv)
 
 #if defined(OS_WIN)
 TestSuite::TestSuite(int argc, wchar_t** argv)
-    : initialized_command_line_(false), created_feature_list_(false) {
+    : initialized_command_line_(false) {
   PreInitialize();
   InitializeFromCommandLine(argc, argv);
   // Logging must be initialized before any thread has a chance to call logging
@@ -342,13 +341,19 @@ void TestSuite::Initialize() {
     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());
+  // an error that it's not set. it will be cleared automatically.

[Ilya Sherman, 2017/06/07 21:32:36]

nit: s/it/It

+  scoped_feature_list_.InitFromCommandLine(
+      CommandLine::ForCurrentProcess()->GetSwitchValueASCII("enable-features"),
+      CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          "disable-features"));
+  // Remove enable-features and disable-features here and DCHECK in
+  // BrowserTestBase. Ensure we don't try to use command line to modify
+  // FeatureList

[Ilya Sherman, 2017/06/07 21:32:36]

nit: It's a bit odd to mention BrowserTestBase from this file, since that file
depends on this one.  I'd replace this comment with something like:

  // The enable-features and disable-features flags were just slurped into a
  // FeatureList, so remove them from the command line. Tests should enable and
  // disable features via the ScopedFeatureList API rather than command-line
  // flags.

+  CommandLine::ForCurrentProcess()->RemoveSwitchASCIIForTesting(
+      "enable-features");
+  CommandLine::ForCurrentProcess()->RemoveSwitchASCIIForTesting(
+      "disable-features");
 
 #if defined(OS_IOS)
   InitIOSTestMessageLoop();
@@ -412,10 +417,6 @@ void TestSuite::Initialize() {
 
 void TestSuite::Shutdown() {
   base::debug::StopProfiling();
-
-  // Clear the FeatureList that was created by Initialize().
-  if (created_feature_list_)
-    FeatureList::ClearInstanceForTesting();
 }
 
 }  // namespace base