Diff
Modified: branches/safari-536.28-branch/LayoutTests/ChangeLog (133087 => 133088)
--- branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-10-31 22:34:47 UTC (rev 133088)
@@ -1,5 +1,19 @@
2012-10-31 Lucas Forschler <[email protected]>
+ Merge r121001
+
+ 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-10-31 Lucas Forschler <[email protected]>
+
Merge r118249
2012-05-23 Abhishek Arya <[email protected]>
@@ -10489,3 +10503,4 @@
.
.
.
+.
Copied: branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt (from rev 121001, trunk/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt) (0 => 133088)
--- branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash-expected.txt 2012-10-31 22:34:47 UTC (rev 133088)
@@ -0,0 +1,3 @@
+WebKit Bug 89599 - Crash in RenderBlock::layoutPositionedObjects.
+Test passes if it does not crash.
+
Copied: branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash.html (from rev 121001, trunk/LayoutTests/fast/table/table-split-positioned-object-crash.html) (0 => 133088)
--- branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash.html (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/fast/table/table-split-positioned-object-crash.html 2012-10-31 22:34:47 UTC (rev 133088)
@@ -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>
Modified: branches/safari-536.28-branch/Source/WebCore/ChangeLog (133087 => 133088)
--- branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-10-31 22:34:47 UTC (rev 133088)
@@ -1,5 +1,32 @@
2012-10-31 Lucas Forschler <[email protected]>
+ Merge r121001
+
+ 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-10-31 Lucas Forschler <[email protected]>
+
Merge r118249
2012-05-23 Abhishek Arya <[email protected]>
@@ -205565,3 +205592,4 @@
.
.
.
+.
Modified: branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.cpp (133087 => 133088)
--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.cpp 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.cpp 2012-10-31 22:34:47 UTC (rev 133088)
@@ -579,14 +579,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);
@@ -640,12 +632,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: branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.h (133087 => 133088)
--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.h 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderBlock.h 2012-10-31 22:34:47 UTC (rev 133088)
@@ -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: branches/safari-536.28-branch/Source/WebCore/rendering/RenderBox.cpp (133087 => 133088)
--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderBox.cpp 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderBox.cpp 2012-10-31 22:34:47 UTC (rev 133088)
@@ -4037,7 +4037,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: branches/safari-536.28-branch/Source/WebCore/rendering/RenderBoxModelObject.cpp (133087 => 133088)
--- branches/safari-536.28-branch/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-10-31 22:32:04 UTC (rev 133087)
+++ branches/safari-536.28-branch/Source/WebCore/rendering/RenderBoxModelObject.cpp 2012-10-31 22:34:47 UTC (rev 133088)
@@ -3047,6 +3047,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())) {
@@ -3059,6 +3064,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.
