Enrollment recovery: Add UMA stats.

chrome/browser/chromeos/login/enrollment/enrollment_screen.cc

Index: chrome/browser/chromeos/login/enrollment/enrollment_screen.cc
diff --git a/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc b/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc
index f68556c931ce96ae56f35803a2cd425ea3a54895..9612e1dc2ceb3911bf871ba1c12027732aafce61 100644
--- a/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc
+++ b/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc
@@ -21,22 +21,11 @@
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
-#include "components/policy/core/common/cloud/enterprise_metrics.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 
 namespace chromeos {
 
-namespace {
-
-void UMA(int sample) {
-  UMA_HISTOGRAM_ENUMERATION(policy::kMetricEnrollment,
-                            sample,
-                            policy::kMetricEnrollmentSize);
-}
-
-}  // namespace
-
 EnrollmentScreen::EnrollmentScreen(
     ScreenObserver* observer,
     EnrollmentScreenActor* actor)
@@ -74,6 +63,7 @@ void EnrollmentScreen::Show() {
     actor_->ShowEnrollmentSpinnerScreen();
     actor_->FetchOAuthToken();
   } else {
+    UMA(policy::kMetricEnrollmentTriggered);
     actor_->ResetAuth(base::Bind(&EnrollmentScreen::ShowSigninScreen,
                                  weak_ptr_factory_.GetWeakPtr()));
   }
@@ -91,15 +81,18 @@ std::string EnrollmentScreen::GetName() const {
 void EnrollmentScreen::OnLoginDone(const std::string& user) {
   user_ = gaia::CanonicalizeEmail(user);
 
-  UMA(is_auto_enrollment() ? policy::kMetricEnrollmentAutoRetried
-                           : policy::kMetricEnrollmentStarted);
+  if (is_auto_enrollment())
+    UMA(policy::kMetricEnrollmentAutoRetried);
+  else if (enrollment_failed_once_)
+    UMA(policy::kMetricEnrollmentRetried);
+  else
+    UMA(policy::kMetricEnrollmentStarted);
 
   actor_->ShowEnrollmentSpinnerScreen();
   actor_->FetchOAuthToken();
 }
 
-void EnrollmentScreen::OnAuthError(
-    const GoogleServiceAuthError& error) {
+void EnrollmentScreen::OnAuthError(const GoogleServiceAuthError& error) {
   enrollment_failed_once_ = true;
   actor_->ShowAuthError(error);
 
@@ -145,7 +138,8 @@ void EnrollmentScreen::OnRetry() {
 }
 
 void EnrollmentScreen::OnCancel() {
-  if (enrollment_mode_ == EnrollmentScreenActor::ENROLLMENT_MODE_FORCED) {
+  if (enrollment_mode_ == EnrollmentScreenActor::ENROLLMENT_MODE_FORCED ||
+      enrollment_mode_ == EnrollmentScreenActor::ENROLLMENT_MODE_RECOVERY) {

[pastarmovj, 2014/07/14 08:46:06]

Is this change part of simply adding the UMAs? It seems to do more than that.

[Thiemo Nagel, 2014/07/14 10:30:35]

This CL introduces ENROLLMENT_MODE_RECOVERY which didn't previously exist.  (I
used to piggy-back on ENROLLMENT_MODE_FORCED but this turns out to not work very
well when recovery-specific logging comes into play.)  The control flow should
be unchanged.

     actor_->ResetAuth(
         base::Bind(&ScreenObserver::OnExit,
                    base::Unretained(get_screen_observer()),
@@ -308,7 +302,17 @@ void EnrollmentScreen::ReportEnrollmentStatus(
   UMAFailure(policy::kMetricEnrollmentOtherFailed);
 }
 
-void EnrollmentScreen::UMAFailure(int sample) {
+void EnrollmentScreen::UMA(policy::MetricEnrollment sample) {
+  const char* histogram;
+  if (enrollment_mode_ == EnrollmentScreenActor::ENROLLMENT_MODE_RECOVERY)
+    histogram = policy::kMetricEnrollmentRecovery;
+  else
+    histogram = policy::kMetricEnrollment;
+  UMA_HISTOGRAM_ENUMERATION(histogram, sample,
+                            policy::kMetricEnrollmentSize);
+}
+
+void EnrollmentScreen::UMAFailure(policy::MetricEnrollment sample) {
   if (is_auto_enrollment())

[pastarmovj, 2014/07/14 08:46:06]

Since auto enrollment is going away anyway can you instead merge this in a
single UMA function. It seems to me there is no need to have two.

[Thiemo Nagel, 2014/07/14 10:30:35]

I fully agree, but I'd rather do that in a separate CL (which will not be
back-merged).

     sample = policy::kMetricEnrollmentAutoFailed;
   UMA(sample);