Refactor MetricsLogChromeOS to ChromeOSMetricsProvider.

Turns MetricsLogChromeOS into a metrics::MetricsProvider. Splits the
ChromeOS-specific unittests out from the MetricsLog test into separate
ChromeOSMetricsProvider tests. Also moves LogChromeOSCrash() from
MetricsService to ChromeOSMetricsProvider.

BUG=374221
R=asvitkine@chromium.org
TBR=derat, pam

Review URL: https://codereview.chromium.org/292433015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272708 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/external_metrics.cc

@@ -20,6 +20,7 @@
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/metrics/statistics_recorder.h"
@@ -28,7 +29,7 @@
 #include "base/time/time.h"
 #include "base/timer/elapsed_timer.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/user_metrics.h"
 
@@ -149,9 +150,7 @@ void ExternalMetrics::RecordAction(const char* action) {
 }
 
 void ExternalMetrics::RecordCrashUI(const std::string& crash_kind) {
-  if (g_browser_process && g_browser_process->metrics_service()) {
-    g_browser_process->metrics_service()->LogChromeOSCrash(crash_kind);
-  }
+  ChromeOSMetricsProvider::LogCrash(crash_kind);
 }
 
 void ExternalMetrics::RecordCrash(const std::string& crash_kind) {

chrome/browser/metrics/metrics_log_chromeos.cc → chrome/browser/metrics/chromeos_metrics_provider.cc

@@ -2,13 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/metrics/metrics_log_chromeos.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.h"
 
+#include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/users/user_manager.h"
+#include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/common/pref_names.h"
 #include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
 #include "device/bluetooth/bluetooth_adapter.h"
@@ -80,17 +83,57 @@ void WriteExternalTouchscreensProto(SystemProfileProto::Hardware* hardware) {
 #endif  // defined(USE_X11)
 }
 
+void IncrementPrefValue(const char* path) {
+  PrefService* pref = g_browser_process->local_state();
+  DCHECK(pref);
+  int value = pref->GetInteger(path);
+  pref->SetInteger(path, value + 1);
+}
+
 }  // namespace
 
-MetricsLogChromeOS::~MetricsLogChromeOS() {
+ChromeOSMetricsProvider::ChromeOSMetricsProvider()
+    : registered_user_count_at_log_initialization_(false),
+      user_count_at_log_initialization_(0) {
+}
+
+ChromeOSMetricsProvider::~ChromeOSMetricsProvider() {
+}
+
+// static
+void ChromeOSMetricsProvider::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterIntegerPref(prefs::kStabilityOtherUserCrashCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityKernelCrashCount, 0);
+  registry->RegisterIntegerPref(prefs::kStabilitySystemUncleanShutdownCount, 0);
 }
 
