Enrollment recovery: Add UMA stats.

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

 // Copyright (c) 2012 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/chromeos/login/enrollment/enrollment_screen.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/login_utils.h"
 #include "chrome/browser/chromeos/login/screens/screen_observer.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/chromeos/policy/auto_enrollment_client.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_cloud_policy_initializer.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #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)
     : WizardScreen(observer),
       actor_(actor),
       enrollment_mode_(EnrollmentScreenActor::ENROLLMENT_MODE_MANUAL),
       enrollment_failed_once_(false),
       lockbox_init_duration_(0),
       weak_ptr_factory_(this) {
   // Init the TPM if it has not been done until now (in debug build we might
@@ skipping 12 matching lines @@
   user_ = user.empty() ? user : gaia::CanonicalizeEmail(user);
   actor_->SetParameters(this, enrollment_mode_, management_domain);
 }
 
 void EnrollmentScreen::PrepareToShow() {
   actor_->PrepareToShow();
 }
 
 void EnrollmentScreen::Show() {
   if (is_auto_enrollment() && !enrollment_failed_once_) {
     actor_->Show();

[Joao da Silva, 2014/07/14 10:15:50]

Nothing is sampled in this case?

[Thiemo Nagel, 2014/07/14 11:04:30]

This code path is just for auto-enrollment which I don't care about and which
will go away soon, anyways.

     UMA(policy::kMetricEnrollmentAutoStarted);
     actor_->ShowEnrollmentSpinnerScreen();
     actor_->FetchOAuthToken();
   } else {
+    UMA(policy::kMetricEnrollmentTriggered);
     actor_->ResetAuth(base::Bind(&EnrollmentScreen::ShowSigninScreen,
                                  weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
 void EnrollmentScreen::Hide() {
   actor_->Hide();
   weak_ptr_factory_.InvalidateWeakPtrs();
 }
 
 std::string EnrollmentScreen::GetName() const {
   return WizardController::kEnrollmentScreenName;
 }
 
 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);
 
   switch (error.state()) {
     case GoogleServiceAuthError::NONE:
     case GoogleServiceAuthError::CAPTCHA_REQUIRED:
     case GoogleServiceAuthError::TWO_FACTOR:
     case GoogleServiceAuthError::HOSTED_NOT_ALLOWED:
     case GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS:
     case GoogleServiceAuthError::REQUEST_CANCELED:
@@ skipping 25 matching lines @@
     const std::string& token) {
   RegisterForDevicePolicy(token);
 }
 
 void EnrollmentScreen::OnRetry() {
   actor_->ResetAuth(base::Bind(&EnrollmentScreen::ShowSigninScreen,
                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 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()),
                    ScreenObserver::ENTERPRISE_ENROLLMENT_BACK));
     return;
   }
 
   if (is_auto_enrollment())
     policy::AutoEnrollmentClient::CancelAutoEnrollment();
   UMA(is_auto_enrollment() ? policy::kMetricEnrollmentAutoCancelled
@@ skipping 142 matching lines @@
       return;
     case policy::EnrollmentStatus::STATUS_SUCCESS:
       NOTREACHED();
       return;
   }
 
   NOTREACHED();
   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);
 }
 
 void EnrollmentScreen::ShowSigninScreen() {
   actor_->Show();
   actor_->ShowSigninScreen();
 }
 
 }  // namespace chromeos