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

Source/core/rendering/RenderFlexibleBox.cpp

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 593 matching lines @@
     return flexBasis.isAuto() || (flexBasis.isFixed() && !flexBasis.value() && hasInfiniteLineLength);
 }
 
 bool RenderFlexibleBox::childPreferredMainAxisContentExtentRequiresLayout(RenderBox* child, bool hasInfiniteLineLength) const
 {
     return preferredMainAxisExtentDependsOnLayout(flexBasisForChild(child), hasInfiniteLineLength) && hasOrthogonalFlow(child);
 }
 
 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);
 
     Length flexBasis = flexBasisForChild(child);
     if (preferredMainAxisExtentDependsOnLayout(flexBasis, hasInfiniteLineLength)) {
         LayoutUnit mainAxisExtent;
         if (hasOrthogonalFlow(child)) {
             if (child->needsLayout() || relayoutChildren) {
                 m_intrinsicSizeAlongMainAxis.remove(child);
@@ skipping 361 matching lines @@
 }
 
 void RenderFlexibleBox::setLogicalOverrideSize(RenderBox* child, LayoutUnit childPreferredSize)
 {
     if (hasOrthogonalFlow(child))
         child->setOverrideLogicalContentHeight(childPreferredSize - child->borderAndPaddingLogicalHeight());
     else
         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());
     child->containingBlock()->insertPositionedObject(child);
     RenderLayer* childLayer = child->layer();
     LayoutUnit inlinePosition = isColumnFlow() ? crossAxisOffset : mainAxisOffset;
     if (layoutMode == FlipForRowReverse && style()->flexDirection() == FlowRowReverse)
         inlinePosition = mainAxisExtent() - mainAxisOffset;
     childLayer->setStaticInlinePosition(inlinePosition);
 
@@ skipping 340 matching lines @@
 
 void RenderFlexibleBox::applyStretchAlignmentToChild(RenderBox* child, LayoutUnit lineCrossAxisExtent)
 {
     if (!isColumnFlow() && child->style()->logicalHeight().isAuto()) {
         // FIXME: If the child has orthogonal flow, then it already has an override height set, so use it.
         if (!hasOrthogonalFlow(child)) {
             LayoutUnit heightBeforeStretching = needToStretchChildLogicalHeight(child) ? constrainedChildIntrinsicContentLogicalHeight(child) : child->logicalHeight();
             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();
             }
         }
     } else if (isColumnFlow() && child->style()->logicalWidth().isAuto()) {
         // FIXME: If the child doesn't have orthogonal flow, then it already has an override width set, so use it.
         if (hasOrthogonalFlow(child)) {
             LayoutUnit childWidth = std::max<LayoutUnit>(0, lineCrossAxisExtent - crossAxisMarginExtentForChild(child));
             childWidth = child->constrainLogicalWidthByMinMax(childWidth, childWidth, this);
 
@@ skipping 31 matching lines @@
             ASSERT(child);
             LayoutUnit lineCrossAxisExtent = lineContexts[lineNumber].crossAxisExtent;
             LayoutUnit originalOffset = lineContexts[lineNumber].crossAxisOffset - crossAxisStartEdge;
             LayoutUnit newOffset = contentExtent - originalOffset - lineCrossAxisExtent;
             adjustAlignmentForChild(child, newOffset - originalOffset);
         }
     }
 }
 
 }