Battery Status API: add UMA logging for Linux.

content/browser/battery_status/battery_status_manager_linux.cc

 // Copyright 2014 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.
 
 #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"
 #include "content/public/browser/browser_thread.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
 #include "dbus/property.h"
 #include "dbus/values_util.h"
@@ skipping 16 matching lines @@
   UPOWER_DEVICE_TYPE_LINE_POWER = 1,
   UPOWER_DEVICE_TYPE_BATTERY = 2,
   UPOWER_DEVICE_TYPE_UPS = 3,
   UPOWER_DEVICE_TYPE_MONITOR = 4,
   UPOWER_DEVICE_TYPE_MOUSE = 5,
   UPOWER_DEVICE_TYPE_KEYBOARD = 6,
   UPOWER_DEVICE_TYPE_PDA = 7,
   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,
                            const std::string& property_name,
                            double default_value) {
   double value = default_value;
   return dictionary.GetDouble(property_name, &value) ? value : default_value;
 }
 
 bool GetPropertyAsBoolean(const base::DictionaryValue& dictionary,
@@ skipping 35 matching lines @@
       proxy->CallMethodAndBlock(&method_call,
                                 dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
 
   if (response) {
     dbus::MessageReader reader(response.get());
     reader.PopArrayOfObjectPaths(paths.get());
   }
   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 {
  public:
   BatteryStatusNotificationThread(
       const BatteryStatusService::BatteryUpdateCallback& callback)
       : base::Thread(kBatteryNotifierThreadName),
         callback_(callback),
         battery_proxy_(NULL) {}
 
@@ skipping 15 matching lines @@
     DCHECK(OnWatcherThread());
 
     if (system_bus_)
       return;
 
     InitDBus();
     dbus::ObjectProxy* power_proxy =
         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);
       dbus::ObjectProxy* device_proxy = system_bus_->GetObjectProxy(
           kUPowerServiceName, device_path);
       scoped_ptr<base::DictionaryValue> dictionary =
           GetPropertiesAsDictionary(device_proxy);
 
       if (!dictionary)
         continue;
 
       bool is_present = GetPropertyAsBoolean(*dictionary, "IsPresent", false);
       uint32 type = static_cast<uint32>(
           GetPropertyAsDouble(*dictionary, "Type", UPOWER_DEVICE_TYPE_UNKNOWN));
 
       if (!is_present || type != UPOWER_DEVICE_TYPE_BATTERY) {
         system_bus_->RemoveObjectProxy(kUPowerServiceName,
                                        device_path,
                                        base::Bind(&base::DoNothing));
         continue;
       }
 
       if (battery_proxy_) {
         // 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;
     }
 
     battery_proxy_->ConnectToSignal(
         kUPowerDeviceName,
         kUPowerDeviceSignalChanged,
         base::Bind(&BatteryStatusNotificationThread::BatteryChanged,
                    base::Unretained(this)),
@@ skipping 187 matching lines @@
 }
 
 // static
 scoped_ptr<BatteryStatusManager> BatteryStatusManager::Create(
     const BatteryStatusService::BatteryUpdateCallback& callback) {
   return scoped_ptr<BatteryStatusManager>(
       new BatteryStatusManagerLinux(callback));
 }
 
 }  // namespace content