Title: [282320] trunk
Revision
282320
Author
mmaxfi...@apple.com
Date
2021-09-12 20:22:54 -0700 (Sun, 12 Sep 2021)

Log Message

[Cocoa] Drawing the rounded system ui font into canvas causes a crash
https://bugs.webkit.org/show_bug.cgi?id=230187
<rdar://problem/81436658>

Reviewed by Wenson Hsieh.

Source/WebKit:

It turns out that r281792 didn't do the right thing. It was trying to differentiate between
system fonts and installed fonts by looking at various values in the font's attribute dictionary.
However, the right way to do this is to treat the dictionary as opaque, and let
kCTFontOptionsSystemUIFont do the heavy lifting to make sure the font round-trips properly.

Test: fast/text/canvas-fonts.html

* Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:
(IPC::createCTFont):
(IPC::findFontDescriptor):

LayoutTests:

* fast/text/canvas-fonts-expected.txt: Added.
* fast/text/canvas-fonts.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (282319 => 282320)


--- trunk/LayoutTests/ChangeLog	2021-09-13 02:14:13 UTC (rev 282319)
+++ trunk/LayoutTests/ChangeLog	2021-09-13 03:22:54 UTC (rev 282320)
@@ -1,3 +1,14 @@
+2021-09-12  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Cocoa] Drawing the rounded system ui font into canvas causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=230187
+        <rdar://problem/81436658>
+
+        Reviewed by Wenson Hsieh.
+
+        * fast/text/canvas-fonts-expected.txt: Added.
+        * fast/text/canvas-fonts.html: Added.
+
 2021-09-12  Fujii Hironori  <hironori.fu...@sony.com>
 
         [WinCairo] Unreviewed test gardening

Added: trunk/LayoutTests/fast/text/canvas-fonts-expected.txt (0 => 282320)


--- trunk/LayoutTests/fast/text/canvas-fonts-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/canvas-fonts-expected.txt	2021-09-13 03:22:54 UTC (rev 282320)
@@ -0,0 +1,3 @@
+This test makes sure that drawing a variety of different kinds of fonts into canvas doesn't cause a crash. The test passes if there is no crash.
+
+
Property changes on: trunk/LayoutTests/fast/text/canvas-fonts-expected.txt
___________________________________________________________________

Added: svn:eol-style

+native \ No newline at end of property

Added: svn:keywords

+Author Date Id Rev URL \ No newline at end of property

Added: trunk/LayoutTests/fast/text/canvas-fonts.html (0 => 282320)


--- trunk/LayoutTests/fast/text/canvas-fonts.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/canvas-fonts.html	2021-09-13 03:22:54 UTC (rev 282320)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+<style>
+@font-face {
+    font-family: "WebFont";
+    src: url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+<p>
+This test makes sure that drawing a variety of different kinds of fonts into canvas doesn't cause a crash. The test passes if there is no crash.
+</p>
+<canvas id="canvas" width="400" height="200"></canvas>
+<script>
+let context = document.getElementById("canvas").getContext("2d");
+document.fonts.keys().next().value.load().then(function() {
+    for (const font of ["WebFont", "Helvetica", "system-ui", "ui-serif", "ui-monospace", "ui-rounded", "-apple-menu", "-apple-status-bar", "lastresort", "-apple-system-monospaced-numbers"]) {
+        for (const weight of ["normal", "bold"]) {
+            context.fillStyle = font == "WebFont" ? "black" : "green";
+            context.font = `${weight} 48px ${font}`;
+            context.fillText("Hello", 0, 150);
+        }
+    }
+    for (const font of ["body", "headline", "subheadline", "caption1", "caption2", "footnote", "short-body", "short-headline", "short-subheadline", "short-caption1", "short-footnote", "tall-body", "title0", "title1", "title2", "title3", "title4"]) {
+        context.font = `-apple-system-${font}`;
+        context.fillText("Hello", 0, 150);
+    }
+    if (window.testRunner)
+        testRunner.notifyDone();
+});
+</script>
+</body>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (282319 => 282320)


--- trunk/Source/WebKit/ChangeLog	2021-09-13 02:14:13 UTC (rev 282319)
+++ trunk/Source/WebKit/ChangeLog	2021-09-13 03:22:54 UTC (rev 282320)
@@ -1,3 +1,22 @@
+2021-09-12  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Cocoa] Drawing the rounded system ui font into canvas causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=230187
+        <rdar://problem/81436658>
+
+        Reviewed by Wenson Hsieh.
+
+        It turns out that r281792 didn't do the right thing. It was trying to differentiate between
+        system fonts and installed fonts by looking at various values in the font's attribute dictionary.
+        However, the right way to do this is to treat the dictionary as opaque, and let
+        kCTFontOptionsSystemUIFont do the heavy lifting to make sure the font round-trips properly.
+
+        Test: fast/text/canvas-fonts.html
+
+        * Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:
+        (IPC::createCTFont):
+        (IPC::findFontDescriptor):
+
 2021-09-12  David Kilzer  <ddkil...@apple.com>
 
         REGRESSION (r280758): WebKit project won't open in Xcode 11.4

