Title: [201358] trunk
Revision
201358
Author
mmaxfi...@apple.com
Date
2016-05-24 15:04:07 -0700 (Tue, 24 May 2016)

Log Message

[Font Loading] Crash during font download failure after garbage collection
https://bugs.webkit.org/show_bug.cgi?id=158013
<rdar://problem/25148032>

Reviewed by Darin Adler.

Source/WebCore:

Usually, ownership during font loading is top-down - _javascript_ owns a JSFontFace,
which owns a FontFace, which owns a CSSFontFace. However, when we receive the
asynchronous callback that a font finished loading, the call comes in from the
bottom - it is delivered from the CSSFontFaceSource to the CSSFontFace, and then
to the FontFace. If a garbage collection had previously run, we might remove
the last reference to ourself during this asynchronous callback. A simple guard
makes sure this doesn't happen.

Test: fast/text/font-face-crash.html

* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::fontLoaded):

LayoutTests:

* fast/text/font-face-crash-expected.txt: Added.
* fast/text/font-face-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (201357 => 201358)


--- trunk/LayoutTests/ChangeLog	2016-05-24 22:03:30 UTC (rev 201357)
+++ trunk/LayoutTests/ChangeLog	2016-05-24 22:04:07 UTC (rev 201358)
@@ -1,3 +1,14 @@
+2016-05-24  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Font Loading] Crash during font download failure after garbage collection
+        https://bugs.webkit.org/show_bug.cgi?id=158013
+        <rdar://problem/25148032>
+
+        Reviewed by Darin Adler.
+
+        * fast/text/font-face-crash-expected.txt: Added.
+        * fast/text/font-face-crash.html: Added.
+
 201-05-24  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r201349.

Added: trunk/LayoutTests/fast/text/font-face-crash-expected.txt (0 => 201358)


--- trunk/LayoutTests/fast/text/font-face-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-crash-expected.txt	2016-05-24 22:04:07 UTC (rev 201358)
@@ -0,0 +1 @@
+This test makes sure that our failure load callback doesn't cause a crash if all the references to the FontFace have died. The test passes if there is no crash.
Property changes on: trunk/LayoutTests/fast/text/font-face-crash-expected.txt
___________________________________________________________________

Added: svn:keywords

Added: svn:eol-style

Added: trunk/LayoutTests/fast/text/font-face-crash.html (0 => 201358)


--- trunk/LayoutTests/fast/text/font-face-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/font-face-crash.html	2016-05-24 22:04:07 UTC (rev 201358)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+(function() {
+    var font = new FontFace("WebFont", "url('notarealfont')", {});
+    font.load();
+})();
+if (window.GCController)
+    GCController.collect();
+</script>
+<div style="font-family: WebFont;">This test makes sure that our failure load callback doesn't cause a crash if all the references to the FontFace have died. The test passes if there is no crash.</div>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (201357 => 201358)


--- trunk/Source/WebCore/ChangeLog	2016-05-24 22:03:30 UTC (rev 201357)
+++ trunk/Source/WebCore/ChangeLog	2016-05-24 22:04:07 UTC (rev 201358)
@@ -1,3 +1,24 @@
+2016-05-24  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Font Loading] Crash during font download failure after garbage collection
+        https://bugs.webkit.org/show_bug.cgi?id=158013
+        <rdar://problem/25148032>
+
+        Reviewed by Darin Adler.
+
+        Usually, ownership during font loading is top-down - _javascript_ owns a JSFontFace,
+        which owns a FontFace, which owns a CSSFontFace. However, when we receive the
+        asynchronous callback that a font finished loading, the call comes in from the
+        bottom - it is delivered from the CSSFontFaceSource to the CSSFontFace, and then
+        to the FontFace. If a garbage collection had previously run, we might remove
+        the last reference to ourself during this asynchronous callback. A simple guard
+        makes sure this doesn't happen.
+
+        Test: fast/text/font-face-crash.html
+
+        * css/CSSFontFace.cpp:
+        (WebCore::CSSFontFace::fontLoaded):
+
 2016-05-24  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r201349.

Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (201357 => 201358)


--- trunk/Source/WebCore/css/CSSFontFace.cpp	2016-05-24 22:03:30 UTC (rev 201357)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp	2016-05-24 22:04:07 UTC (rev 201358)
@@ -421,6 +421,8 @@
 
 void CSSFontFace::fontLoaded(CSSFontFaceSource&)
 {
+    Ref<CSSFontFace> protectedThis(*this);
+
     // If the font is already in the cache, CSSFontFaceSource may report it's loaded before it is added here as a source.
     // Let's not pump the state machine until we've got all our sources. font() and load() are smart enough to act correctly
     // when a source is failed or succeeded before we have asked it to load.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to