Bluetooth: gather usage metrics

chrome/browser/metrics/metrics_log.cc

Index: chrome/browser/metrics/metrics_log.cc
diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index ec39326a7784b74f401734e5e355c8cc2f10b78a..bd4228d29daca62dec632f6d7d8e9a3e9e03c1ae 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
@@ -44,6 +45,8 @@
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/gpu_info.h"
+#include "device/bluetooth/bluetooth_adapter.h"
+#include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/screen.h"
 #include "webkit/plugins/webplugininfo.h"
@@ -61,6 +64,10 @@
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "device/bluetooth/bluetooth_device_experimental_chromeos.h"
+#endif
+
 using content::GpuDataManager;
 using metrics::OmniboxEventProto;
 using metrics::PerfDataProto;
@@ -303,6 +310,52 @@ void WriteScreenDPIInformationProto(SystemProfileProto::Hardware* hardware) {
 
 #endif  // defined(OS_WIN)
 
+#if defined(OS_CHROMEOS)
+// Pattern to match USB modaliases from BlueZ.
+const char kUsbModaliasPattern[] =
+    "usb:p[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]"

[Ilya Sherman, 2013/04/20 04:54:02]

nit: Might help with readability to add a line break after "usb:".

+    "v[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]"
+    "d[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]";
+
+SystemProfileProto::Bluetooth::PairedDevice::Type AsBluetoothDeviceType(
+    enum device::BluetoothDevice::DeviceType device_type) {
+  switch (device_type) {
+    case device::BluetoothDevice::DEVICE_UNKNOWN:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_UNKNOWN;
+    case device::BluetoothDevice::DEVICE_COMPUTER:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_COMPUTER;
+    case device::BluetoothDevice::DEVICE_PHONE:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_PHONE;
+    case device::BluetoothDevice::DEVICE_MODEM:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_MODEM;
+    case device::BluetoothDevice::DEVICE_AUDIO:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_AUDIO;
+    case device::BluetoothDevice::DEVICE_CAR_AUDIO:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_CAR_AUDIO;
+    case device::BluetoothDevice::DEVICE_VIDEO:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_VIDEO;
+    case device::BluetoothDevice::DEVICE_PERIPHERAL:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_PERIPHERAL;
+    case device::BluetoothDevice::DEVICE_JOYSTICK:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_JOYSTICK;
+    case device::BluetoothDevice::DEVICE_GAMEPAD:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_GAMEPAD;
+    case device::BluetoothDevice::DEVICE_KEYBOARD:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_KEYBOARD;
+    case device::BluetoothDevice::DEVICE_MOUSE:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_MOUSE;
+    case device::BluetoothDevice::DEVICE_TABLET:
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_TABLET;
+    case device::BluetoothDevice::DEVICE_KEYBOARD_MOUSE_COMBO:
+      return
+       SystemProfileProto::Bluetooth::PairedDevice::DEVICE_KEYBOARD_MOUSE_COMBO;

[Ilya Sherman, 2013/04/20 04:54:02]

nit: This indentation is off.  Perhaps add a "using
SystemProfileProto::Bluetooth" declaration to the top of this file to make all
of these lines shorter and easier to scan?

[keybuk, 2013/04/22 19:30:51]

You can't "using" a class member like that in C++

[Ilya Sherman, 2013/04/23 00:09:43]

How about a typedef, then?

+    default:

[Ilya Sherman, 2013/04/20 04:54:02]

nit: Please omit the default case, so that the compiler can catch missing case
stmts.

[keybuk, 2013/04/22 19:30:51]

Done.

+      NOTREACHED();
+      return SystemProfileProto::Bluetooth::PairedDevice::DEVICE_UNKNOWN;
+  }
+}
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace
 
 GoogleUpdateMetrics::GoogleUpdateMetrics() : is_system_install(false) {}
@@ -877,6 +930,10 @@ void MetricsLog::RecordEnvironmentProto(
   PerfDataProto perf_data_proto;
   if (perf_provider_.GetPerfData(&perf_data_proto))
     uma_proto()->add_perf_data()->Swap(&perf_data_proto);
+
+  device::BluetoothAdapterFactory::GetAdapter(
+      base::Bind(&MetricsLog::WriteBluetoothProto,
+                 base::Unretained(this)));

[Ilya Sherman, 2013/04/20 04:54:02]

Why is base::Unretained() safe?  That is, what guarantees that this class will
not have been destroyed yet by the time the callback is fired?

[keybuk, 2013/04/22 19:30:51]

Done.

 #endif
 }
 
@@ -1087,3 +1144,40 @@ void MetricsLog::WriteGoogleUpdateProto(
   }
 #endif  // defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
 }
+
+void MetricsLog::WriteBluetoothProto(
+    scoped_refptr<device::BluetoothAdapter> adapter) {
+#if defined(OS_CHROMEOS)
+  SystemProfileProto::Bluetooth* bluetooth =
+      uma_proto()->mutable_system_profile()->mutable_bluetooth();
+
+  bluetooth->set_is_present(adapter->IsPresent());
+  bluetooth->set_is_enabled(adapter->IsPowered());
+
+  device::BluetoothAdapter::DeviceList devices = adapter->GetDevices();
+  for (device::BluetoothAdapter::DeviceList::iterator iter =
+       devices.begin(); iter != devices.end(); ++iter) {

[Ilya Sherman, 2013/04/20 04:54:02]

nit: Indent this line four more spaces, since it's wrapped from the previous
one.

[keybuk, 2013/04/22 19:30:51]

Done.

+    SystemProfileProto::Bluetooth::PairedDevice* paired_device =
+      bluetooth->add_paired_device();
+
+    chromeos::BluetoothDeviceExperimentalChromeOS* device =
+        static_cast<chromeos::BluetoothDeviceExperimentalChromeOS*>(*iter);

[Ilya Sherman, 2013/04/20 04:54:02]

How do you know that this cast is safe?  If it's always safe, why not just
structure the code so that the cast is not necessary?

[keybuk, 2013/04/22 19:30:51]

Done.

+
+    paired_device->set_bluetooth_class(device->GetBluetoothClass());
+    paired_device->set_type(AsBluetoothDeviceType(device->GetDeviceType()));
+
+    std::string modalias = device->GetModalias();
+    if (MatchPattern(modalias, kUsbModaliasPattern)) {

[Ilya Sherman, 2013/04/20 04:54:02]

Hmm, is this meant to be a regex pattern match?  It looks like you're using
string_util.h::MatchPattern(), which AFAIK does not actually support full regex
syntax like what you're using.

[keybuk, 2013/04/22 19:30:51]

Done.

+      // usb:vXXXXpXXXXdXXXX
+      uint64 vendor_id = 0, product_id = 0, bcd_device = 0;
+      base::HexStringToUInt64(modalias.substr(5, 4), &vendor_id);
+      base::HexStringToUInt64(modalias.substr(10, 4), &product_id);
+      base::HexStringToUInt64(modalias.substr(15, 4), &bcd_device);
+
+      paired_device->set_vendor_id(vendor_id);
+      paired_device->set_product_id(product_id);
+      paired_device->set_bcd_device(bcd_device);
+    }
+  }
+#endif  // defined(OS_CHROMEOS)
+}