- Revision
- 169189
- Author
- [email protected]
- Date
- 2014-05-21 23:12:34 -0700 (Wed, 21 May 2014)
Log Message
REGRESSION(r167870): Crash in simple line layout code with :after
https://bugs.webkit.org/show_bug.cgi?id=133155
Source/WebCore:
<rdar://problem/16977696>
Reviewed by Darin Adler.
Fix https://bugs.webkit.org/show_bug.cgi?id=132241 in a safer way.
The underline behavior is tested by the existing fast/text/simple-lines-hover-underline.html
Test: fast/text/simple-lines-hover-after.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::invalidateLineLayoutPath): Deleted.
Move to RenderBlockFlow.
* rendering/RenderBlock.h:
(WebCore::RenderBlock::invalidateLineLayoutPath):
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::styleDidChange):
Invalidate layout if style changes in a manner that makes us ineligible to use the simple line layout path.
(WebCore::RenderBlockFlow::invalidateLineLayoutPath):
Drop the simple line layout on path invalidation if it exists. It may not be valid anymore.
Also invalidate the layout if this happens so we'll reconstruct the lines later.
(WebCore::RenderBlockFlow::simpleLineLayout): Deleted.
(WebCore::RenderBlockFlow::ensureLineBoxes):
(WebCore::RenderBlockFlow::createLineBoxes): Deleted.
Revert some of the changes made it r167870.
* rendering/RenderBlockFlow.h:
(WebCore::RenderBlockFlow::simpleLineLayout):
Add strong validity assert.
LayoutTests:
Reviewed by Darin Adler.
* fast/text/simple-lines-hover-after-expected.html: Added.
* fast/text/simple-lines-hover-after.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (169188 => 169189)
--- trunk/LayoutTests/ChangeLog 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/LayoutTests/ChangeLog 2014-05-22 06:12:34 UTC (rev 169189)
@@ -1,3 +1,13 @@
+2014-05-21 Antti Koivisto <[email protected]>
+
+ REGRESSION(r167870): Crash in simple line layout code with :after
+ https://bugs.webkit.org/show_bug.cgi?id=133155
+
+ Reviewed by Darin Adler.
+
+ * fast/text/simple-lines-hover-after-expected.html: Added.
+ * fast/text/simple-lines-hover-after.html: Added.
+
2014-05-21 Zalan Bujtas <[email protected]>
Unreviewed Mac gardening after r169161.
Added: trunk/LayoutTests/fast/text/simple-lines-hover-after-expected.html (0 => 169189)
--- trunk/LayoutTests/fast/text/simple-lines-hover-after-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/simple-lines-hover-after-expected.html 2014-05-22 06:12:34 UTC (rev 169189)
@@ -0,0 +1 @@
+target after
Added: trunk/LayoutTests/fast/text/simple-lines-hover-after.html (0 => 169189)
--- trunk/LayoutTests/fast/text/simple-lines-hover-after.html (rev 0)
+++ trunk/LayoutTests/fast/text/simple-lines-hover-after.html 2014-05-22 06:12:34 UTC (rev 169189)
@@ -0,0 +1,22 @@
+<style>
+a { display: none; }
+a:after { content: "after"; }
+div:hover a { display: inline-block }
+</style>
+<script>
+function test()
+{
+ var target = document.getElementById("target");
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+ if (window.eventSender)
+ eventSender.mouseMoveTo(target.offsetLeft + 10, target.offsetTop + 10);
+}
+function done()
+{
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+</script>
+<body _onload_="test()">
+<div id="target" _onmouseenter_="done()">target <a></a></div>
Modified: trunk/Source/WebCore/ChangeLog (169188 => 169189)
--- trunk/Source/WebCore/ChangeLog 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/Source/WebCore/ChangeLog 2014-05-22 06:12:34 UTC (rev 169189)
@@ -1,3 +1,44 @@
+2014-05-21 Antti Koivisto <[email protected]>
+
+ REGRESSION(r167870): Crash in simple line layout code with :after
+ https://bugs.webkit.org/show_bug.cgi?id=133155
+ <rdar://problem/16977696>
+
+ Reviewed by Darin Adler.
+
+ Fix https://bugs.webkit.org/show_bug.cgi?id=132241 in a safer way.
+ The underline behavior is tested by the existing fast/text/simple-lines-hover-underline.html
+
+ Test: fast/text/simple-lines-hover-after.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::invalidateLineLayoutPath): Deleted.
+
+ Move to RenderBlockFlow.
+
+ * rendering/RenderBlock.h:
+ (WebCore::RenderBlock::invalidateLineLayoutPath):
+ * rendering/RenderBlockFlow.cpp:
+ (WebCore::RenderBlockFlow::styleDidChange):
+
+ Invalidate layout if style changes in a manner that makes us ineligible to use the simple line layout path.
+
+ (WebCore::RenderBlockFlow::invalidateLineLayoutPath):
+
+ Drop the simple line layout on path invalidation if it exists. It may not be valid anymore.
+ Also invalidate the layout if this happens so we'll reconstruct the lines later.
+
+ (WebCore::RenderBlockFlow::simpleLineLayout): Deleted.
+ (WebCore::RenderBlockFlow::ensureLineBoxes):
+ (WebCore::RenderBlockFlow::createLineBoxes): Deleted.
+
+ Revert some of the changes made it r167870.
+
+ * rendering/RenderBlockFlow.h:
+ (WebCore::RenderBlockFlow::simpleLineLayout):
+
+ Add strong validity assert.
+
2014-05-21 Eric Carlson <[email protected]>
[iOS] two media control button strings are not localized
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (169188 => 169189)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-05-22 06:12:34 UTC (rev 169189)
@@ -626,13 +626,6 @@
cache->recomputeIsIgnored(this);
}
-void RenderBlock::invalidateLineLayoutPath()
-{
- if (m_lineLayoutPath == ForceLineBoxesPath)
- return;
- m_lineLayoutPath = UndeterminedPath;
-}
-
void RenderBlock::makeChildrenNonInline(RenderObject* insertionPoint)
{
// makeChildrenNonInline takes a block whose children are *all* inline and it
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (169188 => 169189)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2014-05-22 06:12:34 UTC (rev 169189)
@@ -81,7 +81,7 @@
virtual void layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeight = 0);
- void invalidateLineLayoutPath();
+ virtual void invalidateLineLayoutPath() { }
void insertPositionedObject(RenderBox&);
static void removePositionedObject(RenderBox&);
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (169188 => 169189)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2014-05-22 06:12:34 UTC (rev 169189)
@@ -1962,9 +1962,12 @@
if (auto fragment = renderNamedFlowFragment())
fragment->setStyle(RenderNamedFlowFragment::createStyle(style()));
- if (diff >= StyleDifferenceRepaint)
- invalidateLineLayoutPath();
-
+ if (diff >= StyleDifferenceRepaint) {
+ // FIXME: This could use a cheaper style-only test instead of SimpleLineLayout::canUseFor.
+ if (selfNeedsLayout() || !m_simpleLineLayout || !SimpleLineLayout::canUseFor(*this))
+ invalidateLineLayoutPath();
+ }
+
if (multiColumnFlowThread()) {
for (auto child = firstChildBox();
child && (child->isInFlowRenderFlowThread() || child->isRenderMultiColumnSet());
@@ -3390,6 +3393,27 @@
return lineBoxes().firstLineBox();
}
+void RenderBlockFlow::invalidateLineLayoutPath()
+{
+ switch (m_lineLayoutPath) {
+ case UndeterminedPath:
+ case ForceLineBoxesPath:
+ ASSERT(!m_simpleLineLayout);
+ return;
+ case LineBoxesPath:
+ ASSERT(!m_simpleLineLayout);
+ m_lineLayoutPath = UndeterminedPath;
+ return;
+ case SimpleLinesPath:
+ // The simple line layout may have become invalid.
+ m_simpleLineLayout = nullptr;
+ setNeedsLayout();
+ m_lineLayoutPath = UndeterminedPath;
+ return;
+ }
+ ASSERT_NOT_REACHED();
+}
+
void RenderBlockFlow::layoutSimpleLines(LayoutUnit& repaintLogicalTop, LayoutUnit& repaintLogicalBottom)
{
ASSERT(!m_lineBoxes.firstLineBox());
@@ -3412,28 +3436,9 @@
toRenderText(firstChild())->deleteLineBoxesBeforeSimpleLineLayout();
}
-const SimpleLineLayout::Layout* RenderBlockFlow::simpleLineLayout() const
-{
- if (m_lineLayoutPath == UndeterminedPath)
- const_cast<RenderBlockFlow&>(*this).m_lineLayoutPath = SimpleLineLayout::canUseFor(*this) ? SimpleLinesPath : LineBoxesPath;
-
- if (m_lineLayoutPath == SimpleLinesPath)
- return m_simpleLineLayout.get();
-
- const_cast<RenderBlockFlow&>(*this).createLineBoxes();
- return nullptr;
-}
-
void RenderBlockFlow::ensureLineBoxes()
{
m_lineLayoutPath = ForceLineBoxesPath;
- createLineBoxes();
-}
-
-void RenderBlockFlow::createLineBoxes()
-{
- ASSERT(m_lineLayoutPath == LineBoxesPath || m_lineLayoutPath == ForceLineBoxesPath);
-
if (!m_simpleLineLayout)
return;
m_simpleLineLayout = nullptr;
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (169188 => 169189)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.h 2014-05-22 04:11:15 UTC (rev 169188)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h 2014-05-22 06:12:34 UTC (rev 169189)
@@ -347,6 +347,7 @@
RootInlineBox* lastRootBox() const { return toRootInlineBox(m_lineBoxes.lastLineBox()); }
virtual bool hasLines() const override final;
+ virtual void invalidateLineLayoutPath() override final;
// Helper methods for computing line counts and heights for line counts.
RootInlineBox* lineAtIndex(int) const;
@@ -525,8 +526,6 @@
virtual VisiblePosition positionForPointWithInlineChildren(const LayoutPoint& pointInLogicalContents, const RenderRegion*) override;
virtual void addFocusRingRectsForInlineChildren(Vector<IntRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*) override;
- void createLineBoxes();
-
// FIXME-BLOCKFLOW: These methods have implementations in
// RenderBlockLineLayout. They should be moved to the proper header once the
// line layout code is separated from RenderBlock and RenderBlockFlow.
@@ -629,6 +628,12 @@
return isRenderBlockFlow() && toRenderBlockFlow(this)->renderNamedFlowFragment();
}
+inline const SimpleLineLayout::Layout* RenderBlockFlow::simpleLineLayout() const
+{
+ ASSERT(m_lineLayoutPath == SimpleLinesPath || !m_simpleLineLayout);
+ return m_simpleLineLayout.get();
+}
+
} // namespace WebCore
#endif // RenderBlockFlow_h
