Blink doesn't honor percent heights on children of "align-self:stretch" flex items in a fixed-height

Source/core/rendering/RenderBox.cpp

Index: Source/core/rendering/RenderBox.cpp
diff --git a/Source/core/rendering/RenderBox.cpp b/Source/core/rendering/RenderBox.cpp
index 28737657d2809226a6b456bb89102c2f1c1d550d..b50a23ba3229874b172c31d3654004ade4a056d5 100644
--- a/Source/core/rendering/RenderBox.cpp
+++ b/Source/core/rendering/RenderBox.cpp
@@ -2273,14 +2273,17 @@ LayoutUnit RenderBox::computeLogicalWidthUsing(SizeType widthType, const Length&
     return logicalWidthResult;
 }
 
-static bool columnFlexItemHasStretchAlignment(const RenderObject* flexitem)
+static bool flexItemHasStretchAlignment(const RenderObject* flexitem)
 {
     RenderObject* parent = flexitem->parent();
-    // auto margins mean we don't stretch. Note that this function will only be used for
-    // widths, so we don't have to check marginBefore/marginAfter.
-    ASSERT(parent->style()->isColumnFlexDirection());
-    if (flexitem->style()->marginStart().isAuto() || flexitem->style()->marginEnd().isAuto())
-        return false;
+    // auto margins mean we don't stretch.
+    if (parent->style()->isColumnFlexDirection()) {
+        if (flexitem->style()->marginStart().isAuto() || flexitem->style()->marginEnd().isAuto())
+            return false;
+    } else {
+        if (flexitem->style()->marginBefore().isAuto() || flexitem->style()->marginAfter().isAuto())

[ojan, 2014/08/28 03:23:09]

I think you just want this. You don't need the isColumnFlexDirection block at
all. Both code paths can call this. Calling marginStart/End was just a premature
performance optimization I think.

[harpreet.sk, 2014/08/28 14:30:19]

I tried removing it but because of it following layout test is getting failed
"css3/flexbox/columns-center-with-margins.html"

In above LayoutTest the flex-item of colmun flexbox having auto margins is not
getting centered.

I think it's not correct to remove isColumnFlexDirection block .

+            return false;
+    }
     return flexitem->style()->alignSelf() == ItemPositionStretch || (flexitem->style()->alignSelf() == ItemPositionAuto && parent->style()->alignItems() == ItemPositionStretch);
 }
 
@@ -2291,7 +2294,7 @@ static bool isStretchingColumnFlexItem(const RenderObject* flexitem)
         return true;
 
     // We don't stretch multiline flexboxes because they need to apply line spacing (align-content) first.
-    if (parent->isFlexibleBox() && parent->style()->flexWrap() == FlexNoWrap && parent->style()->isColumnFlexDirection() && columnFlexItemHasStretchAlignment(flexitem))
+    if (parent->isFlexibleBox() && parent->style()->flexWrap() == FlexNoWrap && parent->style()->isColumnFlexDirection() && flexItemHasStretchAlignment(flexitem))
         return true;
     return false;
 }
@@ -2324,7 +2327,7 @@ bool RenderBox::sizesLogicalWidthToFitContent(const Length& logicalWidth) const
         // For multiline columns, we need to apply align-content first, so we can't stretch now.
         if (!parent()->style()->isColumnFlexDirection() || parent()->style()->flexWrap() != FlexNoWrap)
             return true;
-        if (!columnFlexItemHasStretchAlignment(this))
+        if (!flexItemHasStretchAlignment(this))
             return true;
     }
 
@@ -2628,11 +2631,13 @@ LayoutUnit RenderBox::computePercentageLogicalHeight(const Length& height) const
 
     bool includeBorderPadding = isTable();
 
-    if (isHorizontalWritingMode() != cb->isHorizontalWritingMode())
+    if (isHorizontalWritingMode() != cb->isHorizontalWritingMode()) {
         availableHeight = containingBlockChild->containingBlockLogicalWidthForContent();
-    else if (hasOverrideContainingBlockLogicalHeight())
+    } else if (hasOverrideContainingBlockLogicalHeight()) {
         availableHeight = overrideContainingBlockContentLogicalHeight();
-    else if (cb->isTableCell()) {
+    } else if (cb->isFlexItem() && flexItemHasStretchAlignment(cb) && cb->hasOverrideHeight()) {
+        availableHeight = cb->overrideLogicalContentHeight();
+    } else if (cb->isTableCell()) {
         if (!skippedAutoHeightContainingBlock) {
             // Table cells violate what the CSS spec says to do with heights. Basically we
             // don't care if the cell specified a height or not. We just always make ourselves