Title: [265603] trunk
Revision
265603
Author
[email protected]
Date
2020-08-13 01:13:05 -0700 (Thu, 13 Aug 2020)

Log Message

Font loads quickly followed by navigations may fail indefinitely
https://bugs.webkit.org/show_bug.cgi?id=215435
<rdar://problem/65560550>

Reviewed by Darin Adler.

Source/WebCore:

Font loads are coalesced using a zero-delay timer. However, that zero-delay timer
can fire while the page is in the middle of a navigation, which will cause the font
loads to fail. Then, the second page can request those same fonts, which are marked
as failed, and as such will never actually load/use the desired web font.

This patch just stops the zero-delay timer during navigations, and resumes it
when resuming the document. This means:

1. The second page in the above story will not see that the font has failed, or
even started, and will then re-request the font and load it successfully
2. If the user goes "back" to the previous page, the zero-delay timer is restarted,
the CachedFont realizes it's already succeeded, and the previous page is rendered
as expected.

Test: fast/loader/font-load-timer.html

* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::suspendFontLoadingTimer):
(WebCore::CSSFontSelector::restartFontLoadingTimer):
* css/CSSFontSelector.h:
* dom/Document.cpp:
(WebCore::Document::resume):
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::stopLoading):

LayoutTests:

1) The page has some content that has “font-family: WebFont” but there are no @font-face blocks on the page
2) In script, after the page has loaded, add an @font-face rule to the page with “font-family: WebFont” and some valid font URL
3) Synchronously, within the same turn of the run loop, trigger a synchronous layout of the element (using offsetWidth or something). This will add the font to the 0-delay time work list.
4) Synchronously, within the same turn of the run loop, navigate to a second page that doesn’t use the web font.
5) The second page waits some small-but-positive amount of time. This will cause the 0-delay timer to fire, but because the page is in the middle of navigating, the font load should fail.
6) The second page adds the same @font-face rule to itself using script. This should pull the same (failed) CachedResource object out of the memory cache.
7) Use the CSS Font Loading API to wait for the font load to complete
8) Make sure that the font is used on the second page (as a reference test). Today, the second page’s font load will fail because it pulled the failed font out of the memory cache. The test makes sure the second page’s font load succeeds.

* fast/loader/font-load-timer-expected.html: Added.
* fast/loader/font-load-timer.html: Added.
* fast/loader/resources/font-load-timer-navigation-destination.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (265602 => 265603)


--- trunk/LayoutTests/ChangeLog	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/LayoutTests/ChangeLog	2020-08-13 08:13:05 UTC (rev 265603)
@@ -1,3 +1,24 @@
+2020-08-12  Myles C. Maxfield  <[email protected]>
+
+        Font loads quickly followed by navigations may fail indefinitely
+        https://bugs.webkit.org/show_bug.cgi?id=215435
+        <rdar://problem/65560550>
+
+        Reviewed by Darin Adler.
+
+        1) The page has some content that has “font-family: WebFont” but there are no @font-face blocks on the page
+        2) In script, after the page has loaded, add an @font-face rule to the page with “font-family: WebFont” and some valid font URL
+        3) Synchronously, within the same turn of the run loop, trigger a synchronous layout of the element (using offsetWidth or something). This will add the font to the 0-delay time work list.
+        4) Synchronously, within the same turn of the run loop, navigate to a second page that doesn’t use the web font. 
+        5) The second page waits some small-but-positive amount of time. This will cause the 0-delay timer to fire, but because the page is in the middle of navigating, the font load should fail.
+        6) The second page adds the same @font-face rule to itself using script. This should pull the same (failed) CachedResource object out of the memory cache.
+        7) Use the CSS Font Loading API to wait for the font load to complete
+        8) Make sure that the font is used on the second page (as a reference test). Today, the second page’s font load will fail because it pulled the failed font out of the memory cache. The test makes sure the second page’s font load succeeds.
+
+        * fast/loader/font-load-timer-expected.html: Added.
+        * fast/loader/font-load-timer.html: Added.
+        * fast/loader/resources/font-load-timer-navigation-destination.html: Added.
+
 2020-08-12  Lauro Moura  <[email protected]>
 
         Highpass Biquads use old formulas

