Simplify Writing-Mode Related Calculation of Margins

Remove the very complicated logic from computeLogicalHeight() and
computeLogicalWidth() whose sole purpose was to use the writing mode of
the child vs the containing block when appropriate. The complexity was
unnecessary and the same result can be achieved by ensuring we use the
child's writing mode when computing the size of the box and the parent's
writing mode when computing the margins for position (e.g. for margin
collapsing).

Review URL: https://codereview.chromium.org/298563002

git-svn-id: svn://svn.chromium.org/blink/trunk@176154 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/rendering/RenderBox.cpp

@@ -2227,15 +2227,8 @@ void RenderBox::computeLogicalWidth(LogicalExtentComputedValues& computedValues)
     }
 
     // Margin calculations.
-    if (hasPerpendicularContainingBlock || isFloating() || isInline()) {
-        computedValues.m_margins.m_start = minimumValueForLength(styleToUse->marginStart(), containerLogicalWidth);
-        computedValues.m_margins.m_end = minimumValueForLength(styleToUse->marginEnd(), containerLogicalWidth);
-    } else {
-        bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection();
-        computeInlineDirectionMargins(cb, containerLogicalWidth, computedValues.m_extent,
-            hasInvertedDirection ? computedValues.m_margins.m_end : computedValues.m_margins.m_start,
-            hasInvertedDirection ? computedValues.m_margins.m_start : computedValues.m_margins.m_end);
-    }
+    computeMarginsForDirection(InlineDirection, cb, containerLogicalWidth, computedValues.m_extent, computedValues.m_margins.m_start,
+        computedValues.m_margins.m_end, style()->marginStart(), style()->marginEnd());
 
     if (!hasPerpendicularContainingBlock && containerLogicalWidth && containerLogicalWidth != (computedValues.m_extent + computedValues.m_margins.m_start + computedValues.m_margins.m_end)
         && !isFloating() && !isInline() && !cb->isFlexibleBoxIncludingDeprecated() && !cb->isRenderGrid()) {
@@ -2403,13 +2396,19 @@ bool RenderBox::autoWidthShouldFitContent() const
         || isHTMLTextAreaElement(*node()) || (isHTMLLegendElement(*node()) && !style()->hasOutOfFlowPosition()));
 }
 
