Changeset 138770 in webkit

Timestamp:

Jan 3, 2013, 5:21:07 PM (12 years ago)

Author:

tony@chromium.org

Message:

incorrect flexbox relayout with overflow, padding and absolute positioning
​https://bugs.webkit.org/show_bug.cgi?id=106022

Reviewed by Ojan Vafai.

Source/WebCore:

The problem was we were entering simplified layout, which doesn't apply the stretch
behavior for flex children. That should be fine if we had kept the override size from
the previous layout. So clear the override size during layout rather than after layout.

Not clearing the override size re-triggers bug 98611. Fix this by always forcing a
layout of an image if it has an override size set. The presence of an override size means
we did some special layout that may not be covered by re-computing the logical width and
height.

Test: css3/flexbox/stretch-simplified-layout.html

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::layoutBlock): Don't clear the override size after layout.
(WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Clear the override size right before layout.

• rendering/RenderFlexibleBox.h:

(RenderFlexibleBox): Remove clearChildOverrideSizes() since it's not called anymore.

• rendering/RenderImage.cpp:

(WebCore::RenderImage::imageDimensionsChanged): If an override size is present, force a relayout.
Also cleaned up the code a bit to avoid computing the width and height unless we need to.

LayoutTests:

• css3/flexbox/stretch-simplified-layout-expected.txt: Added.
• css3/flexbox/stretch-simplified-layout.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/stretch-simplified-layout-expected.txt (added)
• LayoutTests/css3/flexbox/stretch-simplified-layout.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (1 diff)
• Source/WebCore/rendering/RenderImage.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-03  Tony Chang  <tony@chromium.org>
+
+        incorrect flexbox relayout with overflow, padding and absolute positioning
+        https://bugs.webkit.org/show_bug.cgi?id=106022
+
+        Reviewed by Ojan Vafai.
+
+        * css3/flexbox/stretch-simplified-layout-expected.txt: Added.
+        * css3/flexbox/stretch-simplified-layout.html: Added.
+
 2013-01-03  Terry Anderson  <tdanderson@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-03  Tony Chang  <tony@chromium.org>
+
+        incorrect flexbox relayout with overflow, padding and absolute positioning
+        https://bugs.webkit.org/show_bug.cgi?id=106022
+
+        Reviewed by Ojan Vafai.
+
+        The problem was we were entering simplified layout, which doesn't apply the stretch
+        behavior for flex children. That should be fine if we had kept the override size from
+        the previous layout. So clear the override size during layout rather than after layout.
+
+        Not clearing the override size re-triggers bug 98611. Fix this by always forcing a
+        layout of an image if it has an override size set. The presence of an override size means
+        we did some special layout that may not be covered by re-computing the logical width and
+        height.
+
+        Test: css3/flexbox/stretch-simplified-layout.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::layoutBlock): Don't clear the override size after layout.
+        (WebCore::RenderFlexibleBox::computeMainAxisPreferredSizes): Clear the override size right before layout.
+        * rendering/RenderFlexibleBox.h:
+        (RenderFlexibleBox): Remove clearChildOverrideSizes() since it's not called anymore.
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::imageDimensionsChanged): If an override size is present, force a relayout.
+        Also cleaned up the code a bit to avoid computing the width and height unless we need to.
+
 2013-01-03  Adam Klein  <adamk@chromium.org>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -343 +343 @@
 
     computeRegionRangeForBlock();
-    clearChildOverrideSizes();
 
     repaintChildrenDuringLayoutIfMoved(oldChildRects);
@@ -413 +412 @@
     // direction:rtl + flex-direction:column means the cross-axis direction is flipped.
     flipForRightToLeftColumn();
-}
-
-void RenderFlexibleBox::clearChildOverrideSizes()
-{
-    for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox())
-        child->clearOverrideSize();
 }
 
@@ -858 +851 @@
         if (child->isOutOfFlowPositioned())
             continue;
+
+        child->clearOverrideSize();
 
         // Only need to layout here if we will need to get the logicalHeight of the child in computeNextFlexLine.

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -140 +140 @@
     bool updateAutoMarginsInCrossAxis(RenderBox* child, LayoutUnit availableAlignmentSpace);
     void repositionLogicalHeightDependentFlexItems(Vector<LineContext>&, LayoutUnit& oldClientAfterEdge);
-    void clearChildOverrideSizes();
     void appendChildFrameRects(ChildFrameRects&);
     void repaintChildrenDuringLayoutIfMoved(const ChildFrameRects&);

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -223 +223 @@
         if (!preferredLogicalWidthsDirty())
             setPreferredLogicalWidthsDirty(true);
-        LogicalExtentComputedValues computedValues;
-        computeLogicalWidthInRegion(computedValues);
-        LayoutUnit newWidth = computedValues.m_extent;
-        computeLogicalHeight(height(), 0, computedValues);
-        LayoutUnit newHeight = computedValues.m_extent;
+
+        bool hasOverrideSize = hasOverrideHeight() || hasOverrideWidth();
+        if (!hasOverrideSize && !imageSizeChanged) {
+            LogicalExtentComputedValues computedValues;
+            computeLogicalWidthInRegion(computedValues);
+            LayoutUnit newWidth = computedValues.m_extent;
+            computeLogicalHeight(height(), 0, computedValues);
+            LayoutUnit newHeight = computedValues.m_extent;
+
+            imageSizeChanged = width() != newWidth || height() != newHeight;
+        }
 
         // FIXME: We only need to recompute the containing block's preferred size
@@ -233 +239 @@
         // There's no easy way to detect that shrink-to-fit is needed, always force a layout.
         bool containingBlockNeedsToRecomputePreferredSize =
-            style()->logicalWidth().type() == Percent
-            || style()->logicalMaxWidth().type() == Percent
-            || style()->logicalMinWidth().type() == Percent;
-
-        if (imageSizeChanged || width() != newWidth || height() != newHeight || containingBlockNeedsToRecomputePreferredSize) {
+            style()->logicalWidth().isPercent()
+            || style()->logicalMaxWidth().isPercent()
+            || style()->logicalMinWidth().isPercent();
+
+        if (imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize) {
             shouldRepaint = false;
             if (!selfNeedsLayout())