Replace doubles by base::Time in RemoteFontFaceSource

We were doing some unneccesary double <-> base::Time conversions. We can just
store base::Time and then convert it to doubles for the histograms.

Bug: 979137
Change-Id: Ifa93dd5d91ddfbfc3dd6eca50115bc9e402ce183
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1678483
Commit-Queue: Sergio Villar <svillar@igalia.com>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676943}

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

@@ -321,14 +321,16 @@ void RemoteFontFaceSource::Trace(blink::Visitor* visitor) {
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::LoadStarted() {
-  if (!load_start_time_)
-    load_start_time_ = CurrentTimeMS();
+  if (load_start_time_.is_null())
+    load_start_time_ = base::TimeTicks::Now();
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::FallbackFontPainted(
     DisplayPeriod period) {
-  if (period == kBlockPeriod && !blank_paint_time_)
-    blank_paint_time_ = CurrentTimeMS();
+  if (period == kBlockPeriod && blank_paint_time_.is_null()) {
+    blank_paint_time_ = base::TimeTicks::Now();
+    blank_paint_time_recorded_ = false;
+  }
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::LongLimitExceeded() {
@@ -337,14 +339,16 @@ void RemoteFontFaceSource::FontLoadHistograms::LongLimitExceeded() {
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::RecordFallbackTime() {
-  if (blank_paint_time_ <= 0)
+  if (blank_paint_time_.is_null() || blank_paint_time_recorded_)
     return;
-  int duration = static_cast<int>(CurrentTimeMS() - blank_paint_time_);
+  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(duration);
-  blank_paint_time_ = -1;
+  blank_text_shown_time_histogram.Count(
+      base::saturated_cast<base::HistogramBase::Sample>(
+          duration.InMilliseconds()));
+  blank_paint_time_recorded_ = true;
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::RecordRemoteFont(
@@ -356,9 +360,8 @@ void RemoteFontFaceSource::FontLoadHistograms::RecordRemoteFont(
   cache_hit_histogram.Count(DataSourceMetricsValue());
 
   if (data_source_ == kFromDiskCache || data_source_ == kFromNetwork) {
-    DCHECK_NE(load_start_time_, 0);
-    int duration = static_cast<int>(CurrentTimeMS() - load_start_time_);
-    RecordLoadTimeHistogram(font, duration);
+    DCHECK(!load_start_time_.is_null());
+    RecordLoadTimeHistogram(font, base::TimeTicks::Now() - load_start_time_);
   }
 }
 
@@ -369,7 +372,7 @@ void RemoteFontFaceSource::FontLoadHistograms::MaySetDataSource(
   // Classify as memory cache hit if |load_start_time_| is not set, i.e.
   // this RemoteFontFaceSource instance didn't trigger FontResource
   // loading.
-  if (load_start_time_ == 0)
+  if (load_start_time_.is_null())
     data_source_ = kFromMemoryCache;
   else
     data_source_ = data_source;
@@ -377,9 +380,11 @@ void RemoteFontFaceSource::FontLoadHistograms::MaySetDataSource(
 
 void RemoteFontFaceSource::FontLoadHistograms::RecordLoadTimeHistogram(
     const FontResource* font,
-    int duration) {
+    base::TimeDelta delta) {
   CHECK_NE(kFromUnknown, data_source_);
 
+  int duration =
+      base::saturated_cast<base::HistogramBase::Sample>(delta.InMilliseconds());
   if (font->ErrorOccurred()) {
     DEFINE_THREAD_SAFE_STATIC_LOCAL(
         CustomCountHistogram, load_error_histogram,

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

@@ -82,8 +82,7 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
     };
 
     FontLoadHistograms()
-        : load_start_time_(0),
-          blank_paint_time_(0),
+        : blank_paint_time_recorded_(false),
           is_long_limit_exceeded_(false),
           data_source_(kFromUnknown) {}
     void LoadStarted();
@@ -91,7 +90,7 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
     void LongLimitExceeded();
     void RecordFallbackTime();
     void RecordRemoteFont(const FontResource*);
-    bool HadBlankText() { return blank_paint_time_; }
+    bool HadBlankText() { return !blank_paint_time_.is_null(); }
     DataSource GetDataSource() { return data_source_; }
     void MaySetDataSource(DataSource);
 
@@ -102,10 +101,14 @@ class RemoteFontFaceSource final : public CSSFontFaceSource,
     }
 
    private:
-    void RecordLoadTimeHistogram(const FontResource*, int duration);
+    void RecordLoadTimeHistogram(const FontResource*, base::TimeDelta duration);
     CacheHitMetrics DataSourceMetricsValue();
-    double load_start_time_;
-    double blank_paint_time_;
+    base::TimeTicks load_start_time_;
+    base::TimeTicks blank_paint_time_;
+    // |blank_paint_time_recorded_| is used to prevent
+    // WebFont.BlankTextShownTime to be reported incorrectly when the web font
+    // fallbacks immediately. See https://crbug.com/591304
+    bool blank_paint_time_recorded_;
     bool is_long_limit_exceeded_;
     DataSource data_source_;
   };