Webfonts should not block load event for more than one frame

Source/core/css/FontLoader.cpp

Index: Source/core/css/FontLoader.cpp
diff --git a/Source/core/css/FontLoader.cpp b/Source/core/css/FontLoader.cpp
index 8646bfac6081351779dfab144ff835b6477edbc5..fda6bdc21dbbb48e38a0669eda03499b9f2bf03f 100644
--- a/Source/core/css/FontLoader.cpp
+++ b/Source/core/css/FontLoader.cpp
@@ -6,6 +6,7 @@
 #include "core/css/FontLoader.h"
 
 #include "core/css/CSSFontSelector.h"
+#include "core/dom/Document.h"
 #include "core/fetch/FontResource.h"
 #include "core/fetch/ResourceFetcher.h"
 
@@ -53,11 +54,26 @@ void FontLoader::loadPendingFonts()
 {
     ASSERT(m_resourceFetcher);
 
+    if (m_fontsToBeginLoading.isEmpty())
+        return;
+
     FontsToLoadVector fontsToBeginLoading;
     fontsToBeginLoading.swap(m_fontsToBeginLoading);
+    bool loaded = false;
     for (FontsToLoadVector::iterator it = fontsToBeginLoading.begin(); it != fontsToBeginLoading.end(); ++it) {
         FontResource* fontResource = it->first.get();
-        fontResource->beginLoadIfNeeded(m_resourceFetcher);
+        if (fontResource->stillNeedsLoad()) {
+            fontResource->beginLoadIfNeeded(m_resourceFetcher);
+            loaded = true;
+        }
+    }
+
+    if (!loaded) {
+        // Since we have prevented load event by RequestCountTracker but did not
+        // start any load, schedule a check for load event here.
+        Document* document = m_resourceFetcher->document();
+        if (document && !document->loadEventFinished() && !document->isDelayingLoadEvent())
+            document->checkLoadEventSoon();

[Nate Chapin, 2014/08/11 22:06:15]

Do we need to block the load event on webfonts in general?

Assuming we do, this is not very intuitive. Why are we putting fonts in
m_fontsToBeginLoading if we don't need to load them?

Would it be possible to ensure we don't add fonts to m_fontsToBeginLoading if
they are already started, and to remove them if another frame starts them
loading? That would presumably require changing m_fontsToBeginLoading to a
HashMap instead of a Vector, but probably more readable in the long run.

[Kunihiko Sakamoto, 2014/08/12 03:12:09]

It worked this way for years and I think many websites depends on this behavior.
We don't add them; if a font is already started fontResource->stillNeedsLoad()
at line 39 returns false.
Adding an entry to m_fontsToBeginLoading creates an instance of
ResourceLoader::RequestCountTracker, that prevent the load event. The destructor
of ResourceLoader::RequestCountTracker just decrements the request count and
doesn't trigger the load event.

Perhaps we should schedule load event in ResourceLoader::~RequestCountTracker()
if the request count became zero?

     }
 
     // When the local fontsToBeginLoading vector goes out of scope it will