Self-collapsing blocks shouldn't get layout bit sit when handling margins

Source/core/layout/LayoutBlockFlow.cpp

 /*
  * Copyright (C) 2013 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 77 matching lines @@
     // This flag is set when we know we're examining bottom margins and we know we're at the bottom of the block.
     bool m_atAfterSideOfBlock : 1;
 
     // These variables are used to detect quirky margins that we need to collapse away (in table cells
     // and in the body element).
     bool m_hasMarginBeforeQuirk : 1;
     bool m_hasMarginAfterQuirk : 1;
     bool m_determinedMarginBeforeQuirk : 1;
 
     bool m_discardMargin : 1;
+    bool m_lastChildIsSelfCollapsingBlockWithClearance : 1;
 
     // These flags track the previous maximal positive and negative margins.
     LayoutUnit m_positiveMargin;
     LayoutUnit m_negativeMargin;
 
 public:
     MarginInfo(LayoutBlockFlow*, LayoutUnit beforeBorderPadding, LayoutUnit afterBorderPadding);
 
     void setAtBeforeSideOfBlock(bool b) { m_atBeforeSideOfBlock = b; }
     void setAtAfterSideOfBlock(bool b) { m_atAfterSideOfBlock = b; }
@@ skipping 32 matching lines @@
     bool canCollapseMarginAfterWithChildren() const { return m_canCollapseMarginAfterWithChildren; }
     bool canCollapseMarginAfterWithLastChild() const { return m_canCollapseMarginAfterWithLastChild; }
     bool quirkContainer() const { return m_quirkContainer; }
     bool determinedMarginBeforeQuirk() const { return m_determinedMarginBeforeQuirk; }
     bool hasMarginBeforeQuirk() const { return m_hasMarginBeforeQuirk; }
     bool hasMarginAfterQuirk() const { return m_hasMarginAfterQuirk; }
     LayoutUnit positiveMargin() const { return m_positiveMargin; }
     LayoutUnit negativeMargin() const { return m_negativeMargin; }
     bool discardMargin() const { return m_discardMargin; }
     LayoutUnit margin() const { return m_positiveMargin - m_negativeMargin; }
+    void setLastChildIsSelfCollapsingBlockWithClearance(bool value) { m_lastChildIsSelfCollapsingBlockWithClearance = value; }
+    bool lastChildIsSelfCollapsingBlockWithClearance() const { return m_lastChildIsSelfCollapsingBlockWithClearance; }
 };
 static bool inNormalFlow(LayoutBox* child)
 {
     LayoutBlock* curr = child->containingBlock();
     LayoutView* layoutView = child->view();
     while (curr && curr != layoutView) {
         if (curr->isLayoutFlowThread())
             return true;
         if (curr->isFloatingOrOutOfFlowPositioned())
             return false;
@@ skipping 870 matching lines @@
 
 // Our MarginInfo state used when laying out block children.
 MarginInfo::MarginInfo(LayoutBlockFlow* blockFlow, LayoutUnit beforeBorderPadding, LayoutUnit afterBorderPadding)
     : m_canCollapseMarginAfterWithLastChild(true)
     , m_atBeforeSideOfBlock(true)
     , m_atAfterSideOfBlock(false)
     , m_hasMarginBeforeQuirk(false)
     , m_hasMarginAfterQuirk(false)
     , m_determinedMarginBeforeQuirk(false)
     , m_discardMargin(false)
+    , m_lastChildIsSelfCollapsingBlockWithClearance(false)
 {
     const ComputedStyle& blockStyle = blockFlow->styleRef();
     ASSERT(blockFlow->isLayoutView() || blockFlow->parent());
     m_canCollapseWithChildren = !blockFlow->createsNewFormattingContext() && !blockFlow->isLayoutFlowThread() && !blockFlow->isLayoutView();
 
     m_canCollapseMarginBeforeWithChildren = m_canCollapseWithChildren && !beforeBorderPadding && blockStyle.marginBeforeCollapse() != MSEPARATE;
 
     // If any height other than auto is specified in CSS, then we don't collapse our bottom
     // margins with our children's margins. To do otherwise would be to risk odd visual
     // effects when the children overflow out of the parent block and yet still collapse
@@ skipping 137 matching lines @@
 
     LayoutUnit beforeCollapseLogicalTop = logicalHeight();
     LayoutUnit logicalTop = beforeCollapseLogicalTop;
 
     LayoutUnit clearanceForSelfCollapsingBlock;
     LayoutObject* prev = child.previousSibling();
     LayoutBlockFlow* previousBlockFlow =  prev && prev->isLayoutBlockFlow() && !prev->isFloatingOrOutOfFlowPositioned() ? toLayoutBlockFlow(prev) : 0;
     // If the child's previous sibling is a self-collapsing block that cleared a float then its top border edge has been set at the bottom border edge
     // of the float. Since we want to collapse the child's top margin with the self-collapsing block's top and bottom margins we need to adjust our parent's height to match the
     // margin top of the self-collapsing block. If the resulting collapsed margin leaves the child still intruding into the float then we will want to clear it.
-    if (!marginInfo.canCollapseWithMarginBefore() && previousBlockFlow && previousBlockFlow->isSelfCollapsingBlock()) {
-        clearanceForSelfCollapsingBlock = previousBlockFlow->marginOffsetForSelfCollapsingBlock();
+    if (!marginInfo.canCollapseWithMarginBefore() && previousBlockFlow && marginInfo.lastChildIsSelfCollapsingBlockWithClearance()) {
+        clearanceForSelfCollapsingBlock = marginValuesForChild(*previousBlockFlow).positiveMarginBefore();
         setLogicalHeight(logicalHeight() - clearanceForSelfCollapsingBlock);
     }
 
     if (childIsSelfCollapsing) {
         // For a self collapsing block both the before and after margins get discarded. The block doesn't contribute anything to the height of the block.
         // Also, the child's top position equals the logical height of the container.
         if (!childDiscardMarginBefore && !marginInfo.discardMargin()) {
             // This child has no height. We need to compute our
             // position before we collapse the child's margins together,
             // so that we can get an accurate position for the zero-height block.
@@ skipping 94 matching lines @@
     if (childLayer->staticBlockPosition() != logicalTop) {
         childLayer->setStaticBlockPosition(logicalTop);
         if (hasStaticBlockPosition)
             child.setChildNeedsLayout(MarkOnlyThis);
     }
 }
 
 LayoutUnit LayoutBlockFlow::clearFloatsIfNeeded(LayoutBox& child, MarginInfo& marginInfo, LayoutUnit oldTopPosMargin, LayoutUnit oldTopNegMargin, LayoutUnit yPos, bool childIsSelfCollapsing)
 {
     LayoutUnit heightIncrease = getClearDelta(&child, yPos);
+    marginInfo.setLastChildIsSelfCollapsingBlockWithClearance(false);
+
     if (!heightIncrease)
         return yPos;
 
     if (childIsSelfCollapsing) {
+        marginInfo.setLastChildIsSelfCollapsingBlockWithClearance(true);
         bool childDiscardMargin = mustDiscardMarginBeforeForChild(child) || mustDiscardMarginAfterForChild(child);
         marginInfo.setDiscardMargin(childDiscardMargin);
 
         // For self-collapsing blocks that clear, they can still collapse their
         // margins with following siblings. Reset the current margins to represent
         // the self-collapsing block's margins only.
         // If DISCARD is specified for -webkit-margin-collapse, reset the margin values.
         LayoutBlockFlow::MarginValues childMargins = marginValuesForChild(child);
         if (!childDiscardMargin) {
             marginInfo.setPositiveMargin(std::max(childMargins.positiveMarginBefore(), childMargins.positiveMarginAfter()));
@@ skipping 165 matching lines @@
         // For replaced elements and scrolled elements, we want to shift them to the next page if they don't fit on the current one.
         logicalTopEstimate = adjustForUnsplittableChild(child, logicalTopEstimate);
 
         if (!child.selfNeedsLayout() && child.isLayoutBlockFlow())
             logicalTopEstimate += toLayoutBlockFlow(&child)->paginationStrut();
     }
 
     return logicalTopEstimate;
 }
 
-LayoutUnit LayoutBlockFlow::marginOffsetForSelfCollapsingBlock()
-{
-    ASSERT(isSelfCollapsingBlock());
-    LayoutBlockFlow* parentBlock = toLayoutBlockFlow(parent());
-    if (parentBlock && style()->clear() && parentBlock->getClearDelta(this, logicalHeight()))
-        return marginValuesForChild(*this).positiveMarginBefore();
-    return LayoutUnit();
-}
-
 void LayoutBlockFlow::adjustFloatingBlock(const MarginInfo& marginInfo)
 {
     // The float should be positioned taking into account the bottom margin
     // of the previous flow. We add that margin into the height, get the
     // float positioned properly, and then subtract the margin out of the
     // height again. In the case of self-collapsing blocks, we always just
     // use the top margins, since the self-collapsing block collapsed its
     // own bottom margin into its top margin.
     //
     // Note also that the previous flow may collapse its margin into the top of
     // our block. If this is the case, then we do not add the margin in to our
     // height when computing the position of the float. This condition can be tested
     // for by simply calling canCollapseWithMarginBefore. See
     // http://www.hixie.ch/tests/adhoc/css/box/block/margin-collapse/046.html for
     // an example of this scenario.
     LayoutUnit marginOffset = marginInfo.canCollapseWithMarginBefore() ? LayoutUnit() : marginInfo.margin();
     setLogicalHeight(logicalHeight() + marginOffset);
     positionNewFloats();
     setLogicalHeight(logicalHeight() - marginOffset);
 }
 
 void LayoutBlockFlow::handleAfterSideOfBlock(LayoutBox* lastChild, LayoutUnit beforeSide, LayoutUnit afterSide, MarginInfo& marginInfo)
 {
     marginInfo.setAtAfterSideOfBlock(true);
 
     // If our last child was a self-collapsing block with clearance then our logical height is flush with the
     // bottom edge of the float that the child clears. The correct vertical position for the margin-collapsing we want
     // to perform now is at the child's margin-top - so adjust our height to that position.
-    if (lastChild && lastChild->isLayoutBlockFlow() && lastChild->isSelfCollapsingBlock())
-        setLogicalHeight(logicalHeight() - toLayoutBlockFlow(lastChild)->marginOffsetForSelfCollapsingBlock());
+    if (marginInfo.lastChildIsSelfCollapsingBlockWithClearance()) {
+        ASSERT(lastChild);
+        setLogicalHeight(logicalHeight() - marginValuesForChild(*lastChild).positiveMarginBefore());
+    }
 
     if (marginInfo.canCollapseMarginAfterWithChildren() && !marginInfo.canCollapseMarginAfterWithLastChild())
         marginInfo.setCanCollapseMarginAfterWithChildren(false);
 
     // If we can't collapse with children then go ahead and add in the bottom margin.
     if (!marginInfo.discardMargin() && (!marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()
         && (!document().inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.hasMarginAfterQuirk())))
         setLogicalHeight(logicalHeight() + marginInfo.margin());
 
     // Now add in our bottom border/padding.
@@ skipping 276 matching lines @@
         break;
     case CBOTH:
         logicalBottom = lowestFloatLogicalBottom();
         break;
     }
 
     // We also clear floats if we are too big to sit on the same line as a float (and wish to avoid floats by default).
     LayoutUnit result = clearSet ? std::max<LayoutUnit>(0, logicalBottom - logicalTop) : LayoutUnit();
     if (!result && child->avoidsFloats()) {
         LayoutUnit newLogicalTop = logicalTop;
+        LayoutRect borderBox = child->borderBoxRect();

[leviw_travelin_and_unemployed, 2015/06/16 17:52:59]

This was just uselessly inside the loop before? *sigh*.

+        LayoutUnit childLogicalWidthAtOldLogicalTopOffset = isHorizontalWritingMode() ? borderBox.width() : borderBox.height();
         while (true) {
             LayoutUnit availableLogicalWidthAtNewLogicalTopOffset = availableLogicalWidthForLine(newLogicalTop, false, logicalHeightForChild(*child));
             if (availableLogicalWidthAtNewLogicalTopOffset == availableLogicalWidthForContent())
                 return newLogicalTop - logicalTop;
 
-            LayoutRect borderBox = child->borderBoxRect();
-            LayoutUnit childLogicalWidthAtOldLogicalTopOffset = isHorizontalWritingMode() ? borderBox.width() : borderBox.height();
-
             LogicalExtentComputedValues computedValues;
             child->logicalExtentAfterUpdatingLogicalWidth(newLogicalTop, computedValues);
             LayoutUnit childLogicalWidthAtNewLogicalTopOffset = computedValues.m_extent;
 
             if (childLogicalWidthAtNewLogicalTopOffset <= availableLogicalWidthAtNewLogicalTopOffset) {
                 // Even though we may not be moving, if the logical width did shrink because of the presence of new floats, then
                 // we need to force a relayout as though we shifted. This happens because of the dynamic addition of overhanging floats
                 // from previous siblings when negative margins exist on a child (see the addOverhangingFloats call at the end of collapseMargins).
                 if (childLogicalWidthAtOldLogicalTopOffset != childLogicalWidthAtNewLogicalTopOffset)
                     child->setChildNeedsLayout(MarkOnlyThis);
@@ skipping 1225 matching lines @@
     FrameView* frameView = document().view();
     LayoutUnit top = (style()->position() == FixedPosition) ? 0 : frameView->scrollOffset().height();
     int visibleHeight = frameView->visibleContentRect(IncludeScrollbars).height();
     if (size().height() < visibleHeight)
         top += (visibleHeight - size().height()) / 2;
     setY(top);
     dialog->setCentered(top);
 }
 
 } // namespace blink