<!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>[243828] 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/243828">243828</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2019-04-03 14:46:55 -0700 (Wed, 03 Apr 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>Documents can be destroyed before their CSSFontFaceSet is destroyed
https://bugs.webkit.org/show_bug.cgi?id=195830

Reviewed by Darin Adler.

Source/WebCore:

CSSFontFaceSet has a raw pointer to its owning document. JS can keep the CSSFontFaceSet alive (by using FontFaceSet)
and can destroy the document at any time. When the document is destroyed, the link between the two objects needs to
be severed.

Test: fast/text/font-face-set-destroy-document.html

* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::CSSFontFace):
* css/CSSFontFace.h:
* css/CSSFontFaceSet.cpp:
(WebCore::CSSFontFaceSet::CSSFontFaceSet):
(WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
* css/CSSFontFaceSet.h:
* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::CSSFontSelector):
(WebCore::CSSFontSelector::addFontFaceRule):
* css/CSSFontSelector.h:
* css/FontFace.cpp:
(WebCore::FontFace::FontFace):

LayoutTests:

* fast/text/font-face-set-destroy-document-expected.html: Added.
* fast/text/font-face-set-destroy-document.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="#trunkSourceWebCorecssCSSFontFaceh">trunk/Source/WebCore/css/CSSFontFace.h</a></li>
<li><a href="#trunkSourceWebCorecssCSSFontFaceSetcpp">trunk/Source/WebCore/css/CSSFontFaceSet.cpp</a></li>
<li><a href="#trunkSourceWebCorecssCSSFontFaceSeth">trunk/Source/WebCore/css/CSSFontFaceSet.h</a></li>
<li><a href="#trunkSourceWebCorecssCSSFontSelectorh">trunk/Source/WebCore/css/CSSFontSelector.h</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextfontfacesetdestroydocumentexpectedhtml">trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasttextfontfacesetdestroydocumenthtml">trunk/LayoutTests/fast/text/font-face-set-destroy-document.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/LayoutTests/ChangeLog 2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2019-04-03  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Documents can be destroyed before their CSSFontFaceSet is destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=195830
+
+        Reviewed by Darin Adler.
+
+        * fast/text/font-face-set-destroy-document-expected.html: Added.
+        * fast/text/font-face-set-destroy-document.html: Added.
+
</ins><span class="cx"> 2019-04-03  Shawn Roberts  <sroberts@apple.com>
</span><span class="cx"> 
</span><span class="cx">         http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html is a flaky timeout
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacesetdestroydocumentexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html (0 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html                         (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-destroy-document-expected.html    2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+<html>
+This test makes sure FontFaceSets don't access deleted documents. The test passes if there is no crash when running under ASan.
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextfontfacesetdestroydocumenthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/font-face-set-destroy-document.html (0 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/font-face-set-destroy-document.html                          (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-set-destroy-document.html     2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -0,0 +1,15 @@
</span><ins>+<html>
+This test makes sure FontFaceSets don't access deleted documents. The test passes if there is no crash when running under ASan.
+<script>
+
+d = document.implementation.createDocument(null, '');
+f = new FontFace('f', 'local(F)', {});
+ffs = d.fonts;
+delete d;
+// gc();
+GCController.collect();
+
+// trigger use after free
+ffs.add(f);
+</script>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/ChangeLog      2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2019-04-03  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Documents can be destroyed before their CSSFontFaceSet is destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=195830
+
+        Reviewed by Darin Adler.
+
+        CSSFontFaceSet has a raw pointer to its owning document. JS can keep the CSSFontFaceSet alive (by using FontFaceSet)
+        and can destroy the document at any time. When the document is destroyed, the link between the two objects needs to
+        be severed.
+
+        Test: fast/text/font-face-set-destroy-document.html
+
+        * css/CSSFontFace.cpp:
+        (WebCore::CSSFontFace::CSSFontFace):
+        * css/CSSFontFace.h:
+        * css/CSSFontFaceSet.cpp:
+        (WebCore::CSSFontFaceSet::CSSFontFaceSet):
+        (WebCore::CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered):
+        * css/CSSFontFaceSet.h:
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::CSSFontSelector):
+        (WebCore::CSSFontSelector::addFontFaceRule):
+        * css/CSSFontSelector.h:
+        * css/FontFace.cpp:
+        (WebCore::FontFace::FontFace):
+
</ins><span class="cx"> 2019-04-03  Sihui Liu  <sihui_liu@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Follow up fix for r243807: Use MarkedArgumentBuffer instead of Vector for JSValue
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSFontFaceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSFontFace.h (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSFontFace.h   2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFace.h      2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -189,7 +189,7 @@
</span><span class="cx">     FontLoadingBehavior m_loadingBehavior { FontLoadingBehavior::Auto };
</span><span class="cx"> 
</span><span class="cx">     Vector<std::unique_ptr<CSSFontFaceSource>, 0, CrashOnOverflow, 0> m_sources;
</span><del>-    RefPtr<CSSFontSelector> m_fontSelector;
</del><ins>+    RefPtr<CSSFontSelector> m_fontSelector; // FIXME: https://bugs.webkit.org/show_bug.cgi?id=196437 There's a retain cycle: CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
</ins><span class="cx">     RefPtr<StyleRuleFontFace> m_cssConnection;
</span><span class="cx">     HashSet<Client*> m_clients;
</span><span class="cx">     WeakPtr<FontFace> m_wrapper;
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSFontFaceSetcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSFontFaceSet.cpp (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSFontFaceSet.cpp      2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.cpp 2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -42,7 +42,7 @@
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><span class="cx"> CSSFontFaceSet::CSSFontFaceSet(CSSFontSelector* owningFontSelector)
</span><del>-    : m_owningFontSelector(owningFontSelector)
</del><ins>+    : m_owningFontSelector(makeWeakPtr(owningFontSelector))
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -113,7 +113,7 @@
</span><span class="cx"> 
</span><span class="cx">     Vector<Ref<CSSFontFace>> faces;
</span><span class="cx">     for (auto item : capabilities) {
</span><del>-        Ref<CSSFontFace> face = CSSFontFace::create(m_owningFontSelector, nullptr, nullptr, true);
</del><ins>+        Ref<CSSFontFace> face = CSSFontFace::create(m_owningFontSelector.get(), nullptr, nullptr, true);
</ins><span class="cx">         
</span><span class="cx">         Ref<CSSValueList> familyList = CSSValueList::createCommaSeparated();
</span><span class="cx">         familyList->append(CSSValuePool::singleton().createFontFamilyValue(familyName));
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSFontFaceSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSFontFaceSet.h (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSFontFaceSet.h        2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontFaceSet.h   2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -120,7 +120,7 @@
</span><span class="cx">     size_t m_facesPartitionIndex { 0 }; // All entries in m_faces before this index are CSS-connected.
</span><span class="cx">     Status m_status { Status::Loaded };
</span><span class="cx">     HashSet<CSSFontFaceSetClient*> m_clients;
</span><del>-    CSSFontSelector* m_owningFontSelector;
</del><ins>+    WeakPtr<CSSFontSelector> m_owningFontSelector;
</ins><span class="cx">     unsigned m_activeCount { 0 };
</span><span class="cx"> };
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSFontSelectorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSFontSelector.h (243827 => 243828)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSFontSelector.h       2019-04-03 21:29:23 UTC (rev 243827)
+++ trunk/Source/WebCore/css/CSSFontSelector.h  2019-04-03 21:46:55 UTC (rev 243828)
</span><span class="lines">@@ -47,7 +47,7 @@
</span><span class="cx"> class Document;
</span><span class="cx"> class StyleRuleFontFace;
</span><span class="cx"> 
</span><del>-class CSSFontSelector final : public FontSelector, public CSSFontFaceSetClient {
</del><ins>+class CSSFontSelector final : public FontSelector, public CSSFontFaceSetClient, public CanMakeWeakPtr<CSSFontSelector> {
</ins><span class="cx"> public:
</span><span class="cx">     static Ref<CSSFontSelector> create(Document& document)
</span><span class="cx">     {
</span></span></pre>
</div>
</div>

</body>
</html>