Refactor MetricsLogChromeOS to ChromeOSMetricsProvider.

chrome/browser/metrics/chromeos_metrics_provider.cc

Index: chrome/browser/metrics/chromeos_metrics_provider.cc
diff --git a/chrome/browser/metrics/metrics_log_chromeos.cc b/chrome/browser/metrics/chromeos_metrics_provider.cc
similarity index 67%
rename from chrome/browser/metrics/metrics_log_chromeos.cc
rename to chrome/browser/metrics/chromeos_metrics_provider.cc
index ecab03038ab8f19057fa1042177784aa893f500d..f59871b7ad31fe042d4e10a3f6a81f5c102b472a 100644
--- a/chrome/browser/metrics/metrics_log_chromeos.cc
+++ b/chrome/browser/metrics/chromeos_metrics_provider.cc
@@ -2,13 +2,15 @@
 // 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"
@@ -80,17 +82,50 @@ 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()
+    : user_count_at_log_initialization_(0) {
+}
+
+ChromeOSMetricsProvider::~ChromeOSMetricsProvider() {
+}
+
+// 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();
 }
 
-MetricsLogChromeOS::MetricsLogChromeOS(ChromeUserMetricsExtension* uma_proto)
-    : uma_proto_(uma_proto) {
-  UpdateMultiProfileUserCount();
+void ChromeOSMetricsProvider::OnDidCreateMetricsLog() {
+  if (chromeos::UserManager::IsInitialized()) {
+    user_count_at_log_initialization_ =
+        chromeos::UserManager::Get()->GetLoggedInUsers().size();
+  }
 }
 
-void MetricsLogChromeOS::LogChromeOSMetrics() {
+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 +135,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 +148,37 @@ 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_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 =
@@ -154,8 +188,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 +203,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 +232,21 @@ 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 (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;
 }