Combine boolean MediaDeviceNotifications histograms to an enumerated histogram.

chrome/browser/system_monitor/media_storage_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // chrome::MediaStorageUtil implementation.
 
 #include "chrome/browser/system_monitor/media_storage_util.h"
 
 #include <vector>
 
 #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"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/system_monitor/media_transfer_protocol_device_observer_chromeos.h"
 #include "chrome/browser/system_monitor/removable_device_notifications_chromeos.h"
 #elif defined(OS_LINUX)
 #include "chrome/browser/system_monitor/removable_device_notifications_linux.h"
 #elif defined(OS_MACOSX)
 #include "chrome/browser/system_monitor/removable_device_notifications_mac.h"
 #elif defined(OS_WIN)
 #include "chrome/browser/system_monitor/removable_device_notifications_window_win.h"
 #endif
 
 using base::SystemMonitor;
 using content::BrowserThread;
 
 namespace chrome {
 
 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:";
 const char kFixedMassStoragePrefix[] = "path:";
 const char kMtpPtpPrefix[] = "mtp:";
 
 static bool (*g_test_get_device_info_from_path_function)(
     const FilePath& path, std::string* device_id, string16* device_name,
     FilePath* relative_path) = NULL;
 
@@ skipping 222 matching lines @@
     return FilePath::FromUTF8Unsafe(unique_id);
   }
 
   DCHECK(type == MTP_OR_PTP ||
          type == REMOVABLE_MASS_STORAGE_WITH_DCIM ||
          type == REMOVABLE_MASS_STORAGE_NO_DCIM);
   return FilePath(FindRemovableStorageLocationById(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;
 }
 
 MediaStorageUtil::MediaStorageUtil() {}
 
 }  // namespace chrome