FontLoadHistograms: classify as memory cache hit if FontResource loading not triggered

third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp

Index: third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
diff --git a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
index 638504edc43eb4d7de3a6f295989eb95929a4d1c..94d4ea9725e08e08d81be7b68078bafc3558a19d 100644
--- a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
+++ b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
@@ -271,18 +271,25 @@ void RemoteFontFaceSource::FontLoadHistograms::recordFallbackTime(const FontReso
 
 void RemoteFontFaceSource::FontLoadHistograms::recordRemoteFont(const FontResource* font)
 {
-    DEFINE_STATIC_LOCAL(EnumerationHistogram, cacheHitHistogram, ("WebFont.CacheHit", CacheHitEnumMax));
-    cacheHitHistogram.count(dataSourceMetricsValue());
-
     if (m_dataSource == FromDiskCache || m_dataSource == FromNetwork) {
-        int duration = static_cast<int>(currentTimeMS() - m_loadStartTime);
-        recordLoadTimeHistogram(font, duration);
-
-        enum { CORSFail, CORSSuccess, CORSEnumMax };
-        int corsValue = font->isCORSFailed() ? CORSFail : CORSSuccess;
-        DEFINE_STATIC_LOCAL(EnumerationHistogram, corsHistogram, ("WebFont.CORSSuccess", CORSEnumMax));
-        corsHistogram.count(corsValue);
+        // Collect load time only if |m_loadStartTime| was set, i.e. the owning
+        // RemoteFontFaceSource instance started the loading of |m_font|.
+        // Otherwise we classify as memory cache hit.
+        if (m_loadStartTime) {
+            int duration = static_cast<int>(currentTimeMS() - m_loadStartTime);
+            recordLoadTimeHistogram(font, duration);
+
+            enum { CORSFail, CORSSuccess, CORSEnumMax };
+            int corsValue = font->isCORSFailed() ? CORSFail : CORSSuccess;
+            DEFINE_STATIC_LOCAL(EnumerationHistogram, corsHistogram, ("WebFont.CORSSuccess", CORSEnumMax));
+            corsHistogram.count(corsValue);
+        } else {
+            m_dataSource = FromMemoryCache;

[Kunihiko Sakamoto, 2016/09/23 07:39:18]

Before this patch, we only had transitions from FromUnknown to others. This
introduces a path from From{DiskCache,Network} to FromMemoryCache - I think this
makes it hard to reason about this field.

Instead, can we do this in the maySetDataSource() call at line 110?

[Shao-Chuan Lee, 2016/09/23 08:41:06]

Done.

+        }
     }
+
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, cacheHitHistogram, ("WebFont.CacheHit", CacheHitEnumMax));
+    cacheHitHistogram.count(dataSourceMetricsValue());
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::recordLoadTimeHistogram(const FontResource* font, int duration)