Attachment 348448 Details for Bug 189119 – Patch

[patch] Patch

bug-189119-20180829164953.patch (text/plain), 7.69 KB, created by Daniel Bates on 2018-08-29 16:49:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-08-29 16:49:54 PDT

Size: 7.69 KB

patch

obsolete

>Subversion Revision: 235381
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2673bda26340d546a9b715aade6a0a240a7e5d05..6a95f1606c50b7c063f08c3b88c80a14ad5e5150 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-08-29  Daniel Bates  <dabates@apple.com>
>+
>+        Do not create inline text boxes for empty combined text
>+        https://bugs.webkit.org/show_bug.cgi?id=189119
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        An empty inline text box does not visually effect the rendering of the page. We should not
>+        create such text boxes both to reduce memory and to make it easier to reason about the line
>+        box tree.
>+
>+        * rendering/InlineTextBox.cpp:
>+        (WebCore::InlineTextBox::isEmpty const): Added. Returns whether an inline text box is empty.
>+        We do not need to consider hypenation since hypens are an embellishment (i.e. they are not
>+        part of the markup of the page).
>+        (WebCore::InlineTextBox::paint): Write in terms of isEmpty().
>+        (WebCore::InlineTextBox::subdivideAndResolveStyle): Assert that WebCore::subdivide() always
>+        returns a non-empty list of subdivisions. A non-empty text box should always have at least
>+        one subdivision, say for the unmarked text. I left the existing conditonal (though marked
>+        it as UNLIKELY()) so as to be forgiving and avoid a bad user experience should WebCore::subdivide()
>+        return an empty vector in a non-debug build.
>+        * rendering/InlineTextBox.h:
>+        * rendering/RenderBlockLineLayout.cpp:
>+        (WebCore::RenderBlockFlow::computeBlockDirectionPositionsForLine): Write in terms of InlineTextBox::isEmpty()
>+        so that we remove empty inline text boxes associated with combined text.
>+        * rendering/RenderText.cpp:
>+        (WebCore::RenderText::positionLineBox): Write in terms of InlineTextBox::isEmpty().
>+
> 2018-08-29  Daniel Bates  <dabates@apple.com>
> 
>         REGRESSION (r226138): WebCore::subdivide() may return an empty an empty vector; Web process can crash when performing find in Epiphany
>diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp
>index 59a7565652cfe8c1d65fba60bd469deb2c326c10..b5ed38c78a3673abe926dd2a1b8761b861cbd1dd 100644
>--- a/Source/WebCore/rendering/InlineTextBox.cpp
>+++ b/Source/WebCore/rendering/InlineTextBox.cpp
>@@ -76,6 +76,15 @@ InlineTextBox::~InlineTextBox()
>     TextPainter::removeGlyphDisplayList(*this);
> }
> 
>+bool InlineTextBox::isEmpty() const
>+{
>+    if (!m_len)
>+        return true;
>+    if (auto* combinedText = this->combinedText())
>+        return combinedText->combinedStringForRendering().isEmpty();
>+    return false;
>+}
>+
> void InlineTextBox::markDirty(bool dirty)
> {
>     if (dirty) {
>@@ -437,7 +446,7 @@ static MarkedText createMarkedTextFromSelectionInBox(const InlineTextBox& box)
> void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit /*lineTop*/, LayoutUnit /*lineBottom*/)
> {
>     if (isLineBreak() || !paintInfo.shouldPaintWithinRoot(renderer()) || renderer().style().visibility() != Visibility::Visible
>-        || m_truncation == cFullTruncation || paintInfo.phase == PaintPhase::Outline || !m_len)
>+        || m_truncation == cFullTruncation || paintInfo.phase == PaintPhase::Outline || isEmpty())
>         return;
> 
>     ASSERT(paintInfo.phase != PaintPhase::SelfOutline && paintInfo.phase != PaintPhase::ChildOutlines);
>@@ -792,7 +801,8 @@ auto InlineTextBox::subdivideAndResolveStyle(const Vector<MarkedText>& textsToSu
>         return { };
> 
>     auto markedTexts = subdivide(textsToSubdivide);
>-    if (markedTexts.isEmpty())
>+    ASSERT(!markedTexts.isEmpty());
>+    if (UNLIKELY(markedTexts.isEmpty()))
>         return { };
> 
>     // Compute frontmost overlapping styled marked texts.
>diff --git a/Source/WebCore/rendering/InlineTextBox.h b/Source/WebCore/rendering/InlineTextBox.h
>index 9156bac891ef2078c53a61aef6f4eec7871fdfad..e079d36180a7d9f5d2b297801bf75a63b932d72b 100644
>--- a/Source/WebCore/rendering/InlineTextBox.h
>+++ b/Source/WebCore/rendering/InlineTextBox.h
>@@ -57,6 +57,8 @@ public:
>     void setNextTextBox(InlineTextBox* n) { m_nextTextBox = n; }
>     void setPreviousTextBox(InlineTextBox* p) { m_prevTextBox = p; }
> 
>+    bool isEmpty() const;
>+
>     // FIXME: These accessors should ASSERT(!isDirty()). See https://bugs.webkit.org/show_bug.cgi?id=97264
>     unsigned start() const { return m_start; }
>     unsigned end() const { return m_len ? m_start + m_len - 1 : m_start; }
>diff --git a/Source/WebCore/rendering/RenderBlockLineLayout.cpp b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
>index b188a46e269b5c7087a1625d1731fbd5fb9b1134..5cf26a69ea81b8a117e8f3ee6ab3fd7c17c4976e 100644
>--- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp
>+++ b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
>@@ -1001,7 +1001,7 @@ void RenderBlockFlow::computeBlockDirectionPositionsForLine(RootInlineBox* lineB
>         if (is<RenderText>(renderer)) {
>             auto& inlineTextBox = downcast<InlineTextBox>(*run->box());
>             downcast<RenderText>(renderer).positionLineBox(inlineTextBox);
>-            inlineBoxIsRedundant = !inlineTextBox.len();
>+            inlineBoxIsRedundant = inlineTextBox.isEmpty();
>         } else if (is<RenderBox>(renderer)) {
>             downcast<RenderBox>(renderer).positionLineBox(downcast<InlineElementBox>(*run->box()));
>             inlineBoxIsRedundant = renderer.isOutOfFlowPositioned();
>diff --git a/Source/WebCore/rendering/RenderText.cpp b/Source/WebCore/rendering/RenderText.cpp
>index ff05b72d26fe4967534011ceaadcfd3f4fd4caf2..4e166530675da738b736cbed6f5e12731e3b0c92 100644
>--- a/Source/WebCore/rendering/RenderText.cpp
>+++ b/Source/WebCore/rendering/RenderText.cpp
>@@ -1304,7 +1304,7 @@ std::unique_ptr<InlineTextBox> RenderText::createTextBox()
> 
> void RenderText::positionLineBox(InlineTextBox& textBox)
> {
>-    if (!textBox.len())
>+    if (textBox.isEmpty())
>         return;
>     m_containsReversedText |= !textBox.isLeftToRightDirection();
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 3ec5d8cb8e51748e91e764549322a0c3fedf5787..4985194c1369bf2b4cb8b5cdb79e2cf87c17965d 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-08-29  Daniel Bates  <dabates@apple.com>
>+
>+        Do not create inline text boxes for empty combined text
>+        https://bugs.webkit.org/show_bug.cgi?id=189119
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update expected result now that we do not create an inline text box associated with combined text
>+        when we do not have any combined text to render.
>+
>+        * fast/text/text-combine-surroundContents-crash-expected.txt:
>+
> 2018-08-29  Daniel Bates  <dabates@apple.com>
> 
>         REGRESSION (r226138): WebCore::subdivide() may return an empty an empty vector; Web process can crash when performing find in Epiphany
>diff --git a/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt b/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt
>index 8fcf441a4c67b324eac5001e9a98b9c1eb344391..53f12c69bce8d7262ebfa72b26b75a85bbf0ed2a 100644
>--- a/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt
>+++ b/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt
>@@ -4,8 +4,7 @@ layer at (0,0) size 800x76
>   RenderBlock {HTML} at (0,0) size 800x76
>     RenderBody {BODY} at (8,8) size 784x60
>       RenderBlock {DIV} at (0,0) size 20x40
>-        RenderCombineText {#text} at (0,0) size 20x20
>-          text run at (0,0) width 20: "\x{FFFC}"
>+        RenderCombineText {#text} at (0,0) size 0x0
>         RenderInline {SPAN} at (0,0) size 20x0
>         RenderCombineText {#text} at (0,20) size 20x20
>           text run at (0,20) width 20: "\x{FFFC}"

Actions: View | Formatted Diff | Diff

Attachments on bug 189119: 348448 | 348510