-void RenderBox::computeInlineDirectionMargins(RenderBlock* containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const
+void RenderBox::computeMarginsForDirection(MarginDirection flowDirection, const RenderBlock* containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd, Length marginStartLength, Length marginEndLength) const
 {
-    const RenderStyle* containingBlockStyle = containingBlock->style();
-    Length marginStartLength = style()->marginStartUsing(containingBlockStyle);
-    Length marginEndLength = style()->marginEndUsing(containingBlockStyle);
+    if (flowDirection == BlockDirection || isFloating() || isInline()) {
+        if (isTableCell() && flowDirection == BlockDirection) {
+            // FIXME: Not right if we allow cells to have different directionality than the table. If we do allow this, though,
+            // we may just do it with an extra anonymous block inside the cell.
+            marginStart = 0;
+            marginEnd = 0;
+            return;
+        }
 
-    if (isFloating() || isInline()) {
+        // Margins are calculated with respect to the logical width of
+        // the containing block (8.3)
         // Inline blocks/tables and floats don't have their margins increased.
         marginStart = minimumValueForLength(marginStartLength, containerWidth);
         marginEnd = minimumValueForLength(marginEndLength, containerWidth);
@@ -2445,8 +2444,9 @@ void RenderBox::computeInlineDirectionMargins(RenderBlock* containingBlock, Layo
 
     // CSS 2.1: "If both 'margin-left' and 'margin-right' are 'auto', their used values are equal. This horizontally centers the element
     // with respect to the edges of the containing block."
+    const RenderStyle* containingBlockStyle = containingBlock->style();
     if ((marginStartLength.isAuto() && marginEndLength.isAuto() && marginBoxWidth < availableWidth)
-        || (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlock->style()->textAlign() == WEBKIT_CENTER)) {
+        || (!marginStartLength.isAuto() && !marginEndLength.isAuto() && containingBlockStyle->textAlign() == WEBKIT_CENTER)) {
         // Other browsers center the margin box for align=center elements so we match them here.
         LayoutUnit centeredMarginBoxStart = max<LayoutUnit>(0, (availableWidth - childWidth - marginStartWidth - marginEndWidth) / 2);
         marginStart = centeredMarginBoxStart + marginStartWidth;
@@ -2474,32 +2474,6 @@ void RenderBox::computeInlineDirectionMargins(RenderBlock* containingBlock, Layo
     marginEnd = marginEndWidth;
 }
 
-static bool shouldFlipBeforeAfterMargins(const RenderStyle* containingBlockStyle, const RenderStyle* childStyle)
-{
-    ASSERT(containingBlockStyle->isHorizontalWritingMode() != childStyle->isHorizontalWritingMode());
-    WritingMode childWritingMode = childStyle->writingMode();
-    bool shouldFlip = false;
-    switch (containingBlockStyle->writingMode()) {
-    case TopToBottomWritingMode:
-        shouldFlip = (childWritingMode == RightToLeftWritingMode);
-        break;
-    case BottomToTopWritingMode:
-        shouldFlip = (childWritingMode == RightToLeftWritingMode);
-        break;
-    case RightToLeftWritingMode:
-        shouldFlip = (childWritingMode == BottomToTopWritingMode);
-        break;
-    case LeftToRightWritingMode:
-        shouldFlip = (childWritingMode == BottomToTopWritingMode);
-        break;
-    }
-
-    if (!containingBlockStyle->isLeftToRightDirection())
-        shouldFlip = !shouldFlip;
-
-    return shouldFlip;
-}
-
 void RenderBox::updateLogicalHeight()
 {
     m_intrinsicContentLogicalHeight = contentLogicalHeight();
@@ -2527,23 +2501,20 @@ void RenderBox::computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logica
         computePositionedLogicalHeight(computedValues);
     else {
         RenderBlock* cb = containingBlock();
-        bool hasPerpendicularContainingBlock = cb->isHorizontalWritingMode() != isHorizontalWritingMode();
 
-        if (!hasPerpendicularContainingBlock) {
-            bool shouldFlipBeforeAfter = cb->style()->writingMode() != style()->writingMode();
-            computeBlockDirectionMargins(cb,
-                shouldFlipBeforeAfter ? computedValues.m_margins.m_after : computedValues.m_margins.m_before,
-                shouldFlipBeforeAfter ? computedValues.m_margins.m_before : computedValues.m_margins.m_after);
-        }
+        // If we are perpendicular to our containing block then we need to resolve our block-start and block-end margins so that if they
+        // are 'auto' we are centred or aligned within the inline flow containing block: this is done by computing the margins as though they are inline.
+        // Note that as this is the 'sizing phase' we are using our own writing mode rather than the containing block's. We use the containing block's
+        // writing mode when figuring out the block-direction margins for positioning in |computeAndSetBlockDirectionMargins| (i.e. margin collapsing etc.).
+        // See http://www.w3.org/TR/2014/CR-css-writing-modes-3-20140320/#orthogonal-flows
+        MarginDirection flowDirection = isHorizontalWritingMode() != cb->isHorizontalWritingMode() ? InlineDirection : BlockDirection;
 
         // For tables, calculate margins only.
         if (isTable()) {
-            if (hasPerpendicularContainingBlock) {
-                bool shouldFlipBeforeAfter = shouldFlipBeforeAfterMargins(cb->style(), style());
-                computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), computedValues.m_extent,
-                    shouldFlipBeforeAfter ? computedValues.m_margins.m_after : computedValues.m_margins.m_before,
-                    shouldFlipBeforeAfter ? computedValues.m_margins.m_before : computedValues.m_margins.m_after);
-            }
+            // FIXME: RenderTable::layout() calls updateLogicalHeight() when an empty table has no height yet, so auto margins can come out wrong here when
+            // we are perpendicular to our containing block.
+            computeMarginsForDirection(flowDirection, cb, containingBlockLogicalWidthForContent(), computedValues.m_extent, computedValues.m_margins.m_before,
+                computedValues.m_margins.m_after, style()->marginBefore(), style()->marginAfter());
             return;
         }
 
@@ -2590,13 +2561,8 @@ void RenderBox::computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logica
         }
 
         computedValues.m_extent = heightResult;
-
-        if (hasPerpendicularContainingBlock) {
-            bool shouldFlipBeforeAfter = shouldFlipBeforeAfterMargins(cb->style(), style());
-            computeInlineDirectionMargins(cb, containingBlockLogicalWidthForContent(), heightResult,
-                shouldFlipBeforeAfter ? computedValues.m_margins.m_after : computedValues.m_margins.m_before,
-                shouldFlipBeforeAfter ? computedValues.m_margins.m_before : computedValues.m_margins.m_after);
-        }
+        computeMarginsForDirection(flowDirection, cb, containingBlockLogicalWidthForContent(), computedValues.m_extent, computedValues.m_margins.m_before,
+            computedValues.m_margins.m_after, style()->marginBefore(), style()->marginAfter());
     }
 
     // WinIE quirk: The <html> block always fills the entire canvas in quirks mode.  The <body> always fills the
@@ -2949,29 +2915,15 @@ LayoutUnit RenderBox::availableLogicalHeightUsing(const Length& h, AvailableLogi
     return availableHeight;
 }
 
-void RenderBox::computeBlockDirectionMargins(const RenderBlock* containingBlock, LayoutUnit& marginBefore, LayoutUnit& marginAfter) const
-{
-    if (isTableCell()) {
-        // FIXME: Not right if we allow cells to have different directionality than the table. If we do allow this, though,
-        // we may just do it with an extra anonymous block inside the cell.
-        marginBefore = 0;
-        marginAfter = 0;
-        return;
-    }
-
-    // Margins are calculated with respect to the logical width of
-    // the containing block (8.3)
-    LayoutUnit cw = containingBlockLogicalWidthForContent();
-    RenderStyle* containingBlockStyle = containingBlock->style();
-    marginBefore = minimumValueForLength(style()->marginBeforeUsing(containingBlockStyle), cw);
-    marginAfter = minimumValueForLength(style()->marginAfterUsing(containingBlockStyle), cw);
-}
-
 void RenderBox::computeAndSetBlockDirectionMargins(const RenderBlock* containingBlock)
 {
     LayoutUnit marginBefore;
     LayoutUnit marginAfter;
-    computeBlockDirectionMargins(containingBlock, marginBefore, marginAfter);
+    computeMarginsForDirection(BlockDirection, containingBlock, containingBlockLogicalWidthForContent(), logicalHeight(), marginBefore, marginAfter,
+        style()->marginBeforeUsing(containingBlock->style()),
+        style()->marginAfterUsing(containingBlock->style()));
+    // Note that in this 'positioning phase' of the layout we are using the containing block's writing mode rather than our own when calculating margins.
+    // See http://www.w3.org/TR/2014/CR-css-writing-modes-3-20140320/#orthogonal-flows
     containingBlock->setMarginBeforeForChild(this, marginBefore);
     containingBlock->setMarginAfterForChild(this, marginAfter);
 }

third_party/WebKit/Source/core/rendering/RenderBox.h

@@ -36,6 +36,7 @@ struct PaintInfo;
 enum SizeType { MainOrPreferredSize, MinSize, MaxSize };
 enum AvailableLogicalHeightType { ExcludeMarginBorderPadding, IncludeMarginBorderPadding };
 enum OverlayScrollbarSizeRelevancy { IgnoreOverlayScrollbarSize, IncludeOverlayScrollbarSize };
+enum MarginDirection { BlockDirection, InlineDirection };
 
 enum ShouldComputePreferred { ComputeActual, ComputePreferred };
 
@@ -385,12 +386,11 @@ public:
         LayoutUnit m_position;
         ComputedMarginValues m_margins;
     };
-    // Resolve auto margins in the inline direction of the containing block so that objects can be pushed to the start, middle or end
+    // Resolve auto margins in the chosen direction of the containing block so that objects can be pushed to the start, middle or end
     // of the containing block.
-    void computeInlineDirectionMargins(RenderBlock* containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd) const;
+    void computeMarginsForDirection(MarginDirection forDirection, const RenderBlock* containingBlock, LayoutUnit containerWidth, LayoutUnit childWidth, LayoutUnit& marginStart, LayoutUnit& marginEnd, Length marginStartLength, Length marginStartEnd) const;
 
     // Used to resolve margins in the containing block's block-flow direction.
-    void computeBlockDirectionMargins(const RenderBlock* containingBlock, LayoutUnit& marginBefore, LayoutUnit& marginAfter) const;
     void computeAndSetBlockDirectionMargins(const RenderBlock* containingBlock);
 
     virtual LayoutUnit offsetFromLogicalTopOfFirstPage() const;

third_party/WebKit/Source/core/rendering/RenderTable.cpp

@@ -302,20 +302,10 @@ void RenderTable::updateLogicalWidth()
     }
 
     // Finally, with our true width determined, compute our margins for real.
-    setMarginStart(0);
-    setMarginEnd(0);
-    if (!hasPerpendicularContainingBlock) {
     ComputedMarginValues marginValues;
-        bool hasInvertedDirection =  cb->style()->isLeftToRightDirection() == style()->isLeftToRightDirection();
-        computeInlineDirectionMargins(cb, availableLogicalWidth, logicalWidth(),
-            hasInvertedDirection ? marginValues.m_start : marginValues.m_end,
-            hasInvertedDirection ? marginValues.m_end : marginValues.m_start);
+    computeMarginsForDirection(InlineDirection, cb, availableLogicalWidth, logicalWidth(), marginValues.m_start, marginValues.m_end, style()->marginStart(), style()->marginEnd());
     setMarginStart(marginValues.m_start);
     setMarginEnd(marginValues.m_end);
-    } else {
-        setMarginStart(minimumValueForLength(style()->marginStart(), availableLogicalWidth));
-        setMarginEnd(minimumValueForLength(style()->marginEnd(), availableLogicalWidth));
-    }
 
     // We should NEVER shrink the table below the min-content logical width, or else the table can't accomodate
     // its own content which doesn't match CSS nor what authors expect.