Merge duplicate histogram naming by SecurityLevel.

This CL removes three similar sections of code that generate histogram
names with a SecurityLevel suffix and replaces them with
a shared implementation in the security_state namespace. This is to
facilitate future growth of that histogram naming pattern.

Bug: 852479
Change-Id: Ic27bc65539108e672e64168c34dd80fa27a34b02
Reviewed-on: https://chromium-review.googlesource.com/c/1318695Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Adrienne Porter Felt <felt@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606549}

chrome/browser/page_load_metrics/observers/security_state_page_load_metrics_observer.cc

@@ -32,30 +32,6 @@ const char kTimeOnPagePrefix[] = "Security.TimeOnPage";
 const char kSecurityLevelOnCommit[] = "Security.SecurityLevel.OnCommit";
 const char kSecurityLevelOnComplete[] = "Security.SecurityLevel.OnComplete";
 
-std::string GetHistogramSuffixForSecurityLevel(
-    security_state::SecurityLevel level) {
-  switch (level) {
-    case security_state::EV_SECURE:
-      return "EV_SECURE";
-    case security_state::SECURE:
-      return "SECURE";
-    case security_state::NONE:
-      return "NONE";
-    case security_state::HTTP_SHOW_WARNING:
-      return "HTTP_SHOW_WARNING";
-    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
-      return "SECURE_WITH_POLICY_INSTALLED_CERT";
-    case security_state::DANGEROUS:
-      return "DANGEROUS";
-    default:
-      return "OTHER";
-  }
-}
-
-std::string GetHistogramName(const char* prefix,
-                             security_state::SecurityLevel level) {
-  return std::string(prefix) + "." + GetHistogramSuffixForSecurityLevel(level);
-}
 }  // namespace
 
 // static
@@ -77,21 +53,24 @@ SecurityStatePageLoadMetricsObserver::MaybeCreateForProfile(
 std::string
 SecurityStatePageLoadMetricsObserver::GetEngagementDeltaHistogramNameForTesting(
     security_state::SecurityLevel level) {
-  return GetHistogramName(kEngagementDeltaPrefix, level);
+  return security_state::GetSecurityLevelHistogramName(
+      kEngagementDeltaPrefix, level);
 }
 
 // static
 std::string
 SecurityStatePageLoadMetricsObserver::GetEngagementFinalHistogramNameForTesting(
     security_state::SecurityLevel level) {
-  return GetHistogramName(kEngagementFinalPrefix, level);
+  return security_state::GetSecurityLevelHistogramName(
+      kEngagementFinalPrefix, level);
 }
 
 // static
 std::string
 SecurityStatePageLoadMetricsObserver::GetPageEndReasonHistogramNameForTesting(
     security_state::SecurityLevel level) {
-  return GetHistogramName(kPageEndReasonPrefix, level);
+  return security_state::GetSecurityLevelHistogramName(
+      kPageEndReasonPrefix, level);
 }
 
 SecurityStatePageLoadMetricsObserver::SecurityStatePageLoadMetricsObserver(
@@ -189,18 +168,22 @@ void SecurityStatePageLoadMetricsObserver::OnComplete(
     int delta = std::round(
         (final_engagement_score - initial_engagement_score_ + 100) / 2);
     base::UmaHistogramExactLinear(
-        GetHistogramName(kEngagementDeltaPrefix, current_security_level_),
+        security_state::GetSecurityLevelHistogramName(
+            kEngagementDeltaPrefix, current_security_level_),
         delta, 100);
     base::UmaHistogramExactLinear(
-        GetHistogramName(kEngagementFinalPrefix, current_security_level_),
+        security_state::GetSecurityLevelHistogramName(
+            kEngagementFinalPrefix, current_security_level_),
         final_engagement_score, 100);
   }
 
   base::UmaHistogramEnumeration(
-      GetHistogramName(kPageEndReasonPrefix, current_security_level_),
+      security_state::GetSecurityLevelHistogramName(
+          kPageEndReasonPrefix, current_security_level_),
       extra_info.page_end_reason, page_load_metrics::PAGE_END_REASON_COUNT);
   base::UmaHistogramCustomTimes(
-      GetHistogramName(kTimeOnPagePrefix, current_security_level_),
+      security_state::GetSecurityLevelHistogramName(
+          kTimeOnPagePrefix, current_security_level_),
       foreground_time_, base::TimeDelta::FromMilliseconds(1),
       base::TimeDelta::FromHours(1), 100);
   base::UmaHistogramEnumeration(kSecurityLevelOnComplete,

chrome/browser/ui/page_info/page_info.cc

@@ -309,31 +309,6 @@ const char kPageInfoTimeActionPrefix[] = "Security.PageInfo.TimeOpen.Action";
 const char kPageInfoTimeNoActionPrefix[] =
     "Security.PageInfo.TimeOpen.NoAction";
 
-std::string GetHistogramSuffixForSecurityLevel(
-    security_state::SecurityLevel level) {
-  switch (level) {
-    case security_state::EV_SECURE:
-      return "EV_SECURE";
-    case security_state::SECURE:
-      return "SECURE";
-    case security_state::NONE:
-      return "NONE";
-    case security_state::HTTP_SHOW_WARNING:
-      return "HTTP_SHOW_WARNING";
-    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
-      return "SECURE_WITH_POLICY_INSTALLED_CERT";
-    case security_state::DANGEROUS:
-      return "DANGEROUS";
-    default:
-      return "OTHER";
-  }
-}
-
-std::string GetHistogramName(const char* prefix,
-                             security_state::SecurityLevel level) {
-  return std::string(prefix) + "." + GetHistogramSuffixForSecurityLevel(level);
-}
-
 }  // namespace
 
 PageInfo::PageInfo(PageInfoUI* ui,
@@ -394,18 +369,21 @@ PageInfo::~PageInfo() {
   // Record the total time the Page Info UI was open for all opens as well as
   // split between whether any action was taken.
   base::UmaHistogramCustomTimes(
-      GetHistogramName(kPageInfoTimePrefix, security_level_),
+      security_state::GetSecurityLevelHistogramName(
+          kPageInfoTimePrefix, security_level_),
       base::TimeTicks::Now() - start_time_,
       base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1), 100);
   if (did_perform_action_) {
     base::UmaHistogramCustomTimes(
-        GetHistogramName(kPageInfoTimeActionPrefix, security_level_),
+        security_state::GetSecurityLevelHistogramName(
+            kPageInfoTimeActionPrefix, security_level_),
         base::TimeTicks::Now() - start_time_,
         base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1),
         100);
   } else {
     base::UmaHistogramCustomTimes(
-        GetHistogramName(kPageInfoTimeNoActionPrefix, security_level_),
+        security_state::GetSecurityLevelHistogramName(
+            kPageInfoTimeNoActionPrefix, security_level_),
         base::TimeTicks::Now() - start_time_,
         base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1),
         100);

components/autofill/core/browser/autofill_metrics.cc

@@ -90,31 +90,6 @@ int64_t HashFieldSignature(autofill::FieldSignature field_signature) {
   return static_cast<uint64_t>(field_signature) % 1021;
 }
 
-std::string GetHistogramSuffixForSecurityLevel(
-    security_state::SecurityLevel level) {
-  switch (level) {
-    case security_state::EV_SECURE:
-      return "EV_SECURE";
-    case security_state::SECURE:
-      return "SECURE";
-    case security_state::NONE:
-      return "NONE";
-    case security_state::HTTP_SHOW_WARNING:
-      return "HTTP_SHOW_WARNING";
-    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
-      return "SECURE_WITH_POLICY_INSTALLED_CERT";
-    case security_state::DANGEROUS:
-      return "DANGEROUS";
-    default:
-      return "OTHER";
-  }
-}
-
-std::string GetSecurityLevelHistogramName(const std::string prefix,
-                                          security_state::SecurityLevel level) {
-  return prefix + "." + GetHistogramSuffixForSecurityLevel(level);
-}
-
 }  // namespace
 
 // First, translates |field_type| to the corresponding logical |group| from
@@ -808,8 +783,9 @@ void AutofillMetrics::LogSaveCardPromptMetricBySecurityLevel(
   }
 
   base::UmaHistogramEnumeration(
-      GetSecurityLevelHistogramName(histogram_name, security_level), metric,
-      NUM_SAVE_CARD_PROMPT_METRICS);
+      security_state::GetSecurityLevelHistogramName(
+          histogram_name, security_level),
+      metric, NUM_SAVE_CARD_PROMPT_METRICS);
 }
 
 // static
