arc: Handle metrics for Auth operations.

chrome/browser/chromeos/arc/arc_optin_uma.cc

Index: chrome/browser/chromeos/arc/arc_optin_uma.cc
diff --git a/chrome/browser/chromeos/arc/arc_optin_uma.cc b/chrome/browser/chromeos/arc/arc_optin_uma.cc
index cea4e1c4a8762618f5ec4217a37c07fd0251ae4b..afed82857662f672b73ba6936e6bc496ae77ca3f 100644
--- a/chrome/browser/chromeos/arc/arc_optin_uma.cc
+++ b/chrome/browser/chromeos/arc/arc_optin_uma.cc
@@ -51,6 +51,20 @@ void UpdateProvisioningTiming(const base::TimeDelta& elapsed_time,
       ->AddTime(elapsed_time);
 }
 
+void UpdateAuthTiming(const char* histogram_name,
+                      base::TimeDelta elapsed_time) {
+  base::Histogram::FactoryTimeGet(
+      histogram_name, base::TimeDelta::FromSeconds(1) /* minimum */,
+      base::TimeDelta::FromMinutes(3) /* maximum */, 50 /* bucket_count */,
+      base::HistogramBase::kUmaTargetedHistogramFlag)
+      ->AddTime(elapsed_time);

[Ilya Sherman, 2017/02/23 21:23:34]

Please use the convenience wrapper base::UmaHistogramCustomTimes from
histogram_functions.h.

[khmel, 2017/02/23 21:57:45]

Done.

+}
+
+void UpdateAuthAttempts(const char* histogram_name, int32_t value) {
+  UMA_HISTOGRAM_CUSTOM_COUNTS(histogram_name, value, 0 /* minimum */,
+                              5 /* maximum */, 6 /* bucket_count */);

[Ilya Sherman, 2017/02/23 21:23:34]

Please use the convenience wrapper base::UmaHistogramExactLinear or
UMA_HISTOGRAM_SPARSE_SLOWLY if you need to record to multiple named histograms. 
Note that it's not safe to pass a runtime-variable name to most of the
UMA_HISTOGRAM macros (the one exception being UMA_HISTOGRAM_SPARSE_SLOWLY).

However, it looks like you are actually only ever passing a single histogram
name to this function.  Why pass in the name as a parameter at all?  I'd
recommend writing this function as

void UpdateAuthAttempts(int32_t num_attempts) {
  UMA_HISTOGRAM_SPARSE_SLOWLY("ArcAuth.CheckinAttempts", num_attempts);
}

(Note that I also renamed the generically named "value" to the clearer
"num_attempts".)

[khmel, 2017/02/23 21:57:45]

Thanks for detailed comment. I reserved it for possible extension and to keep it
aligned with prev function. I am ok to use name here.
Done

+}
+
 void UpdateSilentAuthCodeUMA(OptInSilentAuthCode state) {
   UMA_HISTOGRAM_ENUMERATION("Arc.OptInSilentAuthCode", static_cast<int>(state),
                             static_cast<int>(OptInSilentAuthCode::SIZE));