linux: Detect additional window managers.

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

Index: chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
diff --git a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
index fa1789f60460a8972216c0da4b73d5d385d50e98..4c478450659275766c8c7667d0f5340b2d80af4d 100644
--- a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
+++ b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
@@ -53,7 +53,7 @@ enum UMALinuxGlibcVersion {
 enum UMALinuxWindowManager {
   UMA_LINUX_WINDOW_MANAGER_OTHER,
   UMA_LINUX_WINDOW_MANAGER_BLACKBOX,
-  UMA_LINUX_WINDOW_MANAGER_CHROME_OS,
+  UMA_LINUX_WINDOW_MANAGER_CHROME_OS,  // Deprecated.
   UMA_LINUX_WINDOW_MANAGER_COMPIZ,
   UMA_LINUX_WINDOW_MANAGER_ENLIGHTENMENT,
   UMA_LINUX_WINDOW_MANAGER_ICE_WM,
@@ -63,7 +63,16 @@ enum UMALinuxWindowManager {
   UMA_LINUX_WINDOW_MANAGER_MUTTER,
   UMA_LINUX_WINDOW_MANAGER_OPENBOX,
   UMA_LINUX_WINDOW_MANAGER_XFWM4,
-  // NOTE: Add new window managers above this line and update the enum list in
+  UMA_LINUX_WINDOW_MANAGER_AWESOME,
+  UMA_LINUX_WINDOW_MANAGER_I3,
+  UMA_LINUX_WINDOW_MANAGER_ION3,
+  UMA_LINUX_WINDOW_MANAGER_MATCHBOX,
+  UMA_LINUX_WINDOW_MANAGER_NOTION,
+  UMA_LINUX_WINDOW_MANAGER_QTILE,
+  UMA_LINUX_WINDOW_MANAGER_RATPOISON,
+  UMA_LINUX_WINDOW_MANAGER_STUMPWM,
+  // NOTE: Append new window managers to the list above this line (i.e. don't
+  // renumber) and update LinuxWindowManagerName in
   // tools/metrics/histograms/histograms.xml accordingly.
   UMA_LINUX_WINDOW_MANAGER_COUNT
 };
@@ -133,52 +142,32 @@ void RecordLinuxGlibcVersion() {
 #endif
 }
 
-void RecordLinuxWindowManager() {
 #if defined(USE_X11) && !defined(OS_CHROMEOS)
-  ui::WindowManagerName name = ui::GuessWindowManager();
-  UMALinuxWindowManager uma_name = UMA_LINUX_WINDOW_MANAGER_OTHER;
-  switch (name) {
-    case ui::WM_UNKNOWN:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_OTHER;
-      break;
-    case ui::WM_BLACKBOX:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_BLACKBOX;
-      break;
-    case ui::WM_CHROME_OS:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_CHROME_OS;
-      break;
-    case ui::WM_COMPIZ:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_COMPIZ;
-      break;
-    case ui::WM_ENLIGHTENMENT:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_ENLIGHTENMENT;
-      break;
-    case ui::WM_ICE_WM:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_ICE_WM;
-      break;
-    case ui::WM_KWIN:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_KWIN;
-      break;
-    case ui::WM_METACITY:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_METACITY;
-      break;
-    case ui::WM_MUFFIN:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_MUFFIN;
-      break;
-    case ui::WM_MUTTER:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_MUTTER;
-      break;
-    case ui::WM_OPENBOX:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_OPENBOX;
-      break;
-    case ui::WM_XFWM4:
-      uma_name = UMA_LINUX_WINDOW_MANAGER_XFWM4;
-      break;
+UMALinuxWindowManager GetLinuxWindowManager() {
+  switch (ui::GuessWindowManager()) {
+    case ui::WM_UNKNOWN:       return UMA_LINUX_WINDOW_MANAGER_OTHER;

[Matt Giuca, 2014/09/19 00:48:33]

Continuing the discussion from https://codereview.chromium.org/578443004/.

I see your link to the style guide; OK it seems like it's acceptable, but I
still think you should change it.

1. clang-format formats the return statements on their own line. In general,
Chromium code should match the output of clang-format.
2. This layout is brittle. If you ever wanted to add a window manager whose name
is longer than "enlightenment", you would have to edit every other line, and
this would probably cause most of the lines to go over 80 chars.
3. You could argue that the example in the style guide is just for doing a quick
"break" after a case, not for writing a regular line of code.

Please change it to having returns on their own line.

[Daniel Erat, 2014/09/19 15:54:49]

you clearly feel much more strongly about this than i do; done.

+    case ui::WM_AWESOME:       return UMA_LINUX_WINDOW_MANAGER_AWESOME;
+    case ui::WM_BLACKBOX:      return UMA_LINUX_WINDOW_MANAGER_BLACKBOX;
+    case ui::WM_COMPIZ:        return UMA_LINUX_WINDOW_MANAGER_COMPIZ;
+    case ui::WM_ENLIGHTENMENT: return UMA_LINUX_WINDOW_MANAGER_ENLIGHTENMENT;
+    case ui::WM_I3:            return UMA_LINUX_WINDOW_MANAGER_I3;
+    case ui::WM_ICE_WM:        return UMA_LINUX_WINDOW_MANAGER_ICE_WM;
+    case ui::WM_ION3:          return UMA_LINUX_WINDOW_MANAGER_ION3;
+    case ui::WM_KWIN:          return UMA_LINUX_WINDOW_MANAGER_KWIN;
+    case ui::WM_MATCHBOX:      return UMA_LINUX_WINDOW_MANAGER_MATCHBOX;
+    case ui::WM_METACITY:      return UMA_LINUX_WINDOW_MANAGER_METACITY;
+    case ui::WM_MUFFIN:        return UMA_LINUX_WINDOW_MANAGER_MUFFIN;
+    case ui::WM_MUTTER:        return UMA_LINUX_WINDOW_MANAGER_MUTTER;
+    case ui::WM_NOTION:        return UMA_LINUX_WINDOW_MANAGER_NOTION;
+    case ui::WM_OPENBOX:       return UMA_LINUX_WINDOW_MANAGER_OPENBOX;
+    case ui::WM_QTILE:         return UMA_LINUX_WINDOW_MANAGER_QTILE;
+    case ui::WM_RATPOISON:     return UMA_LINUX_WINDOW_MANAGER_RATPOISON;
+    case ui::WM_STUMPWM:       return UMA_LINUX_WINDOW_MANAGER_STUMPWM;
+    case ui::WM_XFWM4:         return UMA_LINUX_WINDOW_MANAGER_XFWM4;
   }
-  UMA_HISTOGRAM_ENUMERATION("Linux.WindowManager", uma_name,
-                            UMA_LINUX_WINDOW_MANAGER_COUNT);
-#endif
+  return UMA_LINUX_WINDOW_MANAGER_OTHER;
 }
+#endif
 
 void RecordTouchEventState() {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
@@ -225,7 +214,11 @@ void ChromeBrowserMainExtraPartsMetrics::PreBrowserStart() {
 
 void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
   RecordLinuxGlibcVersion();
-  RecordLinuxWindowManager();
+#if defined(USE_X11) && !defined(OS_CHROMEOS)
+  UMA_HISTOGRAM_ENUMERATION("Linux.WindowManager",
+                            GetLinuxWindowManager(),
+                            UMA_LINUX_WINDOW_MANAGER_COUNT);
+#endif
   RecordTouchEventState();
 
   const int kStartupMetricsGatheringDelaySeconds = 45;