Added: trunk/LayoutTests/fast/loader/font-load-timer-expected.html (0 => 265603)


--- trunk/LayoutTests/fast/loader/font-load-timer-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/loader/font-load-timer-expected.html	2020-08-13 08:13:05 UTC (rev 265603)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style id="style">
+@font-face {
+    font-family: 'WebFont';
+    src: url('../../resources/Ahem.ttf') format('truetype');
+}
+</style>
+</head>
+<body>
+<span id="target" style="font: 48px 'WebFont';">HelloWorld!</span>
+</body>
+</html>

Added: trunk/LayoutTests/fast/loader/font-load-timer.html (0 => 265603)


--- trunk/LayoutTests/fast/loader/font-load-timer.html	                        (rev 0)
+++ trunk/LayoutTests/fast/loader/font-load-timer.html	2020-08-13 08:13:05 UTC (rev 265603)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style id="style"></style>
+</head>
+<body>
+<span id="target" style="font: 48px 'WebFont';">Hello, World!</span>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+if (window.internals) {
+    internals.invalidateFontCache();
+    internals.clearMemoryCache();
+}
+
+document.getElementById("style").sheet.insertRule("@font-face { font-family: 'WebFont'; src: url('../../resources/Ahem.ttf') format('truetype'); }");
+document.getElementById("target").offsetWidth;
+window.location = "resources/font-load-timer-navigation-destination.html";
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/fast/loader/resources/font-load-timer-navigation-destination.html (0 => 265603)


--- trunk/LayoutTests/fast/loader/resources/font-load-timer-navigation-destination.html	                        (rev 0)
+++ trunk/LayoutTests/fast/loader/resources/font-load-timer-navigation-destination.html	2020-08-13 08:13:05 UTC (rev 265603)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style id="style"></style>
+</head>
+<body>
+<span id="target" style="font: 48px 'WebFont';">HelloWorld!</span>
+<script>
+window.setTimeout(function() {
+    document.getElementById("style").sheet.insertRule("@font-face { font-family: 'WebFont'; src: url('../../../resources/Ahem.ttf') format('truetype'); }");
+    document.fonts.entries().next().value.load().then(function() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, function() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+}, 100);
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (265602 => 265603)


--- trunk/Source/WebCore/ChangeLog	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/Source/WebCore/ChangeLog	2020-08-13 08:13:05 UTC (rev 265603)
@@ -1,3 +1,36 @@
+2020-08-12  Myles C. Maxfield  <[email protected]>
+
+        Font loads quickly followed by navigations may fail indefinitely
+        https://bugs.webkit.org/show_bug.cgi?id=215435
+        <rdar://problem/65560550>
+
+        Reviewed by Darin Adler.
+
+        Font loads are coalesced using a zero-delay timer. However, that zero-delay timer
+        can fire while the page is in the middle of a navigation, which will cause the font
+        loads to fail. Then, the second page can request those same fonts, which are marked
+        as failed, and as such will never actually load/use the desired web font.
+
+        This patch just stops the zero-delay timer during navigations, and resumes it
+        when resuming the document. This means:
+
+        1. The second page in the above story will not see that the font has failed, or
+        even started, and will then re-request the font and load it successfully
+        2. If the user goes "back" to the previous page, the zero-delay timer is restarted,
+        the CachedFont realizes it's already succeeded, and the previous page is rendered
+        as expected.
+
+        Test: fast/loader/font-load-timer.html
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::suspendFontLoadingTimer):
+        (WebCore::CSSFontSelector::restartFontLoadingTimer):
+        * css/CSSFontSelector.h:
+        * dom/Document.cpp:
+        (WebCore::Document::resume):
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::stopLoading):
+
 2020-08-12  Lauro Moura  <[email protected]>
 
         Highpass Biquads use old formulas

