Use base histogram functions in //third_party/blink/renderer/core/css

Blink allows use of the //base histogram helpers, so there's no point in
having two ways of doing the same thing. Just use //base, which has
standardized definitions for recording "short" times (1ms - 10s), et
cetera.

Bug: 1047547
Change-Id: I8c573e4f29db102fe357ccf3d6eb95230c0abb0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033927
Auto-Submit: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738868}

third_party/blink/renderer/core/css/font_face_set_document.cc

@@ -25,6 +25,7 @@
 
 #include "third_party/blink/renderer/core/css/font_face_set_document.h"
 
+#include "base/metrics/histogram_functions.h"
 #include "third_party/blink/renderer/bindings/core/v8/dictionary.h"
 #include "third_party/blink/renderer/core/css/css_font_selector.h"
 #include "third_party/blink/renderer/core/css/css_property_value_set.h"
@@ -41,7 +42,6 @@
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
-#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
 
 namespace blink {
 
@@ -224,9 +224,7 @@ void FontFaceSetDocument::FontLoadHistogram::UpdateStatus(FontFace* font_face) {
 
 void FontFaceSetDocument::FontLoadHistogram::Record() {
   if (status_ == kHadBlankText || status_ == kDidNotHaveBlankText) {
-    DEFINE_STATIC_LOCAL(EnumerationHistogram, had_blank_text_histogram,
-                        ("WebFont.HadBlankText", 2));
-    had_blank_text_histogram.Count(status_ == kHadBlankText ? 1 : 0);
+    base::UmaHistogramBoolean("WebFont.HadBlankText", status_ == kHadBlankText);
     status_ = kReported;
   }
 }

third_party/blink/renderer/core/css/local_font_face_source.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/css/local_font_face_source.h"
 
+#include "base/metrics/histogram_functions.h"
 #include "build/build_config.h"
 #include "third_party/blink/renderer/core/css/css_custom_font_data.h"
 #include "third_party/blink/renderer/core/css/css_font_face.h"
@@ -14,7 +15,6 @@
 #include "third_party/blink/renderer/platform/fonts/font_unique_name_lookup.h"
 #include "third_party/blink/renderer/platform/fonts/simple_font_data.h"
 #include "third_party/blink/renderer/platform/heap/persistent.h"
-#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 
 namespace blink {
@@ -138,9 +138,7 @@ void LocalFontFaceSource::LocalFontHistograms::Record(bool load_success) {
   if (reported_)
     return;
   reported_ = true;
-  DEFINE_STATIC_LOCAL(EnumerationHistogram, local_font_used_histogram,
-                      ("WebFont.LocalFontUsed", 2));
-  local_font_used_histogram.Count(load_success ? 1 : 0);
+  base::UmaHistogramBoolean("WebFont.LocalFontUsed", load_success);
 }
 
 void LocalFontFaceSource::Trace(blink::Visitor* visitor) {

third_party/blink/renderer/core/css/remote_font_face_source.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/css/remote_font_face_source.h"
 
+#include "base/metrics/histogram_functions.h"
 #include "third_party/blink/public/mojom/feature_policy/feature_policy_feature.mojom-blink.h"
 #include "third_party/blink/public/platform/task_type.h"
 #include "third_party/blink/public/platform/web_effective_connection_type.h"
@@ -19,7 +20,6 @@
 #include "third_party/blink/renderer/platform/fonts/font_description.h"
 #include "third_party/blink/renderer/platform/fonts/font_selector.h"
 #include "third_party/blink/renderer/platform/fonts/simple_font_data.h"
-#include "third_party/blink/renderer/platform/instrumentation/histogram.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_load_priority.h"
 #include "third_party/blink/renderer/platform/network/network_state_notifier.h"
@@ -341,13 +341,10 @@ void RemoteFontFaceSource::FontLoadHistograms::LongLimitExceeded() {
 void RemoteFontFaceSource::FontLoadHistograms::RecordFallbackTime() {
   if (blank_paint_time_.is_null() || blank_paint_time_recorded_)
     return;
+  // TODO(https://crbug.com/1049257): This time should be recorded using a more
+  // appropriate UMA helper, since >1% of samples are in the overflow bucket.
   base::TimeDelta duration = base::TimeTicks::Now() - blank_paint_time_;
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(CustomCountHistogram,
-                                  blank_text_shown_time_histogram,
-                                  ("WebFont.BlankTextShownTime", 0, 10000, 50));
-  blank_text_shown_time_histogram.Count(
-      base::saturated_cast<base::HistogramBase::Sample>(
-          duration.InMilliseconds()));
+  base::UmaHistogramTimes("WebFont.BlankTextShownTime", duration);
   blank_paint_time_recorded_ = true;
 }
 
@@ -355,9 +352,7 @@ void RemoteFontFaceSource::FontLoadHistograms::RecordRemoteFont(
     const FontResource* font) {
   MaySetDataSource(DataSourceForLoadFinish(font));
 
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, cache_hit_histogram,
-                                  ("WebFont.CacheHit", kCacheHitEnumMax));
-  cache_hit_histogram.Count(DataSourceMetricsValue());
+  base::UmaHistogramEnumeration("WebFont.CacheHit", DataSourceMetricsValue());
 
   if (data_source_ == kFromDiskCache || data_source_ == kFromNetwork) {
     DCHECK(!load_start_time_.is_null());
@@ -383,98 +378,74 @@ void RemoteFontFaceSource::FontLoadHistograms::RecordLoadTimeHistogram(
     base::TimeDelta delta) {
   CHECK_NE(kFromUnknown, data_source_);
 
-  int duration =
-      base::saturated_cast<base::HistogramBase::Sample>(delta.InMilliseconds());
+  // TODO(https://crbug.com/1049257): These times should be recorded using a
+  // more appropriate UMA helper, since >1% of samples are in the overflow
+  // bucket.
   if (font->ErrorOccurred()) {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, load_error_histogram,
-        ("WebFont.DownloadTime.LoadError", 0, 10000, 50));
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, missed_cache_load_error_histogram,
-        ("WebFont.MissedCache.DownloadTime.LoadError", 0, 10000, 50));
-    load_error_histogram.Count(duration);
-    if (data_source_ == kFromNetwork)
-      missed_cache_load_error_histogram.Count(duration);
+    base::UmaHistogramTimes("WebFont.DownloadTime.LoadError", delta);
+    if (data_source_ == kFromNetwork) {
+      base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.LoadError",
+                              delta);
+    }
     return;
   }
 
   size_t size = font->EncodedSize();
   if (size < 10 * 1024) {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, under10k_histogram,
-        ("WebFont.DownloadTime.0.Under10KB", 0, 10000, 50));
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, missed_cache_under10k_histogram,
-        ("WebFont.MissedCache.DownloadTime.0.Under10KB", 0, 10000, 50));
-    under10k_histogram.Count(duration);
-    if (data_source_ == kFromNetwork)
-      missed_cache_under10k_histogram.Count(duration);
+    base::UmaHistogramTimes("WebFont.DownloadTime.0.Under10KB", delta);
+    if (data_source_ == kFromNetwork) {
+      base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.0.Under10KB",
+                              delta);
+    }
     return;
   }
   if (size < 50 * 1024) {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, under50k_histogram,
-        ("WebFont.DownloadTime.1.10KBTo50KB", 0, 10000, 50));
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, missed_cache_under50k_histogram,
-        ("WebFont.MissedCache.DownloadTime.1.10KBTo50KB", 0, 10000, 50));
-    under50k_histogram.Count(duration);
-    if (data_source_ == kFromNetwork)
-      missed_cache_under50k_histogram.Count(duration);
+    base::UmaHistogramTimes("WebFont.DownloadTime.1.10KBTo50KB", delta);
+    if (data_source_ == kFromNetwork) {
+      base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.1.10KBTo50KB",
+                              delta);
+    }
     return;
   }
   if (size < 100 * 1024) {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, under100k_histogram,
-        ("WebFont.DownloadTime.2.50KBTo100KB", 0, 10000, 50));
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, missed_cache_under100k_histogram,
-        ("WebFont.MissedCache.DownloadTime.2.50KBTo100KB", 0, 10000, 50));
-    under100k_histogram.Count(duration);
-    if (data_source_ == kFromNetwork)
-      missed_cache_under100k_histogram.Count(duration);
+    base::UmaHistogramTimes("WebFont.DownloadTime.2.50KBTo100KB", delta);
+    if (data_source_ == kFromNetwork) {
+      base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.2.50KBTo100KB",
+                              delta);
+    }
     return;
   }
   if (size < 1024 * 1024) {
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, under1mb_histogram,
-        ("WebFont.DownloadTime.3.100KBTo1MB", 0, 10000, 50));
-    DEFINE_THREAD_SAFE_STATIC_LOCAL(
-        CustomCountHistogram, missed_cache_under1mb_histogram,
-        ("WebFont.MissedCache.DownloadTime.3.100KBTo1MB", 0, 10000, 50));
-    under1mb_histogram.Count(duration);
-    if (data_source_ == kFromNetwork)
-      missed_cache_under1mb_histogram.Count(duration);
+    base::UmaHistogramTimes("WebFont.DownloadTime.3.100KBTo1MB", delta);
+    if (data_source_ == kFromNetwork) {
+      base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.3.100KBTo1MB",
+                              delta);
+    }
     return;
   }
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(
-      CustomCountHistogram, over1mb_histogram,
-      ("WebFont.DownloadTime.4.Over1MB", 0, 10000, 50));
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(
-      CustomCountHistogram, missed_cache_over1mb_histogram,
-      ("WebFont.MissedCache.DownloadTime.4.Over1MB", 0, 10000, 50));
-  over1mb_histogram.Count(duration);
-  if (data_source_ == kFromNetwork)
-    missed_cache_over1mb_histogram.Count(duration);
+  base::UmaHistogramTimes("WebFont.DownloadTime.4.Over1MB", delta);
+  if (data_source_ == kFromNetwork) {
+    base::UmaHistogramTimes("WebFont.MissedCache.DownloadTime.4.Over1MB",
+                            delta);
+  }
 }
 
 RemoteFontFaceSource::FontLoadHistograms::CacheHitMetrics
 RemoteFontFaceSource::FontLoadHistograms::DataSourceMetricsValue() {
   switch (data_source_) {
     case kFromDataURL:
-      return kDataUrl;
+      return CacheHitMetrics::kDataUrl;
     case kFromMemoryCache:
-      return kMemoryHit;
+      return CacheHitMetrics::kMemoryHit;
     case kFromDiskCache:
-      return kDiskHit;
+      return CacheHitMetrics::kDiskHit;
     case kFromNetwork:
-      return kMiss;
+      return CacheHitMetrics::kMiss;
     case kFromUnknown:
-    // Fall through.
-    default:
-      NOTREACHED();
+      return CacheHitMetrics::kMiss;
   }
-  return kMiss;
+  NOTREACHED();
+  return CacheHitMetrics::kMiss;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/remote_font_face_source.h

@@ -66,12 +66,12 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
    public:
     // Should not change following order in CacheHitMetrics to be used for
     // metrics values.
-    enum CacheHitMetrics {
+    enum class CacheHitMetrics {
       kMiss,
       kDiskHit,
       kDataUrl,
       kMemoryHit,
-      kCacheHitEnumMax
+      kMaxValue = kMemoryHit,
     };
     enum DataSource {
       kFromUnknown,