Changeset 50623 in webkit

Timestamp:

Nov 8, 2009, 9:32:07 AM (15 years ago)

Author:

mitz@apple.com

Message:

<rdar://problem/7363434> Crash inside RenderObject::localToAbsolute
below FrameView::layout
​https://bugs.webkit.org/show_bug.cgi?id=31093

Reviewed by Simon Fraser.

WebCore:

Test: fast/block/positioning/relative-positioned-inline-container.html

In <​http://trac.webkit.org/changeset/19148>, setStaticY() was changed
to mark the object for layout, doing so without marking its ancestors.
However, RenderBlock::skipLeadingWhitespace and
RenderBlock::skipTrailingWhitespace() call setStaticY() on a relative-
positioned inline container, causing it to be marked for layout without
ever going back to give it layout, and thus layout could end with a
dirty object still in the tree, leading to all sorts of badness.

The fix is to revert setStaticY() to not marking the object dirty, and
instead do it in the call sites that require it, which are in
RenderBlock and RenderFlexibleBox.

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::adjustPositionedBlock):

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::layoutHorizontalBox):
(WebCore::RenderFlexibleBox::layoutVerticalBox):

• rendering/RenderLayer.cpp:
• rendering/RenderLayer.h:

(WebCore::RenderLayer::setStaticY):

LayoutTests:

• fast/block/positioning/relative-positioned-inline-container-expected.checksum: Added.
• fast/block/positioning/relative-positioned-inline-container-expected.png: Added.
• fast/block/positioning/relative-positioned-inline-container-expected.txt: Added.
• fast/block/positioning/relative-positioned-inline-container.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/block/positioning/relative-positioned-inline-container-expected.checksum (added)
• LayoutTests/fast/block/positioning/relative-positioned-inline-container-expected.png (added)
• LayoutTests/fast/block/positioning/relative-positioned-inline-container-expected.txt (added)
• LayoutTests/fast/block/positioning/relative-positioned-inline-container.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/rendering/RenderBlock.cpp (modified) (1 diff)
• WebCore/rendering/RenderFlexibleBox.cpp (modified) (2 diffs)
• WebCore/rendering/RenderLayer.cpp (modified) (1 diff)
• WebCore/rendering/RenderLayer.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-11-08  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        <rdar://problem/7363434> Crash inside RenderObject::localToAbsolute
+        below FrameView::layout
+        https://bugs.webkit.org/show_bug.cgi?id=31093
+
+        * fast/block/positioning/relative-positioned-inline-container-expected.checksum: Added.
+        * fast/block/positioning/relative-positioned-inline-container-expected.png: Added.
+        * fast/block/positioning/relative-positioned-inline-container-expected.txt: Added.
+        * fast/block/positioning/relative-positioned-inline-container.html: Added.
+
 2009-11-07  Dirk Pranke  <dpranke@chromium.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-11-08  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        <rdar://problem/7363434> Crash inside RenderObject::localToAbsolute
+        below FrameView::layout
+        https://bugs.webkit.org/show_bug.cgi?id=31093
+
+        Test: fast/block/positioning/relative-positioned-inline-container.html
+
+        In <http://trac.webkit.org/changeset/19148>, setStaticY() was changed
+        to mark the object for layout, doing so without marking its ancestors.
+        However, RenderBlock::skipLeadingWhitespace and
+        RenderBlock::skipTrailingWhitespace() call setStaticY() on a relative-
+        positioned inline container, causing it to be marked for layout without
+        ever going back to give it layout, and thus layout could end with a
+        dirty object still in the tree, leading to all sorts of badness.
+
+        The fix is to revert setStaticY() to not marking the object dirty, and
+        instead do it in the call sites that require it, which are in
+        RenderBlock and RenderFlexibleBox.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::adjustPositionedBlock):
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutHorizontalBox):
+        (WebCore::RenderFlexibleBox::layoutVerticalBox):
+        * rendering/RenderLayer.cpp:
+        * rendering/RenderLayer.h:
+        (WebCore::RenderLayer::setStaticY):
+
 2009-11-07  Daniel Bates  <dbates@webkit.org>
 

trunk/WebCore/rendering/RenderBlock.cpp

@@ -868 +868 @@
             y += (collapsedTopPos - collapsedTopNeg) - marginTop;
         }
-        child->layer()->setStaticY(y);
+        RenderLayer* childLayer = child->layer();
+        if (childLayer->staticY() != y) {
+            child->layer()->setStaticY(y);
+            child->setChildNeedsLayout(true, false);
+        }
     }
 }

trunk/WebCore/rendering/RenderFlexibleBox.cpp

@@ -411 +411 @@
                     else child->layer()->setStaticX(width() - xPos);
                 }
-                if (child->style()->hasStaticY())
-                    child->layer()->setStaticY(yPos);
+                if (child->style()->hasStaticY()) {
+                    RenderLayer* childLayer = child->layer();
+                    if (childLayer->staticY() != yPos) {
+                        child->layer()->setStaticY(yPos);
+                        child->setChildNeedsLayout(true, false);
+                    }
+                }
                 child = iterator.next();
                 continue;
@@ -770 +775 @@
                         child->layer()->setStaticX(borderRight()+paddingRight());
                 }
-                if (child->style()->hasStaticY())
-                    child->layer()->setStaticY(height());
+                if (child->style()->hasStaticY()) {
+                    RenderLayer* childLayer = child->layer();
+                    if (childLayer->staticY() != height()) {
+                        child->layer()->setStaticY(height());
+                        child->setChildNeedsLayout(true, false);
+                    }
+                }
                 child = iterator.next();
                 continue;

trunk/WebCore/rendering/RenderLayer.cpp

@@ -244 +244 @@
 }
 
-void RenderLayer::setStaticY(int staticY)
-{
-    if (m_staticY == staticY)
-        return;
-    m_staticY = staticY;
-    renderer()->setChildNeedsLayout(true, false);
-}
-
 void RenderLayer::updateLayerPositions(UpdateLayerPositionsFlags flags)
 {

trunk/WebCore/rendering/RenderLayer.h

@@ -391 +391 @@
     int staticY() const { return m_staticY; }
     void setStaticX(int staticX) { m_staticX = staticX; }
-    void setStaticY(int staticY);
+    void setStaticY(int staticY) { m_staticY = staticY; }
 
     bool hasTransform() const { return renderer()->hasTransform(); }