Modified: trunk/Source/WebCore/css/CSSFontSelector.cpp (265602 => 265603)


--- trunk/Source/WebCore/css/CSSFontSelector.cpp	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp	2020-08-13 08:13:05 UTC (rev 265603)
@@ -364,9 +364,30 @@
     // decrementRequestCount() in beginLoadTimerFired() and in clearDocument().
     m_document->cachedResourceLoader().incrementRequestCount(font);
 
-    m_beginLoadingTimer.startOneShot(0_s);
+    if (!m_fontLoadingTimerIsSuspended)
+        m_beginLoadingTimer.startOneShot(0_s);
 }
 
+void CSSFontSelector::suspendFontLoadingTimer()
+{
+    if (m_fontLoadingTimerIsSuspended)
+        return;
+
+    m_beginLoadingTimer.cancel();
+    m_fontLoadingTimerIsSuspended = true;
+}
+
+void CSSFontSelector::restartFontLoadingTimer()
+{
+    if (!m_fontLoadingTimerIsSuspended)
+        return;
+
+    if (!m_fontsToBeginLoading.isEmpty())
+        m_beginLoadingTimer.startOneShot(0_s);
+
+    m_fontLoadingTimerIsSuspended = false;
+}
+
 void CSSFontSelector::beginLoadTimerFired()
 {
     Vector<CachedResourceHandle<CachedFont>> fontsToBeginLoading;

Modified: trunk/Source/WebCore/css/CSSFontSelector.h (265602 => 265603)


--- trunk/Source/WebCore/css/CSSFontSelector.h	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/Source/WebCore/css/CSSFontSelector.h	2020-08-13 08:13:05 UTC (rev 265603)
@@ -80,6 +80,8 @@
     Document* document() const { return m_document.get(); }
 
     void beginLoadingFontSoon(CachedFont&);
+    void suspendFontLoadingTimer();
+    void restartFontLoadingTimer();
 
     FontFaceSet* fontFaceSetIfExists();
     FontFaceSet& fontFaceSet();
@@ -119,6 +121,7 @@
     unsigned m_computingRootStyleFontCount { 0 };
     bool m_creatingFont { false };
     bool m_buildIsUnderway { false };
+    bool m_fontLoadingTimerIsSuspended { false };
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/dom/Document.cpp (265602 => 265603)


--- trunk/Source/WebCore/dom/Document.cpp	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/Source/WebCore/dom/Document.cpp	2020-08-13 08:13:05 UTC (rev 265603)
@@ -5449,6 +5449,8 @@
     if (renderView())
         renderView()->setIsInWindow(true);
 
+    fontSelector().restartFontLoadingTimer();
+
     ASSERT(page());
     page()->lockAllOverlayScrollbarsToHidden(false);
 

Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (265602 => 265603)


--- trunk/Source/WebCore/loader/DocumentLoader.cpp	2020-08-13 06:09:30 UTC (rev 265602)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp	2020-08-13 08:13:05 UTC (rev 265603)
@@ -33,6 +33,7 @@
 #include "ApplicationCacheHost.h"
 #include "Archive.h"
 #include "ArchiveResourceCollection.h"
+#include "CSSFontSelector.h"
 #include "CachedPage.h"
 #include "CachedRawResource.h"
 #include "CachedResourceLoader.h"
@@ -313,6 +314,9 @@
     // Always cancel multipart loaders
     cancelAll(m_multipartSubresourceLoaders);
 
+    if (auto* document = m_frame->document())
+        document->fontSelector().suspendFontLoadingTimer();
+
     // Appcache uses ResourceHandle directly, DocumentLoader doesn't count these loads.
     m_applicationCacheHost->stopLoadingInFrame(*m_frame);
     
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to