Changeset 249497 in webkit


Ignore:
Timestamp:
Sep 4, 2019 1:42:08 PM (5 years ago)
Author:
Alan Bujtas
Message:

[LFC] Assert on FormattingContext escaping
​https://bugs.webkit.org/show_bug.cgi?id=201464
<rdar://problem/55029574>

Reviewed by Antti Koivisto.

This patch asserts on accidental formatting context escaping. This is only a correctness issue at the moment,
since we don't support multithreaded subtree layout yet.
Normally we should not need to access display boxes in different formatting contexts during layout, but there are a few, justified cases when it is required.

  • layout/FormattingContext.cpp:

(WebCore::Layout::FormattingContext::displayBoxForLayoutBox const):

  • layout/FormattingContext.h:

(WebCore::Layout::FormattingContext::displayBoxForLayoutBox const): Deleted.

  • layout/FormattingContextGeometry.cpp:

(WebCore::Layout::FormattingContext::Geometry::contentHeightForFormattingContextRoot const):
(WebCore::Layout::FormattingContext::Geometry::staticVerticalPositionForOutOfFlowPositioned const):
(WebCore::Layout::FormattingContext::Geometry::staticHorizontalPositionForOutOfFlowPositioned const):

  • layout/FormattingContextQuirks.cpp:

(WebCore::Layout::FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFixedHeight):

  • layout/floats/FloatingContext.cpp:

(WebCore::Layout::FloatingContext::absoluteDisplayBoxCoordinates const):
(WebCore::Layout::FloatingContext::mapToFloatingStateRoot const):
(WebCore::Layout::FloatingContext::mapTopToFloatingStateRoot const):
(WebCore::Layout::FloatingContext::mapPointFromFormattingContextRootToFloatingStateRoot const):

  • page/FrameViewLayoutContext.cpp:

