Title: [226668] trunk
Revision
226668
Author
[email protected]
Date
2018-01-09 16:44:49 -0800 (Tue, 09 Jan 2018)

Log Message

font-display:fallback can cause a visual flash (which is supposed to be impossible)
https://bugs.webkit.org/show_bug.cgi?id=181374

Reviewed by Simon Fraser.

Source/WebCore:

A FontCascade represents an entire font-family fallback list, but sometimes we need to pull out a single
representative font from the list to calculate things like line height. Previously, if the first item in
the font-family list was in the middle of being downloaded, this representative font was hardcoded to be
Times. However, when actually laying out and drawing the glyphs, we have logic to skip the interstitial
Times if there are any installed fonts present in the font-family list (so you wouldn't ever actually
see Times). This means that line height (among other things) was being calculated as if Times was used,
but in reality, some other font from the font-family list was being used.

Alone, this isn't a huge problem, but font-display:fallback makes a font transition between "timed out"
and "failed," and when the font hits the failed state, the representative font skips over the cancelled
item and hits the next item in the fallback list. This means that line heights will change, which causes
a visual flash, even when font-display:fallback is specified.

The solution is simply to educate the logic which identifies this representative font so that it
understands what to do for currently-loading fonts.

Tests: fast/text/font-display/swap-flash.html

* platform/graphics/FontCascadeFonts.h:
(WebCore::FontCascadeFonts::primaryFont):
* rendering/line/BreakingContext.h:
(WebCore::textWidth):

Tools:

The test requires Palatino.

* DumpRenderTree/mac/DumpRenderTree.mm:
(allowedFontFamilySet):
* WebKitTestRunner/mac/TestControllerMac.mm:
(WTR::allowedFontFamilySet):

LayoutTests:

Move font-display tests into their common subfolder.

* fast/text/font-display/block-finish-expected.html: Renamed from LayoutTests/fast/text/loading-block-finish-expected.html.
* fast/text/font-display/block-finish.html: Renamed from LayoutTests/fast/text/loading-block-finish.html.
* fast/text/font-display/block-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-block-nofinish-expected.html.
* fast/text/font-display/block-nofinish.html: Renamed from LayoutTests/fast/text/loading-block-nofinish.html.
* fast/text/font-display/failure-finish-expected.html: Renamed from LayoutTests/fast/text/loading-failure-finish-expected.html.
* fast/text/font-display/failure-finish.html: Renamed from LayoutTests/fast/text/loading-failure-finish.html.
* fast/text/font-display/failure-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-failure-nofinish-expected.html.
* fast/text/font-display/failure-nofinish.html: Renamed from LayoutTests/fast/text/loading-failure-nofinish.html.
* fast/text/font-display/swap-finish-expected.html: Renamed from LayoutTests/fast/text/loading-swap-finish-expected.html.
* fast/text/font-display/swap-finish.html: Renamed from LayoutTests/fast/text/loading-swap-finish.html.
* fast/text/font-display/swap-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-swap-nofinish-expected.html.
* fast/text/font-display/swap-nofinish.html: Renamed from LayoutTests/fast/text/loading-swap-nofinish.html.
* fast/text/font-display/swap-flash-expected.html: Added.
* fast/text/font-display/swap-flash.html: Added.
* platform/win/TestExpectations:

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (226667 => 226668)


