Explicit values for UMA enums in LoginAuthRecorder

1. Provide explicit values for those enums in the LoginAuthRecorder
   class that are logged as UMA metric values.
   This makes it more clear that those values shouldn't be renumbered,
   and makes it easier to find and verify the correspondence between
   the enum values in the C++ code and the <enum> values in enums.xml.

2. Use new-style UMA utils that deduct the enum size automatically
   based on the "kMaxValue" item.

Bug: none
Change-Id: I853e5cae5eaeb39765d02b4256af59b2d2c2eb93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724087
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682919}

chrome/browser/chromeos/login/login_auth_recorder.cc

@@ -15,7 +15,7 @@ using AuthMethodSwitchType = LoginAuthRecorder::AuthMethodSwitchType;
 
 namespace {
 
-AuthMethodSwitchType SwitchFromPasswordTo(AuthMethod current) {
+base::Optional<AuthMethodSwitchType> SwitchFromPasswordTo(AuthMethod current) {
   DCHECK_NE(AuthMethod::kPassword, current);
   switch (current) {
     case AuthMethod::kPin:
@@ -27,13 +27,12 @@ AuthMethodSwitchType SwitchFromPasswordTo(AuthMethod current) {
     case AuthMethod::kChallengeResponse:
       return AuthMethodSwitchType::kPasswordToChallengeResponse;
     case AuthMethod::kPassword:
-    case AuthMethod::kMethodCount:
       NOTREACHED();
-      return AuthMethodSwitchType::kSwitchTypeCount;
+      return base::nullopt;
   }
 }
 
-AuthMethodSwitchType SwitchFromPinTo(AuthMethod current) {
+base::Optional<AuthMethodSwitchType> SwitchFromPinTo(AuthMethod current) {
   DCHECK_NE(AuthMethod::kPin, current);
   switch (current) {
     case AuthMethod::kPassword:
@@ -44,13 +43,12 @@ AuthMethodSwitchType SwitchFromPinTo(AuthMethod current) {
       return AuthMethodSwitchType::kPinToFingerprint;
     case AuthMethod::kPin:
     case AuthMethod::kChallengeResponse:
-    case AuthMethod::kMethodCount:
       NOTREACHED();
-      return AuthMethodSwitchType::kSwitchTypeCount;
+      return base::nullopt;
   }
 }
 
-AuthMethodSwitchType SwitchFromSmartlockTo(AuthMethod current) {
+base::Optional<AuthMethodSwitchType> SwitchFromSmartlockTo(AuthMethod current) {
   DCHECK_NE(AuthMethod::kSmartlock, current);
   switch (current) {
     case AuthMethod::kPassword:
@@ -61,13 +59,13 @@ AuthMethodSwitchType SwitchFromSmartlockTo(AuthMethod current) {
       return AuthMethodSwitchType::kSmartlockToFingerprint;
     case AuthMethod::kSmartlock:
     case AuthMethod::kChallengeResponse:
-    case AuthMethod::kMethodCount:
       NOTREACHED();
-      return AuthMethodSwitchType::kSwitchTypeCount;
+      return base::nullopt;
   }
 }
 
-AuthMethodSwitchType SwitchFromFingerprintTo(AuthMethod current) {
+base::Optional<AuthMethodSwitchType> SwitchFromFingerprintTo(
+    AuthMethod current) {
   DCHECK_NE(AuthMethod::kFingerprint, current);
   switch (current) {
     case AuthMethod::kPassword:
@@ -78,13 +76,13 @@ AuthMethodSwitchType SwitchFromFingerprintTo(AuthMethod current) {
       return AuthMethodSwitchType::kFingerprintToPin;
     case AuthMethod::kFingerprint:
     case AuthMethod::kChallengeResponse:
-    case AuthMethod::kMethodCount:
       NOTREACHED();
-      return AuthMethodSwitchType::kSwitchTypeCount;
+      return base::nullopt;
   }
 }
 
-AuthMethodSwitchType FindSwitchType(AuthMethod previous, AuthMethod current) {
+base::Optional<AuthMethodSwitchType> FindSwitchType(AuthMethod previous,
+                                                    AuthMethod current) {
   DCHECK_NE(previous, current);
   switch (previous) {
     case AuthMethod::kPassword:
@@ -96,9 +94,8 @@ AuthMethodSwitchType FindSwitchType(AuthMethod previous, AuthMethod current) {
     case AuthMethod::kFingerprint:
       return SwitchFromFingerprintTo(current);
     case AuthMethod::kChallengeResponse:
-    case AuthMethod::kMethodCount:
       NOTREACHED();
-      return AuthMethodSwitchType::kSwitchTypeCount;
+      return base::nullopt;
   }
 }
 
@@ -113,7 +110,6 @@ LoginAuthRecorder::~LoginAuthRecorder() {
 }
 
 void LoginAuthRecorder::RecordAuthMethod(AuthMethod method) {
-  DCHECK_NE(method, AuthMethod::kMethodCount);
   if (session_manager::SessionManager::Get()->session_state() !=
       session_manager::SessionState::LOCKED) {
     return;
@@ -123,17 +119,20 @@ void LoginAuthRecorder::RecordAuthMethod(AuthMethod method) {
   const bool is_tablet_mode = TabletModeClient::Get()->tablet_mode_enabled();
   if (is_tablet_mode) {
     UMA_HISTOGRAM_ENUMERATION("Ash.Login.Lock.AuthMethod.Used.TabletMode",
-                              method, AuthMethod::kMethodCount);
+                              method);
   } else {
     UMA_HISTOGRAM_ENUMERATION("Ash.Login.Lock.AuthMethod.Used.ClamShellMode",
-                              method, AuthMethod::kMethodCount);
+                              method);
   }
 
   if (last_auth_method_ != method) {
     // Record switching between unlock methods.
-    UMA_HISTOGRAM_ENUMERATION("Ash.Login.Lock.AuthMethod.Switched",
-                              FindSwitchType(last_auth_method_, method),
-                              AuthMethodSwitchType::kSwitchTypeCount);
+    const base::Optional<AuthMethodSwitchType> switch_type =
+        FindSwitchType(last_auth_method_, method);
+    if (switch_type) {
+      UMA_HISTOGRAM_ENUMERATION("Ash.Login.Lock.AuthMethod.Switched",
+                                *switch_type);
+    }
 
     last_auth_method_ = method;
   }

chrome/browser/chromeos/login/login_auth_recorder.h

@@ -19,34 +19,34 @@ class LoginAuthRecorder : public session_manager::SessionManagerObserver {
  public:
   // Authentication method to unlock the screen. This enum is used to back an
   // UMA histogram and new values should be inserted immediately above
-  // kMethodCount.
+  // kMaxValue.
   enum class AuthMethod {
     kPassword = 0,
-    kPin,
-    kSmartlock,
-    kFingerprint,
-    kChallengeResponse,
-    kMethodCount,
+    kPin = 1,
+    kSmartlock = 2,
+    kFingerprint = 3,
+    kChallengeResponse = 4,
+    kMaxValue = kChallengeResponse,
   };
 
   // The type of switching between auth methods. This enum is used to back an
   // UMA histogram and new values should be inserted immediately above
-  // kSwitchTypeCount.
+  // kMaxValue.
   enum class AuthMethodSwitchType {
     kPasswordToPin = 0,
-    kPasswordToSmartlock,
-    kPinToPassword,
-    kPinToSmartlock,
-    kSmartlockToPassword,
-    kSmartlockToPin,
-    kPasswordToFingerprint,
-    kPinToFingerprint,
-    kSmartlockToFingerprint,
-    kFingerprintToPassword,
-    kFingerprintToPin,
-    kFingerprintToSmartlock,
-    kPasswordToChallengeResponse,
-    kSwitchTypeCount,
+    kPasswordToSmartlock = 1,
+    kPinToPassword = 2,
+    kPinToSmartlock = 3,
+    kSmartlockToPassword = 4,
+    kSmartlockToPin = 5,
+    kPasswordToFingerprint = 6,
+    kPinToFingerprint = 7,
+    kSmartlockToFingerprint = 8,
+    kFingerprintToPassword = 9,
+    kFingerprintToPin = 10,
+    kFingerprintToSmartlock = 11,
+    kPasswordToChallengeResponse = 12,
+    kMaxValue = kPasswordToChallengeResponse,
   };
 
   LoginAuthRecorder();