(WebCore::layoutUsingFormattingContext):

Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r249494 r249497  
     12019-09-04  Zalan Bujtas  <zalan@apple.com>
     2
     3        [LFC] Assert on FormattingContext escaping
     4        https://bugs.webkit.org/show_bug.cgi?id=201464
     5        <rdar://problem/55029574>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        This patch asserts on accidental formatting context escaping. This is only a correctness issue at the moment,
     10        since we don't support multithreaded subtree layout yet.
     11        Normally we should not need to access display boxes in different formatting contexts during layout, but there are a few, justified cases when it is required.
     12
     13        * layout/FormattingContext.cpp:
     14        (WebCore::Layout::FormattingContext::displayBoxForLayoutBox const):
     15        * layout/FormattingContext.h:
     16        (WebCore::Layout::FormattingContext::displayBoxForLayoutBox const): Deleted.
     17        * layout/FormattingContextGeometry.cpp:
     18        (WebCore::Layout::FormattingContext::Geometry::contentHeightForFormattingContextRoot const):
     19        (WebCore::Layout::FormattingContext::Geometry::staticVerticalPositionForOutOfFlowPositioned const):
     20        (WebCore::Layout::FormattingContext::Geometry::staticHorizontalPositionForOutOfFlowPositioned const):
     21        * layout/FormattingContextQuirks.cpp:
     22        (WebCore::Layout::FormattingContext::Quirks::heightValueOfNearestContainingBlockWithFixedHeight):
     23        * layout/floats/FloatingContext.cpp:
     24        (WebCore::Layout::FloatingContext::absoluteDisplayBoxCoordinates const):
     25        (WebCore::Layout::FloatingContext::mapToFloatingStateRoot const):
     26        (WebCore::Layout::FloatingContext::mapTopToFloatingStateRoot const):
     27        (WebCore::Layout::FloatingContext::mapPointFromFormattingContextRootToFloatingStateRoot const):
     28        * page/FrameViewLayoutContext.cpp:
     29        (WebCore::layoutUsingFormattingContext):
     30
    1312019-09-04  Zalan Bujtas  <zalan@apple.com>
    232

  • trunk/Source/WebCore/layout/FormattingContext.cpp

    r249494 r249497  
    184184}
    185185
     186Display::Box& FormattingContext::displayBoxForLayoutBox(const Box& layoutBox, Optional<EscapeType> escapeType) const
     187{
     188    UNUSED_PARAM(escapeType);
     189
     190#ifndef NDEBUG
     191    auto isOkToAccessDisplayBox = [&] {
     192        // 1. Highly common case of accessing the formatting root's display box itself. This is formatting context escaping in the strict sense, since
     193        // the formatting context root box lives in the parent formatting context.
     194        // This happens e.g. when a block level box box needs to stretch horizontally and checks its containing block for horizontal space (this should probably be limited to reading horizontal constraint values).
     195        if (&layoutBox == &root())
     196            return true;
     197
     198        // 2. Special case when accessing the ICB's display box
     199        if (layoutBox.isInitialContainingBlock()) {
     200            // There has to be a valid reason to access the ICB.
     201            if (!escapeType)
     202                return false;
     203            return *escapeType == EscapeType::AccessParentFormattingContext || *escapeType == EscapeType::AccessAncestorFormattingContext;
     204        }
     205
     206        // 3. Most common case of accessing box/containing block display box within the same formatting context tree.
     207        if (&layoutBox.formattingContextRoot() == &root())
     208            return true;
     209
     210        if (!escapeType)
     211            return false;
     212
     213        // 4. Accessing child formatting context subtree is relatively rare. It happens when e.g a shrink to fit (out-of-flow block level) box checks the content width.
     214        // Checking the content width means to get display boxes from the established formatting context (we try to access display boxes in a child formatting context)
     215        if (*escapeType == EscapeType::AccessChildFormattingContext && &layoutBox.formattingContextRoot().formattingContextRoot() == &root())
     216            return true;
     217
     218        // 5. Float box top/left values are mapped relative to the FloatState's root. Inline formatting contexts(A) inherit floats from parent
     219        // block formatting contexts(B). Floats in these inline formatting contexts(A) need to be mapped to the parent, block formatting context(B).
     220        if (*escapeType == EscapeType::AccessParentFormattingContext && &layoutBox.formattingContextRoot() == &root().formattingContextRoot())
     221            return true;
     222
     223        // 6. Finding the first containing block with fixed height quirk. See Quirks::heightValueOfNearestContainingBlockWithFixedHeight
     224        if (*escapeType == EscapeType::AccessAncestorFormattingContext) {
     225            auto& targetFormattingRoot = layoutBox.formattingContextRoot();
     226            auto* ancestorFormattingContextRoot = &root().formattingContextRoot();
     227            while (true) {
     228                if (&targetFormattingRoot == ancestorFormattingContextRoot)
     229                    return true;
     230                if (ancestorFormattingContextRoot->isInitialContainingBlock())
     231                    return false;
     232                ancestorFormattingContextRoot = &ancestorFormattingContextRoot->formattingContextRoot();
     233            }
     234
     235        }
     236        return false;
     237    };
     238#endif
     239    ASSERT(isOkToAccessDisplayBox());
     240    return layoutState().displayBoxForLayoutBox(layoutBox);
     241}
     242
    186243#ifndef NDEBUG
    187244void FormattingContext::validateGeometryConstraintsAfterLayout() const

  • trunk/Source/WebCore/layout/FormattingContext.h

    r249494 r249497  
    7070    bool isTableFormattingContext() const { return root().establishesTableFormattingContext(); }
    7171
    72     Display::Box& displayBoxForLayoutBox(const Box& layoutBox) const { return layoutState().displayBoxForLayoutBox(layoutBox); }
     72    enum class EscapeType {
     73        AccessChildFormattingContext,
     74        AccessParentFormattingContext,
     75        AccessAncestorFormattingContext
     76    };
     77    Display::Box& displayBoxForLayoutBox(const Box&, Optional<EscapeType> = WTF::nullopt) const;
    7378    bool hasDisplayBox(const Box& layoutBox) const { return layoutState().hasDisplayBox(layoutBox); }
    7479

  • trunk/Source/WebCore/layout/FormattingContextGeometry.cpp

    r249357 r249497  
    121121    } else if (formattingRootContainer.establishesBlockFormattingContext() || layoutBox.isDocumentBox()) {
    122122        if (formattingRootContainer.hasInFlowChild()) {
    123             auto& firstDisplayBox = formattingContext.displayBoxForLayoutBox(*formattingRootContainer.firstInFlowChild());
    124             auto& lastDisplayBox = formattingContext.displayBoxForLayoutBox(*formattingRootContainer.lastInFlowChild());
     123            auto& firstDisplayBox = formattingContext.displayBoxForLayoutBox(*formattingRootContainer.firstInFlowChild(), EscapeType::AccessChildFormattingContext);
     124            auto& lastDisplayBox = formattingContext.displayBoxForLayoutBox(*formattingRootContainer.lastInFlowChild(), EscapeType::AccessChildFormattingContext);
    125125            top = firstDisplayBox.rectWithMargin().top();
    126126            bottom = lastDisplayBox.rectWithMargin().bottom();
    … …  
    206206    if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
    207207        // Add sibling offset
    208         auto& previousInFlowDisplayBox = formattingContext.displayBoxForLayoutBox(*previousInFlowSibling);
     208        auto& previousInFlowDisplayBox = formattingContext.displayBoxForLayoutBox(*previousInFlowSibling, FormattingContext::EscapeType::AccessChildFormattingContext);
    209209        top += previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.nonCollapsedMarginAfter();
    210210    } else {
    211211        ASSERT(layoutBox.parent());
    212         top = formattingContext.displayBoxForLayoutBox(*layoutBox.parent()).contentBoxTop();
     212        top = formattingContext.displayBoxForLayoutBox(*layoutBox.parent(), FormattingContext::EscapeType::AccessChildFormattingContext).contentBoxTop();
    213213    }
    214214
    … …  
    217217    // Start with the parent since we pretend that this box is normal flow.
    218218    for (auto* container = layoutBox.parent(); container != &containingBlock; container = container->containingBlock()) {
    219         auto& displayBox = formattingContext.displayBoxForLayoutBox(*container);
     219        auto& displayBox = formattingContext.displayBoxForLayoutBox(*container, FormattingContext::EscapeType::AccessChildFormattingContext);
    220220        // Display::Box::top is the border box top position in its containing block's coordinate system.
    221221        top += displayBox.top();
    … …  
    235235    auto& formattingContext = this->formattingContext();
    236236    ASSERT(layoutBox.parent());
    237     auto left = formattingContext.displayBoxForLayoutBox(*layoutBox.parent()).contentBoxLeft();
     237    auto left = formattingContext.displayBoxForLayoutBox(*layoutBox.parent(), FormattingContext::EscapeType::AccessChildFormattingContext).contentBoxLeft();
    238238
    239239    // Resolve left all the way up to the containing block.
    … …  
    241241    // Start with the parent since we pretend that this box is normal flow.
    242242    for (auto* container = layoutBox.parent(); container != &containingBlock; container = container->containingBlock()) {
    243         auto& displayBox = formattingContext.displayBoxForLayoutBox(*container);
     243        auto& displayBox = formattingContext.displayBoxForLayoutBox(*container, FormattingContext::EscapeType::AccessChildFormattingContext);
    244244        // Display::Box::left is the border box left position in its containing block's coordinate system.
    245245        left += displayBox.left();
    … …  
    247247    }
    248248    // Move the static position relative to the padding box. This is very specific to abolutely positioned boxes.
    249     auto paddingBoxLeft = formattingContext.displayBoxForLayoutBox(containingBlock).paddingBoxLeft();
     249    auto paddingBoxLeft = formattingContext.displayBoxForLayoutBox(containingBlock, FormattingContext::EscapeType::AccessChildFormattingContext).paddingBoxLeft();
    250250    return left - paddingBoxLeft;
    251251}

  • trunk/Source/WebCore/layout/FormattingContextQuirks.cpp

    r249357 r249497  
    5151        // -and it's totally insane because now we freely travel across formatting context boundaries and computed margins are nonexistent.
    5252        if (containingBlock->isBodyBox() || containingBlock->isDocumentBox()) {
    53             auto& displayBox = formattingContext.displayBoxForLayoutBox(*containingBlock);
     53            auto& displayBox = formattingContext.displayBoxForLayoutBox(*containingBlock, FormattingContext::EscapeType::AccessAncestorFormattingContext);
    5454
    55             auto usedValues = UsedHorizontalValues { formattingContext.displayBoxForLayoutBox(*containingBlock->containingBlock()).contentBoxWidth() };
     55            auto usedValues = UsedHorizontalValues { formattingContext.displayBoxForLayoutBox(*containingBlock->containingBlock(), FormattingContext::EscapeType::AccessAncestorFormattingContext).contentBoxWidth() };
    5656            auto verticalMargin = formattingContext.geometry().computedVerticalMargin(*containingBlock, usedValues);
    5757            auto verticalPadding = displayBox.paddingTop().valueOr(0) + displayBox.paddingBottom().valueOr(0);
    … …  
    6363    }
    6464    // Initial containing block has to have a height.
    65     return formattingContext.displayBoxForLayoutBox(layoutBox.initialContainingBlock()).contentBox().height() - bodyAndDocumentVerticalMarginPaddingAndBorder;
     65    return formattingContext.displayBoxForLayoutBox(layoutBox.initialContainingBlock(), FormattingContext::EscapeType::AccessAncestorFormattingContext).contentBox().height() - bodyAndDocumentVerticalMarginPaddingAndBorder;
    6666}
    6767

  • trunk/Source/WebCore/layout/floats/FloatingContext.cpp

    r249494 r249497  
    426426
    427427    if (&containingBlock == &floatingState().root()) {
    428         auto containingBlockDisplayBox = formattingContext().displayBoxForLayoutBox(containingBlock);
     428        auto containingBlockDisplayBox = formattingContext().displayBoxForLayoutBox(containingBlock, FormattingContext::EscapeType::AccessParentFormattingContext);
    429429        return { displayBox, { }, {  containingBlockDisplayBox.contentBoxLeft(), containingBlockDisplayBox.contentBoxRight() } };
    430430    }
    … …  
    437437{
    438438    auto& floatingStateRoot = floatingState().root();
    439     auto& displayBox = formattingContext().displayBoxForLayoutBox(floatBox);
     439    auto& displayBox = formattingContext().displayBoxForLayoutBox(floatBox, FormattingContext::EscapeType::AccessParentFormattingContext);
    440440    auto topLeft = displayBox.topLeft();
    441441    for (auto* containingBlock = floatBox.containingBlock(); containingBlock && containingBlock != &floatingStateRoot; containingBlock = containingBlock->containingBlock())
    442         topLeft.moveBy(formattingContext().displayBoxForLayoutBox(*containingBlock).topLeft());
     442        topLeft.moveBy(formattingContext().displayBoxForLayoutBox(*containingBlock, FormattingContext::EscapeType::AccessParentFormattingContext).topLeft());
    443443
    444444    auto mappedDisplayBox = Display::Box(displayBox);
    … …  
    450450{
    451451    auto& floatingStateRoot = floatingState().root();
    452     auto top = formattingContext().displayBoxForLayoutBox(floatBox).top();
     452    auto top = formattingContext().displayBoxForLayoutBox(floatBox, FormattingContext::EscapeType::AccessParentFormattingContext).top();
    453453    for (auto* container = floatBox.containingBlock(); container && container != &floatingStateRoot; container = container->containingBlock())
    454         top += formattingContext().displayBoxForLayoutBox(*container).top();
     454        top += formattingContext().displayBoxForLayoutBox(*container, FormattingContext::EscapeType::AccessParentFormattingContext).top();
    455455    return top;
    456456}
    … …  
    464464    auto mappedPosition = position;
    465465    for (auto* container = &from; container && container != &to; container = container->containingBlock())
    466         mappedPosition.moveBy(formattingContext().displayBoxForLayoutBox(*container).topLeft());
     466        mappedPosition.moveBy(formattingContext().displayBoxForLayoutBox(*container, FormattingContext::EscapeType::AccessParentFormattingContext).topLeft());
    467467    return mappedPosition;
    468468}
Note: See TracChangeset for help on using the changeset viewer.