Title: [208976] trunk
Revision
208976
Author
mmaxfi...@apple.com
Date
2016-11-25 09:35:06 -0800 (Fri, 25 Nov 2016)

Log Message

[CSS Font Loading] FontFace.load() promises don't always fire
https://bugs.webkit.org/show_bug.cgi?id=165037

Reviewed by Simon Fraser.

Source/WebCore:

We currently handle web fonts in two phases. The first phase is building up
StyleRuleFontFace objects which reflect the style on the page. The second is creating
CSSFontFace objects from those StyleRuleFontFace objects. When script modifies the
style on the page, we can often update the CSSFontFace objects, but there are some
modifications which we don't know how to model. For these operations, we destroy the
CSSFontFace objects and rebuild them from the newly modified StyleRuleFontFace objects.

Normally, this is fine. However, with the CSS font loading API, the CSSFontFaces back
_javascript_ objects which will persist across the rebuilding step mentioned above. This
means that the FontFace objects need to adopt the new CSSFontFace objects and forget
the old CSSFontFace objects.

This gets a little tricky because the operation which caused the rebuild may actually
be a modification to the specific @font-face block which backs a _javascript_ FontFace
object. Because the CSSOM can be used to change the src: attribute of the FontFace
object, I decided in r201971 to clear the FontFace's promise in case an old load would
cause the promise to resolve. However, this would never happen because the old
CSSFontFace is unparented during the FontFace::adopt()ion of the new CSSFontFace.
Therefore, old loads may still complete, but the signal would never make it to the
FontFace and therefore would not cause the promise to resolve. In addition, clearing
the promise during a rebuild is problematic because that rebuild may be caused by
operations which have nothing to do with the specific FontFace object in question (so
the FontFace object should be observably uneffected.)

Because of the above reasons, this patch simply stops clearing the promise during the
rebuild phase.

Tests: fast/text/fontface-rebuild-during-loading.html
       fast/text/fontface-rebuild-during-loading-2.html

* css/FontFace.cpp:
(WebCore::FontFace::adopt):

LayoutTests:

* fast/text/fontfaceset-rebuild-during-loading-2-expected.txt: Added.
* fast/text/fontfaceset-rebuild-during-loading-2.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (208975 => 208976)


--- trunk/LayoutTests/ChangeLog	2016-11-25 14:59:07 UTC (rev 208975)
+++ trunk/LayoutTests/ChangeLog	2016-11-25 17:35:06 UTC (rev 208976)
@@ -1,3 +1,13 @@
+2016-11-25  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [CSS Font Loading] FontFace.load() promises don't always fire
+        https://bugs.webkit.org/show_bug.cgi?id=165037
+
+        Reviewed by Simon Fraser.
+
+        * fast/text/fontfaceset-rebuild-during-loading-2-expected.txt: Added.
+        * fast/text/fontfaceset-rebuild-during-loading-2.html: Added.
+
 2016-11-22  Antti Koivisto  <an...@apple.com>
 
         CrashTracer: [USER] com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::ExtensionStyleSheets::pageUserSheet + 14

Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2-expected.txt (0 => 208976)


--- trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2-expected.txt	2016-11-25 17:35:06 UTC (rev 208976)
@@ -0,0 +1,11 @@
+This test makes sure that FontFace.load promises still fire when the src attribute of the @font-face rule changes during a load.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Initial then-block should succeed
+PASS loadedFont.family is "WebFont2"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+asdf

Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2.html (0 => 208976)


--- trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2.html	2016-11-25 17:35:06 UTC (rev 208976)
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style id="teststyle">
+@font-face {
+	font-family: "WebFont";
+	src: url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+<div class="test">asdf</div>
+<script>
+if (window.internals) {
+    internals.clearMemoryCache();
+    internals.invalidateFontCache();
+}
+
+window.jsTestIsAsync = true;
+
+description("This test makes sure that FontFace.load promises still fire when the src attribute of the @font-face rule changes during a load.");
+
+function completeTest() {
+	finishJSTest();
+}
+var counter = 0;
+
+document.fonts.keys().next().value.load().then(function() {
+	testPassed("Initial then-block should succeed");
+	++counter;
+	if (counter == 2)
+		completeTest();
+}, function() {
+	testFailed("Initial then-block should not fail");
+	finishJSTest();
+});
+
+var testStyle = document.getElementById("teststyle");
+testStyle.sheet.rules[0].style.fontFamily = "WebFont2";
+testStyle.sheet.rules[0].style.src = "" format('opentype')";
+
+var loadedFont;
+document.fonts.keys().next().value.load().then(function(f) {
+	loadedFont = f;
+	shouldBeEqualToString("loadedFont.family", "WebFont2");
+	++counter;
+	if (counter == 2)
+		completeTest();
+}, function() {
+	testFailed("Secondary then-block should not fail");
+	finishJSTest();
+});
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file

Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-expected.txt (0 => 208976)


--- trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-expected.txt	2016-11-25 17:35:06 UTC (rev 208976)
@@ -0,0 +1,10 @@
+This test makes sure that FontFace.load promises still fire when the CSSFontSelector has been rebuilt during a load.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Font loaded.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+asdf

Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading.html (0 => 208976)


--- trunk/LayoutTests/fast/text/fontface-rebuild-during-loading.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/fontface-rebuild-during-loading.html	2016-11-25 17:35:06 UTC (rev 208976)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+@font-face {
+	font-family: "WebFont";
+	src: url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+<style id="teststyle">
+.test {
+	font: 300px "WebFont";
+}
+</style>
+</head>
+<body>
+<div class="test">asdf</div>
+<script>
+if (window.internals) {
+    internals.clearMemoryCache();
+    internals.invalidateFontCache();
+}
+
+window.jsTestIsAsync = true;
+
+description("This test makes sure that FontFace.load promises still fire when the CSSFontSelector has been rebuilt during a load.");
+
+document.fonts.keys().next().value.load().then(function() {
+	testPassed("Font loaded.");
+	finishJSTest();
+}, function() {
+	testFailed("Font should load");
+	finishJSTest();
+});
+
+var testStyle = document.getElementById("teststyle");
+testStyle.media = "print";
+</script>
+<script src=""
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (208975 => 208976)


--- trunk/Source/WebCore/ChangeLog	2016-11-25 14:59:07 UTC (rev 208975)
+++ trunk/Source/WebCore/ChangeLog	2016-11-25 17:35:06 UTC (rev 208976)
@@ -1,3 +1,43 @@
+2016-11-25  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [CSS Font Loading] FontFace.load() promises don't always fire
+        https://bugs.webkit.org/show_bug.cgi?id=165037
+
+        Reviewed by Simon Fraser.
+
+        We currently handle web fonts in two phases. The first phase is building up
+        StyleRuleFontFace objects which reflect the style on the page. The second is creating
+        CSSFontFace objects from those StyleRuleFontFace objects. When script modifies the
+        style on the page, we can often update the CSSFontFace objects, but there are some
+        modifications which we don't know how to model. For these operations, we destroy the
+        CSSFontFace objects and rebuild them from the newly modified StyleRuleFontFace objects.
+
+        Normally, this is fine. However, with the CSS font loading API, the CSSFontFaces back
+        _javascript_ objects which will persist across the rebuilding step mentioned above. This
+        means that the FontFace objects need to adopt the new CSSFontFace objects and forget
+        the old CSSFontFace objects.
+
+        This gets a little tricky because the operation which caused the rebuild may actually
+        be a modification to the specific @font-face block which backs a _javascript_ FontFace
+        object. Because the CSSOM can be used to change the src: attribute of the FontFace
+        object, I decided in r201971 to clear the FontFace's promise in case an old load would
+        cause the promise to resolve. However, this would never happen because the old
+        CSSFontFace is unparented during the FontFace::adopt()ion of the new CSSFontFace.
+        Therefore, old loads may still complete, but the signal would never make it to the
+        FontFace and therefore would not cause the promise to resolve. In addition, clearing
+        the promise during a rebuild is problematic because that rebuild may be caused by
+        operations which have nothing to do with the specific FontFace object in question (so
+        the FontFace object should be observably uneffected.)
+
+        Because of the above reasons, this patch simply stops clearing the promise during the
+        rebuild phase.
+
+        Tests: fast/text/fontface-rebuild-during-loading.html
+               fast/text/fontface-rebuild-during-loading-2.html
+
+        * css/FontFace.cpp:
+        (WebCore::FontFace::adopt):
+
 2016-11-25  Andreas Kling  <akl...@apple.com>
 
         MemoryPressureHandler should only trigger synchronous GC on iOS

Modified: trunk/Source/WebCore/css/FontFace.cpp (208975 => 208976)


--- trunk/Source/WebCore/css/FontFace.cpp	2016-11-25 14:59:07 UTC (rev 208975)
+++ trunk/Source/WebCore/css/FontFace.cpp	2016-11-25 17:35:06 UTC (rev 208976)
@@ -356,7 +356,6 @@
 
 void FontFace::adopt(CSSFontFace& newFace)
 {
-    m_promise = Nullopt;
     m_backing->removeClient(*this);
     m_backing = newFace;
     m_backing->addClient(*this);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to