-MetricsLogChromeOS::MetricsLogChromeOS(ChromeUserMetricsExtension* uma_proto)
-    : uma_proto_(uma_proto) {
-  UpdateMultiProfileUserCount();
+// static
+void ChromeOSMetricsProvider::LogCrash(const std::string& crash_type) {
+  if (crash_type == "user")
+    IncrementPrefValue(prefs::kStabilityOtherUserCrashCount);
+  else if (crash_type == "kernel")
+    IncrementPrefValue(prefs::kStabilityKernelCrashCount);
+  else if (crash_type == "uncleanshutdown")
+    IncrementPrefValue(prefs::kStabilitySystemUncleanShutdownCount);
+  else
+    NOTREACHED() << "Unexpected Chrome OS crash type " << crash_type;
+
+  // Wake up metrics logs sending if necessary now that new
+  // log data is available.
+  g_browser_process->metrics_service()->OnApplicationNotIdle();
 }
 
-void MetricsLogChromeOS::LogChromeOSMetrics() {
+void ChromeOSMetricsProvider::OnDidCreateMetricsLog() {
+  registered_user_count_at_log_initialization_ = false;
+  if (chromeos::UserManager::IsInitialized()) {
+    registered_user_count_at_log_initialization_ = true;
+    user_count_at_log_initialization_ =
+        chromeos::UserManager::Get()->GetLoggedInUsers().size();
+  }
+}
+
+void ChromeOSMetricsProvider::ProvideSystemProfileMetrics(
+    metrics::SystemProfileProto* system_profile_proto) {
   std::vector<PerfDataProto> perf_data;
   if (perf_provider_.GetPerfData(&perf_data)) {
     for (std::vector<PerfDataProto>::iterator iter = perf_data.begin();
@@ -100,11 +143,11 @@ void MetricsLogChromeOS::LogChromeOSMetrics() {
     }
   }
 
-  WriteBluetoothProto();
-  UpdateMultiProfileUserCount();
+  WriteBluetoothProto(system_profile_proto);
+  UpdateMultiProfileUserCount(system_profile_proto);
 
-  SystemProfileProto::Hardware* hardware =
-      uma_proto_->mutable_system_profile()->mutable_hardware();
+  metrics::SystemProfileProto::Hardware* hardware =
+      system_profile_proto->mutable_hardware();
   gfx::Display::TouchSupport has_touch = ui::GetInternalDisplayTouchSupport();
   if (has_touch == gfx::Display::TOUCH_SUPPORT_AVAILABLE)
     hardware->set_internal_display_supports_touch(true);
@@ -113,38 +156,39 @@ void MetricsLogChromeOS::LogChromeOSMetrics() {
   WriteExternalTouchscreensProto(hardware);
 }
 
-void MetricsLogChromeOS::WriteRealtimeStabilityAttributes(PrefService* pref) {
-  SystemProfileProto::Stability* stability =
-      uma_proto_->mutable_system_profile()->mutable_stability();
-
+void ChromeOSMetricsProvider::ProvideStabilityMetrics(
+    metrics::SystemProfileProto* system_profile_proto) {
+  metrics::SystemProfileProto::Stability* stability_proto =
+      system_profile_proto->mutable_stability();
+  PrefService* pref = g_browser_process->local_state();
   int count = pref->GetInteger(prefs::kStabilityOtherUserCrashCount);
   if (count) {
-    stability->set_other_user_crash_count(count);
+    stability_proto->set_other_user_crash_count(count);
     pref->SetInteger(prefs::kStabilityOtherUserCrashCount, 0);
   }
 
   count = pref->GetInteger(prefs::kStabilityKernelCrashCount);
   if (count) {
-    stability->set_kernel_crash_count(count);
+    stability_proto->set_kernel_crash_count(count);
     pref->SetInteger(prefs::kStabilityKernelCrashCount, 0);
   }
 
   count = pref->GetInteger(prefs::kStabilitySystemUncleanShutdownCount);
   if (count) {
-    stability->set_unclean_system_shutdown_count(count);
+    stability_proto->set_unclean_system_shutdown_count(count);
     pref->SetInteger(prefs::kStabilitySystemUncleanShutdownCount, 0);
   }
 }
 
-void MetricsLogChromeOS::WriteBluetoothProto() {
-  SystemProfileProto::Hardware* hardware =
-      uma_proto_->mutable_system_profile()->mutable_hardware();
+void ChromeOSMetricsProvider::WriteBluetoothProto(
+    metrics::SystemProfileProto* system_profile_proto) {
+  metrics::SystemProfileProto::Hardware* hardware =
+      system_profile_proto->mutable_hardware();
 
   // BluetoothAdapterFactory::GetAdapter is synchronous on Chrome OS; if that
   // changes this will fail at the DCHECK().
-  device::BluetoothAdapterFactory::GetAdapter(
-      base::Bind(&MetricsLogChromeOS::SetBluetoothAdapter,
-                 base::Unretained(this)));
+  device::BluetoothAdapterFactory::GetAdapter(base::Bind(
+      &ChromeOSMetricsProvider::SetBluetoothAdapter, base::Unretained(this)));
   DCHECK(adapter_.get());
 
   SystemProfileProto::Hardware::Bluetooth* bluetooth =
@@ -154,8 +198,9 @@ void MetricsLogChromeOS::WriteBluetoothProto() {
   bluetooth->set_is_enabled(adapter_->IsPowered());
 
   device::BluetoothAdapter::DeviceList devices = adapter_->GetDevices();
-  for (device::BluetoothAdapter::DeviceList::iterator iter =
-           devices.begin(); iter != devices.end(); ++iter) {
+  for (device::BluetoothAdapter::DeviceList::iterator iter = devices.begin();
+       iter != devices.end();
+       ++iter) {
     device::BluetoothDevice* device = *iter;
     // Don't collect information about LE devices yet.
     if (!device->IsPaired())
@@ -168,8 +213,8 @@ void MetricsLogChromeOS::WriteBluetoothProto() {
     // |address| is xx:xx:xx:xx:xx:xx, extract the first three components and
     // pack into a uint32.
     std::string address = device->GetAddress();
-    if (address.size() > 9 &&
-        address[2] == ':' && address[5] == ':' && address[8] == ':') {
+    if (address.size() > 9 && address[2] == ':' && address[5] == ':' &&
+        address[8] == ':') {
       std::string vendor_prefix_str;
       uint64 vendor_prefix;
 
@@ -197,24 +242,22 @@ void MetricsLogChromeOS::WriteBluetoothProto() {
   }
 }
 
-void MetricsLogChromeOS::UpdateMultiProfileUserCount() {
-  metrics::SystemProfileProto* system_profile =
-      uma_proto_->mutable_system_profile();
-
+void ChromeOSMetricsProvider::UpdateMultiProfileUserCount(
+    metrics::SystemProfileProto* system_profile_proto) {
   if (chromeos::UserManager::IsInitialized()) {
     size_t user_count = chromeos::UserManager::Get()->GetLoggedInUsers().size();
 
     // We invalidate the user count if it changed while the log was open.
-    if (system_profile->has_multi_profile_user_count() &&
-        user_count != system_profile->multi_profile_user_count()) {
+    if (registered_user_count_at_log_initialization_ &&
+        user_count != user_count_at_log_initialization_) {
       user_count = 0;
     }
 
-    system_profile->set_multi_profile_user_count(user_count);
+    system_profile_proto->set_multi_profile_user_count(user_count);
   }
 }
 
-void MetricsLogChromeOS::SetBluetoothAdapter(
+void ChromeOSMetricsProvider::SetBluetoothAdapter(
     scoped_refptr<device::BluetoothAdapter> adapter) {
   adapter_ = adapter;
 }

chrome/browser/metrics/metrics_log_chromeos.h → chrome/browser/metrics/chromeos_metrics_provider.h

@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_METRICS_METRICS_LOG_CHROMEOS_H_
-#define CHROME_BROWSER_METRICS_METRICS_LOG_CHROMEOS_H_
+#ifndef CHROME_BROWSER_METRICS_CHROMEOS_METRICS_PROVIDER_H_
+#define CHROME_BROWSER_METRICS_CHROMEOS_METRICS_PROVIDER_H_
 
 #include "chrome/browser/metrics/perf_provider_chromeos.h"
+#include "components/metrics/metrics_provider.h"
 
 namespace device {
 class BluetoothAdapter;
@@ -15,35 +16,40 @@ namespace metrics {
 class ChromeUserMetricsExtension;
 }
 
+class PrefRegistrySimple;
 class PrefService;
 
 // Performs ChromeOS specific metrics logging.
-class MetricsLogChromeOS {
+class ChromeOSMetricsProvider : public metrics::MetricsProvider {
  public:
-  explicit MetricsLogChromeOS(metrics::ChromeUserMetricsExtension* uma_proto);
-  virtual ~MetricsLogChromeOS();
+  ChromeOSMetricsProvider();
+  virtual ~ChromeOSMetricsProvider();
 
-  // Logs ChromeOS specific metrics which don't need to be updated immediately.
-  void LogChromeOSMetrics();
+  static void RegisterPrefs(PrefRegistrySimple* registry);
 
-  // Within the stability group, write ChromeOS specific attributes that need to
-  // be updated asap and can't be delayed until the user decides to restart
-  // chromium.  Delaying these stats would bias metrics away from happy long
-  // lived chromium processes (ones that don't crash, and keep on running).
-  void WriteRealtimeStabilityAttributes(PrefService* pref);
+  // Records a crash.
+  static void LogCrash(const std::string& crash_type);
+
+  // metrics::MetricsProvider:
+  virtual void OnDidCreateMetricsLog() OVERRIDE;
+  virtual void ProvideSystemProfileMetrics(
+      metrics::SystemProfileProto* system_profile_proto) OVERRIDE;
+  virtual void ProvideStabilityMetrics(
+      metrics::SystemProfileProto* system_profile_proto) OVERRIDE;
 
  private:
   // Update the number of users logged into a multi-profile session.
   // If the number of users change while the log is open, the call invalidates
   // the user count value.
-  void UpdateMultiProfileUserCount();
+  void UpdateMultiProfileUserCount(
+      metrics::SystemProfileProto* system_profile_proto);
 
   // Sets the Bluetooth Adapter instance used for the WriteBluetoothProto()
   // call.
   void SetBluetoothAdapter(scoped_refptr<device::BluetoothAdapter> adapter);
 
   // Writes info about paired Bluetooth devices on this system.
-  virtual void WriteBluetoothProto();
+  void WriteBluetoothProto(metrics::SystemProfileProto* system_profile_proto);
 
   metrics::PerfProvider perf_provider_;
 
@@ -51,7 +57,15 @@ class MetricsLogChromeOS {
   scoped_refptr<device::BluetoothAdapter> adapter_;
   metrics::ChromeUserMetricsExtension* uma_proto_;
 
-  DISALLOW_COPY_AND_ASSIGN(MetricsLogChromeOS);
+  // Whether the user count was registered at the last log initialization.
+  bool registered_user_count_at_log_initialization_;
+
+  // The user count at the time that a log was last initialized. Contains a
+  // valid value only if |registered_user_count_at_log_initialization_| is
+  // true.
+  uint64 user_count_at_log_initialization_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeOSMetricsProvider);
 };
 
-#endif  // CHROME_BROWSER_METRICS_METRICS_LOG_CHROMEOS_H_
+#endif  // CHROME_BROWSER_METRICS_CHROMEOS_METRICS_PROVIDER_H_

chrome/browser/metrics/chromeos_metrics_provider_unittest.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 "chrome/browser/metrics/chromeos_metrics_provider.h"
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "chrome/browser/chromeos/login/users/fake_user_manager.h"
+#include "chrome/browser/chromeos/login/users/user_manager.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.h"
+#include "chromeos/dbus/fake_bluetooth_adapter_client.h"
+#include "chromeos/dbus/fake_bluetooth_agent_manager_client.h"
+#include "chromeos/dbus/fake_bluetooth_device_client.h"
+#include "chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h"
+#include "chromeos/dbus/fake_bluetooth_gatt_descriptor_client.h"
+#include "chromeos/dbus/fake_bluetooth_gatt_service_client.h"
+#include "chromeos/dbus/fake_bluetooth_input_client.h"
+#include "chromeos/dbus/fake_dbus_thread_manager.h"
+#include "components/metrics/proto/system_profile.pb.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using chromeos::DBusThreadManager;
+using chromeos::BluetoothAdapterClient;
+using chromeos::BluetoothAgentManagerClient;
+using chromeos::BluetoothDeviceClient;
+using chromeos::BluetoothGattCharacteristicClient;
+using chromeos::BluetoothGattDescriptorClient;
+using chromeos::BluetoothGattServiceClient;
+using chromeos::BluetoothInputClient;
+using chromeos::FakeBluetoothAdapterClient;
+using chromeos::FakeBluetoothAgentManagerClient;
+using chromeos::FakeBluetoothDeviceClient;
+using chromeos::FakeBluetoothGattCharacteristicClient;
+using chromeos::FakeBluetoothGattDescriptorClient;
+using chromeos::FakeBluetoothGattServiceClient;
+using chromeos::FakeBluetoothInputClient;
+using chromeos::FakeDBusThreadManager;
+
+class ChromeOSMetricsProviderTest : public testing::Test {
+ public:
+  ChromeOSMetricsProviderTest() {}
+
+ protected:
+  virtual void SetUp() OVERRIDE {
+    // Set up the fake Bluetooth environment,
+    scoped_ptr<FakeDBusThreadManager> fake_dbus_thread_manager(
+        new FakeDBusThreadManager);
+    fake_dbus_thread_manager->SetBluetoothAdapterClient(
+        scoped_ptr<BluetoothAdapterClient>(new FakeBluetoothAdapterClient));
+    fake_dbus_thread_manager->SetBluetoothDeviceClient(
+        scoped_ptr<BluetoothDeviceClient>(new FakeBluetoothDeviceClient));
+    fake_dbus_thread_manager->SetBluetoothGattCharacteristicClient(
+        scoped_ptr<BluetoothGattCharacteristicClient>(
+            new FakeBluetoothGattCharacteristicClient));
+    fake_dbus_thread_manager->SetBluetoothGattDescriptorClient(
+        scoped_ptr<BluetoothGattDescriptorClient>(
+            new FakeBluetoothGattDescriptorClient));
+    fake_dbus_thread_manager->SetBluetoothGattServiceClient(
+        scoped_ptr<BluetoothGattServiceClient>(
+            new FakeBluetoothGattServiceClient));
+    fake_dbus_thread_manager->SetBluetoothInputClient(
+        scoped_ptr<BluetoothInputClient>(new FakeBluetoothInputClient));
+    fake_dbus_thread_manager->SetBluetoothAgentManagerClient(
+        scoped_ptr<BluetoothAgentManagerClient>(
+            new FakeBluetoothAgentManagerClient));
+    DBusThreadManager::InitializeForTesting(fake_dbus_thread_manager.release());
+
+    // Grab pointers to members of the thread manager for easier testing.
+    fake_bluetooth_adapter_client_ = static_cast<FakeBluetoothAdapterClient*>(
+        DBusThreadManager::Get()->GetBluetoothAdapterClient());
+    fake_bluetooth_device_client_ = static_cast<FakeBluetoothDeviceClient*>(
+        DBusThreadManager::Get()->GetBluetoothDeviceClient());
+  }
+
+  virtual void TearDown() OVERRIDE { DBusThreadManager::Shutdown(); }
+
+ protected:
+  FakeBluetoothAdapterClient* fake_bluetooth_adapter_client_;
+  FakeBluetoothDeviceClient* fake_bluetooth_device_client_;
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeOSMetricsProviderTest);
+};
+
+TEST_F(ChromeOSMetricsProviderTest, MultiProfileUserCount) {
+  std::string user1("user1@example.com");
+  std::string user2("user2@example.com");
+  std::string user3("user3@example.com");
+
+  // |scoped_enabler| takes over the lifetime of |user_manager|.
+  chromeos::FakeUserManager* user_manager = new chromeos::FakeUserManager();
+  chromeos::ScopedUserManagerEnabler scoped_enabler(user_manager);
+  user_manager->AddKioskAppUser(user1);
+  user_manager->AddKioskAppUser(user2);
+  user_manager->AddKioskAppUser(user3);
+
+  user_manager->LoginUser(user1);
+  user_manager->LoginUser(user3);
+
+  ChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(2u, system_profile.multi_profile_user_count());
+}
+
+TEST_F(ChromeOSMetricsProviderTest, MultiProfileCountInvalidated) {
+  std::string user1("user1@example.com");
+  std::string user2("user2@example.com");
+  std::string user3("user3@example.com");
+
+  // |scoped_enabler| takes over the lifetime of |user_manager|.
+  chromeos::FakeUserManager* user_manager = new chromeos::FakeUserManager();
+  chromeos::ScopedUserManagerEnabler scoped_enabler(user_manager);
+  user_manager->AddKioskAppUser(user1);
+  user_manager->AddKioskAppUser(user2);
+  user_manager->AddKioskAppUser(user3);
+
+  user_manager->LoginUser(user1);
+
+  ChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(1u, system_profile.multi_profile_user_count());
+
+  user_manager->LoginUser(user2);
+  provider.ProvideSystemProfileMetrics(&system_profile);
+  EXPECT_EQ(0u, system_profile.multi_profile_user_count());
+}
+
+TEST_F(ChromeOSMetricsProviderTest, BluetoothHardwareDisabled) {
+  ChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+
+  EXPECT_TRUE(system_profile.has_hardware());
+  EXPECT_TRUE(system_profile.hardware().has_bluetooth());
+
+  EXPECT_TRUE(system_profile.hardware().bluetooth().is_present());
+  EXPECT_FALSE(system_profile.hardware().bluetooth().is_enabled());
+}
+
+TEST_F(ChromeOSMetricsProviderTest, BluetoothHardwareEnabled) {
+  FakeBluetoothAdapterClient::Properties* properties =
+      fake_bluetooth_adapter_client_->GetProperties(
+          dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath));
+  properties->powered.ReplaceValue(true);
+
+  ChromeOSMetricsProvider provider;
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+
+  EXPECT_TRUE(system_profile.has_hardware());
+  EXPECT_TRUE(system_profile.hardware().has_bluetooth());
+
+  EXPECT_TRUE(system_profile.hardware().bluetooth().is_present());
+  EXPECT_TRUE(system_profile.hardware().bluetooth().is_enabled());
+}
+
+TEST_F(ChromeOSMetricsProviderTest, BluetoothPairedDevices) {
+  // The fake bluetooth adapter class already claims to be paired with one
+  // device when initialized. Add a second and third fake device to it so we
+  // can test the cases where a device is not paired (LE device, generally)
+  // and a device that does not have Device ID information.
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(FakeBluetoothDeviceClient::kRequestPinCodePath));
+
+  fake_bluetooth_device_client_->CreateDevice(
+      dbus::ObjectPath(FakeBluetoothAdapterClient::kAdapterPath),
+      dbus::ObjectPath(FakeBluetoothDeviceClient::kConfirmPasskeyPath));
+
+  FakeBluetoothDeviceClient::Properties* properties =
+      fake_bluetooth_device_client_->GetProperties(
+          dbus::ObjectPath(FakeBluetoothDeviceClient::kConfirmPasskeyPath));
+  properties->paired.ReplaceValue(true);
+
+  ChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+
+  ASSERT_TRUE(system_profile.has_hardware());
+  ASSERT_TRUE(system_profile.hardware().has_bluetooth());
+
+  // Only two of the devices should appear.
+  EXPECT_EQ(2, system_profile.hardware().bluetooth().paired_device_size());
+
+  typedef metrics::SystemProfileProto::Hardware::Bluetooth::PairedDevice
+      PairedDevice;
+
+  // First device should match the Paired Device object, complete with
+  // parsed Device ID information.
+  PairedDevice device1 = system_profile.hardware().bluetooth().paired_device(0);
+
+  EXPECT_EQ(FakeBluetoothDeviceClient::kPairedDeviceClass,
+            device1.bluetooth_class());
+  EXPECT_EQ(PairedDevice::DEVICE_COMPUTER, device1.type());
+  EXPECT_EQ(0x001122U, device1.vendor_prefix());
+  EXPECT_EQ(PairedDevice::VENDOR_ID_USB, device1.vendor_id_source());
+  EXPECT_EQ(0x05ACU, device1.vendor_id());
+  EXPECT_EQ(0x030DU, device1.product_id());
+  EXPECT_EQ(0x0306U, device1.device_id());
+
+  // Second device should match the Confirm Passkey object, this has
+  // no Device ID information.
+  PairedDevice device2 = system_profile.hardware().bluetooth().paired_device(1);
+
+  EXPECT_EQ(FakeBluetoothDeviceClient::kConfirmPasskeyClass,
+            device2.bluetooth_class());
+  EXPECT_EQ(PairedDevice::DEVICE_PHONE, device2.type());
+  EXPECT_EQ(0x207D74U, device2.vendor_prefix());
+  EXPECT_EQ(PairedDevice::VENDOR_ID_UNKNOWN, device2.vendor_id_source());
+}

chrome/browser/metrics/metrics_log.cc

@@ -46,10 +46,6 @@
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 #endif
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/metrics/metrics_log_chromeos.h"
-#endif  // OS_CHROMEOS
-
 using metrics::MetricsLogBase;
 using metrics::ProfilerEventProto;
 using metrics::SystemProfileProto;
@@ -193,10 +189,6 @@ MetricsLog::MetricsLog(const std::string& client_id,
       client_(client),
       creation_time_(base::TimeTicks::Now()) {
   uma_proto()->mutable_system_profile()->set_channel(client_->GetChannel());
-
-#if defined(OS_CHROMEOS)
-  metrics_log_chromeos_.reset(new MetricsLogChromeOS(uma_proto()));
-#endif  // OS_CHROMEOS
 }
 
 MetricsLog::~MetricsLog() {}
@@ -317,10 +309,6 @@ void MetricsLog::WriteRealtimeStabilityAttributes(
     pref->SetInteger(prefs::kStabilityChildProcessCrashCount, 0);
   }
 
-#if defined(OS_CHROMEOS)
-  metrics_log_chromeos_->WriteRealtimeStabilityAttributes(pref);
-#endif  // OS_CHROMEOS
-
   const uint64 incremental_uptime_sec = incremental_uptime.InSeconds();
   if (incremental_uptime_sec)
     stability->set_incremental_uptime_sec(incremental_uptime_sec);
@@ -390,10 +378,6 @@ void MetricsLog::RecordEnvironment(
   WriteFieldTrials(field_trial_ids, system_profile);
   WriteFieldTrials(synthetic_trials, system_profile);
 
-#if defined(OS_CHROMEOS)
-  metrics_log_chromeos_->LogChromeOSMetrics();
-#endif  // OS_CHROMEOS
-
   for (size_t i = 0; i < metrics_providers.size(); ++i)
     metrics_providers[i]->ProvideSystemProfileMetrics(system_profile);
 

chrome/browser/metrics/metrics_log.h

@@ -18,10 +18,6 @@
 
 class PrefService;
 
-#if defined(OS_CHROMEOS)
-class MetricsLogChromeOS;
-#endif
-
 namespace base {
 class DictionaryValue;
 }
@@ -113,11 +109,6 @@ class MetricsLog : public metrics::MetricsLogBase {
   virtual void GetFieldTrialIds(
       std::vector<variations::ActiveGroupId>* field_trial_ids) const;
 
-  // Exposed to allow dependency injection for tests.
-#if defined(OS_CHROMEOS)
-  scoped_ptr<MetricsLogChromeOS> metrics_log_chromeos_;
-#endif
-
  private:
   FRIEND_TEST_ALL_PREFIXES(MetricsLogTest, ChromeOSStabilityData);
 

chrome/browser/metrics/metrics_service.cc

@@ -210,6 +210,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.h"
 #include "chromeos/system/statistics_provider.h"
 #endif
 
@@ -336,11 +337,6 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
                                 0);
   registry->RegisterIntegerPref(prefs::kStabilityDebuggerPresent, 0);
   registry->RegisterIntegerPref(prefs::kStabilityDebuggerNotPresent, 0);
-#if defined(OS_CHROMEOS)
-  registry->RegisterIntegerPref(prefs::kStabilityOtherUserCrashCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilityKernelCrashCount, 0);
-  registry->RegisterIntegerPref(prefs::kStabilitySystemUncleanShutdownCount, 0);
-#endif  // OS_CHROMEOS
 
   registry->RegisterStringPref(prefs::kStabilitySavedSystemProfile,
                                std::string());
@@ -421,6 +417,11 @@ MetricsService::MetricsService(metrics::MetricsStateManager* state_manager,
   RegisterMetricsProvider(scoped_ptr<metrics::MetricsProvider>(
       plugin_metrics_provider_));
 #endif
+
+#if defined(OS_CHROMEOS)
+  RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(new ChromeOSMetricsProvider));
+#endif
 }
 
 MetricsService::~MetricsService() {
@@ -822,6 +823,8 @@ void MetricsService::NotifyOnDidCreateMetricsLog() {
   DCHECK(thread_checker_.CalledOnValidThread());
   FOR_EACH_OBSERVER(
       MetricsServiceObserver, observers_, OnDidCreateMetricsLog());
+  for (size_t i = 0; i < metrics_providers_.size(); ++i)
+    metrics_providers_[i]->OnDidCreateMetricsLog();
 }
 
 //------------------------------------------------------------------------------
@@ -1402,22 +1405,6 @@ void MetricsService::LogCleanShutdown() {
                    MetricsService::SHUTDOWN_COMPLETE);
 }
 
-#if defined(OS_CHROMEOS)
-void MetricsService::LogChromeOSCrash(const std::string &crash_type) {
-  if (crash_type == "user")
-    IncrementPrefValue(prefs::kStabilityOtherUserCrashCount);
-  else if (crash_type == "kernel")
-    IncrementPrefValue(prefs::kStabilityKernelCrashCount);
-  else if (crash_type == "uncleanshutdown")
-    IncrementPrefValue(prefs::kStabilitySystemUncleanShutdownCount);
-  else
-    NOTREACHED() << "Unexpected Chrome OS crash type " << crash_type;
-  // Wake up metrics logs sending if necessary now that new
-  // log data is available.
-  HandleIdleSinceLastTransmission(false);
-}
-#endif  // OS_CHROMEOS
-
 void MetricsService::LogPluginLoadingError(const base::FilePath& plugin_path) {
 #if defined(ENABLE_PLUGINS)
   // TODO(asvitkine): Move this out of here.

chrome/browser/metrics/metrics_service.h

@@ -201,11 +201,6 @@ class MetricsService
   // This count is eventually send via UMA logs.
   void RecordBreakpadHasDebugger(bool has_debugger);
 
-#if defined(OS_CHROMEOS)
-  // Records a Chrome OS crash.
-  void LogChromeOSCrash(const std::string &crash_type);
-#endif
-
   bool recording_active() const;
   bool reporting_active() const;
 

chrome/browser/metrics/metrics_service_unittest.cc

@@ -19,10 +19,6 @@
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/metrics/metrics_log_chromeos.h"
-#endif  // OS_CHROMEOS
-
 namespace {
 
 using metrics::MetricsLogManager;
@@ -40,32 +36,14 @@ class TestMetricsService : public MetricsService {
   DISALLOW_COPY_AND_ASSIGN(TestMetricsService);
 };
 
-#if defined(OS_CHROMEOS)
-class TestMetricsLogChromeOS : public MetricsLogChromeOS {
- public:
-  explicit TestMetricsLogChromeOS(
-      metrics::ChromeUserMetricsExtension* uma_proto)
-      : MetricsLogChromeOS(uma_proto) {
-  }
-
- protected:
-  // Don't touch bluetooth information, as it won't be correctly initialized.
-  virtual void WriteBluetoothProto() OVERRIDE {
-  }
-};
-#endif  // OS_CHROMEOS
-
 class TestMetricsLog : public MetricsLog {
  public:
   TestMetricsLog(const std::string& client_id,
                  int session_id,
                  metrics::MetricsServiceClient* client)
       : MetricsLog(client_id, session_id, MetricsLog::ONGOING_LOG, client) {
-#if defined(OS_CHROMEOS)
-    metrics_log_chromeos_.reset(new TestMetricsLogChromeOS(
-        MetricsLog::uma_proto()));
-#endif  // OS_CHROMEOS
   }
+
   virtual ~TestMetricsLog() {}
 
  private:

chrome/browser/prefs/browser_prefs.cc

@@ -160,6 +160,7 @@
 #include "chrome/browser/chromeos/status/data_promo_notification.h"
 #include "chrome/browser/chromeos/system/automatic_reboot_manager.h"
 #include "chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.h"
 #include "chrome/browser/ui/webui/chromeos/charger_replacement_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/hid_detection_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/network_screen_handler.h"
@@ -280,6 +281,7 @@ void RegisterLocalState(PrefRegistrySimple* registry) {
 #endif  // !defined(OS_ANDROID)
 
 #if defined(OS_CHROMEOS)
+  ChromeOSMetricsProvider::RegisterPrefs(registry);
   chromeos::AudioDevicesPrefHandlerImpl::RegisterPrefs(registry);
   chromeos::ChargerReplacementHandler::RegisterPrefs(registry);
   chromeos::DataPromoNotification::RegisterPrefs(registry);

chrome/chrome_browser.gypi

@@ -1209,8 +1209,6 @@
         'browser/metrics/metric_event_duration_details.h',
         'browser/metrics/metrics_log.cc',
         'browser/metrics/metrics_log.h',
-        'browser/metrics/metrics_log_chromeos.cc',
-        'browser/metrics/metrics_log_chromeos.h',
         'browser/metrics/metrics_service.cc',
         'browser/metrics/metrics_service.h',
         'browser/metrics/metrics_service_accessor.cc',
@@ -2971,6 +2969,10 @@
           'dependencies': [
             'browser_chromeos',
           ],
+          'sources': [
+            'browser/metrics/chromeos_metrics_provider.cc',
+            'browser/metrics/chromeos_metrics_provider.h',
+          ],
           'sources!': [
             'browser/first_run/upgrade_util.cc',
             'browser/first_run/upgrade_util.h',

chrome/chrome_tests_unit.gypi

@@ -2300,6 +2300,7 @@
           ],
           'sources': [
             'browser/extensions/updater/local_extension_cache_unittest.cc',
+            'browser/metrics/chromeos_metrics_provider_unittest.cc',
           ],
           'sources/': [
             ['exclude', '^browser/ui/views/app_list/linux/'],

components/metrics/metrics_provider.h

@@ -20,6 +20,9 @@ class MetricsProvider {
   MetricsProvider() {}
   virtual ~MetricsProvider() {}
 
+  // Called when a new MetricsLog is created.
+  virtual void OnDidCreateMetricsLog() {}
+
   // Called when metrics recording has been enabled.
   virtual void OnRecordingEnabled() {}