- Revision
- 236139
- Author
- [email protected]
- Date
- 2018-09-18 08:59:18 -0700 (Tue, 18 Sep 2018)
Log Message
Merge r235615 - Remove redundant inline text boxes for empty combined text
https://bugs.webkit.org/show_bug.cgi?id=189119
Reviewed by Zalan Bujtas.
Source/WebCore:
We should consider inline text boxes that have a combined text renderer (RenderCombineText)
whose composed string is empty as "redundant" just as we do for inline text boxes that have
a non-combined text renderer that have zero length so that we remove them. Such boxes are
visibly empty and do not take up space visually. By removing them we reduce memory and make
it easier to reason about the line box tree.
Currently RenderBlockFlow::computeBlockDirectionPositionsForLine() tests if an inline text
box is empty by checking if it has a zero length (InlineTextBox::len()). However an inline
text box associated with a RenderCombineText always has length 1 regardless of whether the
composed string it represents is the empty string. Instead we should expose a way to check
if an inline text box is visually empty and have RenderBlockFlow::computeBlockDirectionPositionsForLine()
query the inline text box for this answer.
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::hasTextContent const): Added. Returns whether an inline text box
has text content. 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 hasTextContent().
(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::hasTextContent()
so that we remove empty inline text boxes associated with combined text.
* rendering/RenderText.cpp:
(WebCore::RenderText::positionLineBox): Write in terms of InlineTextBox::hasTextContent().
LayoutTests:
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:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog 2018-09-18 15:59:18 UTC (rev 236139)
@@ -1,3 +1,15 @@
+2018-09-04 Daniel Bates <[email protected]>
+
+ Remove redundant inline text boxes for empty combined text
+ https://bugs.webkit.org/show_bug.cgi?id=189119
+
+ Reviewed by Zalan Bujtas.
+
+ 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-09-03 Youenn Fablet <[email protected]>
REGRESSION: Layout Test http/tests/security/bypassing-cors-checks-for-extension-urls.html is Flaky
Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt 2018-09-18 15:59:18 UTC (rev 236139)
@@ -4,8 +4,7 @@
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}"
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog 2018-09-18 15:59:18 UTC (rev 236139)
@@ -1,3 +1,40 @@
+2018-09-04 Daniel Bates <[email protected]>
+
+ Remove redundant inline text boxes for empty combined text
+ https://bugs.webkit.org/show_bug.cgi?id=189119
+
+ Reviewed by Zalan Bujtas.
+
+ We should consider inline text boxes that have a combined text renderer (RenderCombineText)
+ whose composed string is empty as "redundant" just as we do for inline text boxes that have
+ a non-combined text renderer that have zero length so that we remove them. Such boxes are
+ visibly empty and do not take up space visually. By removing them we reduce memory and make
+ it easier to reason about the line box tree.
+
+ Currently RenderBlockFlow::computeBlockDirectionPositionsForLine() tests if an inline text
+ box is empty by checking if it has a zero length (InlineTextBox::len()). However an inline
+ text box associated with a RenderCombineText always has length 1 regardless of whether the
+ composed string it represents is the empty string. Instead we should expose a way to check
+ if an inline text box is visually empty and have RenderBlockFlow::computeBlockDirectionPositionsForLine()
+ query the inline text box for this answer.
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::hasTextContent const): Added. Returns whether an inline text box
+ has text content. 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 hasTextContent().
+ (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::hasTextContent()
+ so that we remove empty inline text boxes associated with combined text.
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::positionLineBox): Write in terms of InlineTextBox::hasTextContent().
+
2018-09-03 Youenn Fablet <[email protected]>
REGRESSION: Layout Test http/tests/security/bypassing-cors-checks-for-extension-urls.html is Flaky
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.cpp (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.cpp 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.cpp 2018-09-18 15:59:18 UTC (rev 236139)
@@ -76,6 +76,17 @@
TextPainter::removeGlyphDisplayList(*this);
}
+bool InlineTextBox::hasTextContent() const
+{
+ if (m_len > 1)
+ return true;
+ if (auto* combinedText = this->combinedText()) {
+ ASSERT(m_len == 1);
+ return !combinedText->combinedStringForRendering().isEmpty();
+ }
+ return false;
+}
+
void InlineTextBox::markDirty(bool dirty)
{
if (dirty) {
@@ -437,7 +448,7 @@
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 || !hasTextContent())
return;
ASSERT(paintInfo.phase != PaintPhase::SelfOutline && paintInfo.phase != PaintPhase::ChildOutlines);
@@ -792,7 +803,8 @@
return { };
auto markedTexts = subdivide(textsToSubdivide);
- if (markedTexts.isEmpty())
+ ASSERT(!markedTexts.isEmpty());
+ if (UNLIKELY(markedTexts.isEmpty()))
return { };
// Compute frontmost overlapping styled marked texts.
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.h (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.h 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/InlineTextBox.h 2018-09-18 15:59:18 UTC (rev 236139)
@@ -57,7 +57,14 @@
void setNextTextBox(InlineTextBox* n) { m_nextTextBox = n; }
void setPreviousTextBox(InlineTextBox* p) { m_prevTextBox = p; }
+ bool hasTextContent() const;
+
+ // These functions do not account for combined text. For combined text this box will always have len() == 1
+ // regardless of whether the resulting composition is the empty string. Use hasTextContent() if you want to
+ // know whether this box has text content.
+ //
// FIXME: These accessors should ASSERT(!isDirty()). See https://bugs.webkit.org/show_bug.cgi?id=97264
+ // Note len() == 1 for combined text regardless of whether the composition is empty. Use hasTextContent() to
unsigned start() const { return m_start; }
unsigned end() const { return m_len ? m_start + m_len - 1 : m_start; }
unsigned len() const { return m_len; }
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderBlockLineLayout.cpp (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2018-09-18 15:59:18 UTC (rev 236139)
@@ -1001,7 +1001,7 @@
if (is<RenderText>(renderer)) {
auto& inlineTextBox = downcast<InlineTextBox>(*run->box());
downcast<RenderText>(renderer).positionLineBox(inlineTextBox);
- inlineBoxIsRedundant = !inlineTextBox.len();
+ inlineBoxIsRedundant = !inlineTextBox.hasTextContent();
} else if (is<RenderBox>(renderer)) {
downcast<RenderBox>(renderer).positionLineBox(downcast<InlineElementBox>(*run->box()));
inlineBoxIsRedundant = renderer.isOutOfFlowPositioned();
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderText.cpp (236138 => 236139)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderText.cpp 2018-09-18 15:41:31 UTC (rev 236138)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/rendering/RenderText.cpp 2018-09-18 15:59:18 UTC (rev 236139)
@@ -1304,7 +1304,7 @@
void RenderText::positionLineBox(InlineTextBox& textBox)
{
- if (!textBox.len())
+ if (!textBox.hasTextContent())
return;
m_containsReversedText |= !textBox.isLeftToRightDirection();
}