@@ -1192,8 +1168,9 @@ void AutofillMetrics::LogUserHappinessBySecurityLevel(
   }
 
   base::UmaHistogramEnumeration(
-      GetSecurityLevelHistogramName(histogram_name, security_level), metric,
-      NUM_USER_HAPPINESS_METRICS);
+      security_state::GetSecurityLevelHistogramName(
+          histogram_name, security_level),
+      metric, NUM_USER_HAPPINESS_METRICS);
 }
 
 // static

components/security_state/core/security_state.cc

@@ -288,6 +288,26 @@ void SecurityInfoForRequest(
       visible_security_state.insecure_input_events;
 }
 
+std::string GetHistogramSuffixForSecurityLevel(
+    security_state::SecurityLevel level) {
+  switch (level) {
+    case EV_SECURE:
+      return "EV_SECURE";
+    case SECURE:
+      return "SECURE";
+    case NONE:
+      return "NONE";
+    case HTTP_SHOW_WARNING:
+      return "HTTP_SHOW_WARNING";
+    case SECURE_WITH_POLICY_INSTALLED_CERT:
+      return "SECURE_WITH_POLICY_INSTALLED_CERT";
+    case DANGEROUS:
+      return "DANGEROUS";
+    default:
+      return "OTHER";
+  }
+}
+
 }  // namespace
 
 SecurityInfo::SecurityInfo()
@@ -354,4 +374,10 @@ bool IsSslCertificateValid(SecurityLevel security_level) {
          security_level == SECURE_WITH_POLICY_INSTALLED_CERT;
 }
 
+std::string GetSecurityLevelHistogramName(
+    const std::string& prefix,
+    security_state::SecurityLevel level) {
+  return prefix + "." + GetHistogramSuffixForSecurityLevel(level);
+}
+
 }  // namespace security_state

components/security_state/core/security_state.h

@@ -241,6 +241,10 @@ bool IsOriginLocalhostOrFile(const GURL& url);
 // SECURE, and SECURE_WITH_POLICY_INSTALLED_CERT are considered valid.
 bool IsSslCertificateValid(security_state::SecurityLevel security_level);
 
+// Returns the given prefix suffixed with a dot and the current security level.
+std::string GetSecurityLevelHistogramName(
+    const std::string& prefix, security_state::SecurityLevel level);
+
 }  // namespace security_state
 
 #endif  // COMPONENTS_SECURITY_STATE_CORE_SECURITY_STATE_H_