- Revision
- 121001
- Author
- [email protected]
- Date
- 2012-06-21 21:01:57 -0700 (Thu, 21 Jun 2012)
Log Message
Crash in RenderBlock::layoutPositionedObjects.
https://bugs.webkit.org/show_bug.cgi?id=89599
Reviewed by Julien Chaffraix.
Source/WebCore:
Test: fast/table/table-split-positioned-object-crash.html
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::splitBlocks): no longer need to explicitly call
removePositionedObjects, since it is part of moveChildrenTo.
* rendering/RenderBlock.h:
(WebCore::RenderBlock::hasPositionedObjects): helper to tell if we have
positioned objects in our list.
* rendering/RenderBox.cpp:
(WebCore::RenderBox::splitAnonymousBoxesAroundChild): Like r102263, this
condition was wrong and while moving children across completely different
trees, we need fullRemoveInsert as true.
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::moveChildTo): see code comment.
(WebCore::RenderBoxModelObject::moveChildrenTo): see code comment.
LayoutTests:
* fast/table/table-split-positioned-object-crash-expected.txt: Added.
* fast/table/table-split-positioned-object-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (121000 => 121001)
--- trunk/LayoutTests/ChangeLog 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/LayoutTests/ChangeLog 2012-06-22 04:01:57 UTC (rev 121001)
@@ -1,3 +1,13 @@
+2012-06-21 Abhishek Arya <[email protected]>
+
+ Crash in RenderBlock::layoutPositionedObjects.
+ https://bugs.webkit.org/show_bug.cgi?id=89599
+
+ Reviewed by Julien Chaffraix.
+
+ * fast/table/table-split-positioned-object-crash-expected.txt: Added.
+ * fast/table/table-split-positioned-object-crash.html: Added.
+
2012-06-21 Tony Gentilcore <[email protected]>
[Chromium][Win] Missing trailing newlines in one text test expectation, debug only
Added: trunk/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt (0 => 121001)
--- trunk/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt 2012-06-22 04:01:57 UTC (rev 121001)
@@ -0,0 +1,3 @@
+WebKit Bug 89599 - Crash in RenderBlock::layoutPositionedObjects.
+Test passes if it does not crash.
+
Added: trunk/LayoutTests/fast/table/table-split-positioned-object-crash.html (0 => 121001)
--- trunk/LayoutTests/fast/table/table-split-positioned-object-crash.html (rev 0)
+++ trunk/LayoutTests/fast/table/table-split-positioned-object-crash.html 2012-06-22 04:01:57 UTC (rev 121001)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+WebKit Bug 89599 - Crash in RenderBlock::layoutPositionedObjects.<br />
+Test passes if it does not crash.
+<style>
+table { position: relative; }
+.span:last-child { position: relative; }
+</style>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+function crash() {
+ table = document.createElement('table');
+ document.documentElement.appendChild(table);
+
+ span1 = document.createElement('span');
+ span2 = document.createElement('span');
+ span3 = document.createElement('span');
+ span3.setAttribute('class', 'span');
+ span4 = document.createElement('span');
+ span4.style.position = 'absolute';
+ span3.appendChild(span4);
+
+ table.appendChild(span1);
+ table.appendChild(span2);
+ table.appendChild(span3);
+
+ document.documentElement.offsetTop;
+ span2.style.display = 'table-header-group';
+ span3.style.display = 'block'
+}
+window._onload_ = crash;
+</script>
+</html>
Property changes on: trunk/LayoutTests/fast/table/table-split-positioned-object-crash.html
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (121000 => 121001)
--- trunk/Source/WebCore/ChangeLog 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/Source/WebCore/ChangeLog 2012-06-22 04:01:57 UTC (rev 121001)
@@ -1,3 +1,26 @@
+2012-06-21 Abhishek Arya <[email protected]>
+
+ Crash in RenderBlock::layoutPositionedObjects.
+ https://bugs.webkit.org/show_bug.cgi?id=89599
+
+ Reviewed by Julien Chaffraix.
+
+ Test: fast/table/table-split-positioned-object-crash.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::splitBlocks): no longer need to explicitly call
+ removePositionedObjects, since it is part of moveChildrenTo.
+ * rendering/RenderBlock.h:
+ (WebCore::RenderBlock::hasPositionedObjects): helper to tell if we have
+ positioned objects in our list.
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::splitAnonymousBoxesAroundChild): Like r102263, this
+ condition was wrong and while moving children across completely different
+ trees, we need fullRemoveInsert as true.
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::moveChildTo): see code comment.
+ (WebCore::RenderBoxModelObject::moveChildrenTo): see code comment.
+
2012-06-21 Kwang Yul Seo <[email protected]>
Make HTMLDocumentParser::create(DocumentFragment*,Element*, FragmentScriptingPermission) private.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (121000 => 121001)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-06-22 04:01:57 UTC (rev 121001)
@@ -568,14 +568,6 @@
if (beforeChild && childrenInline())
deleteLineBoxTree();
- // We have to remove the descendant child from our positioned objects list
- // before we do the split and move some of the children to cloneBlock. Since
- // we are doing layout anyway, it is easier to blow away the entire list, than
- // traversing down the subtree looking for positioned childs and then remove them
- // from our positioned objects list.
- if (beforeChild)
- removePositionedObjects(0);
-
// Now take all of the children from beforeChild to the end and remove
// them from |this| and place them in the clone.
moveChildrenTo(cloneBlock, beforeChild, 0, true);
@@ -629,12 +621,6 @@
currChildNextSibling = 0; // We destroyed the last child, so now we need to update
// the value of currChildNextSibling.
- // It is possible that positioned objects under blockCurr are going to be moved to cloneBlock.
- // Since we are doing layout anyway, it is easier to blow away the entire list, than
- // traversing down the subtree looking for positioned children and then remove them
- // from our positioned objects list.
- blockCurr->removePositionedObjects(0);
-
// Now we need to take all of the children starting from the first child
// *after* currChild and append them all to the clone.
blockCurr->moveChildrenTo(cloneBlock, currChildNextSibling, 0, true);
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (121000 => 121001)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2012-06-22 04:01:57 UTC (rev 121001)
@@ -104,6 +104,7 @@
typedef ListHashSet<RenderBox*, 4> PositionedObjectsListHashSet;
PositionedObjectsListHashSet* positionedObjects() const { return m_positionedObjects.get(); }
+ bool hasPositionedObjects() const { return m_positionedObjects && !m_positionedObjects->isEmpty(); }
void addPercentHeightDescendant(RenderBox*);
static void removePercentHeightDescendant(RenderBox*);
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (121000 => 121001)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-06-22 04:01:57 UTC (rev 121001)
@@ -3979,7 +3979,7 @@
postBox->setChildrenInline(boxToSplit->childrenInline());
RenderBox* parentBox = toRenderBox(boxToSplit->parent());
parentBox->virtualChildren()->insertChildNode(parentBox, postBox, boxToSplit->nextSibling());
- boxToSplit->moveChildrenTo(postBox, beforeChild, 0, boxToSplit->hasLayer());
+ boxToSplit->moveChildrenTo(postBox, beforeChild, 0, true);
markBoxForRelayoutAfterSplit(boxToSplit);
markBoxForRelayoutAfterSplit(postBox);
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (121000 => 121001)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-06-22 03:51:01 UTC (rev 121000)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-06-22 04:01:57 UTC (rev 121001)
@@ -2758,6 +2758,11 @@
void RenderBoxModelObject::moveChildTo(RenderBoxModelObject* toBoxModelObject, RenderObject* child, RenderObject* beforeChild, bool fullRemoveInsert)
{
+ // FIXME: We need a performant way to handle clearing positioned objects from our list that are
+ // in |child|'s subtree so we could just clear them here. Because of this, we assume that callers
+ // have cleared their positioned objects list for child moves (!fullRemoveInsert) to avoid any badness.
+ ASSERT(!fullRemoveInsert || !isRenderBlock() || !toRenderBlock(this)->hasPositionedObjects());
+
ASSERT(this == child->parent());
ASSERT(!beforeChild || toBoxModelObject == beforeChild->parent());
if (fullRemoveInsert && (toBoxModelObject->isRenderBlock() || toBoxModelObject->isRenderInline())) {
@@ -2770,6 +2775,15 @@
void RenderBoxModelObject::moveChildrenTo(RenderBoxModelObject* toBoxModelObject, RenderObject* startChild, RenderObject* endChild, RenderObject* beforeChild, bool fullRemoveInsert)
{
+ // This condition is rarely hit since this function is usually called on
+ // anonymous blocks which can no longer carry positioned objects (see r120761)
+ // or when fullRemoveInsert is false.
+ if (fullRemoveInsert && isRenderBlock()) {
+ RenderBlock* block = toRenderBlock(this);
+ if (block->hasPositionedObjects())
+ block->removePositionedObjects(0);
+ }
+
ASSERT(!beforeChild || toBoxModelObject == beforeChild->parent());
for (RenderObject* child = startChild; child && child != endChild; ) {
// Save our next sibling as moveChildTo will clear it.
