[css-flexbox] Use correct aspect ratio for min-size: auto

third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Index: third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
index 3d90d7938790bc1fe97bd44aca3f750dc5b9ec5e..25111556d3a748e3a61c1d39c28442e95ef2434e 100644
--- a/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
@@ -625,6 +625,32 @@ LayoutPoint LayoutFlexibleBox::flowAwareLocationForChild(const LayoutBox& child)
     return isHorizontalFlow() ? child.location() : child.location().transposedPoint();
 }
 
+bool LayoutFlexibleBox::useChildAspectRatio(const LayoutBox& child) const
+{
+    if (!child.isImage() && !child.isCanvas() && !child.isVideo())
+        return false;
+    if (child.intrinsicSize().height() == 0) {
+        // We can't compute a ratio in this case.
+        return false;
+    }
+    Length crossSize;
+    if (isHorizontalFlow())
+        crossSize = child.styleRef().height();

[leviw_travelin_and_unemployed, 2016/01/27 04:53:56]

Do we have tests for both these codepaths?

[cbiesinger, 2016/01/27 21:42:56]

Complicated question...

Short answer, yes, flex-minimum-{width,height}-flex-items-008.xht test the two
cases (here and below).

Long answer, we already passed the width version because of how we compute the
min-content logical width vs the height. The width takes the aspect ratio into
account, the height doesn't.

Your question made me realize that we have a bug in the contentSize calculation.
I've updated the code to fix that bug, and marked some tests as failing, and
added a test for that.

[leviw_travelin_and_unemployed, 2016/01/27 23:18:45]

Word :)

+    else
+        crossSize = child.styleRef().width();
+    return crossAxisLengthIsDefinite(child, crossSize);
+}
+
+LayoutUnit LayoutFlexibleBox::computeMainSizeFromAspectRatio(const LayoutBox& child, LayoutUnit crossSize) const
+{
+    ASSERT(useChildAspectRatio(child));
+    const LayoutSize& childIntrinsicSize = child.intrinsicSize();
+    double ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
+    if (isHorizontalFlow())

[leviw_travelin_and_unemployed, 2016/01/27 04:53:56]

Ditto.

+        return crossSize * ratio;
+    return crossSize / ratio;
+}
+
 void LayoutFlexibleBox::setFlowAwareLocationForChild(LayoutBox& child, const LayoutPoint& location)
 {
     if (isHorizontalFlow())
@@ -650,6 +676,19 @@ bool LayoutFlexibleBox::mainAxisLengthIsDefinite(const LayoutBox& child, const L
     return true;
 }
 
+bool LayoutFlexibleBox::crossAxisLengthIsDefinite(const LayoutBox& child, const Length& length) const

[leviw_travelin_and_unemployed, 2016/01/27 04:53:56]

Damn, it's back :(

[cbiesinger, 2016/01/27 21:42:56]

Hm? What do you mean?

[leviw_travelin_and_unemployed, 2016/01/27 23:18:45]

Is this not a method that existed previously?

[cbiesinger, 2016/01/27 23:25:25]

I don't *believe* so but I'm actually not sure...

+{
+    if (length.isAuto())
+        return false;
+    if (length.hasPercent()) {
+        return hasOrthogonalFlow(child) ?
+            hasDefiniteLogicalWidth() :
+            child.computePercentageLogicalHeight(length) != -1;
+
+    }
+    return true;
+}
+
 bool LayoutFlexibleBox::childFlexBaseSizeRequiresLayout(const LayoutBox& child) const
 {
     return !mainAxisLengthIsDefinite(child, flexBasisForChild(child)) && (
@@ -923,13 +962,26 @@ LayoutUnit LayoutFlexibleBox::adjustChildSizeForMinAndMax(const LayoutBox& child
         if (mainAxisLengthIsDefinite(child, mainSize)) {
             LayoutUnit resolvedMainSize = computeMainAxisExtentForChild(child, MainOrPreferredSize, mainSize);
             ASSERT(resolvedMainSize >= 0);
+            // TODO(cbiesinger): For items with an aspect ratio, clamp this size through the converted cross-size min/max size. crbug.com/249112
             LayoutUnit specifiedSize = maxExtent != -1 ? std::min(resolvedMainSize, maxExtent) : resolvedMainSize;
 
             minExtent = std::min(specifiedSize, contentSize);
+        } else if (useChildAspectRatio(child)) {
+            Length crossSizeLength = isHorizontalFlow() ? child.styleRef().height() : child.styleRef().width();
+            LayoutUnit crossSize;
+            if (crossSizeLength.isFixed()) {
+                crossSize = crossSizeLength.value();
+            } else {
+                ASSERT(crossSizeLength.hasPercent());
+                crossSize = hasOrthogonalFlow(child) ?
+                    adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth())) :
+                    child.computePercentageLogicalHeight(crossSizeLength);
+            }
+            LayoutUnit transferredSize = computeMainSizeFromAspectRatio(child, crossSize);
+            minExtent = std::min(transferredSize, contentSize);
         } else {
             minExtent = contentSize;
         }
-        // TODO(cbiesinger): Implement aspect ratio handling (here, transferred size) - crbug.com/249112
     }
     ASSERT(minExtent >= 0);
     return std::max(childSize, minExtent);
@@ -1174,6 +1226,7 @@ bool LayoutFlexibleBox::needToStretchChildLogicalHeight(const LayoutBox& child)
     if (isHorizontalFlow() != child.styleRef().isHorizontalWritingMode())
         return false;
 
+    // TODO(cbiesinger): what about indefinite percentage heights?
     return isHorizontalFlow() ? child.styleRef().height().isAuto() : child.styleRef().width().isAuto();
 }
 
@@ -1203,6 +1256,7 @@ EOverflow LayoutFlexibleBox::crossAxisOverflowForChild(const LayoutBox& child) c
         return child.styleRef().overflowY();
     return child.styleRef().overflowX();
 }
+
 void LayoutFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList& children, const Vector<LayoutUnit, 16>& childSizes, LayoutUnit availableFreeSpace, bool relayoutChildren, SubtreeLayoutScope& layoutScope, Vector<LineContext>& lineContexts)
 {
     ASSERT(childSizes.size() == children.size());