--- trunk/LayoutTests/ChangeLog	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/ChangeLog	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,3 +1,28 @@
+2018-01-09  Myles C. Maxfield  <[email protected]>
+
+        font-display:fallback can cause a visual flash (which is supposed to be impossible)
+        https://bugs.webkit.org/show_bug.cgi?id=181374
+
+        Reviewed by Simon Fraser.
+
+        Move font-display tests into their common subfolder.
+
+        * fast/text/font-display/block-finish-expected.html: Renamed from LayoutTests/fast/text/loading-block-finish-expected.html.
+        * fast/text/font-display/block-finish.html: Renamed from LayoutTests/fast/text/loading-block-finish.html.
+        * fast/text/font-display/block-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-block-nofinish-expected.html.
+        * fast/text/font-display/block-nofinish.html: Renamed from LayoutTests/fast/text/loading-block-nofinish.html.
+        * fast/text/font-display/failure-finish-expected.html: Renamed from LayoutTests/fast/text/loading-failure-finish-expected.html.
+        * fast/text/font-display/failure-finish.html: Renamed from LayoutTests/fast/text/loading-failure-finish.html.
+        * fast/text/font-display/failure-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-failure-nofinish-expected.html.
+        * fast/text/font-display/failure-nofinish.html: Renamed from LayoutTests/fast/text/loading-failure-nofinish.html.
+        * fast/text/font-display/swap-finish-expected.html: Renamed from LayoutTests/fast/text/loading-swap-finish-expected.html.
+        * fast/text/font-display/swap-finish.html: Renamed from LayoutTests/fast/text/loading-swap-finish.html.
+        * fast/text/font-display/swap-nofinish-expected.html: Renamed from LayoutTests/fast/text/loading-swap-nofinish-expected.html.
+        * fast/text/font-display/swap-nofinish.html: Renamed from LayoutTests/fast/text/loading-swap-nofinish.html.
+        * fast/text/font-display/swap-flash-expected.html: Added.
+        * fast/text/font-display/swap-flash.html: Added.
+        * platform/win/TestExpectations:
+
 2018-01-09  Matt Lewis  <[email protected]>
 
         Fixed test expectaions.

