[CSS Font Loading] Fonts are erroneously invisible when the policy says they should...
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Oct 2017 21:16:42 +0000 (21:16 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Oct 2017 21:16:42 +0000 (21:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178238

Reviewed by Simon Fraser.

When implementing font-display, I added testing infrastructure (so we don't have to wait for
3 second timeouts to occur). This testing infrastructure covered up a real bug where the wrong
font would be reported to CSSFontAccessor. This patch reverts the erroneous testing
infrastructure and replaces it with a real fix to the problem. The replacement fix is covered
by the same tests that I wrote when implementing the feature.

Covered by existing tests.

* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::pump):
(WebCore::visibility):
(WebCore::CSSFontFace::font):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223576 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontFace.cpp

index 75d9cf8..03285fa 100644 (file)
@@ -1,3 +1,23 @@
+2017-10-17  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [CSS Font Loading] Fonts are erroneously invisible when the policy says they should be visible
+        https://bugs.webkit.org/show_bug.cgi?id=178238
+
+        Reviewed by Simon Fraser.
+
+        When implementing font-display, I added testing infrastructure (so we don't have to wait for
+        3 second timeouts to occur). This testing infrastructure covered up a real bug where the wrong
+        font would be reported to CSSFontAccessor. This patch reverts the erroneous testing
+        infrastructure and replaces it with a real fix to the problem. The replacement fix is covered
+        by the same tests that I wrote when implementing the feature.
+
+        Covered by existing tests.
+
+        * css/CSSFontFace.cpp:
+        (WebCore::CSSFontFace::pump):
+        (WebCore::visibility):
+        (WebCore::CSSFontFace::font):
+
 2017-10-16  Sam Weinig  <sam@webkit.org>
 
         [Settings] Generate Settings.h/cpp
index 3ae65f2..19c1f31 100644 (file)
@@ -714,10 +714,6 @@ size_t CSSFontFace::pump(ExternalResourceDownloadPolicy policy)
                 if (m_status == Status::Pending)
                     setStatus(Status::Loading);
                 source->load(m_fontSelector.get());
-            } else if (fontLoadTimingOverride(m_fontSelector.get()) != Settings::FontLoadTimingOverride::None && m_status == Status::Pending) {
-                // Similar to above, if a test that has set fontLoadTimingOverride() needs to fail, this needs to happen
-                // eagerly so the FontRanges sees a consistent view of the CSSFontFace.
-                setStatus(Status::Loading);
             }
         }
 
@@ -755,6 +751,21 @@ void CSSFontFace::load()
     pump(ExternalResourceDownloadPolicy::Allow);
 }
 
+static Font::Visibility visibility(CSSFontFace::Status status, CSSFontFace::FontLoadTiming timing)
+{
+    switch (status) {
+    case CSSFontFace::Status::Pending:
+        return timing.blockPeriod == 0_s ? Font::Visibility::Visible : Font::Visibility::Invisible;
+    case CSSFontFace::Status::Loading:
+        return Font::Visibility::Invisible;
+    case CSSFontFace::Status::TimedOut:
+    case CSSFontFace::Status::Failure:
+    case CSSFontFace::Status::Success:
+    default:
+        return Font::Visibility::Visible;
+    }
+}
+
 RefPtr<Font> CSSFontFace::font(const FontDescription& fontDescription, bool syntheticBold, bool syntheticItalic, ExternalResourceDownloadPolicy policy)
 {
     if (computeFailureState())
@@ -777,7 +788,7 @@ RefPtr<Font> CSSFontFace::font(const FontDescription& fontDescription, bool synt
         switch (source->status()) {
         case CSSFontFaceSource::Status::Pending:
         case CSSFontFaceSource::Status::Loading: {
-            Font::Visibility visibility = status() == Status::TimedOut || status() == Status::Failure ? Font::Visibility::Visible : Font::Visibility::Invisible;
+            Font::Visibility visibility = WebCore::visibility(status(), fontLoadTiming());
             return Font::create(FontCache::singleton().lastResortFallbackFont(fontDescription)->platformData(), Font::Origin::Local, Font::Interstitial::Yes, visibility);
         }
         case CSSFontFaceSource::Status::Success: