<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[208976] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/208976">208976</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2016-11-25 09:35:06 -0800 (Fri, 25 Nov 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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 <a href="http://trac.webkit.org/projects/webkit/changeset/201971">r201971</a> 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssFontFacecpp">trunk/Source/WebCore/css/FontFace.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextfontfacerebuildduringloading2expectedtxt">trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasttextfontfacerebuildduringloading2html">trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2.html</a></li>
<li><a href="#trunkLayoutTestsfasttextfontfacerebuildduringloadingexpectedtxt">trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasttextfontfacerebuildduringloadinghtml">trunk/LayoutTests/fast/text/fontface-rebuild-during-loading.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (208975 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-11-25 14:59:07 UTC (rev 208975)
+++ trunk/LayoutTests/ChangeLog        2016-11-25 17:35:06 UTC (rev 208976)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2016-11-25  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
+        [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.
+
</ins><span class="cx"> 2016-11-22  Antti Koivisto  &lt;antti@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         CrashTracer: [USER] com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::ExtensionStyleSheets::pageUserSheet + 14
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacerebuildduringloading2expectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2-expected.txt (0 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+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 &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS Initial then-block should succeed
+PASS loadedFont.family is &quot;WebFont2&quot;
+PASS successfullyParsed is true
+
+TEST COMPLETE
+asdf
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacerebuildduringloading2html"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-2.html (0 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,57 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script src=&quot;../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;style id=&quot;teststyle&quot;&gt;
+@font-face {
+        font-family: &quot;WebFont&quot;;
+        src: url(&quot;../../resources/Ahem.ttf&quot;) format(&quot;truetype&quot;);
+}
+&lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;div class=&quot;test&quot;&gt;asdf&lt;/div&gt;
+&lt;script&gt;
+if (window.internals) {
+    internals.clearMemoryCache();
+    internals.invalidateFontCache();
+}
+
+window.jsTestIsAsync = true;
+
+description(&quot;This test makes sure that FontFace.load promises still fire when the src attribute of the @font-face rule changes during a load.&quot;);
+
+function completeTest() {
+        finishJSTest();
+}
+var counter = 0;
+
+document.fonts.keys().next().value.load().then(function() {
+        testPassed(&quot;Initial then-block should succeed&quot;);
+        ++counter;
+        if (counter == 2)
+                completeTest();
+}, function() {
+        testFailed(&quot;Initial then-block should not fail&quot;);
+        finishJSTest();
+});
+
+var testStyle = document.getElementById(&quot;teststyle&quot;);
+testStyle.sheet.rules[0].style.fontFamily = &quot;WebFont2&quot;;
+testStyle.sheet.rules[0].style.src = &quot;url('../../resources/Ahem.otf') format('opentype')&quot;;
+
+var loadedFont;
+document.fonts.keys().next().value.load().then(function(f) {
+        loadedFont = f;
+        shouldBeEqualToString(&quot;loadedFont.family&quot;, &quot;WebFont2&quot;);
+        ++counter;
+        if (counter == 2)
+                completeTest();
+}, function() {
+        testFailed(&quot;Secondary then-block should not fail&quot;);
+        finishJSTest();
+});
+&lt;/script&gt;
+&lt;script src=&quot;../../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins><span class="cx">\ No newline at end of file
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacerebuildduringloadingexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading-expected.txt (0 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+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 &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS Font loaded.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+asdf
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacerebuildduringloadinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/fontface-rebuild-during-loading.html (0 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,42 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script src=&quot;../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;style&gt;
+@font-face {
+        font-family: &quot;WebFont&quot;;
+        src: url(&quot;../../resources/Ahem.ttf&quot;) format(&quot;truetype&quot;);
+}
+&lt;/style&gt;
+&lt;style id=&quot;teststyle&quot;&gt;
+.test {
+        font: 300px &quot;WebFont&quot;;
+}
+&lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;div class=&quot;test&quot;&gt;asdf&lt;/div&gt;
+&lt;script&gt;
+if (window.internals) {
+    internals.clearMemoryCache();
+    internals.invalidateFontCache();
+}
+
+window.jsTestIsAsync = true;
+
+description(&quot;This test makes sure that FontFace.load promises still fire when the CSSFontSelector has been rebuilt during a load.&quot;);
+
+document.fonts.keys().next().value.load().then(function() {
+        testPassed(&quot;Font loaded.&quot;);
+        finishJSTest();
+}, function() {
+        testFailed(&quot;Font should load&quot;);
+        finishJSTest();
+});
+
+var testStyle = document.getElementById(&quot;teststyle&quot;);
+testStyle.media = &quot;print&quot;;
+&lt;/script&gt;
+&lt;script src=&quot;../../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins><span class="cx">\ No newline at end of file
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (208975 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2016-11-25  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
+        [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):
+
</ins><span class="cx"> 2016-11-25  Andreas Kling  &lt;akling@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         MemoryPressureHandler should only trigger synchronous GC on iOS
</span></span></pre></div>
<a id="trunkSourceWebCorecssFontFacecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/FontFace.cpp (208975 => 208976)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -356,7 +356,6 @@
</span><span class="cx"> 
</span><span class="cx"> void FontFace::adopt(CSSFontFace&amp; newFace)
</span><span class="cx"> {
</span><del>-    m_promise = Nullopt;
</del><span class="cx">     m_backing-&gt;removeClient(*this);
</span><span class="cx">     m_backing = newFace;
</span><span class="cx">     m_backing-&gt;addClient(*this);
</span></span></pre>
</div>
</div>

</body>
</html>