Battery Status API: add UMA logging for Linux.

content/browser/battery_status/battery_status_manager_linux.cc

Index: content/browser/battery_status/battery_status_manager_linux.cc
diff --git a/content/browser/battery_status/battery_status_manager_linux.cc b/content/browser/battery_status/battery_status_manager_linux.cc
index e6abf88ed4eb11b4f53ee7b1e9d771e87caf98d5..ba714fcd89dc8be0d1c8ffa5d3084724cc52d671 100644
--- a/content/browser/battery_status/battery_status_manager_linux.cc
+++ b/content/browser/battery_status/battery_status_manager_linux.cc
@@ -5,6 +5,7 @@
 #include "content/browser/battery_status/battery_status_manager_linux.h"
 
 #include "base/macros.h"
+#include "base/metrics/histogram.h"
 #include "base/threading/thread.h"
 #include "base/values.h"
 #include "content/browser/battery_status/battery_status_manager.h"
@@ -41,6 +42,18 @@ enum UPowerDeviceType {
   UPOWER_DEVICE_TYPE_PHONE = 8,
 };
 
+// This enum is used for histogram. Don't change the order of the existing
+// values.
+enum NumberBatteriesType {

[Michael van Ouwerkerk, 2014/08/21 09:28:18]

This seems kind of over-engineered. Can't we use a simple
UMA_HISTOGRAM_CUSTOM_COUNTS like for Net.Wifi.InterfaceCount ?

Its results are here:
https://uma.googleplex.com/histograms/?dayCount=1&endDate=08-20-2014&version=...

[timvolodine, 2014/08/21 14:52:18]

sure, the CUSTOM_COUNTS are a bit difficult to use though. Note that the
referred Net.Wifi.InterfaceCount example does it incorrectly and declares wrong
number of buckets.

Done.

+  NO_BATTERY = 0,
+  ONE_BATTERY = 1,
+  TWO_BATTERIES = 2,
+  THREE_BATTERIES = 3,
+  FOUR_BATTERIES = 4,
+  FIVE_OR_MORE_BATTERIES = 5,
+  NUMBER_BATTERIES_COUNT = 6,
+};
+
 typedef std::vector<dbus::ObjectPath> PathsVector;
 
 double GetPropertyAsDouble(const base::DictionaryValue& dictionary,
@@ -96,6 +109,13 @@ scoped_ptr<PathsVector> GetPowerSourcesPaths(dbus::ObjectProxy* proxy) {
   return paths.Pass();;
 }
 
+void UpdateNumberBatteriesHistogram(int count) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "BatteryStatus.NumberBatteriesLinux",
+      count < NUMBER_BATTERIES_COUNT ? count : FIVE_OR_MORE_BATTERIES,
+      NUMBER_BATTERIES_COUNT);
+}
+
 // Class that represents a dedicated thread which communicates with DBus to
 // obtain battery information and receives battery change notifications.
 class BatteryStatusNotificationThread : public base::Thread {
@@ -131,6 +151,7 @@ class BatteryStatusNotificationThread : public base::Thread {
         system_bus_->GetObjectProxy(kUPowerServiceName,
                                     dbus::ObjectPath(kUPowerPath));
     scoped_ptr<PathsVector> device_paths = GetPowerSourcesPaths(power_proxy);
+    int num_batteries = 0;
 
     for (size_t i = 0; i < device_paths->size(); ++i) {
       const dbus::ObjectPath& device_path = device_paths->at(i);
@@ -157,14 +178,16 @@ class BatteryStatusNotificationThread : public base::Thread {
         // TODO(timvolodine): add support for multiple batteries. Currently we
         // only collect information from the first battery we encounter
         // (crbug.com/400780).
-        // TODO(timvolodine): add UMA logging for this case.
         LOG(WARNING) << "multiple batteries found, "
                      << "using status data of the first battery only.";
       } else {
         battery_proxy_ = device_proxy;
       }
+      num_batteries++;
     }
 
+    UpdateNumberBatteriesHistogram(num_batteries);
+
     if (!battery_proxy_) {
       callback_.Run(blink::WebBatteryStatus());
       return;