Modified: trunk/Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm (282319 => 282320)


--- trunk/Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm	2021-09-13 02:14:13 UTC (rev 282319)
+++ trunk/Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm	2021-09-13 03:22:54 UTC (rev 282320)
@@ -491,7 +491,6 @@
     if (CFArrayGetCount(fontDescriptors.get()) == 1)
         return static_cast<CTFontDescriptorRef>(CFArrayGetValueAtIndex(fontDescriptors.get(), 0));
 
-    // There's supposed to only be a single item in the array, but we can be defensive here.
     for (CFIndex i = 0; i < CFArrayGetCount(fontDescriptors.get()); ++i) {
         auto fontDescriptor = static_cast<CTFontDescriptorRef>(CFArrayGetValueAtIndex(fontDescriptors.get(), i));
         auto currentPostScriptName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(fontDescriptor, kCTFontNameAttribute)));
@@ -501,22 +500,25 @@
     return nullptr;
 }
 
-static RetainPtr<CTFontRef> createCTFont(CFDictionaryRef attributes, float size, const String& referenceURL, const String& postScriptName)
+static RetainPtr<CTFontRef> createCTFont(CFDictionaryRef attributes, float size, const String& referenceURL, const String& desiredPostScriptName)
 {
-    if (auto name = static_cast<CFStringRef>(CFDictionaryGetValue(attributes, kCTFontNameAttribute))) {
-        if (CFStringHasPrefix(name, CFSTR("."))) {
-            auto fontDescriptor = adoptCF(CTFontDescriptorCreateWithAttributes(attributes));
-            if (!fontDescriptor)
-                return nullptr;
-            return adoptCF(CTFontCreateWithFontDescriptorAndOptions(fontDescriptor.get(), size, nullptr, kCTFontOptionsSystemUIFont));
-        }
+    auto fontDescriptor = adoptCF(CTFontDescriptorCreateWithAttributes(attributes));
+    if (fontDescriptor) {
+        auto font = adoptCF(CTFontCreateWithFontDescriptorAndOptions(fontDescriptor.get(), size, nullptr, kCTFontOptionsSystemUIFont));
+        String actualPostScriptName(adoptCF(CTFontCopyPostScriptName(font.get())).get());
+        if (actualPostScriptName == desiredPostScriptName)
+            return font;
     }
 
-    auto fontDescriptor = findFontDescriptor(referenceURL, postScriptName);
-    if (!fontDescriptor)
-        return nullptr;
-    fontDescriptor = adoptCF(CTFontDescriptorCreateCopyWithAttributes(fontDescriptor.get(), attributes));
-    return adoptCF(CTFontCreateWithFontDescriptor(fontDescriptor.get(), size, nullptr));
+    // CoreText couldn't round-trip the font.
+    // We can fall back to doing our best to find it ourself.
+    fontDescriptor = findFontDescriptor(referenceURL, desiredPostScriptName);
+    if (!fontDescriptor) {
+        ASSERT_NOT_REACHED();
+        fontDescriptor = CTFontDescriptorCreateLastResort();
+    }
+    ASSERT(fontDescriptor);
+    return adoptCF(CTFontCreateWithFontDescriptorAndOptions(fontDescriptor.get(), size, nullptr, kCTFontOptionsSystemUIFont));
 }
 
 std::optional<WebCore::FontPlatformData> ArgumentCoder<Ref<WebCore::Font>>::decodePlatformData(Decoder& decoder)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to