The width of a nullptr TextRun should be zero
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:37:37 +0000 (19:37 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2018 19:37:37 +0000 (19:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189154
<rdar://problem/43685926>

Reviewed by Zalan Bujtas.

Source/WebCore:

If a page has an empty TextRun and attempts to paint it we can crash with a nullptr.

This patch recognizes that an empty TextRun should always produce a zero width, rather than
attempt to compute this value from font data.

Test: fast/text/null-string-textrun.html

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::widthOfTextRange const): An empty TextRun has zero width.
(WebCore::FontCascade::width const): Ditto.
(WebCore::FontCascade::codePath const): ASSERT that the TextRun is non-empty.

LayoutTests:

* fast/text/null-string-textrun-expected.txt: Added.
* fast/text/null-string-textrun.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235516 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text/null-string-textrun-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/null-string-textrun.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontCascade.cpp

index 5aedfbd..f0eb45e 100644 (file)
@@ -1,3 +1,14 @@
+2018-08-30  Brent Fulgham  <bfulgham@apple.com>
+
+        The width of a nullptr TextRun should be zero
+        https://bugs.webkit.org/show_bug.cgi?id=189154
+        <rdar://problem/43685926>
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/text/null-string-textrun-expected.txt: Added.
+        * fast/text/null-string-textrun.html: Added.
+
 2018-08-30  Eric Carlson  <eric.carlson@apple.com>
 
         Mock video devices should only support discrete sizes
diff --git a/LayoutTests/fast/text/null-string-textrun-expected.txt b/LayoutTests/fast/text/null-string-textrun-expected.txt
new file mode 100644 (file)
index 0000000..99d8c89
--- /dev/null
@@ -0,0 +1,6 @@
+This test confirms that a null text run doesn't trigger a crash. It passes if it loads without crashing.
+
+        
+        
+    
+
diff --git a/LayoutTests/fast/text/null-string-textrun.html b/LayoutTests/fast/text/null-string-textrun.html
new file mode 100644 (file)
index 0000000..b145900
--- /dev/null
@@ -0,0 +1,19 @@
+<!doctype html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<head>
+<body>
+    <p>This test confirms that a null text run doesn't trigger a crash. It passes if it loads without crashing.</p>
+    <pre id="pre_tag" dir="RTL" >
+        <style onload="pre_tag.appendChild(meter_tag)"/></style>
+        <select multiple="multiple">
+            <optgroup/>
+        </select>
+    </pre>
+    <label>
+        <meter id="meter_tag">
+    </label>
+</body>
\ No newline at end of file
index 1c4dde3..05033cb 100644 (file)
@@ -1,3 +1,23 @@
+2018-08-30  Brent Fulgham  <bfulgham@apple.com>
+
+        The width of a nullptr TextRun should be zero
+        https://bugs.webkit.org/show_bug.cgi?id=189154
+        <rdar://problem/43685926>
+
+        Reviewed by Zalan Bujtas.
+
+        If a page has an empty TextRun and attempts to paint it we can crash with a nullptr.
+
+        This patch recognizes that an empty TextRun should always produce a zero width, rather than
+        attempt to compute this value from font data.
+
+        Test: fast/text/null-string-textrun.html
+
+        * platform/graphics/FontCascade.cpp:
+        (WebCore::FontCascade::widthOfTextRange const): An empty TextRun has zero width.
+        (WebCore::FontCascade::width const): Ditto.
+        (WebCore::FontCascade::codePath const): ASSERT that the TextRun is non-empty.
+
 2018-08-30  Eric Carlson  <eric.carlson@apple.com>
 
         Mock video devices should only support discrete sizes
index 7648f5b..2fbd006 100644 (file)
@@ -341,6 +341,9 @@ float FontCascade::widthOfTextRange(const TextRun& run, unsigned from, unsigned
     ASSERT(from <= to);
     ASSERT(to <= run.length());
 
+    if (!run.length())
+        return 0;
+
     float offsetBeforeRange = 0;
     float offsetAfterRange = 0;
     float totalWidth = 0;
@@ -385,6 +388,9 @@ float FontCascade::widthOfTextRange(const TextRun& run, unsigned from, unsigned
 
 float FontCascade::width(const TextRun& run, HashSet<const Font*>* fallbackFonts, GlyphOverflow* glyphOverflow) const
 {
+    if (!run.length())
+        return 0;
+
     CodePath codePathToUse = codePath(run);
     if (codePathToUse != Complex) {
         // The complex path is more restrictive about returning fallback fonts than the simple path, so we need an explicit test to make their behaviors match.
@@ -604,6 +610,8 @@ FontCascade::CodePath FontCascade::codePath(const TextRun& run, std::optional<un
     if (s_codePath != Auto)
         return s_codePath;
 
+    ASSERT(run.length());
+
 #if !USE(FREETYPE)
     // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
     if ((enableKerning() || requiresShaping()) && (from.value_or(0) || to.value_or(run.length()) != run.length()))