Collect UMA statistics on which chrome://flags lead to chrome restart on ChromeOS.

chrome/browser/chromeos/login/login_utils.cc

Index: chrome/browser/chromeos/login/login_utils.cc
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index fafe4398fe5abc26626f9aadd4c18c42f612460f..081aafd6f652de3462ea28abcfde80345514e214 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -19,6 +19,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/prefs/pref_member.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
@@ -85,11 +86,45 @@
 #include "url/gurl.h"
 
 using content::BrowserThread;
-
 namespace chromeos {
 
 struct DoBrowserLaunchOnLocaleLoadedData;
 
+namespace {
+
+void ReportCustomFlags(const std::set<std::string>& command_line_difference) {
+  const std::map<std::string, int>& switch_histogram_id =

[Lei Zhang, 2014/06/26 20:03:14]

|switch_histogram_id| -> switch_histogram_ids or switch_histogram_id_map ?

[Alexander Alekseev, 2014/06/27 19:28:37]

Done.

+      about_flags::GetSwitchesHistogramId();
+  for (std::set<std::string>::const_iterator i =

[Lei Zhang, 2014/06/26 20:03:14]

in general: i -> it for iterators

[Alexander Alekseev, 2014/06/27 19:28:37]

Done.

+           command_line_difference.begin();
+       i != command_line_difference.end();
+       ++i) {
+    int uma_id = about_flags::UMA_HISTOGRAM_ID_UNKNOWN_FLAG;
+    if (i->size() > 2) {
+      // skip '--' before switch name
+      const std::string switch_name(&((*i)[2]));

[Lei Zhang, 2014/06/26 20:03:14]

i->substring(2) is a bit more readable than &((*i)[2])

[Alexander Alekseev, 2014/06/27 19:28:37]

Done.

+      const std::map<std::string, int>::const_iterator pos =
+          switch_histogram_id.find(switch_name);
+      if (pos != switch_histogram_id.end()) {
+        uma_id = pos->second;
+      } else {
+        uma_id = about_flags::UMA_HISTOGRAM_ID_UNKNOWN_FLAG;
+        LOG(ERROR) << "ReportCustomFlags(): flag '" << *i << "' ('"
+                   << switch_name << "') is unknown.";
+      }
+    } else {
+      uma_id = about_flags::UMA_HISTOGRAM_ID_BAD_FLAG_FORMAT;
+      LOG(ERROR) << "ReportCustomFlags(): flag '" << *i
+                 << "' has incorrect format.";
+    }
+    LOG(ERROR) << "ReportCustomFlags(): '" << *i << "', uma_id=" << uma_id;
+    VLOG(1) << "ReportCustomFlags(): '" << *i << "', uma_id=" << uma_id;
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Login.CustomFlags", uma_id);
+  }
+}
+
+}  // anonymous namespace

[Lei Zhang, 2014/06/26 20:03:14]

just // namespace

[Alexander Alekseev, 2014/06/27 19:28:37]

Done.

+
 class LoginUtilsImpl
     : public LoginUtils,
       public base::SupportsWeakPtr<LoginUtilsImpl>,
@@ -216,16 +251,29 @@ void LoginUtilsImpl::DoBrowserLaunchOnLocaleLoadedImpl(
   about_flags::PrefServiceFlagsStorage flags_storage_(profile->GetPrefs());
   about_flags::ConvertFlagsToSwitches(&flags_storage_, &user_flags,
                                       about_flags::kAddSentinels);
+
+  // about_flags::AreSwitchesIdenticalToCurrentCommandLine() requires

[Lei Zhang, 2014/06/26 20:03:14]

You're in CrOS-only code, you don't need this comment. Just use
std::set<CommandLine::StringType>

[Alexander Alekseev, 2014/06/27 19:28:37]

Done.

+  // output argument to be std::set<CommandLine::StringType>* .
+  // To report the difference, we need std::set<std::string> .
+  // So here we are silently using the fact, that CommandLine::StringType
+  // is std::string on ChromeOS. Otherwise we would have to decode strings here.
+  std::set<std::string> command_line_difference;
+
   // Only restart if needed and if not going into managed mode.
   // Don't restart browser if it is not first profile in session.
   if (UserManager::Get()->GetLoggedInUsers().size() == 1 &&
       !UserManager::Get()->IsLoggedInAsLocallyManagedUser() &&
       !about_flags::AreSwitchesIdenticalToCurrentCommandLine(
-          user_flags, *CommandLine::ForCurrentProcess())) {
+          user_flags,
+          *CommandLine::ForCurrentProcess(),
+          &command_line_difference)) {
     CommandLine::StringVector flags;
     // argv[0] is the program name |CommandLine::NO_PROGRAM|.
     flags.assign(user_flags.argv().begin() + 1, user_flags.argv().end());
     VLOG(1) << "Restarting to apply per-session flags...";
+
+    ReportCustomFlags(command_line_difference);
+
     DBusThreadManager::Get()->GetSessionManagerClient()->SetFlagsForUser(
         UserManager::Get()->GetActiveUser()->email(), flags);
     AttemptRestart(profile);