Log base::Features in Launch.FlagsAtStartup

chrome/browser/about_flags.cc

Index: chrome/browser/about_flags.cc
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index b57dcedf7dc67ef5c5958274de5fb298a267abac..ed0bd9930f977e49175e2ed32888df3b36a6f366 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -2239,22 +2239,16 @@ void ReportAboutFlagsHistogram(const std::string& uma_histogram_name,
                                const std::set<std::string>& flags) {

[Alexei Svitkine (slow), 2016/09/15 19:59:18]

This function is also called from a different place whose result doesn't come
from GetSwitchesFromFlags(), so these changes are not safe.

I suggest keeping the semantics of the |flags| param the same and adding two
separate params for the enabled and disabled features. Then, for the place that
calls this function that doesn't use GetSwitchesFromFlags() you can pass empty
lists for the extra params.

[lawrencewu, 2016/09/16 15:34:56]

I needed only one extra parameter for all features (since the logic for
distinguishing enabled and disabled features happens in GetFeaturesFromFlags()),
and separated the body of ReportAboutFlagsHistogram() into two functions: one
for switches and one for features.

   for (const std::string& flag : flags) {
     int uma_id = about_flags::testing::kBadSwitchFormatHistogramId;
-    if (base::StartsWith(flag, "--", base::CompareCase::SENSITIVE)) {
-      // Skip '--' before switch name.
-      std::string switch_name(flag.substr(2));
-
-      // Kill value, if any.
-      const size_t value_pos = switch_name.find('=');
-      if (value_pos != std::string::npos)
-        switch_name.resize(value_pos);
-
-      uma_id = GetSwitchUMAId(switch_name);
-    } else {
-      NOTREACHED() << "ReportAboutFlagsHistogram(): flag '" << flag
-                   << "' has incorrect format.";
-    }
+
+    std::string mutable_flag(flag);
+    // Kill value, if any.
+    const size_t value_pos = mutable_flag.find('=');
+    if (value_pos != std::string::npos)
+      mutable_flag.resize(value_pos);
+
+    uma_id = GetSwitchUMAId(mutable_flag);
     DVLOG(1) << "ReportAboutFlagsHistogram(): histogram='" << uma_histogram_name
-             << "' '" << flag << "', uma_id=" << uma_id;
+             << "' '" << mutable_flag << "', uma_id=" << uma_id;
 
     // Sparse histogram macro does not cache the histogram, so it's safe
     // to use macro with non-static histogram name here.