Copied: trunk/LayoutTests/fast/text/font-display/block-finish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-block-finish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/block-finish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/block-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "block" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/block-finish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-block-finish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/block-finish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/block-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Block");
+    internals.settings.setShouldIgnoreFontLoadCompletions(false);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "block" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/block-nofinish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-block-nofinish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/block-nofinish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/block-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "block" period while the font is still loading is rendered as invisible. The test passes if there is no text below.
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/block-nofinish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-block-nofinish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/block-nofinish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/block-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Block");
+    internals.settings.setShouldIgnoreFontLoadCompletions(true);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "block" period while the font is still loading is rendered as invisible. The test passes if there is no text below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/failure-finish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-failure-finish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/failure-finish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/failure-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("garbage") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "failure" period after the font finishes loading is rendered with the old font. The test passes if there is text below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/failure-finish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-failure-finish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/failure-finish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/failure-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Failure");
+    internals.settings.setShouldIgnoreFontLoadCompletions(false);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "failure" period after the font finishes loading is rendered with the old font. The test passes if there is text below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/failure-nofinish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-failure-nofinish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/failure-nofinish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/failure-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontTimeoutIndex(7);
+    internals.settings.setShouldIgnoreFontLoadCompletions(true);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("garbage") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "failure" period while the font is still loading is rendered as visible. The test passes if there is text below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/failure-nofinish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-failure-nofinish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/failure-nofinish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/failure-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Failure");
+    internals.settings.setShouldIgnoreFontLoadCompletions(true);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "failure" period while the font is still loading is rendered as visible. The test passes if there is text below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/swap-finish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-swap-finish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-finish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "swap" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/swap-finish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-swap-finish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-finish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Swap");
+    internals.settings.setShouldIgnoreFontLoadCompletions(false);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "swap" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
+<div style="font: 100px 'MyFont';">Hello</div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/font-display/swap-flash-expected.html (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-flash-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-flash-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Failure");
+    internals.settings.setShouldIgnoreFontLoadCompletions(false);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text rendered in the "swap" period is identical to text rendered in the "failure" period. The test passes if the vertical position of the text below is identital (to the subpixel) between this file and the expected file.
+<div style="font-family: MyFont, Palatino; font-size: 24px; height: 30px; line-height: 28px;"><span style="font-family: MyFont, Palatino; font-size: 24px; line-height: 30px;">Descriptors</span></div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/font-display/swap-flash.html (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-flash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-flash.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Swap");
+    internals.settings.setShouldIgnoreFontLoadCompletions(true);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text rendered in the "swap" period is identical to text rendered in the "failure" period. The test passes if the vertical position of the text below is identital (to the subpixel) between this file and the expected file.
+<div style="font-family: MyFont, Palatino; font-size: 24px; height: 30px; line-height: 28px;"><span style="font-family: MyFont, Palatino; font-size: 24px; line-height: 30px;">Descriptors</span></div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/swap-nofinish-expected.html (from rev 226667, trunk/LayoutTests/fast/text/loading-swap-nofinish-expected.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-nofinish-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("garbage") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "swap" period while the font is still loading is rendered as visible. The test passes if there is text below.
+<div style="font: 100px 'MyFont', 'Helvetica';">Hello<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
+</body>
+</html>

Copied: trunk/LayoutTests/fast/text/font-display/swap-nofinish.html (from rev 226667, trunk/LayoutTests/fast/text/loading-swap-nofinish.html) (0 => 226668)


--- trunk/LayoutTests/fast/text/font-display/swap-nofinish.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-display/swap-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+    internals.settings.setFontLoadTimingOverride("Swap");
+    internals.settings.setShouldIgnoreFontLoadCompletions(true);
+}
+</script>
+<style>
+@font-face {
+    font-family: "MyFont";
+    src: url("../../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that text being drawn during the "swap" period while the font is still loading is rendered as visible. The test passes if there is text below.
+<div style="font: 100px 'MyFont', 'Helvetica';">Hello<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
+</body>
+</html>

Deleted: trunk/LayoutTests/fast/text/loading-block-finish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-block-finish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-block-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,15 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "block" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-block-finish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-block-finish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-block-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Block");
-    internals.settings.setShouldIgnoreFontLoadCompletions(false);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "block" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-block-nofinish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-block-nofinish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-block-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,14 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "block" period while the font is still loading is rendered as invisible. The test passes if there is no text below.
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-block-nofinish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-block-nofinish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-block-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Block");
-    internals.settings.setShouldIgnoreFontLoadCompletions(true);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "block" period while the font is still loading is rendered as invisible. The test passes if there is no text below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-failure-finish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-failure-finish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-failure-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,15 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("garbage") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "failure" period after the font finishes loading is rendered with the old font. The test passes if there is text below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-failure-finish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-failure-finish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-failure-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Failure");
-    internals.settings.setShouldIgnoreFontLoadCompletions(false);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "failure" period after the font finishes loading is rendered with the old font. The test passes if there is text below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-failure-nofinish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-failure-nofinish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-failure-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontTimeoutIndex(7);
-    internals.settings.setShouldIgnoreFontLoadCompletions(true);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("garbage") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "failure" period while the font is still loading is rendered as visible. The test passes if there is text below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-failure-nofinish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-failure-nofinish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-failure-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Failure");
-    internals.settings.setShouldIgnoreFontLoadCompletions(true);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "failure" period while the font is still loading is rendered as visible. The test passes if there is text below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-swap-finish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-swap-finish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-swap-finish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,15 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "swap" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-swap-finish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-swap-finish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-swap-finish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Swap");
-    internals.settings.setShouldIgnoreFontLoadCompletions(false);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "swap" period after the font finishes loading is rendered as visible. The test passes if you see a black rectangle below.
-<div style="font: 100px 'MyFont';">Hello</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-swap-nofinish-expected.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-swap-nofinish-expected.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-swap-nofinish-expected.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,15 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("garbage") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "swap" period while the font is still loading is rendered as visible. The test passes if there is text below.
-<div style="font: 100px 'MyFont', 'Helvetica';">Hello<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
-</body>
-</html>

Deleted: trunk/LayoutTests/fast/text/loading-swap-nofinish.html (226667 => 226668)


--- trunk/LayoutTests/fast/text/loading-swap-nofinish.html	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/fast/text/loading-swap-nofinish.html	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,21 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals) {
-    internals.settings.setFontLoadTimingOverride("Swap");
-    internals.settings.setShouldIgnoreFontLoadCompletions(true);
-}
-</script>
-<style>
-@font-face {
-    font-family: "MyFont";
-    src: url("../../resources/Ahem.ttf") format("truetype");
-}
-</style>
-</head>
-<body>
-This test makes sure that text being drawn during the "swap" period while the font is still loading is rendered as visible. The test passes if there is text below.
-<div style="font: 100px 'MyFont', 'Helvetica';">Hello<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
-</body>
-</html>

Modified: trunk/LayoutTests/platform/win/TestExpectations (226667 => 226668)


--- trunk/LayoutTests/platform/win/TestExpectations	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/LayoutTests/platform/win/TestExpectations	2018-01-10 00:44:49 UTC (rev 226668)
@@ -3789,10 +3789,10 @@
 webkit.org/b/177626 http/tests/preconnect/link-rel-preconnect-http.html [ Skip ]
 webkit.org/b/177626 http/tests/preconnect/link-rel-preconnect-https.html [ Skip ]
 
-webkit.org/b/177964 fast/text/loading-block-nofinish.html [ Pass ImageOnlyFailure ]
-webkit.org/b/177964 fast/text/loading-failure-finish.html [ Pass ImageOnlyFailure ]
-webkit.org/b/177964 fast/text/loading-failure-nofinish.html [ Pass ImageOnlyFailure ]
-webkit.org/b/177964 fast/text/loading-swap-nofinish.html [ Pass ImageOnlyFailure ]
+webkit.org/b/177964 fast/text/font-display/block-nofinish.html [ Pass ImageOnlyFailure ]
+webkit.org/b/177964 fast/text/font-display/failure-finish.html [ Pass ImageOnlyFailure ]
+webkit.org/b/177964 fast/text/font-display/failure-nofinish.html [ Pass ImageOnlyFailure ]
+webkit.org/b/177964 fast/text/font-display/swap-nofinish.html [ Pass ImageOnlyFailure ]
 
 # xhtml tests are failing on some of the EWS bots.
 webkit.org/b/178230 fast/xmlhttprequest/xmlhttprequest-get.xhtml [ Failure ]

Modified: trunk/Source/WebCore/ChangeLog (226667 => 226668)


--- trunk/Source/WebCore/ChangeLog	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Source/WebCore/ChangeLog	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,3 +1,33 @@
+2018-01-09  Myles C. Maxfield  <[email protected]>
+
+        font-display:fallback can cause a visual flash (which is supposed to be impossible)
+        https://bugs.webkit.org/show_bug.cgi?id=181374
+
+        Reviewed by Simon Fraser.
+
+        A FontCascade represents an entire font-family fallback list, but sometimes we need to pull out a single
+        representative font from the list to calculate things like line height. Previously, if the first item in
+        the font-family list was in the middle of being downloaded, this representative font was hardcoded to be
+        Times. However, when actually laying out and drawing the glyphs, we have logic to skip the interstitial
+        Times if there are any installed fonts present in the font-family list (so you wouldn't ever actually
+        see Times). This means that line height (among other things) was being calculated as if Times was used,
+        but in reality, some other font from the font-family list was being used.
+
+        Alone, this isn't a huge problem, but font-display:fallback makes a font transition between "timed out"
+        and "failed," and when the font hits the failed state, the representative font skips over the cancelled
+        item and hits the next item in the fallback list. This means that line heights will change, which causes
+        a visual flash, even when font-display:fallback is specified.
+
+        The solution is simply to educate the logic which identifies this representative font so that it
+        understands what to do for currently-loading fonts.
+
+        Tests: fast/text/font-display/swap-flash.html
+
+        * platform/graphics/FontCascadeFonts.h:
+        (WebCore::FontCascadeFonts::primaryFont):
+        * rendering/line/BreakingContext.h:
+        (WebCore::textWidth):
+
 2018-01-04  Filip Pizlo  <[email protected]>
 
         CodeBlocks should be in IsoSubspaces

Modified: trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h (226667 => 226668)


--- trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h	2018-01-10 00:44:49 UTC (rev 226668)
@@ -126,9 +126,21 @@
     ASSERT(isMainThread());
     if (!m_cachedPrimaryFont) {
         auto& primaryRanges = realizeFallbackRangesAt(description, 0);
-        m_cachedPrimaryFont = primaryRanges.fontForCharacter(' ');
+        m_cachedPrimaryFont = primaryRanges.glyphDataForCharacter(' ', ExternalResourceDownloadPolicy::Allow).font;
         if (!m_cachedPrimaryFont)
             m_cachedPrimaryFont = &primaryRanges.fontForFirstRange();
+        else if (m_cachedPrimaryFont->isInterstitial()) {
+            for (unsigned index = 1; ; ++index) {
+                auto& localRanges = realizeFallbackRangesAt(description, index);
+                if (localRanges.isNull())
+                    break;
+                auto* font = localRanges.glyphDataForCharacter(' ', ExternalResourceDownloadPolicy::Forbid).font;
+                if (font && !font->isInterstitial()) {
+                    m_cachedPrimaryFont = font;
+                    break;
+                }
+            }
+        }
     }
     return *m_cachedPrimaryFont;
 }

Modified: trunk/Source/WebCore/platform/graphics/FontRanges.h (226667 => 226668)


--- trunk/Source/WebCore/platform/graphics/FontRanges.h	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Source/WebCore/platform/graphics/FontRanges.h	2018-01-10 00:44:49 UTC (rev 226668)
@@ -83,7 +83,7 @@
     unsigned size() const { return m_ranges.size(); }
     const Range& rangeAt(unsigned i) const { return m_ranges[i]; }
 
-    GlyphData glyphDataForCharacter(UChar32, ExternalResourceDownloadPolicy) const;
+    WEBCORE_EXPORT GlyphData glyphDataForCharacter(UChar32, ExternalResourceDownloadPolicy) const;
     WEBCORE_EXPORT const Font* fontForCharacter(UChar32) const;
     WEBCORE_EXPORT const Font& fontForFirstRange() const;
     bool isLoading() const;

Modified: trunk/Source/WebCore/rendering/line/BreakingContext.h (226667 => 226668)


--- trunk/Source/WebCore/rendering/line/BreakingContext.h	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Source/WebCore/rendering/line/BreakingContext.h	2018-01-10 00:44:49 UTC (rev 226668)
@@ -621,6 +621,7 @@
     const RenderStyle& style = text.style();
 
     GlyphOverflow glyphOverflow;
+    // FIXME: This is not the right level of abstraction for isFixedPitch. Font fallback may make it such that the fixed pitch font is never actually used!
     if (isFixedPitch || (!from && len == text.text().length()) || style.hasTextCombine())
         return text.width(from, len, font, xPos, &fallbackFonts, &glyphOverflow);
 

Modified: trunk/Tools/ChangeLog (226667 => 226668)


--- trunk/Tools/ChangeLog	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Tools/ChangeLog	2018-01-10 00:44:49 UTC (rev 226668)
@@ -1,3 +1,17 @@
+2018-01-09  Myles C. Maxfield  <[email protected]>
+
+        font-display:fallback can cause a visual flash (which is supposed to be impossible)
+        https://bugs.webkit.org/show_bug.cgi?id=181374
+
+        Reviewed by Simon Fraser.
+
+        The test requires Palatino.
+
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (allowedFontFamilySet):
+        * WebKitTestRunner/mac/TestControllerMac.mm:
+        (WTR::allowedFontFamilySet):
+
 2018-01-09  Saam Barati  <[email protected]>
 
         Give some slack in display-profiler-outputs computation of the terminal window's number of columns

Modified: trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm (226667 => 226668)


--- trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm	2018-01-10 00:44:49 UTC (rev 226668)
@@ -394,6 +394,7 @@
         @"New Peninim MT",
         @"Optima",
         @"Osaka",
+        @"Palatino",
         @"Papyrus",
         @"PCMyungjo",
         @"PilGi",

Modified: trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm (226667 => 226668)


--- trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm	2018-01-10 00:30:38 UTC (rev 226667)
+++ trunk/Tools/WebKitTestRunner/mac/TestControllerMac.mm	2018-01-10 00:44:49 UTC (rev 226668)
@@ -235,6 +235,7 @@
         @"New Peninim MT",
         @"Optima",
         @"Osaka",
+        @"Palatino",
         @"Papyrus",
         @"PCMyungjo",
         @"PilGi",
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to