Let MetricsService know about some Android Activities

chrome/browser/metrics/metrics_service_android.cc

Index: chrome/browser/metrics/metrics_service_android.cc
diff --git a/chrome/browser/metrics/metrics_service_android.cc b/chrome/browser/metrics/metrics_service_android.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7c753fe34fe5e70037a1ebadbb89709af9e7b348
--- /dev/null
+++ b/chrome/browser/metrics/metrics_service_android.cc
@@ -0,0 +1,127 @@
+// Copyright 2013 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_service.h"
+
+#include "base/metrics/sparse_histogram.h"
+#include "base/prefs/pref_registry_simple.h"
+#include "base/prefs/pref_service.h"
+#include "chrome/browser/android/activity_type_ids.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/common/pref_names.h"
+
+void MetricsService::OnForegroundActivityChanged(ActivityTypeIds::Type type) {
+  PrefService* pref = g_browser_process->local_state();
+
+  // Record that the Activity is now in the foreground.
+  pref->SetInteger(prefs::kStabilityForegroundActivity, type);
+
+  // Record that the Activity was launched.
+  int launched_activities =
+      pref->GetInteger(prefs::kStabilityLaunchedActivityFlags) | type;
+  pref->SetInteger(prefs::kStabilityLaunchedActivityFlags, launched_activities);
+
+  pref->CommitPendingWrite();
+}

[Ilya Sherman, 2013/12/16 23:54:59]

nit: Please arrange this method in the same order in the impl file as in the
header file.  (I didn't check the other methods, but this comment applies
throughout.)

[gone, 2013/12/17 01:34:31]

Done.

+
+// static
+void MetricsService::RegisterPrefsAndroid(PrefRegistrySimple* registry) {
+  registry->RegisterIntegerPref(prefs::kStabilityForegroundActivity,
+                                ActivityTypeIds::ACTIVITY_NONE);
+  registry->RegisterIntegerPref(prefs::kStabilityLaunchedActivityFlags,
+                                ActivityTypeIds::ACTIVITY_NONE);
+
+  registry->RegisterIntegerPref(prefs::kStabilityLaunchCountMainActivity, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityLaunchCountFullScreenActivity,
+                                0);
+
+  registry->RegisterIntegerPref(prefs::kStabilityCrashCountMainActivity, 0);
+  registry->RegisterIntegerPref(prefs::kStabilityCrashCountFullScreenActivity,
+                                0);
+}
+
+// static
+void MetricsService::DiscardOldStabilityStatsAndroid(PrefService* local_state) {
+  local_state->SetInteger(prefs::kStabilityForegroundActivity,
+                          ActivityTypeIds::ACTIVITY_NONE);
+  local_state->SetInteger(prefs::kStabilityLaunchedActivityFlags,
+                          ActivityTypeIds::ACTIVITY_NONE);
+
+  local_state->SetInteger(prefs::kStabilityLaunchCountMainActivity, 0);
+  local_state->SetInteger(prefs::kStabilityLaunchCountFullScreenActivity, 0);
+
+  local_state->SetInteger(prefs::kStabilityCrashCountMainActivity, 0);
+  local_state->SetInteger(prefs::kStabilityCrashCountFullScreenActivity, 0);
+}
+
+void MetricsService::RecordAndroidStabilityPrefs() {
+  PrefService* pref = g_browser_process->local_state();
+  DCHECK(pref);

[Ilya Sherman, 2013/12/16 23:54:59]

nit: No need for this, since you directly access the object on line 64.

[gone, 2013/12/17 01:34:31]

Done.

+
+  // Track which Activities were launched by the user.
+  int launched_activities =
+      pref->GetInteger(prefs::kStabilityLaunchedActivityFlags);
+  if (launched_activities & ActivityTypeIds::ACTIVITY_MAIN) {
+    IncrementPrefValue(prefs::kStabilityLaunchCountMainActivity);
+  }

[Ilya Sherman, 2013/12/16 23:54:59]

nit: No need for curly braces.

[gone, 2013/12/17 01:34:31]

Done.

+  if (launched_activities & ActivityTypeIds::ACTIVITY_FULLSCREEN) {
+    IncrementPrefValue(prefs::kStabilityLaunchCountFullScreenActivity);
+  }

[Ilya Sherman, 2013/12/16 23:54:59]

nit: No need for curly braces.

[gone, 2013/12/17 01:34:31]

Done.

+  pref->SetInteger(prefs::kStabilityLaunchedActivityFlags,
+                   ActivityTypeIds::ACTIVITY_NONE);
+
+  // If an Activity was in the foreground when Chrome died, then Chrome was
+  // improperly shut down.  Record it as a crash for that particular Activity.
+  int foreground = pref->GetInteger(prefs::kStabilityForegroundActivity);
+  switch (foreground) {
+    case ActivityTypeIds::ACTIVITY_MAIN:
+      IncrementPrefValue(prefs::kStabilityCrashCountMainActivity);
+      break;
+    case ActivityTypeIds::ACTIVITY_FULLSCREEN:
+      IncrementPrefValue(prefs::kStabilityCrashCountFullScreenActivity);
+      break;
+    default:
+      break;
+  }
+  pref->SetInteger(prefs::kStabilityForegroundActivity,
+                   ActivityTypeIds::ACTIVITY_NONE);
+}
+
+void MetricsService::RecordAndroidStabilityHistograms() {
+  PrefService* pref = g_browser_process->local_state();
+
+  // Track which Activities were launched by the user.
+  int main_launch_count =
+      pref->GetInteger(prefs::kStabilityLaunchCountMainActivity);
+  for (int i = 0; i < main_launch_count; ++i) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Chrome.Android.Activity.LaunchCounts",
+                                ActivityTypeIds::ACTIVITY_MAIN);
+  }

[Ilya Sherman, 2013/12/16 23:54:59]

This is somewhat inefficient.  Any reason not to use a regular enumerated
histogram instead?

[gone, 2013/12/17 00:17:28]

No reason other than I'm not familiar with all the different histogram types. 
The stat we want in the end is just the number of times a launch or crash
happened, summed up across all devices.  Each device should just send how many
times a launch or crash happened across all of a user's sessions.  Would the
regular enumerated histogram be fine for that, or would that just result in
bucketing users by how many times they crashed or launched Chrome between
uploads?

[Ilya Sherman, 2013/12/17 00:32:22]

An enumerated histogram should be fine for that.  The difference between an
enumerated histogram and a sparse histogram is mostly whether the histogram is
backed by a dense representation (vector) vs. a sparse representation (map).  In
this case, it looks like you're emitting to a histogram with only two buckets,
so a dense representation ought to be more efficient.

[gone, 2013/12/17 01:34:31]

I think I've adjusted for that use case now.

+  pref->SetInteger(prefs::kStabilityLaunchCountMainActivity, 0);
+
+  int full_screen_launch_count =
+      pref->GetInteger(prefs::kStabilityLaunchCountFullScreenActivity);
+  for (int i = 0; i < full_screen_launch_count; ++i) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Chrome.Android.Activity.LaunchCounts",
+                                ActivityTypeIds::ACTIVITY_FULLSCREEN);
+  }
+  pref->SetInteger(prefs::kStabilityLaunchCountFullScreenActivity, 0);
+
+  // Track which Activities were in the foreground when Chrome died.
+  int main_crash_count =
+      pref->GetInteger(prefs::kStabilityCrashCountMainActivity);
+  for (int i = 0; i < main_crash_count; ++i) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Chrome.Android.Activity.CrashCounts",
+                                ActivityTypeIds::ACTIVITY_MAIN);
+  }
+  pref->SetInteger(prefs::kStabilityCrashCountMainActivity, 0);
+
+  int full_screen_crash_count =
+      pref->GetInteger(prefs::kStabilityCrashCountFullScreenActivity);
+  for (int i = 0; i < full_screen_crash_count; ++i) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Chrome.Android.Activity.CrashCounts",
+                                ActivityTypeIds::ACTIVITY_FULLSCREEN);
+  }
+  pref->SetInteger(prefs::kStabilityCrashCountFullScreenActivity, 0);
+}