[LFC] Layout::Box::containingBlock should return a const ContainerBox&
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Mar 2020 14:02:37 +0000 (14:02 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Mar 2020 14:02:37 +0000 (14:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209381
<rdar://problem/60732278>

Reviewed by Antti Koivisto.

Layout tree is immutable during layout, so every box should be able to return a valid containing block (except the ICB).
(This patch also removes the unused isDescendantOf() function and renames isDescendantOfFormattingRoot to isInFormattingContextOf).

* layout/FormattingContext.cpp:
(WebCore::Layout::FormattingContext::layoutOutOfFlowContent):
(WebCore::Layout::FormattingContext::validateGeometryConstraintsAfterLayout const):
* layout/FormattingContextGeometry.cpp:
(WebCore::Layout::isHeightAuto):
(WebCore::Layout::FormattingContext::Geometry::computedHeightValue const):
(WebCore::Layout::FormattingContext::Geometry::staticVerticalPositionForOutOfFlowPositioned const):
(WebCore::Layout::FormattingContext::Geometry::staticHorizontalPositionForOutOfFlowPositioned const):
(WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
(WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry const):
(WebCore::Layout::FormattingContext::Geometry::inFlowPositionedPositionOffset const):
* layout/FormattingContextQuirks.cpp:
(WebCore::Layout::FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFixedHeight):
* layout/Verification.cpp:
(WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
* layout/blockformatting/BlockFormattingContext.cpp:
(WebCore::Layout::BlockFormattingContext::layoutInFlowContent):
(WebCore::Layout::BlockFormattingContext::usedAvailableWidthForFloatAvoider):
(WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForAncestors):
(WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForBoxAndAncestors):
(WebCore::Layout::BlockFormattingContext::verticalPositionWithMargin const):
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin const):
* layout/blockformatting/BlockFormattingContextQuirks.cpp:
(WebCore::Layout::BlockFormattingContext::Quirks::stretchedInFlowHeight):
(WebCore::Layout::initialContainingBlock): Deleted.
* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginsCollapseThrough const):
* layout/blockformatting/PrecomputedBlockMarginCollapse.cpp:
(WebCore::Layout::BlockFormattingContext::MarginCollapse::precomputedPositiveNegativeValues const):
* layout/displaytree/DisplayPainter.cpp:
(WebCore::Display::absoluteDisplayBox):
* layout/floats/FloatingContext.cpp:
(WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
(WebCore::Layout::FloatingContext::absoluteDisplayBoxCoordinates const):
(WebCore::Layout::FloatingContext::mapToFloatingStateRoot const):
(WebCore::Layout::FloatingContext::mapTopToFloatingStateRoot const):
(WebCore::Layout::FloatingContext::mapPointFromFormattingContextRootToFloatingStateRoot const):
* layout/floats/FloatingState.cpp:
(WebCore::Layout::FloatingState::bottom const):
(WebCore::Layout::FloatingState::top const):
* layout/floats/FloatingState.h:
(WebCore::Layout::FloatingState::FloatItem::isInFormattingContextOf const):
(WebCore::Layout::FloatingState::FloatItem::isDescendantOfFormattingRoot const): Deleted.
* layout/layouttree/LayoutBox.cpp:
(WebCore::Layout::Box::containingBlock const):
(WebCore::Layout::Box::formattingContextRoot const):
(WebCore::Layout::Box::isInFormattingContextOf const):
(WebCore::Layout::Box::isDescendantOf const): Deleted.
(WebCore::Layout::Box::isContainingBlockDescendantOf const): Deleted.
* layout/layouttree/LayoutBox.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@258819 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/layout/FormattingContext.cpp
Source/WebCore/layout/FormattingContextGeometry.cpp
Source/WebCore/layout/FormattingContextQuirks.cpp
Source/WebCore/layout/Verification.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp
Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
Source/WebCore/layout/blockformatting/PrecomputedBlockMarginCollapse.cpp
Source/WebCore/layout/displaytree/DisplayPainter.cpp
Source/WebCore/layout/floats/FloatingContext.cpp
Source/WebCore/layout/floats/FloatingState.cpp
Source/WebCore/layout/floats/FloatingState.h
Source/WebCore/layout/layouttree/LayoutBox.cpp
Source/WebCore/layout/layouttree/LayoutBox.h

index 660cac6..b4e18df 100644 (file)
@@ -1,3 +1,66 @@
+2020-03-22  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC] Layout::Box::containingBlock should return a const ContainerBox&
+        https://bugs.webkit.org/show_bug.cgi?id=209381
+        <rdar://problem/60732278>
+
+        Reviewed by Antti Koivisto.
+
+        Layout tree is immutable during layout, so every box should be able to return a valid containing block (except the ICB).
+        (This patch also removes the unused isDescendantOf() function and renames isDescendantOfFormattingRoot to isInFormattingContextOf).
+
+        * layout/FormattingContext.cpp:
+        (WebCore::Layout::FormattingContext::layoutOutOfFlowContent):
+        (WebCore::Layout::FormattingContext::validateGeometryConstraintsAfterLayout const):
+        * layout/FormattingContextGeometry.cpp:
+        (WebCore::Layout::isHeightAuto):
+        (WebCore::Layout::FormattingContext::Geometry::computedHeightValue const):
+        (WebCore::Layout::FormattingContext::Geometry::staticVerticalPositionForOutOfFlowPositioned const):
+        (WebCore::Layout::FormattingContext::Geometry::staticHorizontalPositionForOutOfFlowPositioned const):
+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry const):
+        (WebCore::Layout::FormattingContext::Geometry::inFlowPositionedPositionOffset const):
+        * layout/FormattingContextQuirks.cpp:
+        (WebCore::Layout::FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFixedHeight):
+        * layout/Verification.cpp:
+        (WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
+        * layout/blockformatting/BlockFormattingContext.cpp:
+        (WebCore::Layout::BlockFormattingContext::layoutInFlowContent):
+        (WebCore::Layout::BlockFormattingContext::usedAvailableWidthForFloatAvoider):
+        (WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForAncestors):
+        (WebCore::Layout::BlockFormattingContext::precomputeVerticalPositionForBoxAndAncestors):
+        (WebCore::Layout::BlockFormattingContext::verticalPositionWithMargin const):
+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin const):
+        * layout/blockformatting/BlockFormattingContextQuirks.cpp:
+        (WebCore::Layout::BlockFormattingContext::Quirks::stretchedInFlowHeight):
+        (WebCore::Layout::initialContainingBlock): Deleted.
+        * layout/blockformatting/BlockMarginCollapse.cpp:
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginsCollapseThrough const):
+        * layout/blockformatting/PrecomputedBlockMarginCollapse.cpp:
+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::precomputedPositiveNegativeValues const):
+        * layout/displaytree/DisplayPainter.cpp:
+        (WebCore::Display::absoluteDisplayBox):
+        * layout/floats/FloatingContext.cpp:
+        (WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
+        (WebCore::Layout::FloatingContext::absoluteDisplayBoxCoordinates const):
+        (WebCore::Layout::FloatingContext::mapToFloatingStateRoot const):
+        (WebCore::Layout::FloatingContext::mapTopToFloatingStateRoot const):
+        (WebCore::Layout::FloatingContext::mapPointFromFormattingContextRootToFloatingStateRoot const):
+        * layout/floats/FloatingState.cpp:
+        (WebCore::Layout::FloatingState::bottom const):
+        (WebCore::Layout::FloatingState::top const):
+        * layout/floats/FloatingState.h:
+        (WebCore::Layout::FloatingState::FloatItem::isInFormattingContextOf const):
+        (WebCore::Layout::FloatingState::FloatItem::isDescendantOfFormattingRoot const): Deleted.
+        * layout/layouttree/LayoutBox.cpp:
+        (WebCore::Layout::Box::containingBlock const):
+        (WebCore::Layout::Box::formattingContextRoot const):
+        (WebCore::Layout::Box::isInFormattingContextOf const):
+        (WebCore::Layout::Box::isDescendantOf const): Deleted.
+        (WebCore::Layout::Box::isContainingBlockDescendantOf const): Deleted.
+        * layout/layouttree/LayoutBox.h:
+
 2020-03-21  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         An animated PNG plays the frames one time more than the image loopCount
index 71a69c4..b673e31 100644 (file)
@@ -139,19 +139,17 @@ void FormattingContext::layoutOutOfFlowContent(InvalidationState& invalidationSt
     collectOutOfFlowDescendantsIfNeeded();
 
     auto horizontalConstraintsForLayoutBox = [&] (const auto& outOfFlowBox) {
-        auto* containingBlock = outOfFlowBox.containingBlock();
-        ASSERT(containingBlock);
-        if (containingBlock == &root())
+        auto& containingBlock = outOfFlowBox.containingBlock();
+        if (&containingBlock == &root())
             return rootHorizontalConstraints;
-        return Geometry::horizontalConstraintsForOutOfFlow(geometryForBox(*containingBlock));
+        return Geometry::horizontalConstraintsForOutOfFlow(geometryForBox(containingBlock));
     };
 
     auto verticalConstraintsForLayoutBox = [&] (const auto& outOfFlowBox) {
-        auto* containingBlock = outOfFlowBox.containingBlock();
-        ASSERT(containingBlock);
-        if (containingBlock == &root())
+        auto& containingBlock = outOfFlowBox.containingBlock();
+        if (&containingBlock == &root())
             return rootVerticalConstraints;
-        return Geometry::verticalConstraintsForOutOfFlow(geometryForBox(*containingBlock));
+        return Geometry::verticalConstraintsForOutOfFlow(geometryForBox(containingBlock));
     };
 
     for (auto& outOfFlowBox : formattingState().outOfFlowBoxes()) {
@@ -303,7 +301,7 @@ void FormattingContext::validateGeometryConstraintsAfterLayout() const
     for (auto& layoutBox : descendantsOfType<Box>(formattingContextRoot)) {
         if (&layoutBox.formattingContextRoot() != &formattingContextRoot)
             continue;
-        auto& containingBlockGeometry = geometryForBox(*layoutBox.containingBlock());
+        auto& containingBlockGeometry = geometryForBox(layoutBox.containingBlock());
         auto& boxGeometry = geometryForBox(layoutBox);
 
         // 10.3.3 Block-level, non-replaced elements in normal flow
index 6d36319..2e1e727 100644 (file)
@@ -54,7 +54,7 @@ static inline bool isHeightAuto(const Box& layoutBox)
         if (layoutBox.isOutOfFlowPositioned())
             return false;
 
-        return !layoutBox.containingBlock()->style().logicalHeight().isFixed();
+        return !layoutBox.containingBlock().style().logicalHeight().isFixed();
     }
 
     return false;
@@ -76,7 +76,7 @@ Optional<LayoutUnit> FormattingContext::Geometry::computedHeightValue(const Box&
         if (layoutState().inQuirksMode())
             containingBlockHeight = formattingContext().quirks().heightValueOfNearestContainingBlockWithFixedHeight(layoutBox);
         else {
-            auto containingBlockHeightFromStyle = layoutBox.containingBlock()->style().logicalHeight();
+            auto containingBlockHeightFromStyle = layoutBox.containingBlock().style().logicalHeight();
             if (containingBlockHeightFromStyle.isFixed())
                 containingBlockHeight = LayoutUnit { containingBlockHeightFromStyle.value() };
         }
@@ -238,13 +238,13 @@ LayoutUnit FormattingContext::Geometry::staticVerticalPositionForOutOfFlowPositi
     }
 
     // Resolve top all the way up to the containing block.
-    auto& containingBlock = *layoutBox.containingBlock();
+    auto& containingBlock = layoutBox.containingBlock();
     // Start with the parent since we pretend that this box is normal flow.
-    for (auto* containerBox = layoutBox.parent(); containerBox != &containingBlock; containerBox = containerBox->containingBlock()) {
-        auto& boxGeometry = formattingContext.geometryForBox(*containerBox, EscapeReason::OutOfFlowBoxNeedsInFlowGeometry);
+    for (auto* ancestor = layoutBox.parent(); ancestor != &containingBlock; ancestor = &ancestor->containingBlock()) {
+        auto& boxGeometry = formattingContext.geometryForBox(*ancestor, EscapeReason::OutOfFlowBoxNeedsInFlowGeometry);
         // Display::Box::top is the border box top position in its containing block's coordinate system.
         top += boxGeometry.top();
-        ASSERT(!containerBox->isPositioned() || layoutBox.isFixedPositioned());
+        ASSERT(!ancestor->isPositioned() || layoutBox.isFixedPositioned());
     }
     // Move the static position relative to the padding box. This is very specific to abolutely positioned boxes.
     return top - verticalConstraints.logicalTop;
@@ -261,13 +261,13 @@ LayoutUnit FormattingContext::Geometry::staticHorizontalPositionForOutOfFlowPosi
     auto left = formattingContext.geometryForBox(*layoutBox.parent(), EscapeReason::OutOfFlowBoxNeedsInFlowGeometry).contentBoxLeft();
 
     // Resolve left all the way up to the containing block.
-    auto& containingBlock = *layoutBox.containingBlock();
+    auto& containingBlock = layoutBox.containingBlock();
     // Start with the parent since we pretend that this box is normal flow.
-    for (auto* containerBox = layoutBox.parent(); containerBox != &containingBlock; containerBox = containerBox->containingBlock()) {
-        auto& boxGeometry = formattingContext.geometryForBox(*containerBox, EscapeReason::OutOfFlowBoxNeedsInFlowGeometry);
+    for (auto* ancestor = layoutBox.parent(); ancestor != &containingBlock; ancestor = &ancestor->containingBlock()) {
+        auto& boxGeometry = formattingContext.geometryForBox(*ancestor, EscapeReason::OutOfFlowBoxNeedsInFlowGeometry);
         // Display::Box::left is the border box left position in its containing block's coordinate system.
         left += boxGeometry.left();
-        ASSERT(!containerBox->isPositioned() || layoutBox.isFixedPositioned());
+        ASSERT(!ancestor->isPositioned() || layoutBox.isFixedPositioned());
     }
     // Move the static position relative to the padding box. This is very specific to abolutely positioned boxes.
     return left - horizontalConstraints.logicalLeft;
@@ -451,7 +451,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
     auto& style = layoutBox.style();
     auto& boxGeometry = formattingContext.geometryForBox(layoutBox);
     auto containingBlockWidth = horizontalConstraints.logicalWidth;
-    auto isLeftToRightDirection = layoutBox.containingBlock()->style().isLeftToRightDirection();
+    auto isLeftToRightDirection = layoutBox.containingBlock().style().isLeftToRightDirection();
     
     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
     auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
@@ -664,7 +664,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
     auto& style = replacedBox.style();
     auto& boxGeometry = formattingContext.geometryForBox(replacedBox);
     auto containingBlockWidth = horizontalConstraints.logicalWidth;
-    auto isLeftToRightDirection = replacedBox.containingBlock()->style().isLeftToRightDirection();
+    auto isLeftToRightDirection = replacedBox.containingBlock().style().isLeftToRightDirection();
 
     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
     auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
@@ -1025,7 +1025,7 @@ LayoutSize FormattingContext::Geometry::inFlowPositionedPositionOffset(const Box
         right = -*left;
     } else {
         // #4
-        auto isLeftToRightDirection = layoutBox.containingBlock()->style().isLeftToRightDirection();
+        auto isLeftToRightDirection = layoutBox.containingBlock().style().isLeftToRightDirection();
         if (isLeftToRightDirection)
             right = -*left;
         else
index 499da0f..8c266fe 100644 (file)
@@ -39,7 +39,7 @@ LayoutUnit FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFix
     // In quirks mode, we go and travers the containing block chain to find a block level box with fixed height value, even if it means leaving
     // the current formatting context. FIXME: surely we need to do some tricks here when block direction support is added.
     auto& formattingContext = this->formattingContext();
-    auto* containingBlock = layoutBox.containingBlock();
+    auto* containingBlock = &layoutBox.containingBlock();
     LayoutUnit bodyAndDocumentVerticalMarginPaddingAndBorder;
     while (containingBlock) {
         auto containingBlockHeight = containingBlock->style().logicalHeight();
@@ -51,7 +51,7 @@ LayoutUnit FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFix
         if (containingBlock->isBodyBox() || containingBlock->isDocumentBox()) {
             auto& boxGeometry = formattingContext.geometryForBox(*containingBlock, FormattingContext::EscapeReason::FindFixedHeightAncestorQuirk);
 
-            auto& containingBlockDisplayBox = formattingContext.geometryForBox(*containingBlock->containingBlock(), FormattingContext::EscapeReason::FindFixedHeightAncestorQuirk);
+            auto& containingBlockDisplayBox = formattingContext.geometryForBox(containingBlock->containingBlock(), FormattingContext::EscapeReason::FindFixedHeightAncestorQuirk);
             auto horizontalConstraints = Geometry::horizontalConstraintsForInFlow(containingBlockDisplayBox);
             auto verticalMargin = formattingContext.geometry().computedVerticalMargin(*containingBlock, horizontalConstraints);
             auto verticalPadding = boxGeometry.paddingTop().valueOr(0) + boxGeometry.paddingBottom().valueOr(0);
@@ -59,7 +59,9 @@ LayoutUnit FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFix
             bodyAndDocumentVerticalMarginPaddingAndBorder += verticalMargin.before.valueOr(0) + verticalMargin.after.valueOr(0) + verticalPadding + verticalBorder;
         }
 
-        containingBlock = containingBlock->containingBlock();
+        if (containingBlock->isInitialContainingBlock())
+            break;
+        containingBlock = &containingBlock->containingBlock();
     }
     // Initial containing block has to have a height.
     return formattingContext.geometryForBox(layoutBox.initialContainingBlock(), FormattingContext::EscapeReason::FindFixedHeightAncestorQuirk).contentBox().height() - bodyAndDocumentVerticalMarginPaddingAndBorder;
index 0de106f..3afa59d 100644 (file)
@@ -251,7 +251,7 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
     if (layoutBox.isTableBox()) {
         // When the <table> is out-of-flow positioned, the wrapper table box has the offset
         // while the actual table box is static, inflow.
-        auto& tableWrapperDisplayBox = context.displayBoxForLayoutBox(*layoutBox.containingBlock());
+        auto& tableWrapperDisplayBox = context.displayBoxForLayoutBox(layoutBox.containingBlock());
         displayBox.moveBy(tableWrapperDisplayBox.topLeft());
     }
 
index 481afcd..879518d 100644 (file)
@@ -91,19 +91,17 @@ void BlockFormattingContext::layoutInFlowContent(InvalidationState& invalidation
     };
 
     auto horizontalConstraintsForLayoutBox = [&] (const auto& layoutBox) {
-        auto* containingBlock = layoutBox.containingBlock();
-        ASSERT(containingBlock);
-        if (containingBlock == &formattingRoot)
+        auto& containingBlock = layoutBox.containingBlock();
+        if (&containingBlock == &formattingRoot)
             return rootHorizontalConstraints;
-        return Geometry::horizontalConstraintsForInFlow(geometryForBox(*containingBlock));
+        return Geometry::horizontalConstraintsForInFlow(geometryForBox(containingBlock));
     };
 
     auto verticalConstraintsForLayoutBox = [&] (const auto& layoutBox) {
-        auto* containingBlock = layoutBox.containingBlock();
-        ASSERT(containingBlock);
-        if (containingBlock == &formattingRoot)
+        auto& containingBlock = layoutBox.containingBlock();
+        if (&containingBlock == &formattingRoot)
             return rootVerticalConstraints;
-        return Geometry::verticalConstraintsForInFlow(geometryForBox(*containingBlock));
+        return Geometry::verticalConstraintsForInFlow(geometryForBox(containingBlock));
     };
 
     // This is a post-order tree traversal layout.
@@ -211,10 +209,10 @@ Optional<LayoutUnit> BlockFormattingContext::usedAvailableWidthForFloatAvoider(c
 
     auto mapLogicalTopToFormattingContextRoot = [&] {
         auto& formattingContextRoot = root();
-        ASSERT(layoutBox.isContainingBlockDescendantOf(formattingContextRoot));
+        ASSERT(layoutBox.isInFormattingContextOf(formattingContextRoot));
         auto top = geometryForBox(layoutBox).top();
-        for (auto* containerBox = layoutBox.containingBlock(); containerBox && containerBox != &formattingContextRoot; containerBox = containerBox->containingBlock())
-            top += geometryForBox(*containerBox).top();
+        for (auto* ancestor = &layoutBox.containingBlock(); ancestor != &formattingContextRoot; ancestor = &ancestor->containingBlock())
+            top += geometryForBox(*ancestor).top();
         return top;
     };
 
@@ -259,7 +257,7 @@ void BlockFormattingContext::computeStaticHorizontalPosition(const Box& layoutBo
 void BlockFormattingContext::precomputeVerticalPositionForAncestors(const Box& layoutBox, const ConstraintsPair<HorizontalConstraints>& horizontalConstraints, const ConstraintsPair<VerticalConstraints>& verticalConstraints)
 {
     ASSERT(layoutBox.isFloatAvoider());
-    precomputeVerticalPositionForBoxAndAncestors(*layoutBox.containingBlock(), horizontalConstraints, verticalConstraints);
+    precomputeVerticalPositionForBoxAndAncestors(layoutBox.containingBlock(), horizontalConstraints, verticalConstraints);
 }
 
 void BlockFormattingContext::precomputeVerticalPositionForBoxAndAncestors(const Box& layoutBox, const ConstraintsPair<HorizontalConstraints>& horizontalConstraints, const ConstraintsPair<VerticalConstraints>& verticalConstraints)
@@ -274,14 +272,14 @@ void BlockFormattingContext::precomputeVerticalPositionForBoxAndAncestors(const
     //
     // The idea here is that as long as we don't cross the block formatting context boundary, we should be able to pre-compute the final top position.
     // FIXME: we currently don't account for the "clear" property when computing the final position for an ancestor.
-    for (auto* ancestor = &layoutBox; ancestor && ancestor != &root(); ancestor = ancestor->containingBlock()) {
+    for (auto* ancestor = &layoutBox; ancestor && ancestor != &root(); ancestor = &ancestor->containingBlock()) {
         auto horizontalConstraintsForAncestor = [&] {
-            auto* containingBlock = ancestor->containingBlock();
-            return containingBlock == &root() ? horizontalConstraints.root : Geometry::horizontalConstraintsForInFlow(geometryForBox(*containingBlock));
+            auto& containingBlock = ancestor->containingBlock();
+            return &containingBlock == &root() ? horizontalConstraints.root : Geometry::horizontalConstraintsForInFlow(geometryForBox(containingBlock));
         };
         auto verticalConstraintsForAncestor = [&] {
-            auto* containingBlock = ancestor->containingBlock();
-            return containingBlock == &root() ? verticalConstraints.root : Geometry::verticalConstraintsForInFlow(geometryForBox(*containingBlock));
+            auto& containingBlock = ancestor->containingBlock();
+            return &containingBlock == &root() ? verticalConstraints.root : Geometry::verticalConstraintsForInFlow(geometryForBox(containingBlock));
         };
 
         auto computedVerticalMargin = geometry().computedVerticalMargin(*ancestor, horizontalConstraintsForAncestor());
@@ -553,7 +551,7 @@ LayoutUnit BlockFormattingContext::verticalPositionWithMargin(const Box& layoutB
         return containingBlockContentBoxTop + verticalMargin.before();
     }
     // At this point this box indirectly (via collapsed through previous in-flow siblings) adjoins the parent. Let's check if it margin collapses with the parent.
-    auto& containingBlock = *layoutBox.containingBlock();
+    auto& containingBlock = layoutBox.containingBlock();
     ASSERT(containingBlock.firstInFlowChild());
     ASSERT(containingBlock.firstInFlowChild() != &layoutBox);
     if (marginCollapse().marginBeforeCollapsesWithParentMarginBefore(*containingBlock.firstInFlowChild()))
index 525e3a9..03d40cc 100644 (file)
@@ -158,7 +158,7 @@ ContentWidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAn
 
         // #2
         if (width && computedHorizontalMargin.start && computedHorizontalMargin.end) {
-            if (layoutBox.containingBlock()->style().isLeftToRightDirection()) {
+            if (layoutBox.containingBlock().style().isLeftToRightDirection()) {
                 usedHorizontalMargin.start = *computedHorizontalMargin.start;
                 usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight);
             } else {
index a7180bb..16d5aae 100644 (file)
 namespace WebCore {
 namespace Layout {
 
-static const ContainerBox& initialContainingBlock(const Box& layoutBox)
-{
-    auto* containingBlock = layoutBox.containingBlock();
-    while (containingBlock->containingBlock())
-        containingBlock = containingBlock->containingBlock();
-    return *containingBlock;
-}
-
 static bool isQuirkContainer(const Box& layoutBox)
 {
     return layoutBox.isBodyBox() || layoutBox.isDocumentBox() || layoutBox.isTableCell();
@@ -71,7 +63,7 @@ LayoutUnit BlockFormattingContext::Quirks::stretchedInFlowHeight(const Box& layo
 
     if (layoutBox.isDocumentBox()) {
         // Let's stretch the inflow document box(<html>) to the height of the initial containing block (view).
-        auto documentBoxContentHeight = formattingContext.geometryForBox(initialContainingBlock(layoutBox), EscapeReason::DocumentBoxStrechesToViewportQuirk).contentBoxHeight();
+        auto documentBoxContentHeight = formattingContext.geometryForBox(layoutBox.initialContainingBlock(), EscapeReason::DocumentBoxStrechesToViewportQuirk).contentBoxHeight();
         // Document box's own vertical margin/border/padding values always shrink the content height.
         auto& documentBoxGeometry = formattingContext.geometryForBox(layoutBox);
         documentBoxContentHeight -= nonCollapsedVerticalMargin + documentBoxGeometry.verticalBorder() + documentBoxGeometry.verticalPadding().valueOr(0);
@@ -80,7 +72,7 @@ LayoutUnit BlockFormattingContext::Quirks::stretchedInFlowHeight(const Box& layo
 
     // Here is the quirky part for body box when it stretches all the way to the ICB even when the document box does not (e.g. out-of-flow positioned).
     ASSERT(layoutBox.isBodyBox());
-    auto& initialContainingBlockGeometry = formattingContext.geometryForBox(initialContainingBlock(layoutBox), EscapeReason::BodyStrechesToViewportQuirk);
+    auto& initialContainingBlockGeometry = formattingContext.geometryForBox(layoutBox.initialContainingBlock(), EscapeReason::BodyStrechesToViewportQuirk);
     // Start the content height with the ICB.
     auto bodyBoxContentHeight = initialContainingBlockGeometry.contentBoxHeight();
     // Body box's own border and padding shrink the content height.
index 1374a87..b49ea7d 100644 (file)
@@ -406,7 +406,7 @@ bool BlockFormattingContext::MarginCollapse::marginsCollapseThrough(const Box& l
                 // Any float box in this formatting context prevents collapsing through.
                 auto& floats = formattingState.floatingState().floats();
                 for (auto& floatItem : floats) {
-                    if (floatItem.isDescendantOfFormattingRoot(containerBox))
+                    if (floatItem.isInFormattingContextOf(containerBox))
                         return false;
                 }
                 return true;
index 809c0d4..1f2ab73 100644 (file)
@@ -44,7 +44,7 @@ PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse
     if (blockFormattingState.hasPositiveAndNegativeVerticalMargin(layoutBox))
         return blockFormattingState.positiveAndNegativeVerticalMargin(layoutBox).before;
 
-    auto horizontalConstraints = Geometry::horizontalConstraintsForInFlow(formattingContext().geometryForBox(*layoutBox.containingBlock()));
+    auto horizontalConstraints = Geometry::horizontalConstraintsForInFlow(formattingContext().geometryForBox(layoutBox.containingBlock()));
     auto computedVerticalMargin = formattingContext().geometry().computedVerticalMargin(layoutBox, horizontalConstraints);
     auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) };
     return precomputedPositiveNegativeMarginBefore(layoutBox, nonCollapsedMargin);
index 661777c..e443021 100644 (file)
@@ -150,8 +150,8 @@ static Box absoluteDisplayBox(const Layout::LayoutState& layoutState, const Layo
         return layoutState.displayBoxForLayoutBox(layoutBox);
 
     auto absoluteBox = Box { layoutState.displayBoxForLayoutBox(layoutBox) };
-    for (auto* containerBox = layoutBox.containingBlock(); containerBox != &layoutBox.initialContainingBlock(); containerBox = containerBox->containingBlock())
-        absoluteBox.moveBy(layoutState.displayBoxForLayoutBox(*containerBox).topLeft());
+    for (auto* containingBlock = &layoutBox.containingBlock(); !containingBlock->isInitialContainingBlock(); containingBlock = &containingBlock->containingBlock())
+        absoluteBox.moveBy(layoutState.displayBoxForLayoutBox(*containingBlock).topLeft());
     return absoluteBox;
 }
 
index edb33e1..2b7edb3 100644 (file)
@@ -251,10 +251,10 @@ FloatingContext::ClearancePosition FloatingContext::verticalPositionWithClearanc
         ASSERT(*floatBottom == rootRelativeTop);
 
         // The return vertical position is in the containing block's coordinate system. Convert it to the formatting root's coordinate system if needed.
-        if (layoutBox.containingBlock() == &m_floatingState.root())
+        if (&layoutBox.containingBlock() == &m_floatingState.root())
             return { Position { rootRelativeTop }, clearance };
 
-        auto containingBlockRootRelativeTop = mapTopToFloatingStateRoot(*layoutBox.containingBlock());
+        auto containingBlockRootRelativeTop = mapTopToFloatingStateRoot(layoutBox.containingBlock());
         return { Position { rootRelativeTop - containingBlockRootRelativeTop }, clearance };
     };
 
@@ -418,7 +418,7 @@ void FloatingContext::findPositionForFormattingContextRoot(FloatAvoider& floatAv
 
 FloatingContext::AbsoluteCoordinateValuesForFloatAvoider FloatingContext::absoluteDisplayBoxCoordinates(const Box& floatAvoider) const
 {
-    auto& containingBlock = *floatAvoider.containingBlock();
+    auto& containingBlock = floatAvoider.containingBlock();
     auto displayBox = mapToFloatingStateRoot(floatAvoider);
 
     if (&containingBlock == &floatingState().root()) {
@@ -435,7 +435,7 @@ Display::Box FloatingContext::mapToFloatingStateRoot(const Box& floatBox) const
     auto& floatingStateRoot = floatingState().root();
     auto& boxGeometry = formattingContext().geometryForBox(floatBox, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates);
     auto topLeft = boxGeometry.topLeft();
-    for (auto* containingBlock = floatBox.containingBlock(); containingBlock && containingBlock != &floatingStateRoot; containingBlock = containingBlock->containingBlock())
+    for (auto* containingBlock = &floatBox.containingBlock(); containingBlock != &floatingStateRoot; containingBlock = &containingBlock->containingBlock())
         topLeft.moveBy(formattingContext().geometryForBox(*containingBlock, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).topLeft());
 
     auto mappedDisplayBox = Display::Box(boxGeometry);
@@ -447,8 +447,8 @@ LayoutUnit FloatingContext::mapTopToFloatingStateRoot(const Box& floatBox) const
 {
     auto& floatingStateRoot = floatingState().root();
     auto top = formattingContext().geometryForBox(floatBox, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).top();
-    for (auto* containerBox = floatBox.containingBlock(); containerBox && containerBox != &floatingStateRoot; containerBox = containerBox->containingBlock())
-        top += formattingContext().geometryForBox(*containerBox, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).top();
+    for (auto* containingBlock = &floatBox.containingBlock(); containingBlock != &floatingStateRoot; containingBlock = &containingBlock->containingBlock())
+        top += formattingContext().geometryForBox(*containingBlock, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).top();
     return top;
 }
 
@@ -459,8 +459,8 @@ Point FloatingContext::mapPointFromFormattingContextRootToFloatingStateRoot(Poin
     if (&from == &to)
         return position;
     auto mappedPosition = position;
-    for (auto* containerBox = &from; containerBox && containerBox != &to; containerBox = containerBox->containingBlock())
-        mappedPosition.moveBy(formattingContext().geometryForBox(*containerBox, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).topLeft());
+    for (auto* containingBlock = &from; containingBlock != &to; containingBlock = &containingBlock->containingBlock())
+        mappedPosition.moveBy(formattingContext().geometryForBox(*containingBlock, FormattingContext::EscapeReason::FloatBoxNeedsToBeInAbsoluteCoordinates).topLeft());
     return mappedPosition;
 }
 
index b4f87ea..94f3b02 100644 (file)
@@ -97,7 +97,7 @@ Optional<PositionInContextRoot> FloatingState::bottom(const ContainerBox& format
     Optional<PositionInContextRoot> bottom;
     for (auto& floatItem : m_floats) {
         // Ignore floats from ancestor formatting contexts when the floating state is inherited.
-        if (!floatItem.isDescendantOfFormattingRoot(formattingContextRoot))
+        if (!floatItem.isInFormattingContextOf(formattingContextRoot))
             continue;
 
         if ((type == Clear::Left && !floatItem.isLeftPositioned())
@@ -122,7 +122,7 @@ Optional<PositionInContextRoot> FloatingState::top(const ContainerBox& formattin
     Optional<PositionInContextRoot> top;
     for (auto& floatItem : m_floats) {
         // Ignore floats from ancestor formatting contexts when the floating state is inherited.
-        if (!floatItem.isDescendantOfFormattingRoot(formattingContextRoot))
+        if (!floatItem.isInFormattingContextOf(formattingContextRoot))
             continue;
 
         auto floatTop = floatItem.rectWithMargin().top();
index acb253f..4f7ab4a 100644 (file)
@@ -66,7 +66,7 @@ public:
         FloatItem(Position, Display::Box absoluteDisplayBox);
 
         bool isLeftPositioned() const { return m_position == Position::Left; }
-        bool isDescendantOfFormattingRoot(const ContainerBox&) const;
+        bool isInFormattingContextOf(const ContainerBox& formattingContextRoot) const { return m_layoutBox->isInFormattingContextOf(formattingContextRoot); }
 
         Display::Rect rectWithMargin() const { return m_absoluteDisplayBox.rectWithMargin(); }
         UsedHorizontalMargin horizontalMargin() const { return m_absoluteDisplayBox.horizontalMargin(); }
@@ -115,12 +115,6 @@ inline Optional<PositionInContextRoot> FloatingState::bottom(const ContainerBox&
     return bottom(formattingContextRoot, Clear::Both);
 }
 
-inline bool FloatingState::FloatItem::isDescendantOfFormattingRoot(const ContainerBox& formattingContextRoot) const
-{
-    ASSERT(formattingContextRoot.establishesFormattingContext());
-    return m_layoutBox->isContainingBlockDescendantOf(downcast<ContainerBox>(formattingContextRoot));
-}
-
 }
 }
 #endif
index d3c2564..118cc53 100644 (file)
@@ -178,43 +178,47 @@ bool Box::isFloatAvoider() const
     return (establishesBlockFormattingContext() && !establishesInlineFormattingContext()) || establishesTableFormattingContext() || hasFloatClear();
 }
 
-const ContainerBox* Box::containingBlock() const
+const ContainerBox& Box::containingBlock() const
 {
     // Finding the containing block by traversing the tree during tree construction could provide incorrect result.
     ASSERT(!Phase::isInTreeBuilding());
+    // If we ever end up here with the ICB, we must be doing something not-so-great.
+    RELEASE_ASSERT(!isInitialContainingBlock());
     // The containing block in which the root element lives is a rectangle called the initial containing block.
     // For other elements, if the element's position is 'relative' or 'static', the containing block is formed by the
     // content edge of the nearest block container ancestor box or which establishes a formatting context.
     // If the element has 'position: fixed', the containing block is established by the viewport
     // If the element has 'position: absolute', the containing block is established by the nearest ancestor with a
     // 'position' of 'absolute', 'relative' or 'fixed'.
-    if (!parent())
-        return nullptr;
-
     if (!isPositioned() || isInFlowPositioned()) {
-        for (auto* nearestBlockContainerOrFormattingContextRoot = parent(); nearestBlockContainerOrFormattingContextRoot; nearestBlockContainerOrFormattingContextRoot = nearestBlockContainerOrFormattingContextRoot->parent()) {
-            if (nearestBlockContainerOrFormattingContextRoot->isBlockContainerBox() || nearestBlockContainerOrFormattingContextRoot->establishesFormattingContext())
-                return nearestBlockContainerOrFormattingContextRoot; 
+        auto* ancestor = parent();
+        for (; !ancestor->isInitialContainingBlock(); ancestor = ancestor->parent()) {
+            if (ancestor->isBlockContainerBox() || ancestor->establishesFormattingContext())
+                return *ancestor;
         }
-        // We should always manage to find the ICB.
-        ASSERT_NOT_REACHED();
-        return nullptr;
+        return *ancestor;
     }
 
     if (isFixedPositioned()) {
         auto* ancestor = parent();
-        for (; ancestor->parent() && !ancestor->style().hasTransform(); ancestor = ancestor->parent()) { }
-        return ancestor;
+        for (; !ancestor->isInitialContainingBlock(); ancestor = ancestor->parent()) {
+            if (ancestor->style().hasTransform())
+                return *ancestor;
+        }
+        return *ancestor;
     }
 
     if (isOutOfFlowPositioned()) {
         auto* ancestor = parent();
-        for (; ancestor->parent() && !ancestor->isPositioned() && !ancestor->style().hasTransform(); ancestor = ancestor->parent()) { }
-        return ancestor;
+        for (; !ancestor->isInitialContainingBlock(); ancestor = ancestor->parent()) {
+            if (ancestor->isPositioned() || ancestor->style().hasTransform())
+                return *ancestor;
+        }
+        return *ancestor;
     }
 
     ASSERT_NOT_REACHED();
-    return nullptr;
+    return initialContainingBlock();
 }
 
 const ContainerBox& Box::formattingContextRoot() const
@@ -230,15 +234,10 @@ const ContainerBox& Box::formattingContextRoot() const
     // <div id=outer style="position: absolute"><div id=inner><span style="position: relative">content</span></div></div>
     // While the relatively positioned inline container (span) is placed relative to its containing block "outer", it lives in the inline
     // formatting context established by "inner".
-    const ContainerBox* ancestor = nullptr;
-    if (isInlineLevelBox() && isInFlowPositioned())
-        ancestor = parent();
-    else
-        ancestor = containingBlock();
-    ASSERT(ancestor);
-    if (ancestor->establishesFormattingContext())
-        return *ancestor;
-    return ancestor->formattingContextRoot();
+    auto& ancestor = isInlineLevelBox() && isInFlowPositioned() ? *parent() : containingBlock();
+    if (ancestor.establishesFormattingContext())
+        return ancestor;
+    return ancestor.formattingContextRoot();
 }
 
 const ContainerBox& Box::initialContainingBlock() const
@@ -252,21 +251,19 @@ const ContainerBox& Box::initialContainingBlock() const
     return *parent;
 }
 
-bool Box::isDescendantOf(const ContainerBox& ancestorCandidate) const
-{
-    for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
-        if (ancestor == &ancestorCandidate)
-            return true;
-    }
-    return false;
-}
-
-bool Box::isContainingBlockDescendantOf(const ContainerBox& ancestorCandidate) const
+bool Box::isInFormattingContextOf(const ContainerBox& formattingContextRoot) const
 { 
-    for (auto* ancestor = containingBlock(); ancestor; ancestor = ancestor->containingBlock()) {
-        if (ancestor == &ancestorCandidate)
+    ASSERT(formattingContextRoot.establishesFormattingContext());
+    ASSERT(!isInitialContainingBlock());
+    auto* ancestor = &containingBlock();
+    while (ancestor) {
+        if (ancestor == &formattingContextRoot)
             return true;
+        if (ancestor->isInitialContainingBlock())
+            return false;
+        ancestor = &ancestor->containingBlock();
     }
+    ASSERT_NOT_REACHED();
     return false;
 }
 
index 79b8092..fe348ea 100644 (file)
@@ -93,12 +93,11 @@ public:
 
     bool isFloatingOrOutOfFlowPositioned() const { return isFloatingPositioned() || isOutOfFlowPositioned(); }
 
-    const ContainerBox* containingBlock() const;
+    const ContainerBox& containingBlock() const;
     const ContainerBox& formattingContextRoot() const;
     const ContainerBox& initialContainingBlock() const;
 
-    bool isDescendantOf(const ContainerBox&) const;
-    bool isContainingBlockDescendantOf(const ContainerBox&) const;
+    bool isInFormattingContextOf(const ContainerBox&) const;
 
     bool isAnonymous() const { return m_isAnonymous; }