- Revision
- 217079
- Author
- [email protected]
- Date
- 2017-05-18 17:37:43 -0700 (Thu, 18 May 2017)
Log Message
Redundant ellipsis box triggers ASSERT_WITH_SECURITY_IMPLICATION in InlineBox::parent().
https://bugs.webkit.org/show_bug.cgi?id=172309
<rdar://problem/32262357>
Reviewed by Simon Fraser.
Source/WebCore:
This patch stops the redundant ellipsis box trigger ASSERT_WITH_SECURITY_IMPLICATION.
In RootInlineBox::placeEllipsis we construct an ellipsis box and append it to a static HashMap which
keeps track of the ellipsis boxes on each line. However when the line already has an ellipsis, we
re-use the existing one and this newly constructed (but redundant) box gets destroyed as we return from this function.
In InlineBox's d'tor, we let the parent know that now it has a dangling child and we assert on it
later, while accessing the children list. However this redundant ellipsis box was never added to the line,
so the assertion hits incorrectly.
Test: fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html
* rendering/EllipsisBox.cpp:
(WebCore::EllipsisBox::EllipsisBox):
* rendering/InlineBox.cpp:
(WebCore::InlineBox::invalidateParentChildList):
* rendering/InlineBox.h:
* rendering/RootInlineBox.cpp:
(WebCore::RootInlineBox::placeEllipsis): Use the newly created ellipsis box instead.
LayoutTests:
* fast/inline/redundant-ellipsis-triggers-assert-incorrectly-expected.txt: Added.
* fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (217078 => 217079)
--- trunk/LayoutTests/ChangeLog 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/LayoutTests/ChangeLog 2017-05-19 00:37:43 UTC (rev 217079)
@@ -1,3 +1,14 @@
+2017-05-18 Zalan Bujtas <[email protected]>
+
+ Redundant ellipsis box triggers ASSERT_WITH_SECURITY_IMPLICATION in InlineBox::parent().
+ https://bugs.webkit.org/show_bug.cgi?id=172309
+ <rdar://problem/32262357>
+
+ Reviewed by Simon Fraser.
+
+ * fast/inline/redundant-ellipsis-triggers-assert-incorrectly-expected.txt: Added.
+ * fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html: Added.
+
2017-05-18 Simon Fraser <[email protected]>
Add a test to ensure that media controls don't trigger composting of ancestors via "isolates blending"
Added: trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly-expected.txt (0 => 217079)
--- trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly-expected.txt 2017-05-19 00:37:43 UTC (rev 217079)
@@ -0,0 +1,2 @@
+PASS if no assert or crash.
+foobaar foobarfoobarfo foobar
Added: trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html (0 => 217079)
--- trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html (rev 0)
+++ trunk/LayoutTests/fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html 2017-05-19 00:37:43 UTC (rev 217079)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that we don't trigger assert incorrectly for ellipsis boxes</title>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+<style>
+@font-face{
+ font-family:"Market Sans"; src:url('');
+}
+
+div {
+ display: -webkit-box;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ -webkit-line-clamp: 2;
+ -webkit-box-orient: vertical;
+ font-family: "Market Sans";
+ max-width: 224px;
+}
+</style>
+</head>
+<body>
+PASS if no assert or crash.
+<div>foobaar foobarfoobarfo foobar</div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (217078 => 217079)
--- trunk/Source/WebCore/ChangeLog 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/Source/WebCore/ChangeLog 2017-05-19 00:37:43 UTC (rev 217079)
@@ -1,3 +1,30 @@
+2017-05-18 Zalan Bujtas <[email protected]>
+
+ Redundant ellipsis box triggers ASSERT_WITH_SECURITY_IMPLICATION in InlineBox::parent().
+ https://bugs.webkit.org/show_bug.cgi?id=172309
+ <rdar://problem/32262357>
+
+ Reviewed by Simon Fraser.
+
+ This patch stops the redundant ellipsis box trigger ASSERT_WITH_SECURITY_IMPLICATION.
+
+ In RootInlineBox::placeEllipsis we construct an ellipsis box and append it to a static HashMap which
+ keeps track of the ellipsis boxes on each line. However when the line already has an ellipsis, we
+ re-use the existing one and this newly constructed (but redundant) box gets destroyed as we return from this function.
+ In InlineBox's d'tor, we let the parent know that now it has a dangling child and we assert on it
+ later, while accessing the children list. However this redundant ellipsis box was never added to the line,
+ so the assertion hits incorrectly.
+
+ Test: fast/inline/redundant-ellipsis-triggers-assert-incorrectly.html
+
+ * rendering/EllipsisBox.cpp:
+ (WebCore::EllipsisBox::EllipsisBox):
+ * rendering/InlineBox.cpp:
+ (WebCore::InlineBox::invalidateParentChildList):
+ * rendering/InlineBox.h:
+ * rendering/RootInlineBox.cpp:
+ (WebCore::RootInlineBox::placeEllipsis): Use the newly created ellipsis box instead.
+
2017-05-18 Andy Estes <[email protected]>
ENABLE(APPLE_PAY_DELEGATE) should be NO on macOS Sierra and earlier
Modified: trunk/Source/WebCore/rendering/EllipsisBox.cpp (217078 => 217079)
--- trunk/Source/WebCore/rendering/EllipsisBox.cpp 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/Source/WebCore/rendering/EllipsisBox.cpp 2017-05-19 00:37:43 UTC (rev 217079)
@@ -38,6 +38,9 @@
, m_str(ellipsisStr)
, m_selectionState(RenderObject::SelectionNone)
{
+#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+ m_isEverInChildList = false;
+#endif
}
void EllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom)
Modified: trunk/Source/WebCore/rendering/InlineBox.cpp (217078 => 217079)
--- trunk/Source/WebCore/rendering/InlineBox.cpp 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/Source/WebCore/rendering/InlineBox.cpp 2017-05-19 00:37:43 UTC (rev 217079)
@@ -43,6 +43,7 @@
#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
unsigned s;
bool f;
+ bool i;
#endif
};
@@ -70,7 +71,7 @@
void InlineBox::invalidateParentChildList()
{
assertNotDeleted();
- if (!m_hasBadParent && m_parent)
+ if (!m_hasBadParent && m_parent && m_isEverInChildList)
m_parent->setHasBadChildList();
}
Modified: trunk/Source/WebCore/rendering/InlineBox.h (217078 => 217079)
--- trunk/Source/WebCore/rendering/InlineBox.h 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/Source/WebCore/rendering/InlineBox.h 2017-05-19 00:37:43 UTC (rev 217079)
@@ -412,6 +412,8 @@
bool extracted() const { return m_bitfields.extracted(); }
#if !ASSERT_WITH_SECURITY_IMPLICATION_DISABLED
+protected:
+ bool m_isEverInChildList { true };
private:
static constexpr unsigned deletionSentinelNotDeletedValue = 0xF0F0F0F0U;
static constexpr unsigned deletionSentinelDeletedValue = 0xF0DEADF0U;
Modified: trunk/Source/WebCore/rendering/RootInlineBox.cpp (217078 => 217079)
--- trunk/Source/WebCore/rendering/RootInlineBox.cpp 2017-05-19 00:00:17 UTC (rev 217078)
+++ trunk/Source/WebCore/rendering/RootInlineBox.cpp 2017-05-19 00:37:43 UTC (rev 217079)
@@ -126,13 +126,9 @@
if (!gEllipsisBoxMap)
gEllipsisBoxMap = new EllipsisBoxMap();
- // Create an ellipsis box.
- auto newEllipsisBox = std::make_unique<EllipsisBox>(blockFlow(), ellipsisStr, this, ellipsisWidth - (markupBox ? markupBox->logicalWidth() : 0), logicalHeight(), y(), !prevRootBox(), isHorizontal(), markupBox);
- auto ellipsisBox = newEllipsisBox.get();
-
- gEllipsisBoxMap->add(this, WTFMove(newEllipsisBox));
+ ASSERT(!hasEllipsisBox());
+ auto* ellipsisBox = gEllipsisBoxMap->set(this, std::make_unique<EllipsisBox>(blockFlow(), ellipsisStr, this, ellipsisWidth - (markupBox ? markupBox->logicalWidth() : 0), logicalHeight(), y(), !prevRootBox(), isHorizontal(), markupBox)).iterator->value.get();
setHasEllipsisBox(true);
-
// FIXME: Do we need an RTL version of this?
if (ltr && (x() + logicalWidth() + ellipsisWidth) <= blockRightEdge) {
ellipsisBox->setX(x() + logicalWidth());
@@ -140,7 +136,7 @@
}
// Now attempt to find the nearest glyph horizontally and place just to the right (or left in RTL)
- // of that glyph. Mark all of the objects that intersect the ellipsis box as not painting (as being
+ // of that glyph. Mark all of the objects that intersect the ellipsis box as not painting (as being
// truncated).
bool foundBox = false;
float truncatedWidth = 0;
