Combine boolean MediaDeviceNotifications histograms to an enumerated histogram.

chrome/browser/system_monitor/media_storage_util.cc

Index: chrome/browser/system_monitor/media_storage_util.cc
diff --git a/chrome/browser/system_monitor/media_storage_util.cc b/chrome/browser/system_monitor/media_storage_util.cc
index c6a03ed6c1b057450d8aa2ff3bc0b9c237412a72..dd7c91e7dc65f75225b5fceaae2a79955b7340df 100644
--- a/chrome/browser/system_monitor/media_storage_util.cc
+++ b/chrome/browser/system_monitor/media_storage_util.cc
@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/system_monitor/system_monitor.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -34,6 +35,19 @@ namespace {
 
 typedef std::vector<SystemMonitor::RemovableStorageInfo> RemovableStorageInfo;
 
+// MediaDeviceNotification.DeviceInfo histogram values.
+enum DeviceInfoHistogramBuckets {
+  MASS_STORAGE_DEVICE_NAME_AND_UUID_AVAILABLE,
+  MASS_STORAGE_DEVICE_UUID_MISSING,
+  MASS_STORAGE_DEVICE_NAME_MISSING,
+  MASS_STORAGE_DEVICE_NAME_AND_UUID_MISSING,
+  MTP_STORAGE_DEVICE_NAME_AND_UUID_AVAILABLE,
+  MTP_STORAGE_DEVICE_UUID_MISSING,
+  MTP_STORAGE_DEVICE_NAME_MISSING,
+  MTP_STORAGE_DEVICE_NAME_AND_UUID_MISSING,
+  DEVICE_INFO_BUCKET_BOUNDARY
+};
+
 // Prefix constants for different device id spaces.
 const char kRemovableMassStorageWithDCIMPrefix[] = "dcim:";
 const char kRemovableMassStorageNoDCIMPrefix[] = "nodcim:";
@@ -276,6 +290,26 @@ FilePath MediaStorageUtil::FindDevicePathById(const std::string& device_id) {
 }
 
 // static
+void MediaStorageUtil::RecordDeviceInfoHistogram(bool mass_storage,
+                                                 const std::string& device_uuid,
+                                                 const string16& device_name) {
+  int event_number = mass_storage ? 0 : 4;

[vandebo (ex-Chrome), 2012/09/17 18:40:07]

nit: Remove the trinary operator.

[Lei Zhang, 2012/09/17 18:46:54]

Why even use integers? Why not just use the enums?

[kmadhusu, 2012/09/17 19:32:49]

Done.

[kmadhusu, 2012/09/17 19:32:49]

I think this function is simple and easier to understand. So I preferred
integers vs using enum values.

[Lei Zhang, 2012/09/17 19:48:27]

I'm not sure it is. Every time |event_number| changes, I have to go up to the
enum to look up what the new value means.

+  if (device_name.empty())
+    event_number += 2;
+
+  if (device_uuid.empty())
+    event_number += 1;
+  enum DeviceInfoHistogramBuckets event =
+      static_cast<enum DeviceInfoHistogramBuckets>(event_number);
+  if (event >= DEVICE_INFO_BUCKET_BOUNDARY) {
+    NOTREACHED();
+    return;
+  }
+  UMA_HISTOGRAM_ENUMERATION("MediaDeviceNotifications.DeviceInfo", event,
+                            DEVICE_INFO_BUCKET_BOUNDARY);
+}
+
+// static
 void MediaStorageUtil::SetGetDeviceInfoFromPathFunctionForTesting(
     GetDeviceInfoFromPathFunction function) {
   g_test_get_device_info_from_path_function = function;