Attachment 348825 Details for Bug 189154 – Patch

[patch] Patch

bug-189154-20180904105043.patch (text/plain), 8.54 KB, created by Brent Fulgham on 2018-09-04 10:50:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2018-09-04 10:50:44 PDT

Size: 8.54 KB

patch

obsolete

>Subversion Revision: 235612
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 37942bc85786d383e5fea2cacfa9490a2102e4bc..a967f5c2c82e1049c357f8f9042963f0d02cb93f 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-09-02  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.
>+
>+        Most accessors in WTFString.cpp, such as isAllASCII(), hash(), etc., perform a nullptr check
>+        before using m_impl, but is8Bit() does not.
>+
>+        This patch adds a check in the is8Bit() implementation to be consistent with other methods,
>+        and to address a small number of crashes observed in testing.
>+
>+        * wtf/text/WTFString.h:
>+        (WTF::String::is8Bit const):
>+
> 2018-08-31  Antti Koivisto  <antti@apple.com>
> 
>         Replace OptionSet |= and -= operators with add() and remove() functions
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d63bca0e358f679ecc711030661f56a4aae59940..5383bab847b718fb62d3ea233384552decd5fc64 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-09-02  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/ComplexTextController.cpp:
>+        (WebCore::TextLayout::isNeeded): An empty RenderText does not need to layout its text.
>+        * 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.
>+        * rendering/RenderText.cpp:
>+        (WebCore::RenderText::computeCanUseSimplifiedTextMeasuring): Don't consider font and CodePath
>+        if the RenderText contains a nullptr string.
>+        * rendering/svg/SVGTextMetricsBuilder.cpp:
>+        (WebCore::SVGTextMetricsBuilder::initializeMeasurementWithTextRenderer): Only consider font
>+        and CodePath if the RenderText contains a non-nullptr string.
>+
> 2018-09-04  Zan Dobersek  <zdobersek@igalia.com> and Ms2ger  <Ms2ger@igalia.com>
> 
>         Implement support for passing ImageBitmap to texImage2D/texSubImage2D
>diff --git a/Source/WTF/wtf/text/WTFString.h b/Source/WTF/wtf/text/WTFString.h
>index b08be530af463f149aaa837e6fc0f72011beb344..fb18428a5eb3873278ef507808fa90b8e6904945 100644
>--- a/Source/WTF/wtf/text/WTFString.h
>+++ b/Source/WTF/wtf/text/WTFString.h
>@@ -154,7 +154,7 @@ public:
>     // Return characters8() or characters16() depending on CharacterType.
>     template<typename CharacterType> const CharacterType* characters() const;
> 
>-    bool is8Bit() const { return m_impl->is8Bit(); }
>+    bool is8Bit() const { return !m_impl || m_impl->is8Bit(); }
> 
>     unsigned sizeInBytes() const { return m_impl ? m_impl->length() * (is8Bit() ? sizeof(LChar) : sizeof(UChar)) : 0; }
> 
>diff --git a/Source/WebCore/platform/graphics/ComplexTextController.cpp b/Source/WebCore/platform/graphics/ComplexTextController.cpp
>index 7d8ef0a3dafc426df51e683fc4df08954c380ffe..7a7ab6c933c1c56e7d696d67fde46b1dc2f8aafa 100644
>--- a/Source/WebCore/platform/graphics/ComplexTextController.cpp
>+++ b/Source/WebCore/platform/graphics/ComplexTextController.cpp
>@@ -70,6 +70,9 @@ class TextLayout {
> public:
>     static bool isNeeded(RenderText& text, const FontCascade& font)
>     {
>+        if (!text.length())
>+            return false;
>+
>         TextRun run = RenderBlock::constructTextRun(text, text.style());
>         return font.codePath(run) == FontCascade::Complex;
>     }
>diff --git a/Source/WebCore/platform/graphics/FontCascade.cpp b/Source/WebCore/platform/graphics/FontCascade.cpp
>index 7648f5bf2693bee26f0598ecebffaf1ac23d042c..08692534a4747c0bc8229d5a4701c40f58060491 100644
>--- a/Source/WebCore/platform/graphics/FontCascade.cpp
>+++ b/Source/WebCore/platform/graphics/FontCascade.cpp
>@@ -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.
>diff --git a/Source/WebCore/rendering/RenderText.cpp b/Source/WebCore/rendering/RenderText.cpp
>index ff05b72d26fe4967534011ceaadcfd3f4fd4caf2..c00eaa80e9d759cecc89c9fe4c5699aeb89100ce 100644
>--- a/Source/WebCore/rendering/RenderText.cpp
>+++ b/Source/WebCore/rendering/RenderText.cpp
>@@ -1242,6 +1242,9 @@ bool RenderText::computeCanUseSimplifiedTextMeasuring() const
>     if (font.wordSpacing() || font.letterSpacing())
>         return false;
> 
>+    if (m_text.isEmpty())
>+        return false;
>+
>     // Additional check on the font codepath.
>     TextRun run(m_text);
>     run.setCharacterScanForCodePath(false);
>diff --git a/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp b/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
>index 324102584c1f113d0eed33214a67f5534ab0f59a..cabba838c9d2f13fba70c6221e286da177a1a120 100644
>--- a/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
>+++ b/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
>@@ -98,7 +98,7 @@ void SVGTextMetricsBuilder::initializeMeasurementWithTextRenderer(RenderSVGInlin
> 
>     const FontCascade& scaledFont = text.scaledFont();
>     m_run = SVGTextMetrics::constructTextRun(text);
>-    m_isComplexText = scaledFont.codePath(m_run) == FontCascade::Complex;
>+    m_isComplexText = text.length() && scaledFont.codePath(m_run) == FontCascade::Complex;
> 
>     if (m_isComplexText)
>         m_simpleWidthIterator = nullptr;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0d7fe7235b4458ce6a3e32e6ef4c55ea9295b7c1..805a676ca8920159e66f6cc2886e168887b36fbd 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-09-02  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-09-04  Zan Dobersek  <zdobersek@igalia.com> and Ms2ger  <Ms2ger@igalia.com>
> 
>         Implement support for passing ImageBitmap to texImage2D/texSubImage2D
>diff --git a/LayoutTests/fast/text/null-string-textrun-expected.txt b/LayoutTests/fast/text/null-string-textrun-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..99d8c8928ee37dbaadfa148b3dc7f111181b3bc4
>--- /dev/null
>+++ b/LayoutTests/fast/text/null-string-textrun-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..b145900dbfe85b5b77aff7171c8cb7cac79f1c41
>--- /dev/null
>+++ b/LayoutTests/fast/text/null-string-textrun.html
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 189154: 348501 | 348509 | 348518 | 348655 | 348743 | 348825 | 348925