WebFonts intervention: Add metrics only for miss-cached loading

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 0e092f1a8060ab6bd021738fee8308d5e0b68567..fc90cc87a57ecfc8b1e14e7e7eadc5a5b75156f2 100644
--- a/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
+++ b/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
@@ -24,33 +24,35 @@ namespace blink {
 
 namespace {
 
-    bool isEffectiveConnectionTypeSlowFor(Document* document)
-    {
-        WebEffectiveConnectionType type = document->frame()->loader().client()->getEffectiveConnectionType();
+bool isEffectiveConnectionTypeSlowFor(Document* document)
+{
+    WebEffectiveConnectionType type = document->frame()->loader().client()->getEffectiveConnectionType();
 
-        WebEffectiveConnectionType thresholdType = RuntimeEnabledFeatures::webFontsInterventionV2With2GEnabled()
-            ? WebEffectiveConnectionType::Type2G
-            : WebEffectiveConnectionType::TypeSlow2G;
+    WebEffectiveConnectionType thresholdType = RuntimeEnabledFeatures::webFontsInterventionV2With2GEnabled()
+        ? WebEffectiveConnectionType::Type2G
+        : WebEffectiveConnectionType::TypeSlow2G;
 
-        return WebEffectiveConnectionType::TypeOffline <= type && type <= thresholdType;
-    }
+    return WebEffectiveConnectionType::TypeOffline <= type && type <= thresholdType;
+}
 
-    bool isConnectionTypeSlow()
-    {
-        return networkStateNotifier().connectionType() == WebConnectionTypeCellular2G;
-    }
+bool isConnectionTypeSlow()
+{
+    return networkStateNotifier().connectionType() == WebConnectionTypeCellular2G;
+}
 
-    bool shouldTriggerWebFontsIntervention(Document* document, FontDisplay display)
-    {
-        if (RuntimeEnabledFeatures::webFontsInterventionTriggerEnabled())
-            return true;
+bool shouldTriggerWebFontsIntervention(Document* document, FontDisplay display, bool isLoadedFromMemoryCache)
+{
+    if (isLoadedFromMemoryCache)
+        return false;

[Kunihiko Sakamoto, 2016/06/27 09:07:45]

So, we no longer trigger the intervention for memory-cached fonts -- could you
update the CL description for this behavior change?

[Takashi Toyoshima, 2016/06/27 09:39:40]

Done.

+    if (RuntimeEnabledFeatures::webFontsInterventionTriggerEnabled())
+        return true;
 
-        bool isV2Enabled = RuntimeEnabledFeatures::webFontsInterventionV2With2GEnabled() || RuntimeEnabledFeatures::webFontsInterventionV2WithSlow2GEnabled();
+    bool isV2Enabled = RuntimeEnabledFeatures::webFontsInterventionV2With2GEnabled() || RuntimeEnabledFeatures::webFontsInterventionV2WithSlow2GEnabled();
 
-        bool networkIsSlow = isV2Enabled ? isEffectiveConnectionTypeSlowFor(document) : isConnectionTypeSlow();
+    bool networkIsSlow = isV2Enabled ? isEffectiveConnectionTypeSlowFor(document) : isConnectionTypeSlow();
 
-        return networkIsSlow && display == FontDisplayAuto;
-    }
+    return networkIsSlow && display == FontDisplayAuto;
+}
 
 } // namespace
 
@@ -60,11 +62,12 @@ RemoteFontFaceSource::RemoteFontFaceSource(FontResource* font, CSSFontSelector*
     , m_display(display)
     , m_period(display == FontDisplaySwap ? SwapPeriod : BlockPeriod)
     , m_isInterventionTriggered(false)
+    , m_isLoadedFromMemoryCache(font->isLoaded())
 {
     ThreadState::current()->registerPreFinalizer(this);
     m_font->addClient(this);
 
-    if (shouldTriggerWebFontsIntervention(m_fontSelector->document(), display)) {
+    if (shouldTriggerWebFontsIntervention(m_fontSelector->document(), display, m_isLoadedFromMemoryCache)) {
 
         m_isInterventionTriggered = true;
         m_period = SwapPeriod;
@@ -114,7 +117,7 @@ bool RemoteFontFaceSource::isValid() const
 void RemoteFontFaceSource::notifyFinished(Resource*)
 {
     m_histograms.recordRemoteFont(m_font.get());
-    m_histograms.fontLoaded(m_isInterventionTriggered);
+    m_histograms.fontLoaded(m_isInterventionTriggered, m_isLoadedFromMemoryCache || m_font->response().wasCached());

[Takashi Toyoshima, 2016/06/27 08:55:10]

wasCached() returns disk cache result.

So, if the first fetch comes from network, m_isLoadedFromMemoryCache and
wasCached() are false. Then the next fetch will be m_isLoadedFromMemoryCache is
true and wasCached() was false because the same response() is cached in the
memory cache.

If the first fetch comes from disk cache, wasCached() is true for both first and
the second fetches.

 
     m_font->ensureCustomFontData();
     // FIXME: Provide more useful message such as OTS rejection reason.
@@ -235,16 +238,16 @@ void RemoteFontFaceSource::FontLoadHistograms::fallbackFontPainted(DisplayPeriod
         m_blankPaintTime = currentTimeMS();
 }
 
-void RemoteFontFaceSource::FontLoadHistograms::fontLoaded(bool isInterventionTriggered)
+void RemoteFontFaceSource::FontLoadHistograms::fontLoaded(bool isInterventionTriggered, bool isLoadedFromCache)
 {
     if (!m_isLongLimitExceeded)
-        recordInterventionResult(isInterventionTriggered);
+        recordInterventionResult(isInterventionTriggered, isLoadedFromCache);
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::longLimitExceeded(bool isInterventionTriggered)
 {
     m_isLongLimitExceeded = true;
-    recordInterventionResult(isInterventionTriggered);
+    recordInterventionResult(isInterventionTriggered, false);
 }
 
 void RemoteFontFaceSource::FontLoadHistograms::recordFallbackTime(const FontResource* font)
@@ -311,7 +314,7 @@ void RemoteFontFaceSource::FontLoadHistograms::recordLoadTimeHistogram(const Fon
     over1mbHistogram.count(duration);
 }
 
-void RemoteFontFaceSource::FontLoadHistograms::recordInterventionResult(bool triggered)
+void RemoteFontFaceSource::FontLoadHistograms::recordInterventionResult(bool triggered, bool loadedFromCache)
 {
     // interventionResult takes 0-3 values.
     int interventionResult = 0;
@@ -322,7 +325,10 @@ void RemoteFontFaceSource::FontLoadHistograms::recordInterventionResult(bool tri
     const int boundary = 1 << 2;
 
     DEFINE_STATIC_LOCAL(EnumerationHistogram, interventionHistogram, ("WebFont.InterventionResult", boundary));
+    DEFINE_STATIC_LOCAL(EnumerationHistogram, missCachedInterventionHistogram, ("WebFont.MissCachedInterventionResult", boundary));
     interventionHistogram.count(interventionResult);
+    if (!loadedFromCache)

[tbansal1, 2016/06/27 15:54:13]

Will the accuracy histogram be logged if the font was base-64 encoded within the
file (like a data URL)? 

[Takashi Toyoshima, 2016/06/28 05:42:52]

That's good point.
Let me check if we do not trigger the intervention for data URL after submitting
this CL.

I expect font->isLoaded() returns true at the constructor.
Once I check the behavior, I will report the result at the crbug.

+        missCachedInterventionHistogram.count(interventionResult);
 }
 
 } // namespace blink