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

Source/core/rendering/RenderFlexibleBox.cpp

Index: Source/core/rendering/RenderFlexibleBox.cpp
diff --git a/Source/core/rendering/RenderFlexibleBox.cpp b/Source/core/rendering/RenderFlexibleBox.cpp
index 7cb1c671c53bc8417cfb52b5fdccc2f719bf66aa..89e2be3db0055f221eefeb26efa5de3ce719928c 100644
--- a/Source/core/rendering/RenderFlexibleBox.cpp
+++ b/Source/core/rendering/RenderFlexibleBox.cpp
@@ -611,7 +611,7 @@ bool RenderFlexibleBox::childPreferredMainAxisContentExtentRequiresLayout(Render
 
 LayoutUnit RenderFlexibleBox::preferredMainAxisContentExtentForChild(RenderBox* child, bool hasInfiniteLineLength, bool relayoutChildren)
 {
-    child->clearOverrideSize();
+    clearLogicalOverrideSize(child);
 
     if (child->style()->hasAspectRatio() || child->isImage() || child->isVideo() || child->isCanvas())
         UseCounter::count(document(), UseCounter::AspectRatioFlexItem);
@@ -993,6 +993,14 @@ void RenderFlexibleBox::setLogicalOverrideSize(RenderBox* child, LayoutUnit chil
         child->setOverrideLogicalContentWidth(childPreferredSize - child->borderAndPaddingLogicalWidth());
 }
 
+void RenderFlexibleBox::clearLogicalOverrideSize(RenderBox* child)

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

LogicalOverrideSize doesn't tell you which size you're clearing.
LocicalCOntentHeight and logicalContentWidth are both logical override sizes.

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

I named this api like this just to keep consistency with above api
"setLogicalOverrideSize" . Since above api is responsible for setting
overrideLogicalHeight/Width based on flow so i thought to named this api as
clearLogicalOverrideSize.

+{
+    if (hasOrthogonalFlow(child))

[cbiesinger, 2014/08/28 03:05:56]

When the flexbox changes from a column to a row flexbox (or vice versa), we have
to make sure to clear both override sizes, right? But you can handle that using
styleDidChange.

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

Added line to clear both override sizes.

+        child->clearOverrideLogicalContentHeight();
+    else
+        child->clearOverrideLogicalContentWidth();
+}
+
 void RenderFlexibleBox::prepareChildForPositionedLayout(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset, PositionedLayoutMode layoutMode)
 {
     ASSERT(child->isOutOfFlowPositioned());
@@ -1353,10 +1361,10 @@ void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox* child, LayoutUni
             LayoutUnit stretchedLogicalHeight = heightBeforeStretching + availableAlignmentSpaceForChildBeforeStretching(lineCrossAxisExtent, child);
             ASSERT(!child->needsLayout());
             LayoutUnit desiredLogicalHeight = child->constrainLogicalHeightByMinMax(stretchedLogicalHeight, heightBeforeStretching - child->borderAndPaddingLogicalHeight());
+            child->setOverrideLogicalContentHeight(desiredLogicalHeight - child->borderAndPaddingLogicalHeight());

[cbiesinger, 2014/08/28 03:05:56]

I think we also have to relayout if the old overrideLogicalContentHeight != the
new logicalContentHeight (at least if hasPercentHeightDecendants()) -- consider
this example:

<div class="flexbox"><div style="height: 100px"><div
height="50%"></div></div></div>

The flexbox height will be identical to the intrinsic <div> height, so the if
below will be false, so the child div will be incorrectly sized, right? I don't
see a testcase for that in your test, either.

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

@Chris: The example which you have stated i think is not related to the bug. It
is because the flex-item (*child* here) have fixed height and you can see above
that our first condition checks
  
     if (!isColumnFlow() && child->style()->logicalHeight().isAuto())

Since style()->logicalHeight() of child is not auto (*fixed* here) so the child
will never stretch. The test case will never enter into this condition.

I cross checked this test case it was working fine in earlier version of chrome
also. Should i add this testcase although it's irrevelant to the bug. 

[cbiesinger, 2014/08/29 00:28:09]

Apologies, you are correct. I tried to make a testcase that hits the
desiredLogicalHeight == child->logicalHeight case below. I still think that your
patch does not handle that case correctly?

 
             // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
             if (desiredLogicalHeight != child->logicalHeight()) {
-                child->setOverrideLogicalContentHeight(desiredLogicalHeight - child->borderAndPaddingLogicalHeight());
                 child->setLogicalHeight(0);
                 child->forceChildLayout();
             }