Attachment 343784 Details for Bug 187129 – Patch

[patch] Patch

bug-187129-20180627195928.patch (text/plain), 16.55 KB, created by zalan on 2018-06-27 19:59:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-27 19:59:29 PDT

Size: 16.55 KB

patch

obsolete

>Subversion Revision: 233297
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0c42e42b7abb6512f831cc96a94b1fa04498f4aa..8e61ebcab6450e7190d1982a2a33dfd88f1c3bf1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-06-27  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Compute both the collapsed and the non-collapsed margin values.
>+        https://bugs.webkit.org/show_bug.cgi?id=187129
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        For validation purposes only at this point.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
>+        * layout/FormattingContext.h:
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedHeightAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedHeightAndMargin):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
>+        * layout/blockformatting/BlockMarginCollapse.cpp:
>+        (WebCore::Layout::isMarginTopCollapsedWithParent):
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):
>+
> 2018-06-27  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Align inFlowNonReplacedHeightAndMargin() style with the rest of the compute functions.
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 7c58b669219b2123e582c6baeeb00f679d2a747f..08f35d974b97ba5b8bc41358c6a5cf3e07f76add 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -55,7 +55,7 @@ void FormattingContext::computeFloatingHeightAndMargin(LayoutContext& layoutCont
> {
>     auto heightAndMargin = Geometry::floatingHeightAndMargin(layoutContext, layoutBox);
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>-    displayBox.moveVertically(heightAndMargin.margin.top);
>+    displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
>     displayBox.setVerticalMargin(heightAndMargin.margin);
> }
> 
>@@ -79,7 +79,8 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(LayoutContext& layoutCo
> {
>     auto verticalGeometry = Geometry::outOfFlowVerticalGeometry(layoutContext, layoutBox);
>     displayBox.setTop(verticalGeometry.top);
>-    displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
>+    displayBox.setContentBoxHeight(verticalGeometry.bottom - verticalGeometry.top);
>+    ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin);
>     displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin);
> }
> 
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index ea17c7bb1b82e9c09f7a7218f4dc4bf539ff265c..05b828870bb1f6a649657c4cabfa721e05fd2d97 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -96,6 +96,7 @@ protected:
>         struct HeightAndMargin {
>             LayoutUnit height;
>             Display::Box::VerticalEdges margin;
>+            std::optional<Display::Box::VerticalEdges> collapsedMargin;
>         };
> 
>          struct HorizontalGeometry {
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 2466f2dec9f54e265e548e84a4e9f27656057ee9..c62b233950976530d0dc79066e17dc2bcace1607 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -227,7 +227,7 @@ FormattingContext::Geometry::VerticalGeometry FormattingContext::Geometry::outOf
>     ASSERT(marginBottom);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow non-replaced -> top(" << *top << "px) bottom("  << *bottom << "px) height(" << *height << "px) margin(" << *marginTop << "px, "  << *marginBottom << "px) layoutBox(" << &layoutBox << ")");
>-    return { *top, *bottom, { *height, { *marginTop, *marginBottom } } };
>+    return { *top, *bottom, { *height, { *marginTop, *marginBottom }, { } } };
> }
> 
> FormattingContext::Geometry::HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -440,7 +440,7 @@ FormattingContext::Geometry::VerticalGeometry FormattingContext::Geometry::outOf
>         bottom = containingBlockHeight - (*top + *marginTop + borderTop + paddingTop + height + paddingBottom + borderBottom + *marginBottom); 
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow replaced -> top(" << *top << "px) bottom("  << *bottom << "px) height(" << height << "px) margin(" << *marginTop << "px, "  << *marginBottom << "px) layoutBox(" << &layoutBox << ")");
>-    return { *top, *bottom, { height, { *marginTop, *marginBottom } } };
>+    return { *top, *bottom, { height, { *marginTop, *marginBottom }, { } } };
> }
> 
> FormattingContext::Geometry::HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -570,7 +570,7 @@ FormattingContext::Geometry::HeightAndMargin FormattingContext::Geometry::floati
>         height = contentHeightForFormattingContextRoot(layoutContext, layoutBox);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> floating non-replaced -> height(" << *height << "px) margin(" << *marginTop << "px, " << *marginBottom << "px) -> layoutBox(" << &layoutBox << ")");
>-    return FormattingContext::Geometry::HeightAndMargin { *height, { *marginTop, *marginBottom } };
>+    return FormattingContext::Geometry::HeightAndMargin { *height, { *marginTop, *marginBottom }, { } };
> }
> 
> FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -699,7 +699,7 @@ FormattingContext::Geometry::HeightAndMargin FormattingContext::Geometry::inline
>     ASSERT(height);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow replaced -> height(" << *height << "px) margin(" << margin.top << "px, " << margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
>-    return { *height, margin };
>+    return { *height, margin, { } };
> }
> 
> FormattingContext::Geometry::WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox,
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 1d342bbcf22fee31a2fd3540b1b18476a29d6ea9..4357735cb9831e9efe51af244053ba3007c55a9f 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -192,7 +192,7 @@ void BlockFormattingContext::computeInFlowHeightAndMargin(LayoutContext& layoutC
> {
>     auto heightAndMargin = Geometry::inFlowHeightAndMargin(layoutContext, layoutBox);
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>-    displayBox.moveVertically(heightAndMargin.margin.top);
>+    displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
>     displayBox.setVerticalMargin(heightAndMargin.margin);
> }
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index d27bdfccecac61a3dd54450d3956b6005bf1d0cc..7edd2cdb1041dacfb954082a102fc7e044d367a7 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -79,20 +79,21 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
>         auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->contentBoxWidth();
>         auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
> 
>-        auto marginTop = FormattingContext::Geometry::computedValueIfNotAuto(style.marginTop(), containingBlockWidth).value_or(0);
>-        auto marginBottom = FormattingContext::Geometry::computedValueIfNotAuto(style.marginBottom(), containingBlockWidth).value_or(0);
>+        Display::Box::VerticalEdges nonCollapsedMargin = { FormattingContext::Geometry::computedValueIfNotAuto(style.marginTop(), containingBlockWidth).value_or(0),
>+            FormattingContext::Geometry::computedValueIfNotAuto(style.marginBottom(), containingBlockWidth).value_or(0) }; 
>+        Display::Box::VerticalEdges collapsedMargin = { MarginCollapse::marginTop(layoutContext, layoutBox), MarginCollapse::marginBottom(layoutContext, layoutBox) };
>         auto borderAndPaddingTop = displayBox.borderTop() + displayBox.paddingTop();
>         
>         if (!style.logicalHeight().isAuto())
>-            return { style.logicalHeight().value(), { marginTop, marginBottom } };
>+            return { style.logicalHeight().value(), nonCollapsedMargin, collapsedMargin };
> 
>         if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
>-            return { 0, { marginTop, marginBottom } };
>+            return { 0, nonCollapsedMargin, collapsedMargin };
> 
>         // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
>         if (layoutBox.establishesInlineFormattingContext()) {
>             // height = lastLineBox().bottom();
>-            return { 0, { marginTop, marginBottom } };
>+            return { 0, nonCollapsedMargin, collapsedMargin };
>         }
> 
>         // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
>@@ -101,7 +102,7 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
>         if (!MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
>             auto* lastInFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*lastInFlowChild);
>             ASSERT(lastInFlowDisplayBox);
>-            return { lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom() - borderAndPaddingTop, { marginTop, marginBottom } };
>+            return { lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };
>         }
> 
>         // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
>@@ -111,29 +112,27 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
>         if (inFlowChild) {
>             auto* inFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*inFlowChild);
>             ASSERT(inFlowDisplayBox);
>-            return { inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height() - borderAndPaddingTop, { marginTop, marginBottom } };
>+            return { inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };
>         }
> 
>         // 4. zero, otherwise
>-        return { 0, { marginTop, marginBottom } };
>+        return { 0, nonCollapsedMargin, collapsedMargin };
>     };
> 
>     auto heightAndMargin = compute();
>-    auto collapsedMarginTop = MarginCollapse::marginTop(layoutContext, layoutBox);
>-    auto collapsedMarginBottom =  MarginCollapse::marginBottom(layoutContext, layoutBox);
> 
>     if (!isStretchedToViewport(layoutContext, layoutBox)) {
>-        LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << collapsedMarginTop << "px, " << collapsedMarginBottom << "px) -> layoutBox(" << &layoutBox << ")");
>-        return { heightAndMargin.height, { collapsedMarginTop, collapsedMarginBottom } };
>+        LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
>+        return heightAndMargin;
>     }
> 
>     auto initialContainingBlockHeight = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBoxHeight();
>     // Stretch but never overstretch with the margins.
>-    if (heightAndMargin.height + collapsedMarginTop + collapsedMarginBottom < initialContainingBlockHeight)
>-        heightAndMargin.height = initialContainingBlockHeight - collapsedMarginTop - collapsedMarginBottom;
>+    if (heightAndMargin.height + heightAndMargin.margin.top + heightAndMargin.margin.bottom < initialContainingBlockHeight)
>+        heightAndMargin.height = initialContainingBlockHeight - heightAndMargin.margin.top - heightAndMargin.margin.bottom;
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << collapsedMarginTop << "px, " << collapsedMarginBottom << "px) -> layoutBox(" << &layoutBox << ")");
>-    return { heightAndMargin.height, { collapsedMarginTop, collapsedMarginBottom } };
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
>+    return heightAndMargin;
> }
> 
> FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox,
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index 42a9eb8e3404b7b70da6feef15a83aa00109c4a1..ea6caa87e75653c042e8a5526d6591168a6b5616 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -78,7 +78,7 @@ static bool isMarginBottomCollapsedWithSibling(const Box& layoutBox)
>     return layoutBox.style().bottom().isAuto();
> }
> 
>-static bool isMarginTopCollapsedWithParent(const Box& layoutBox, const Display::Box& displayBox)
>+static bool isMarginTopCollapsedWithParent(const LayoutContext& layoutContext, const Box& layoutBox)
> {
>     // The first inflow child could propagate its top margin to parent.
>     // https://www.w3.org/TR/CSS21/box.html#collapsing-margins
>@@ -99,13 +99,14 @@ static bool isMarginTopCollapsedWithParent(const Box& layoutBox, const Display::
>         return false;
> 
>     // Margins of the root element's box do not collapse.
>-    if (layoutBox.isInitialContainingBlock())
>+    if (parent.isDocumentBox() || parent.isInitialContainingBlock())
>         return false;
> 
>-    if (displayBox.borderTop())
>+    auto& parentDisplayBox = *layoutContext.displayBoxForLayoutBox(parent);
>+    if (parentDisplayBox.borderTop())
>         return false;
> 
>-    if (!displayBox.paddingTop())
>+    if (parentDisplayBox.paddingTop())
>         return false;
> 
>     return true;
>@@ -118,8 +119,7 @@ LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstCh
>         return 0;
> 
>     auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild();
>-    auto& fistInflowDisplayBox = *layoutContext.displayBoxForLayoutBox(firstInFlowChild);
>-    if (!isMarginTopCollapsedWithParent(firstInFlowChild, fistInflowDisplayBox))
>+    if (!isMarginTopCollapsedWithParent(layoutContext, firstInFlowChild))
>         return 0;
> 
>     // Collect collapsed margin top recursively.
>@@ -162,8 +162,7 @@ LayoutUnit BlockFormattingContext::MarginCollapse::marginTop(const LayoutContext
>         return 0;
> 
>     // TODO: take _hasAdjoiningMarginTopAndBottom() into account.
>-    auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>-    if (isMarginTopCollapsedWithParent(layoutBox, displayBox))
>+    if (isMarginTopCollapsedWithParent(layoutContext, layoutBox))
>         return 0;
> 
>     // Floats and out of flow positioned boxes do not collapse their margins.

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 187129: 343784