Percent height content should respect the fixed height of its containing cell

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 2735 matching lines @@
     LayoutUnit stretchedFlexHeight(-1);
     if (cb->isFlexItem())
         stretchedFlexHeight = toLayoutFlexibleBox(cb->parent())->childLogicalHeightForPercentageResolution(*cb);
 
     if (isHorizontalWritingMode() != cb->isHorizontalWritingMode()) {
         availableHeight = containingBlockChild->containingBlockLogicalWidthForContent();
     } else if (stretchedFlexHeight != LayoutUnit(-1)) {
         availableHeight = stretchedFlexHeight;
     } else if (hasOverrideContainingBlockLogicalHeight()) {
         availableHeight = overrideContainingBlockContentLogicalHeight();
+    } else if (cbstyle.logicalHeight().isFixed()) {
+        LayoutUnit contentBoxHeight = cb->adjustContentBoxLogicalHeightForBoxSizing(cbstyle.logicalHeight().value());
+        availableHeight = std::max(LayoutUnit(), cb->constrainContentBoxLogicalHeightByMinMax(contentBoxHeight - cb->scrollbarLogicalHeight(), LayoutUnit(-1)));

[eae, 2016/06/02 22:50:34]

Please wrap this line. Also consider using clampNegativeToZero.

Perhaps something like
availableHeight = cb>constrainContentBoxLogicalHeightByMinMax(
        contentBoxHeight - cb->scrollbarLogicalHeight(),
        LayoutUnit(-1)
    ).clampNegativeToZero();

or using a local variable

LayoutUnit minHeight = contentBoxHeight - cb->scrollbarLogicalHeight();
availableHeight = cb>constrainContentBoxLogicalHeightByMinMax(minHeight,
    LayoutUnit(-1)).clampNegativeToZero();

+        if (cb->isTableCell()) {
+            includeBorderPadding = true;
+            // We're sizing content to the height from the cell's style so don't involve the intrinsic padding used to align the content.
+            availableHeight -= cb->computedCSSPaddingBefore() + cb->computedCSSPaddingAfter() + cb->borderBefore() + cb->borderAfter();
+        }
     } 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
             // be a percentage of the cell's current content height.
             if (!cb->hasOverrideLogicalContentHeight()) {
                 // Normally we would let the cell size intrinsically, but scrolling overflow has to be
                 // treated differently, since WinIE lets scrolled overflow regions shrink as needed.
                 // While we can't get all cases right, we can at least detect when the cell has a specified
                 // height or when the table has a specified height. In these cases we want to initially have
                 // no size and allow the flexing of the table or the cell to its specified height to cause us
                 // to grow to fill the space. This could end up being wrong in some cases, but it is
                 // preferable to the alternative (sizing intrinsically and making the row end up too big).
                 LayoutTableCell* cell = toLayoutTableCell(cb);
                 if (scrollsOverflowY() && (!cell->style()->logicalHeight().isAuto() || !cell->table()->style()->logicalHeight().isAuto()))
                     return LayoutUnit();
                 return LayoutUnit(-1);
             }
             availableHeight = cb->overrideLogicalContentHeight();
             includeBorderPadding = true;
         }
-    } else if (cbstyle.logicalHeight().isFixed()) {
-        LayoutUnit contentBoxHeight = cb->adjustContentBoxLogicalHeightForBoxSizing(LayoutUnit(cbstyle.logicalHeight().value()));
-        availableHeight = std::max(LayoutUnit(), cb->constrainContentBoxLogicalHeightByMinMax(contentBoxHeight - cb->scrollbarLogicalHeight(), LayoutUnit(-1)));
     } else if (cbstyle.logicalHeight().hasPercent() && !isOutOfFlowPositionedWithSpecifiedHeight) {
         // We need to recur and compute the percentage height for our containing block.
         LayoutUnit heightWithScrollbar = cb->computePercentageLogicalHeight(cbstyle.logicalHeight());
         if (heightWithScrollbar != -1) {
             LayoutUnit contentBoxHeightWithScrollbar = cb->adjustContentBoxLogicalHeightForBoxSizing(heightWithScrollbar);
             // We need to adjust for min/max height because this method does not
             // handle the min/max of the current block, its caller does. So the
             // return value from the recursive call will not have been adjusted
             // yet.
             LayoutUnit contentBoxHeight = cb->constrainContentBoxLogicalHeightByMinMax(contentBoxHeightWithScrollbar - cb->scrollbarLogicalHeight(), LayoutUnit(-1));
@@ skipping 12 matching lines @@
     if (availableHeight == -1)
         return availableHeight;
 
     availableHeight -= rootMarginBorderPaddingHeight;
 
     if (isTable() && isOutOfFlowPositioned())
         availableHeight += cb->paddingLogicalHeight();
 
     LayoutUnit result = valueForLength(height, availableHeight);
     if (includeBorderPadding) {
-        // FIXME: Table cells should default to box-sizing: border-box so we can avoid this hack.
-        // It is necessary to use the border-box to match WinIE's broken
-        // box model. This is essential for sizing inside
-        // table cells using percentage heights.
+        // TODO(rhogan) crbug.com/467378: Doing this for content inside tables cells is wrong, it should fill
+        // whatever height the cell makes available.
         result -= borderAndPaddingLogicalHeight();
         return std::max(LayoutUnit(), result);
     }
     return result;
 }
 
 LayoutUnit LayoutBox::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
     return computeReplacedLogicalWidthRespectingMinMaxWidth(computeReplacedLogicalWidthUsing(MainOrPreferredSize, style()->logicalWidth()), shouldComputePreferred);
 }
@@ skipping 2111 matching lines @@
         m_rareData->m_snapAreas->remove(&snapArea);
     }
 }
 
 SnapAreaSet* LayoutBox::snapAreas() const
 {
     return m_rareData ? m_rareData->m_snapAreas.get() : nullptr;
 }
 
 } // namespace blink