Recategorize ARC auth sign in errors.

chrome/browser/chromeos/arc/arc_optin_uma.h

 // Copyright 2016 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_ARC_OPTIN_UMA_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ARC_OPTIN_UMA_H_
 
 namespace base {
 class TimeDelta;
 }
 
 namespace arc {
 
 // These enums are used to define the buckets for an enumerated UMA histogram
 // and need to be synced with histograms.xml
 enum class OptInActionType : int {
   OPTED_OUT = 0,               // Arc was opted out by user.
   OPTED_IN = 1,                // Arc was opted in by user.
   NOTIFICATION_ACCEPTED = 2,   // Arc OptIn notification was accepted.
   NOTIFICATION_DECLINED = 3,   // Arc OptIn notification was declined.
   NOTIFICATION_TIMED_OUT = 4,  // Arc OptIn notification was timed out.
   RETRY = 5,                   // User asked to retry OptIn.
   SIZE,                        // The size of this enum; keep last.
 };
 
 enum class OptInCancelReason {
-  USER_CANCEL = 0,                  // Canceled by user.
-  UNKNOWN_ERROR = 1,                // Unclassified failure.
-  NETWORK_ERROR = 2,                // Network failure.
-  SERVICE_UNAVAILABLE = 3,          // GMS Services are not available.
-  BAD_AUTHENTICATION = 4,           // Bad authentication returned by server.
-  GMS_CORE_NOT_AVAILABLE = 5,       // GMS Core is not available.
+  USER_CANCEL = 0,          // Canceled by user.
+  UNKNOWN_ERROR = 1,        // Unclassified failure.
+  NETWORK_ERROR = 2,        // Network failure.
+  SERVICE_UNAVAILABLE = 3,  // GMS Services are not available.
+  DEPRECATED_BAD_AUTHENTICATION = 4,
+  DEPRECATED_GMS_CORE_NOT_AVAILABLE = 5,
   CLOUD_PROVISION_FLOW_FAIL = 6,    // Cloud provision flow failed.
   ANDROID_MANAGEMENT_REQUIRED = 7,  // Android management is required for user.
   SIZE,                             // The size of this enum; keep last.
 };
 
-enum class ProvisioningResult {
-  SUCCESS = 0,                    // Provisioning was successful.
-  UNKNOWN_ERROR = 1,              // Unclassified failure.
-  NETWORK_ERROR = 2,              // Network failure.
-  SERVICE_UNAVAILABLE = 3,        // GMS Services are not available.
-  BAD_AUTHENTICATION = 4,         // Bad authentication returned by server.
-  GMS_CORE_NOT_AVAILABLE = 5,     // GMS Core is not available.
-  CLOUD_PROVISION_FLOW_FAIL = 6,  // Cloud provision flow failed.
-  SIZE,                           // The size of this enum; keep last.
+enum class ProvisioningResult : int {
+  // Provisioning was successful.
+  SUCCESS = 0,
+
+  // Unclassified failure.
+  UNKNOWN_ERROR = 1,
+
+  // Network failure in GMS sign in.
+  GMS_NETWORK_ERROR = 2,
+
+  // GMS Services are not available.
+  GMS_SERVICE_UNAVAILABLE = 3,
+
+  // Bad authentication returned by server.
+  GMS_BAD_AUTHENTICATION = 4,
+
+  // Failed to Check in on the device.

[victorhsieh, 2016/07/25 20:30:41]

nit: checkin

[hidehiko, 2016/07/26 13:02:58]

Done.

+  DEVICE_CHECK_IN_FAILED = 5,
+
+  // Cloud provision flow failed.
+  CLOUD_PROVISION_FLOW_FAILED = 6,
+
+  // ArcAuthHost API version mismatch.
+  MOJO_VERSION_MISMATCH = 7,
+
+  // Invocation of ArcAuthHost API is timed out.
+  MOJO_CALL_TIMEOUT = 8,
+
+  // Failed to sign-in on GMS. No detailed info.
+  GMS_SIGN_IN_FAILED = 9,
+
+  // GMS sign-in is timed out.
+  GMS_SIGN_IN_TIMEOUT = 10,
+
+  // Empty consumer package when getting token.
+  GMS_EMPTY_CONSUMER_PKG_OR_SIG = 11,
+
+  // 2-Factor errors.
+  GMS_NEEDS_2F = 12,
+  GMS_NEEDS_POST_SIGN_IN_FLOW = 13,
+  GMS_NEEDS_BROWSER = 14,
+
+  // Cloud provision flow is timed out.
+  CLOUD_PROVISION_FLOW_TIMEOUT = 15,

[victorhsieh, 2016/07/25 20:30:42]

nit/optional: put this next to CLOUD_PROVISION_FLOW_FAILED?

[hidehiko, 2016/07/26 13:02:58]

Can I keep this as is?
1 - 6 are in the order for backward compatibility.

[victorhsieh, 2016/07/26 18:22:11]

Not sure I understand this.  Aren't their value explicitly specificed?  Or are
they kept in order so that it's easier to understand, and if this is the case,
wouldn't a comment be more helpful?

[hidehiko, 2016/07/26 18:50:48]

Sorry for my poor explanation.
This is an enum designed to put "SIZE" at the end as a coding techinque. So,
IMHO shuffling the values (like what I did in .mojom) may look confusing? WDYT?

Note that moving just CLOUD_PROVISION_FLOW_TIMEOUT to just after
CLOUD_PROVISION_FLOW_FAILED seems inconsistent to me, because other GMS_ errors
are kept there.

[victorhsieh, 2016/07/26 19:52:42]

The style is probably subjective, especially this is an enum.  I still prefer to
move CLOUD_PROVISION_FLOW_TIMEOUT, but it's up to you.

[hidehiko, 2016/07/27 17:36:12]

Looks technical stuff, because "putting SIZE at last" requires that the values
is beginning from 0, and consecutively ascending order.

Anyway, let me keep this as is, now. I'm happy to change the code later, but I'd
like to focus on "adding enums" in this CL. Added short comment instead.

[victorhsieh, 2016/07/27 18:03:05]

I thought SIZE's value is derived from the previous assigned value, in this case
16 + 1?  But anyway.

+
+  // ARC instance is stopped during the sign in procedure.
+  ARC_STOPPED = 16,
+
+  // The size of this enum; keep last.
+  SIZE,
 };
 
 void UpdateOptInActionUMA(OptInActionType type);
 void UpdateOptInCancelUMA(OptInCancelReason reason);
 void UpdateEnabledStateUMA(bool enabled);
 void UpdateProvisioningResultUMA(ProvisioningResult result);
 void UpdateProvisioningTiming(const base::TimeDelta& elapsed_time,
                               bool success,
                               bool managed);
 
 }  // namespace arc
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_ARC_OPTIN_UMA_H_