- Revision
- 115343
- Author
- [email protected]
- Date
- 2012-04-26 12:59:05 -0700 (Thu, 26 Apr 2012)
Log Message
Crash from removal of line break object after layout
https://bugs.webkit.org/show_bug.cgi?id=75461
Source/WebCore:
Patch by Ken Buchanan <[email protected]> on 2012-04-26
Reviewed by David Hyatt.
There is a condition where objects can get removed from underneath
inlines while they represent a line break object in a RootInlineBox
of an ancestor block. If an intermediary inline has already been
marked as needing layout, then the line box will not get dirtied
because dirtyLineFromChangedChild thinks it already has been.
This patch introduces a new set in RenderObject to indicate whether
an ancestral line box corresponding to the current line has been
marked dirty or not. dirtyLinesFromChangedChild() can use this set
rather than m_selfNeedsLayout, so it will not be confused if a
container was dirtied for some other reason that did not affect the
line box.
* rendering/RenderLineBoxList.cpp:
(WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Use the new
set rather than m_selfNeedsLayout in the container to determine
whether to continue propagating upward.
* rendering/RenderObject.cpp:
(WebCore::RenderObject::s_ancestorLineboxDirtySet): Instantiate the
static member.
(WebCore::RenderObject::willBeDestroyed): Clears the object from the
linebox set when it is being destroyed.
* rendering/RenderObject.h:
(WebCore::RenderObject::s_ancestorLineboxDirtySet): Added static
member set.
(WebCore::RenderObject::setNeedsLayout): Clears the
object from the linebox set when layout bits are getting cleared.
(WebCore::RenderObject::ancestorLineBoxDirty): Added.
(WebCore::RenderObject::setAncestorLineBoxDirty): Added.
LayoutTests:
Patch by Ken Buchanan <[email protected]> on 2012-04-25
Reviewed by David Hyatt.
Test exercising crashing condition in bug 75461.
* fast/block/line-layout/line-break-obj-removal-crash-expected.txt: Added
* fast/block/line-layout/line-break-obj-removal-crash.html: Added
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (115342 => 115343)
--- trunk/LayoutTests/ChangeLog 2012-04-26 19:54:35 UTC (rev 115342)
+++ trunk/LayoutTests/ChangeLog 2012-04-26 19:59:05 UTC (rev 115343)
@@ -1,3 +1,15 @@
+2012-04-25 Ken Buchanan <[email protected]>
+
+ Crash from removal of line break object after layout
+ https://bugs.webkit.org/show_bug.cgi?id=75461
+
+ Reviewed by David Hyatt.
+
+ Test exercising crashing condition in bug 75461.
+
+ * fast/block/line-layout/line-break-obj-removal-crash-expected.txt: Added
+ * fast/block/line-layout/line-break-obj-removal-crash.html: Added
+
2012-04-26 Robert Hogan <[email protected]>
Update Qt expectations after r115340
Added: trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt (0 => 115343)
--- trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash-expected.txt 2012-04-26 19:59:05 UTC (rev 115343)
@@ -0,0 +1,2 @@
+
+PASS, if no crash or assert in debug
Added: trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html (0 => 115343)
--- trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html (rev 0)
+++ trunk/LayoutTests/fast/block/line-layout/line-break-obj-removal-crash.html 2012-04-26 19:59:05 UTC (rev 115343)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<style>
+.style1:nth-last-child(odd) { padding-right: 100px; }
+.styleCounterBefore:before { position: absolute; content: counter(section); }
+</style>
+<script>
+window._onload_ = function() {
+ divElem = document.createElement('div');
+ divElem.setAttribute('class', 'styleCounterBefore');
+ document.documentElement.appendChild(divElem);
+ brElem = document.createElement('br');
+ document.documentElement.appendChild(brElem);
+ sampElem = document.createElement('samp');
+ sampElem.setAttribute('class', 'style1');
+ document.documentElement.appendChild(sampElem);
+ spanElem = document.createElement('span');
+ spanElem.setAttribute('class', 'styleCounterBefore');
+ trElem = document.createElement('tr');
+ document.documentElement.appendChild(trElem);
+ sampElem.appendChild(spanElem);
+ document.documentElement.offsetHeight;
+ document.documentElement.removeChild(trElem);
+
+ document.documentElement.appendChild(document.createTextNode('PASS, if no crash or assert in debug'));
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+}
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (115342 => 115343)
--- trunk/Source/WebCore/ChangeLog 2012-04-26 19:54:35 UTC (rev 115342)
+++ trunk/Source/WebCore/ChangeLog 2012-04-26 19:59:05 UTC (rev 115343)
@@ -1,3 +1,40 @@
+2012-04-26 Ken Buchanan <[email protected]>
+
+ Crash from removal of line break object after layout
+ https://bugs.webkit.org/show_bug.cgi?id=75461
+
+ Reviewed by David Hyatt.
+
+ There is a condition where objects can get removed from underneath
+ inlines while they represent a line break object in a RootInlineBox
+ of an ancestor block. If an intermediary inline has already been
+ marked as needing layout, then the line box will not get dirtied
+ because dirtyLineFromChangedChild thinks it already has been.
+
+ This patch introduces a new set in RenderObject to indicate whether
+ an ancestral line box corresponding to the current line has been
+ marked dirty or not. dirtyLinesFromChangedChild() can use this set
+ rather than m_selfNeedsLayout, so it will not be confused if a
+ container was dirtied for some other reason that did not affect the
+ line box.
+
+ * rendering/RenderLineBoxList.cpp:
+ (WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Use the new
+ set rather than m_selfNeedsLayout in the container to determine
+ whether to continue propagating upward.
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::s_ancestorLineboxDirtySet): Instantiate the
+ static member.
+ (WebCore::RenderObject::willBeDestroyed): Clears the object from the
+ linebox set when it is being destroyed.
+ * rendering/RenderObject.h:
+ (WebCore::RenderObject::s_ancestorLineboxDirtySet): Added static
+ member set.
+ (WebCore::RenderObject::setNeedsLayout): Clears the
+ object from the linebox set when layout bits are getting cleared.
+ (WebCore::RenderObject::ancestorLineBoxDirty): Added.
+ (WebCore::RenderObject::setAncestorLineBoxDirty): Added.
+
2012-04-26 Christophe Dumez <[email protected]>
[EFL] Enable VIDEO_TRACK feature
Modified: trunk/Source/WebCore/rendering/RenderLineBoxList.cpp (115342 => 115343)
--- trunk/Source/WebCore/rendering/RenderLineBoxList.cpp 2012-04-26 19:54:35 UTC (rev 115342)
+++ trunk/Source/WebCore/rendering/RenderLineBoxList.cpp 2012-04-26 19:59:05 UTC (rev 115343)
@@ -321,9 +321,9 @@
if (!firstBox) {
// For an empty inline, go ahead and propagate the check up to our parent, unless the parent
// is already dirty.
- if (container->isInline() && !container->parent()->selfNeedsLayout()) {
+ if (container->isInline() && !container->parent()->ancestorLineBoxDirty()) {
container->parent()->dirtyLinesFromChangedChild(container);
- container->setNeedsLayout(true); // Mark the container as needing layout to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
+ container->setAncestorLineBoxDirty(); // Mark the container to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
}
return;
}
@@ -361,9 +361,9 @@
// we won't find a previous sibling, but firstBox can be pointing to a following sibling.
// This isn't good enough, since we won't locate the root line box that encloses the removed
// <br>. We have to just over-invalidate a bit and go up to our parent.
- if (!inlineContainer->parent()->selfNeedsLayout()) {
+ if (!inlineContainer->parent()->ancestorLineBoxDirty()) {
inlineContainer->parent()->dirtyLinesFromChangedChild(inlineContainer);
- inlineContainer->setNeedsLayout(true); // Mark the container as needing layout to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
+ inlineContainer->setAncestorLineBoxDirty(); // Mark the container to avoid dirtying the same lines again across multiple destroy() calls of the same subtree.
}
return;
}
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (115342 => 115343)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2012-04-26 19:54:35 UTC (rev 115342)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2012-04-26 19:59:05 UTC (rev 115343)
@@ -103,6 +103,8 @@
bool RenderObject::s_affectsParentBlock = false;
+RenderObjectAncestorLineboxDirtySet* RenderObject::s_ancestorLineboxDirtySet = 0;
+
void* RenderObject::operator new(size_t sz, RenderArena* renderArena)
{
return renderArena->allocate(sz);
@@ -2324,6 +2326,8 @@
toRenderBoxModelObject(this)->destroyLayer();
}
+ setAncestorLineBoxDirty(false);
+
clearLayoutRootIfNeeded();
}
Modified: trunk/Source/WebCore/rendering/RenderObject.h (115342 => 115343)
--- trunk/Source/WebCore/rendering/RenderObject.h 2012-04-26 19:54:35 UTC (rev 115342)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2012-04-26 19:59:05 UTC (rev 115343)
@@ -38,6 +38,7 @@
#include "ScrollBehavior.h"
#include "TextAffinity.h"
#include "TransformationMatrix.h"
+#include <wtf/HashSet.h>
#include <wtf/UnusedParam.h>
#if USE(CG) || USE(CAIRO) || USE(SKIA) || PLATFORM(QT)
@@ -123,6 +124,8 @@
};
#endif
+typedef WTF::HashSet<const RenderObject*> RenderObjectAncestorLineboxDirtySet;
+
#ifndef NDEBUG
const int showTreeCharacterOffset = 39;
#endif
@@ -379,6 +382,22 @@
bool hasColumns() const { return m_bitfields.hasColumns(); }
void setHasColumns(bool b = true) { m_bitfields.setHasColumns(b); }
+ bool ancestorLineBoxDirty() const { return s_ancestorLineboxDirtySet && s_ancestorLineboxDirtySet->contains(this); }
+ void setAncestorLineBoxDirty(bool b = true)
+ {
+ if (b) {
+ if (!s_ancestorLineboxDirtySet)
+ s_ancestorLineboxDirtySet = new RenderObjectAncestorLineboxDirtySet;
+ s_ancestorLineboxDirtySet->add(this);
+ } else if (s_ancestorLineboxDirtySet) {
+ s_ancestorLineboxDirtySet->remove(this);
+ if (s_ancestorLineboxDirtySet->isEmpty()) {
+ delete s_ancestorLineboxDirtySet;
+ s_ancestorLineboxDirtySet = 0;
+ }
+ }
+ }
+
bool inRenderFlowThread() const { return m_bitfields.inRenderFlowThread(); }
void setInRenderFlowThread(bool b = true) { m_bitfields.setInRenderFlowThread(b); }
@@ -904,6 +923,8 @@
RenderObject* m_previous;
RenderObject* m_next;
+ static RenderObjectAncestorLineboxDirtySet* s_ancestorLineboxDirtySet;
+
#ifndef NDEBUG
bool m_hasAXObject : 1;
bool m_setNeedsLayoutForbidden : 1;
@@ -1064,6 +1085,7 @@
setNeedsSimplifiedNormalFlowLayout(false);
setNormalChildNeedsLayout(false);
setNeedsPositionedMovementLayout(false);
+ setAncestorLineBoxDirty(false);
}
}
