https://bugs.webkit.org/show_bug.cgi?id=132337
Reviewed by Simon Fraser.
From Blink r154582 by <jchaffraix@chromium.org>
Source/WebCore:
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.
LayoutTests:
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.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167942
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2014-04-29 Manuel Rego Casasnovas <rego@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 <jchaffraix@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-29 Hans Muller <hmuller@adobe.com>
[CSS Shapes] off-by-one error in Shape::createRasterShape()
--- /dev/null
+This test has passed if it doesn't crash.
+* { display: -webkit-flex; }
+if (window.testRunner) testRunner.dumpAsText(); crashy.offsetLeft; crashy.parentNode.removeChild(crashy);
--- /dev/null
+<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>
+2014-04-29 Manuel Rego Casasnovas <rego@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 <jchaffraix@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-29 Enrica Casucci <enrica@apple.com>
iOS build fix after http://trac.webkit.org/changeset/167937.
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())
RenderBox* first();
RenderBox* next();
+ void invalidate();
+
private:
void reset();
, m_childIndex(0)
, m_allChildrenHaveDefaultOrderValue(true)
{
- m_iterator.m_children.clear();
+ m_iterator.invalidate();
}
~OrderIteratorPopulator();
return isHorizontalFlow();
}
+void RenderFlexibleBox::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
+}
+
}
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;
return "RenderGrid";
}
+void RenderGrid::removeChild(RenderObject& child)
+{
+ RenderBlock::removeChild(child);
+ m_orderIterator.invalidate();
+}
+
} // namespace WebCore
#endif /* ENABLE(CSS_GRID_LAYOUT) */
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; }