flags: don't depend on real flags in tests

The flag 'unsafely-treat-insecure-origin-as-secure' was being used in
this test to test the behavior of ORIGIN_LIST_VALUE_TYPE flags; that
made it impossible for that flag to expire or be removed without
breaking the test. This change:

1) Introduces about_flags::testing::SetFeatureEntries() to override the
   flags list for testing purposes
2) Has AboutFlagsBrowserTest use it to set the flag set to a single test
   flag. As a side-effect, this hugely speeds up this test suite since
   the page now contains far less data.
3) Undoes a bit of confusion in the tests: they treated "switch" and
   "flag" as synonyms throughout, which is not correct.

A followup change will fix https://crbug.com/1010678 and re-enable these
tests on the bots.

Bug: 1010678
Change-Id: I91f39fb8b3e000a824d8cf85fd3daa51d9a247af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872318
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707927}

chrome/browser/about_flags.cc

@@ -4700,7 +4700,9 @@ const FeatureEntry kFeatureEntries[] = {
 class FlagsStateSingleton {
  public:
   FlagsStateSingleton()
-      : flags_state_(kFeatureEntries, base::size(kFeatureEntries)) {}
+      : flags_state_(std::make_unique<flags_ui::FlagsState>(
+            kFeatureEntries,
+            base::size(kFeatureEntries))) {}
   ~FlagsStateSingleton() {}
 
   static FlagsStateSingleton* GetInstance() {
@@ -4708,11 +4710,16 @@ class FlagsStateSingleton {
   }
 
   static flags_ui::FlagsState* GetFlagsState() {
-    return &GetInstance()->flags_state_;
+    return GetInstance()->flags_state_.get();
+  }
+
+  void RebuildState(const std::vector<flags_ui::FeatureEntry>& entries) {
+    flags_state_ =
+        std::make_unique<flags_ui::FlagsState>(entries.data(), entries.size());
   }
 
  private:
-  flags_ui::FlagsState flags_state_;
+  std::unique_ptr<flags_ui::FlagsState> flags_state_;
 
   DISALLOW_COPY_AND_ASSIGN(FlagsStateSingleton);
 };
@@ -4911,11 +4918,27 @@ namespace testing {
 
 const base::HistogramBase::Sample kBadSwitchFormatHistogramId = 0;
 
+std::vector<FeatureEntry>* GetEntriesForTesting() {
+  static base::NoDestructor<std::vector<FeatureEntry>> entries;
+  return entries.get();
+}
+
 const FeatureEntry* GetFeatureEntries(size_t* count) {
+  if (!GetEntriesForTesting()->empty()) {
+    *count = GetEntriesForTesting()->size();
+    return GetEntriesForTesting()->data();
+  }
   *count = base::size(kFeatureEntries);
   return kFeatureEntries;
 }
 
+void SetFeatureEntries(const std::vector<FeatureEntry>& entries) {
+  GetEntriesForTesting()->clear();
+  for (const auto& entry : entries)
+    GetEntriesForTesting()->push_back(entry);
+  FlagsStateSingleton::GetInstance()->RebuildState(*GetEntriesForTesting());
+}
+
 }  // namespace testing
 
 }  // namespace about_flags

chrome/browser/about_flags.h

@@ -118,6 +118,9 @@ namespace testing {
 // Returns the global set of feature entries.
 const flags_ui::FeatureEntry* GetFeatureEntries(size_t* count);
 
+// Sets the global set of feature entries.
+void SetFeatureEntries(const std::vector<flags_ui::FeatureEntry>& entries);
+
 // This value is reported as switch histogram ID if switch name has unknown
 // format.
 extern const base::HistogramBase::Sample kBadSwitchFormatHistogramId;

chrome/browser/about_flags_browsertest.cc

@@ -7,19 +7,22 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
+#include "chrome/browser/about_flags.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/unexpire_flags.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/flags_ui/feature_entry_macros.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "ui/base/window_open_disposition.h"
 
 namespace {
 
-const char kSwitchName[] = "unsafely-treat-insecure-origin-as-secure";
+const char kSwitchName[] = "flag-system-test-switch";
+const char kFlagName[] = "flag-system-test-flag";
 
 // Command line switch containing an invalid origin.
 const char kUnsanitizedCommandLine[] =
@@ -116,6 +119,9 @@ class AboutFlagsBrowserTest : public InProcessBrowserTest,
                               public testing::WithParamInterface<bool> {
  public:
   AboutFlagsBrowserTest() {
+    about_flags::testing::SetFeatureEntries(
+        {{"flag-system-test-flag", "name", "description", -1,
+          ORIGIN_LIST_VALUE_TYPE(kSwitchName, "")}});
     feature_list_.InitWithFeatures({flags::kUnexpireFlagsM76}, {});
   }
 
@@ -164,19 +170,18 @@ IN_PROC_BROWSER_TEST_P(AboutFlagsBrowserTest, PRE_OriginFlagDisabled) {
       browser()->tab_strip_model()->GetActiveWebContents();
 
   // The page should be populated with the sanitized command line value.
-  EXPECT_EQ(GetSanitizedCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+  EXPECT_EQ(GetSanitizedCommandLine(), GetOriginListText(contents, kFlagName));
 
   // Type a value in the experiment's textarea. Since the flag state is
   // "Disabled" by default, command line shouldn't change.
-  SimulateTextType(contents, kSwitchName, kUnsanitizedInput);
+  SimulateTextType(contents, kFlagName, kUnsanitizedInput);
   EXPECT_EQ(kInitialSwitches,
             base::CommandLine::ForCurrentProcess()->GetSwitches());
 
   // Input should be restored after a page reload.
   NavigateToFlagsPage();
   EXPECT_EQ(GetSanitizedInputAndCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+            GetOriginListText(contents, kFlagName));
 }
 
 // Flaky. http://crbug.com/1010678
@@ -190,9 +195,9 @@ IN_PROC_BROWSER_TEST_P(AboutFlagsBrowserTest, DISABLED_OriginFlagDisabled) {
   NavigateToFlagsPage();
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_FALSE(IsDropdownEnabled(contents, kSwitchName));
+  EXPECT_FALSE(IsDropdownEnabled(contents, kFlagName));
   EXPECT_EQ(GetSanitizedInputAndCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+            GetOriginListText(contents, kFlagName));
 }
 
 // Goes to chrome://flags page, types text into an ORIGIN_LIST_VALUE field and
@@ -207,18 +212,17 @@ IN_PROC_BROWSER_TEST_P(AboutFlagsBrowserTest, PRE_OriginFlagEnabled) {
       browser()->tab_strip_model()->GetActiveWebContents();
 
   // The page should be populated with the sanitized command line value.
-  EXPECT_EQ(GetSanitizedCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+  EXPECT_EQ(GetSanitizedCommandLine(), GetOriginListText(contents, kFlagName));
 
   // Type a value in the experiment's textarea. Since the flag state is
   // "Disabled" by default, command line shouldn't change.
-  SimulateTextType(contents, kSwitchName, kUnsanitizedInput);
+  SimulateTextType(contents, kFlagName, kUnsanitizedInput);
   EXPECT_EQ(kInitialSwitches,
             base::CommandLine::ForCurrentProcess()->GetSwitches());
 
   // Enable the experiment. The behavior is different between ChromeOS and
   // non-ChromeOS.
-  ToggleEnableDropdown(contents, kSwitchName, true);
+  ToggleEnableDropdown(contents, kFlagName, true);
 
 #if !defined(OS_CHROMEOS)
   // On non-ChromeOS, the command line is not modified until restart.
@@ -236,7 +240,7 @@ IN_PROC_BROWSER_TEST_P(AboutFlagsBrowserTest, PRE_OriginFlagEnabled) {
   // Input should be restored after a page reload.
   NavigateToFlagsPage();
   EXPECT_EQ(GetSanitizedInputAndCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+            GetOriginListText(contents, kFlagName));
 }
 
 // Flaky. http://crbug.com/1010678
@@ -256,14 +260,14 @@ IN_PROC_BROWSER_TEST_P(AboutFlagsBrowserTest, DISABLED_OriginFlagEnabled) {
   NavigateToFlagsPage();
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
-  EXPECT_TRUE(IsDropdownEnabled(contents, kSwitchName));
+  EXPECT_TRUE(IsDropdownEnabled(contents, kFlagName));
   EXPECT_EQ(GetSanitizedInputAndCommandLine(),
-            GetOriginListText(contents, kSwitchName));
+            GetOriginListText(contents, kFlagName));
 
 #if defined(OS_CHROMEOS)
   // ChromeOS doesn't read chrome://flags values on startup so we explicitly
   // need to disable and re-enable the flag here.
-  ToggleEnableDropdown(contents, kSwitchName, true);
+  ToggleEnableDropdown(contents, kFlagName, true);
 #endif
 
   EXPECT_EQ(