flags: add expiry & ownership metadata

This change:
1) Adds c/b/flag-metadata.json, to store metadata about flags that is needed for
   other tooling but not for the browser;
2) Adds a disabled unit test AboutFlagsTest.EveryFlagHasMetadata, which asserts
   that every flag has an entry in this file.
3) Adds a unit test AboutFlagsTest.OnlyAllowedFlagsNeverExpire, which asserts
   that only a specified set of flags have no expiry milestone.

Next steps are to make AboutFlagsTest.EveryFlagHasMetadata pass by adding
entries for all current flags, then enable the test.

Bug: 897809
Change-Id: Icbf0055cb88106e7212bd6a1f90fcf9350fc7bf4
Reviewed-on: https://chromium-review.googlesource.com/c/1309106
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607955}

chrome/browser/about_flags_unittest.cc

@@ -10,9 +10,13 @@
 #include <set>
 #include <string>
 
+#include "base/base_paths.h"
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/format_macros.h"
+#include "base/json/json_file_value_serializer.h"
+#include "base/path_service.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/metrics/histogram_enum_reader.h"
 #include "base/values.h"
@@ -64,6 +68,39 @@ std::set<std::string> GetAllSwitchesAndFeaturesForTesting() {
   return result;
 }
 
+struct FlagMetadataEntry {
+  std::vector<std::string> owners;
+  int expire_milestone;
+};
+
+using FlagMetadataMap = std::map<std::string, FlagMetadataEntry>;
+
+FlagMetadataMap LoadFlagMetadata() {
+  FlagMetadataMap metadata;
+  base::FilePath metadata_path;
+  base::PathService::Get(base::DIR_SOURCE_ROOT, &metadata_path);
+  JSONFileValueDeserializer deserializer(
+      metadata_path.AppendASCII("chrome").AppendASCII("browser").AppendASCII(
+          "flag-metadata.json"));
+  int error_code;
+  std::string error_message;
+  std::unique_ptr<base::Value> metadata_json =
+      deserializer.Deserialize(&error_code, &error_message);
+  DCHECK(metadata_json) << "Failed to load flag metadata: " << error_code << " "
+                        << error_message;
+
+  for (const auto& entry : metadata_json->GetList()) {
+    std::string name = entry.FindKey("name")->GetString();
+    std::vector<std::string> owners;
+    for (const auto& owner : entry.FindKey("owners")->GetList())
+      owners.push_back(owner.GetString());
+    int expire_milestone = entry.FindKey("expire_milestone")->GetInt();
+    metadata[name] = FlagMetadataEntry{owners, expire_milestone};
+  }
+
+  return metadata;
+}
+
 }  // anonymous namespace
 
 // Makes sure there are no separators in any of the entry names.
@@ -77,6 +114,24 @@ TEST(AboutFlagsTest, NoSeparators) {
   }
 }
 
+// Makes sure that every flag has an owner and an expiry entry in
+// flag-metadata.json.
+TEST(AboutFlagsTest, DISABLED_EveryFlagHasMetadata) {
+  size_t count;
+  const flags_ui::FeatureEntry* entries = testing::GetFeatureEntries(&count);
+  FlagMetadataMap metadata = LoadFlagMetadata();
+
+  std::vector<std::string> missing_flags;
+
+  for (size_t i = 0; i < count; ++i) {
+    if (metadata.count(entries[i].internal_name) == 0)
+      missing_flags.push_back(entries[i].internal_name);
+  }
+
+  EXPECT_EQ(0u, missing_flags.size())
+      << "Missing flags: " << base::JoinString(missing_flags, "\n  ");
+}
+
 class AboutFlagsHistogramTest : public ::testing::Test {
  protected:
   // This is a helper function to check that all IDs in enum LoginCustomFlags in

chrome/browser/flag-metadata.json

+// This file lists metadata for chrome://flags entries. This metadata is not
+// ever used in the built browser or in any compiled code, but is used as part
+// of the review process and to clean up flags that have become obsolete or
+// unused.
+//
+// This file is a list of json objects; each object contains these keys:
+//
+//   name: the internal name of the flag, as present in chrome://flags. This is
+//   used as a primary key. The value is a string.
+//
+//   owners: the person(s) or team(s) responsible for this flag. The value is a
+//   list of strings, each of which is either an email address, or any other
+//   text, in which case it is assumed to be the username part of an
+//   @chromium.org email address.
+//
+//   expiry_milestone: the milestone after which this flag is obsolete.
+//   Specifically, after the milestone with the given number branches, this flag
+//   may freely be deleted and defaulted to either enabled or disabled where
+//   used. The special value -1 means "never expires", which should only be used
+//   in consultation with top-level OWNERS.
+
+[
+  {
+    "name": "ignore-gpu-blacklist",
+    "owners": [ "graphics-dev" ],
+    "expiry_milestone": -1
+  }
+]

chrome/test/BUILD.gn

@@ -2310,6 +2310,10 @@ test("unit_tests") {
     # enums.xml changes.
     "../../tools/metrics/histograms/enums.xml",
 
+    # flag-metadata.json is analyzed by AboutFlagsTest, so this dependency is
+    # needed to re-run unit_tests on changes to that file.
+    "../browser/flag-metadata.json",
+
     # All unittests in browser, common, renderer and service.
     "../browser/about_flags_unittest.cc",
     "../browser/active_use_util_unittest.cc",