Refactor MetricsLogChromeOS to ChromeOSMetricsProvider.

chrome/browser/metrics/chromeos_metrics_provider.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/metrics_log_chromeos.h"
+#include "chrome/browser/metrics/chromeos_metrics_provider.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"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/screen.h"
 
 #if defined(USE_X11)
 #include "ui/events/x/touch_factory_x11.h"
@@ skipping 51 matching lines @@
        it != touchscreen_ids.end();
        ++it) {
     SystemProfileProto::Hardware::TouchScreen* touchscreen =
         hardware->add_external_touchscreen();
     touchscreen->set_vendor_id(it->first);
     touchscreen->set_product_id(it->second);
   }
 #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()
+    : user_count_at_log_initialization_(0) {
 }
 
-MetricsLogChromeOS::MetricsLogChromeOS(ChromeUserMetricsExtension* uma_proto)
-    : uma_proto_(uma_proto) {
-  UpdateMultiProfileUserCount();
+ChromeOSMetricsProvider::~ChromeOSMetricsProvider() {
 }
 
-void MetricsLogChromeOS::LogChromeOSMetrics() {
+// static
+void ChromeOSMetricsProvider::LogChromeOSCrash(const std::string& crash_type) {
+  if (crash_type == "user")
+    IncrementPrefValue(prefs::kStabilityOtherUserCrashCount);
+  else if (crash_type == "kernel")
+    IncrementPrefValue(prefs::kStabilityKernelCrashCount);

[Alexei Svitkine (slow), 2014/05/23 08:41:47]

Can you move the registration of these prefs to a static RegisterPrefs() method
in this class and out of MetricsService?

[blundell, 2014/05/23 12:09:22]

Done.

+  else if (crash_type == "uncleanshutdown")
+    IncrementPrefValue(prefs::kStabilitySystemUncleanShutdownCount);
+  else
+    NOTREACHED() << "Unexpected Chrome OS crash type " << crash_type;
+
+  // TODO(blundell): Should this check be at the beginning to preserve current
+  // behavior?
+  // Wake up metrics logs sending if necessary now that new
+  // log data is available.
+  if (g_browser_process && g_browser_process->metrics_service())
+    g_browser_process->metrics_service()->OnApplicationNotIdle();
+}
+
+void ChromeOSMetricsProvider::OnDidCreateMetricsLog() {
+  if (chromeos::UserManager::IsInitialized()) {
+    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();
          iter != perf_data.end();
          ++iter) {
       uma_proto_->add_perf_data()->Swap(&(*iter));
     }
   }
 
-  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);
   else if (has_touch == gfx::Display::TOUCH_SUPPORT_UNAVAILABLE)
     hardware->set_internal_display_supports_touch(false);
   WriteExternalTouchscreensProto(hardware);
 }
 
-void MetricsLogChromeOS::WriteRealtimeStabilityAttributes(PrefService* pref) {
-  SystemProfileProto::Stability* stability =
-      uma_proto_->mutable_system_profile()->mutable_stability();
-
+void ChromeOSMetricsProvider::ProvideStabilityMetrics(
+    metrics::SystemProfileProto_Stability* stability_proto) {
+  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 =
       hardware->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) {
+  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())
       continue;
 
     PairedDevice* paired_device = bluetooth->add_paired_device();
     paired_device->set_bluetooth_class(device->GetBluetoothClass());
     paired_device->set_type(AsBluetoothDeviceType(device->GetDeviceType()));
 
     // |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;
 
       base::RemoveChars(address.substr(0, 9), ":", &vendor_prefix_str);
       DCHECK_EQ(6U, vendor_prefix_str.size());
       base::HexStringToUInt64(vendor_prefix_str, &vendor_prefix);
 
       paired_device->set_vendor_prefix(vendor_prefix);
     }
 
     switch (device->GetVendorIDSource()) {
       case device::BluetoothDevice::VENDOR_ID_BLUETOOTH:
         paired_device->set_vendor_id_source(PairedDevice::VENDOR_ID_BLUETOOTH);
         break;
       case device::BluetoothDevice::VENDOR_ID_USB:
         paired_device->set_vendor_id_source(PairedDevice::VENDOR_ID_USB);
         break;
       default:
         paired_device->set_vendor_id_source(PairedDevice::VENDOR_ID_UNKNOWN);
     }
 
     paired_device->set_vendor_id(device->GetVendorID());
     paired_device->set_product_id(device->GetProductID());
     paired_device->set_device_id(device->GetDeviceID());
   }
 }
 
-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 (user_count != user_count_at_log_initialization_) {

[Alexei Svitkine (slow), 2014/05/23 08:41:47]

Hmm, what happens if chromeos::UserManager::IsInitialized() is false during
initialization?

The previous code handled that via the
system_profile->has_multi_profile_user_count() check. Perhaps you could add a
bool member to track this to preserve the old behaviour?

[blundell, 2014/05/23 12:09:22]

Done.

       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;
 }