Recategorize ARC auth sign in errors.

chrome/browser/chromeos/arc/arc_optin_uma.h

Index: chrome/browser/chromeos/arc/arc_optin_uma.h
diff --git a/chrome/browser/chromeos/arc/arc_optin_uma.h b/chrome/browser/chromeos/arc/arc_optin_uma.h
index 28246cfb329b9d1ebf8a08697930b459962315df..a2f96a2c00a93d47211413e348a63e021f49d123 100644
--- a/chrome/browser/chromeos/arc/arc_optin_uma.h
+++ b/chrome/browser/chromeos/arc/arc_optin_uma.h
@@ -24,26 +24,67 @@ enum class OptInActionType : int {
 };
 
 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);