Support Using ScopedFeatureList in BrowserTest

content/test/content_browser_test_test.cc

Index: content/test/content_browser_test_test.cc
diff --git a/content/test/content_browser_test_test.cc b/content/test/content_browser_test_test.cc
index 446349da297ec89a96034c706cf6336173bc719d..eb455d3f8a48990a56bb9cfbf0b55d2e1b6c2111 100644
--- a/content/test/content_browser_test_test.cc
+++ b/content/test/content_browser_test_test.cc
@@ -8,8 +8,10 @@
 #include "base/location.h"
 #include "base/process/launch.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/launcher/test_launcher.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
@@ -176,6 +178,61 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTestSanityTest, SingleProcess) {
   Test();
 }
 
+const base::Feature kTestFeatureForContentBrowserTest{
+    "TestFeatureForContentBrowserTest", base::FEATURE_DISABLED_BY_DEFAULT};

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

nit: Please define this in an anonymous namespace.

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

Is it worth having both an enabled and a disabled feature get tested?

+
+class ContentBrowserTestScopedFeatureListTest : public ContentBrowserTest {
+ public:
+  void SetUp() override {
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    enabled_features_ =
+        command_line->GetSwitchValueASCII(switches::kEnableFeatures);
+    disabled_features_ =
+        command_line->GetSwitchValueASCII(switches::kEnableFeatures);

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

This should probably be kDisableFeatures... though I'm actually not sure why
you're saving these at all.  If you don't control them within the test, why are
you including them in what's tested?

+    scoped_feature_list_.InitAndEnableFeature(
+        kTestFeatureForContentBrowserTest);
+    ContentBrowserTest::SetUp();
+  }
+
+  std::string enabled_features_;
+  std::string disabled_features_;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(ContentBrowserTestScopedFeatureListTest,
+                       FeatureListTest) {
+  std::string enabled_features0;
+  std::string disabled_features0;
+
+  base::FeatureList::GetInstance()->GetFeatureOverrides(&enabled_features0,
+                                                        &disabled_features0);
+
+  base::StringPiece enabled_features = enabled_features0;
+  base::StringPiece disabled_features = disabled_features0;
+
+  // Ensure we repected the features from command line.

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

Is there reason to expect that there *are* features on the command line?  It
would be more compelling if you added features to the command-line as part of
the test.

+  std::vector<base::StringPiece> original_enabled_features =
+      base::SplitStringPiece(enabled_features_, ",", base::TRIM_WHITESPACE,
+                             base::SPLIT_WANT_NONEMPTY);
+  std::vector<base::StringPiece> original_disabled_features =
+      base::SplitStringPiece(disabled_features_, ",", base::TRIM_WHITESPACE,
+                             base::SPLIT_WANT_NONEMPTY);
+
+  for (base::StringPiece enabled_feature : original_enabled_features) {
+    EXPECT_NE(enabled_features.find(enabled_feature), base::StringPiece::npos);
+  }
+
+  for (base::StringPiece disabled_feature : original_disabled_features) {
+    EXPECT_NE(disabled_features.find(disabled_feature),
+              base::StringPiece::npos);
+  }
+
+  // Ensure kTestFeatureForContentBrowserTest enabled.
+  EXPECT_TRUE(base::FeatureList::IsEnabled(kTestFeatureForContentBrowserTest));
+}
+
 namespace {
 
 void CallbackChecker(bool* non_nested_task_ran) {