Diff
Modified: tags/Safari-538.32/LayoutTests/ChangeLog (167951 => 167952)
--- tags/Safari-538.32/LayoutTests/ChangeLog 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/LayoutTests/ChangeLog 2014-04-29 20:25:32 UTC (rev 167952)
@@ -1,3 +1,22 @@
+2014-04-29 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r167942
+
+ 2014-04-29 Manuel Rego Casasnovas <r...@igalia.com>
+
+ REGRESSION (r167879): Heap-use-after-free in WebCore::RenderFlexibleBox
+ https://bugs.webkit.org/show_bug.cgi?id=132337
+
+ Reviewed by Simon Fraser.
+
+ From Blink r154582 by <jchaffr...@chromium.org>
+
+ Add new layout test to check that removing a child doesn't cause a crash
+ in OrderIterator.
+
+ * fast/flexbox/order-iterator-crash-expected.txt: Added.
+ * fast/flexbox/order-iterator-crash.html: Added.
+
2014-04-28 Ryosuke Niwa <rn...@webkit.org>
Layout Test fast/events/shadow-event-path[-2].html is failing
Copied: tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt (from rev 167942, trunk/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt) (0 => 167952)
--- tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt (rev 0)
+++ tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash-expected.txt 2014-04-29 20:25:32 UTC (rev 167952)
@@ -0,0 +1,3 @@
+This test has passed if it doesn't crash.
+* { display: -webkit-flex; }
+if (window.testRunner) testRunner.dumpAsText(); crashy.offsetLeft; crashy.parentNode.removeChild(crashy);
Copied: tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash.html (from rev 167942, trunk/LayoutTests/fast/flexbox/order-iterator-crash.html) (0 => 167952)
--- tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash.html (rev 0)
+++ tags/Safari-538.32/LayoutTests/fast/flexbox/order-iterator-crash.html 2014-04-29 20:25:32 UTC (rev 167952)
@@ -0,0 +1,12 @@
+<div>This test has passed if it doesn't crash.</div>
+<style>
+* { display: -webkit-flex; }
+</style>
+<table><td id="crashy"></td></table>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+crashy.offsetLeft;
+crashy.parentNode.removeChild(crashy);
+</script>
Modified: tags/Safari-538.32/Source/WebCore/ChangeLog (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/ChangeLog 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/ChangeLog 2014-04-29 20:25:32 UTC (rev 167952)
@@ -1,3 +1,41 @@
+2014-04-29 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r167942
+
+ 2014-04-29 Manuel Rego Casasnovas <r...@igalia.com>
+
+ REGRESSION (r167879): Heap-use-after-free in WebCore::RenderFlexibleBox
+ https://bugs.webkit.org/show_bug.cgi?id=132337
+
+ Reviewed by Simon Fraser.
+
+ From Blink r154582 by <jchaffr...@chromium.org>
+
+ This is a regression from the changes in OrderIterator. The issue is
+ that we don't invalidate our iterator when a child is removed. This
+ means that we could hold onto free'd memory until the next layout
+ when we will recompute the iterator.
+
+ The solution is simple: just clear the memory when we remove a child.
+
+ Note that RenderGrid is not impacted by this bug as we don't use the
+ iterator outside layout yet, but if we do it at some point the very same
+ problem will arise, so the same treatment was applied to the class.
+
+ Test: fast/flexbox/order-iterator-crash.html
+
+ * rendering/OrderIterator.cpp:
+ (WebCore::OrderIterator::invalidate): Clear m_children Vector.
+ * rendering/OrderIterator.h:
+ (WebCore::OrderIteratorPopulator::OrderIteratorPopulator): Use
+ invalidate() method.
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::removeChild): Invalidate m_orderIterator.
+ * rendering/RenderFlexibleBox.h: Add removeChild() signature.
+ * rendering/RenderGrid.cpp: Invalidate m_orderIterator.
+ (WebCore::RenderGrid::removeChild):
+ * rendering/RenderGrid.h: Add removeChild() header.
+
2014-04-28 Beth Dakin <bda...@apple.com>
Scrollbars do not update properly when topContentInset changes dynamically
Modified: tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.cpp (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.cpp 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.cpp 2014-04-29 20:25:32 UTC (rev 167952)
@@ -56,6 +56,11 @@
return currentChild();
}
+void OrderIterator::invalidate()
+{
+ m_children.clear();
+}
+
static bool compareByOrderValueAndIndex(std::pair<RenderBox*, int> childAndIndex1, std::pair<RenderBox*, int> childAndIndex2)
{
if (childAndIndex1.first->style().order() != childAndIndex2.first->style().order())
Modified: tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.h (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.h 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/OrderIterator.h 2014-04-29 20:25:32 UTC (rev 167952)
@@ -47,6 +47,8 @@
RenderBox* first();
RenderBox* next();
+ void invalidate();
+
private:
void reset();
@@ -61,7 +63,7 @@
, m_childIndex(0)
, m_allChildrenHaveDefaultOrderValue(true)
{
- m_iterator.m_children.clear();
+ m_iterator.invalidate();
}
~OrderIteratorPopulator();
Modified: tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.cpp (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.cpp 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.cpp 2014-04-29 20:25:32 UTC (rev 167952)
@@ -1392,4 +1392,10 @@
return isHorizontalFlow();
}
+void RenderFlexibleBox::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
}
+
+}
Modified: tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.h (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.h 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/RenderFlexibleBox.h 2014-04-29 20:25:32 UTC (rev 167952)
@@ -60,6 +60,8 @@
bool isTopLayoutOverflowAllowed() const override;
bool isLeftLayoutOverflowAllowed() const override;
+ void removeChild(RenderObject&) override;
+
protected:
virtual void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
virtual void computePreferredLogicalWidths() override;
Modified: tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.cpp (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.cpp 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.cpp 2014-04-29 20:25:32 UTC (rev 167952)
@@ -1186,6 +1186,12 @@
return "RenderGrid";
}
+void RenderGrid::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
+}
+
} // namespace WebCore
#endif /* ENABLE(CSS_GRID_LAYOUT) */
Modified: tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.h (167951 => 167952)
--- tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.h 2014-04-29 19:47:25 UTC (rev 167951)
+++ tags/Safari-538.32/Source/WebCore/rendering/RenderGrid.h 2014-04-29 20:25:32 UTC (rev 167952)
@@ -58,6 +58,8 @@
const Vector<LayoutUnit>& columnPositions() const { return m_columnPositions; }
const Vector<LayoutUnit>& rowPositions() const { return m_rowPositions; }
+ void removeChild(RenderObject&) override;
+
private:
virtual const char* renderName() const override;
virtual bool isRenderGrid() const override { return true; }
