Attachment 358309 Details for Bug 193131 – Patch

[patch] Patch

Patch.txt (text/plain), 42.50 KB, created by zalan on 2019-01-03 21:48:17 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-03 21:48:17 PST

Size: 42.50 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7e3275a9b0c..0b93f94499b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2019-01-03  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] ComputedHorizontalMargin should have optional members
>+        https://bugs.webkit.org/show_bug.cgi?id=193131
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Split HorizontalMargin into UsedHorizontalMargin and ComputedHorizontalMargin. ComputedHorizontalMargin's members are optional.
>+        (see computed vs used values)
>+
>+        * layout/FormattingContext.h:
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::computedHorizontalMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::computedNonCollapsedHorizontalMarginValue): Deleted.
>+        * layout/LayoutUnits.h:
>+        * layout/MarginTypes.h:
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::outputMismatchingBlockBoxInformationIfNeeded):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::setHorizontalMargin):
>+        (WebCore::Display::Box::setHorizontalComputedMargin):
>+        (WebCore::Display::Box::computedMarginStart const):
>+        (WebCore::Display::Box::computedMarginEnd const):
>+        * layout/floats/FloatAvoider.h:
>+        (WebCore::Layout::FloatAvoider::marginStart const):
>+        (WebCore::Layout::FloatAvoider::marginEnd const):
>+        * layout/inlineformatting/InlineFormattingContext.cpp:
>+        (WebCore::Layout::InlineFormattingContext::collectInlineContentForSubtree const):
>+        * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
>+        (WebCore::Layout::InlineFormattingContext::Geometry::inlineBlockWidthAndMargin):
>+
> 2019-01-03  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC] Use computedValue and usedValue consistently
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index 7f743cb2c62..50bbba5b618 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -96,7 +96,7 @@ protected:
>         static Edges computedBorder(const LayoutState&, const Box&);
>         static Optional<Edges> computedPadding(const LayoutState&, const Box&);
> 
>-        static HorizontalMargin computedNonCollapsedHorizontalMarginValue(const LayoutState&, const Box&);
>+        static ComputedHorizontalMargin computedHorizontalMargin(const LayoutState&, const Box&);
>         static VerticalMargin::ComputedValues computedNonCollapsedVerticalMarginValue(const LayoutState&, const Box&);
> 
>         static Optional<LayoutUnit> computedValueIfNotAuto(const Length& geometryProperty, LayoutUnit containingBlockWidth);
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index cf2e377f3d8..cba9515c19d 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -400,10 +400,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
>     auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
>     auto width = computedValueIfNotAuto(usedWidth ? Length { usedWidth.value(), Fixed } : style.logicalWidth(), containingBlockWidth);
>-    auto computedMarginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
>-    auto computedMarginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
>-    auto usedMarginStart = computedMarginStart;
>-    auto usedMarginEnd = computedMarginEnd;
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
>+    UsedHorizontalMargin usedHorizontalMargin;
>     auto paddingLeft = displayBox.paddingLeft().valueOr(0);
>     auto paddingRight = displayBox.paddingRight().valueOr(0);
>     auto borderLeft = displayBox.borderLeft();
>@@ -413,8 +411,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>         // If all three of 'left', 'width', and 'right' are 'auto': First set any 'auto' values for 'margin-left' and 'margin-right' to 0.
>         // Then, if the 'direction' property of the element establishing the static-position containing block is 'ltr' set 'left' to the static
>         // position and apply rule number three below; otherwise, set 'right' to the static position and apply rule number one below.
>-        usedMarginStart = usedMarginStart.valueOr(0);
>-        usedMarginEnd = usedMarginEnd.valueOr(0);
>+        usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
> 
>         auto staticHorizontalPosition = staticHorizontalPositionForOutOfFlowPositioned(layoutState, layoutBox);
>         if (isLeftToRightDirection)
>@@ -427,84 +424,80 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>         // solve for 'margin-right' ('margin-left'). If one of 'margin-left' or 'margin-right' is 'auto', solve the equation for that value.
>         // If the values are over-constrained, ignore the value for 'left' (in case the 'direction' property of the containing block is 'rtl') or 'right'
>         // (in case 'direction' is 'ltr') and solve for that value.
>-        if (!usedMarginStart && !usedMarginEnd) {
>+        if (!computedHorizontalMargin.start && !computedHorizontalMargin.end) {
>             auto marginStartAndEnd = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>             if (marginStartAndEnd >= 0)
>-                usedMarginStart = usedMarginEnd = marginStartAndEnd / 2;  
>+                usedHorizontalMargin = { marginStartAndEnd / 2, marginStartAndEnd / 2 };  
>             else {
>                 if (isLeftToRightDirection) {
>-                    usedMarginStart = 0_lu;
>-                    usedMarginEnd = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>+                    usedHorizontalMargin.start = 0_lu;
>+                    usedHorizontalMargin.end = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>                 } else {
>-                    usedMarginEnd = 0_lu;
>-                    usedMarginStart = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd + *right);
>+                    usedHorizontalMargin.end = 0_lu;
>+                    usedHorizontalMargin.start = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>                 }
>             }
>-        } else if (!usedMarginStart) {
>-            usedMarginStart = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd + *right);
>+        } else if (!computedHorizontalMargin.start) {
>+            usedHorizontalMargin.end = *computedHorizontalMargin.end;
>+            usedHorizontalMargin.start = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>             // Overconstrained? Ignore right (left).
>-            if (*usedMarginStart < 0) {
>+            if (usedHorizontalMargin.start < 0) {
>                 if (isLeftToRightDirection)
>-                    usedMarginStart = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
>+                    usedHorizontalMargin.start = containingBlockWidth - (*left + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>                 else
>-                    usedMarginStart = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd + *right);
>+                    usedHorizontalMargin.start = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>             }
>-        } else if (!usedMarginEnd) {
>-            usedMarginEnd = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>+        } else if (!computedHorizontalMargin.end) {
>+            usedHorizontalMargin.start = *computedHorizontalMargin.start;
>+            usedHorizontalMargin.end = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>             // Overconstrained? Ignore right (left).
>-            if (*usedMarginEnd < 0) {
>+            if (usedHorizontalMargin.end < 0) {
>                 if (isLeftToRightDirection)
>-                    usedMarginEnd = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight);
>+                    usedHorizontalMargin.end = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight);
>                 else
>-                    usedMarginEnd = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>+                    usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + *right);
>             }
>         }
>     } else {
>         // Otherwise, set 'auto' values for 'margin-left' and 'margin-right' to 0, and pick the one of the following six rules that applies.
>-        usedMarginStart = usedMarginStart.valueOr(0);
>-        usedMarginEnd = usedMarginEnd.valueOr(0);
>+        usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
>     }
> 
>-    ASSERT(usedMarginStart);
>-    ASSERT(usedMarginEnd);
>-
>     if (!left && !width && right) {
>         // #1
>         width = shrinkToFitWidth(layoutState, layoutBox);
>-        left = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight  + borderRight + *usedMarginEnd + *right);
>+        left = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight  + borderRight + usedHorizontalMargin.end + *right);
>     } else if (!left && !right && width) {
>         // #2
>         auto staticHorizontalPosition = staticHorizontalPositionForOutOfFlowPositioned(layoutState, layoutBox);
>         if (isLeftToRightDirection) {
>             left = staticHorizontalPosition;
>-            right = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
>+            right = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>         } else {
>             right = staticHorizontalPosition;
>-            left = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd + *right);
>+            left = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>         }
>     } else if (!width && !right && left) {
>         // #3
>         width = shrinkToFitWidth(layoutState, layoutBox);
>-        right = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
>+        right = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>     } else if (!left && width && right) {
>         // #4
>-        left = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd + *right);
>+        left = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>     } else if (!width && left && right) {
>         // #5
>-        width = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + paddingRight  + borderRight + *usedMarginEnd + *right);
>+        width = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + paddingRight  + borderRight + usedHorizontalMargin.end + *right);
>     } else if (!right && left && width) {
>         // #6
>-        right = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
>+        right = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>     }
> 
>     ASSERT(left);
>     ASSERT(right);
>     ASSERT(width);
>-    ASSERT(usedMarginStart);
>-    ASSERT(usedMarginEnd);
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow non-replaced -> left(" << *left << "px) right("  << *right << "px) width(" << *width << "px) margin(" << *usedMarginStart << "px, "  << *usedMarginEnd << "px) layoutBox(" << &layoutBox << ")");
>-    return { *left, *right, { *width, { *usedMarginStart, *usedMarginEnd }, { computedMarginStart.valueOr(0), computedMarginEnd.valueOr(0) } } };
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow non-replaced -> left(" << *left << "px) right("  << *right << "px) width(" << *width << "px) margin(" << usedHorizontalMargin.start << "px, "  << usedHorizontalMargin.end << "px) layoutBox(" << &layoutBox << ")");
>+    return { *left, *right, { *width, usedHorizontalMargin, computedHorizontalMargin } };
> }
> 
> VerticalGeometry FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedHeight)
>@@ -604,10 +597,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
> 
>     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
>     auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
>-    auto computedMarginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
>-    auto computedMarginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
>-    auto usedMarginStart = computedMarginStart;
>-    auto usedMarginEnd = computedMarginEnd;
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
>+    UsedHorizontalMargin usedHorizontalMargin;
>     auto width = inlineReplacedWidthAndMargin(layoutState, layoutBox, usedWidth).width;
>     auto paddingLeft = displayBox.paddingLeft().valueOr(0);
>     auto paddingRight = displayBox.paddingRight().valueOr(0);
>@@ -625,55 +616,52 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
> 
>     if (!left || !right) {
>         // #2
>-        usedMarginStart = usedMarginStart.valueOr(0); 
>-        usedMarginEnd = usedMarginEnd.valueOr(0); 
>+        usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) }; 
>     }
> 
>-    if (!usedMarginStart && !usedMarginEnd) {
>+    if (!computedHorizontalMargin.start && !computedHorizontalMargin.end) {
>         // #3
>         auto marginStartAndEnd = containingBlockWidth - (*left + borderLeft + paddingLeft + width + paddingRight + borderRight + *right);
>         if (marginStartAndEnd >= 0)
>-            usedMarginStart = usedMarginEnd = marginStartAndEnd / 2;
>+            usedHorizontalMargin = { marginStartAndEnd / 2, marginStartAndEnd / 2 };
>         else {
>             if (isLeftToRightDirection) {
>-                usedMarginStart = 0_lu;
>-                usedMarginEnd = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *right);
>+                usedHorizontalMargin.start = 0_lu;
>+                usedHorizontalMargin.end = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + *right);
>             } else {
>-                usedMarginEnd = 0_lu;
>-                usedMarginStart = containingBlockWidth - (*left + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd + *right);
>+                usedHorizontalMargin.end = 0_lu;
>+                usedHorizontalMargin.start = containingBlockWidth - (*left + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>             }
>         }
>     }
> 
>     // #4
>     if (!left)
>-        left = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd + *right);
>+        left = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
> 
>     if (!right)
>-        right = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd);
>+        right = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end);
> 
>-    if (!usedMarginStart)
>-        usedMarginStart = containingBlockWidth - (*left + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd + *right);
>+    if (!computedHorizontalMargin.start)
>+        usedHorizontalMargin.start = containingBlockWidth - (*left + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
> 
>-    if (!usedMarginEnd)
>-        usedMarginEnd = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *right);
>+    if (!computedHorizontalMargin.end)
>+        usedHorizontalMargin.end = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + *right);
> 
>-    auto boxWidth = (*left + *usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd + *right);
>+    auto boxWidth = (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>     if (boxWidth > containingBlockWidth) {
>         // #5 Over-constrained?
>         if (isLeftToRightDirection)
>-            right = containingBlockWidth - (*left + *usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd);
>+            right = containingBlockWidth - (*left + usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end);
>         else
>-            left = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + width + paddingRight + borderRight + *usedMarginEnd + *right);
>+            left = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + width + paddingRight + borderRight + usedHorizontalMargin.end + *right);
>     }
> 
>     ASSERT(left);
>     ASSERT(right);
>-    ASSERT(usedMarginStart);
>-    ASSERT(usedMarginEnd);
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow replaced -> left(" << *left << "px) right("  << *right << "px) width(" << width << "px) margin(" << *usedMarginStart << "px, "  << *usedMarginEnd << "px) layoutBox(" << &layoutBox << ")");
>-    return { *left, *right, { width, { *usedMarginStart, *usedMarginEnd }, { computedMarginStart.valueOr(0), computedMarginEnd.valueOr(0) } } };
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow replaced -> left(" << *left << "px) right("  << *right << "px) width(" << width << "px) margin(" << usedHorizontalMargin.start << "px, "  << usedHorizontalMargin.end << "px) layoutBox(" << &layoutBox << ")");
>+    return { *left, *right, { width, usedHorizontalMargin, computedHorizontalMargin } };
> }
> 
> HeightAndMargin FormattingContext::Geometry::complicatedCases(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedHeight)
>@@ -729,14 +717,14 @@ WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(La
>     auto containingBlockWidth = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxWidth();
> 
>     // #1
>-    auto margin = computedNonCollapsedHorizontalMarginValue(layoutState, layoutBox);
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
>     // #2
>     auto width = computedValueIfNotAuto(usedWidth ? Length { usedWidth.value(), Fixed } : layoutBox.style().logicalWidth(), containingBlockWidth);
>     if (!width)
>         width = shrinkToFitWidth(layoutState, layoutBox);
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> floating non-replaced -> width(" << *width << "px) margin(" << margin.start << "px, " << margin.end << "px) -> layoutBox(" << &layoutBox << ")");
>-    return WidthAndMargin { *width, margin, margin };
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> floating non-replaced -> width(" << *width << "px) margin(" << computedHorizontalMargin.start.valueOr(0_lu) << "px, " << computedHorizontalMargin.end.valueOr(0_lu) << "px) -> layoutBox(" << &layoutBox << ")");
>+    return WidthAndMargin { *width, { computedHorizontalMargin.start.valueOr(0_lu), computedHorizontalMargin.end.valueOr(0_lu) }, computedHorizontalMargin };
> }
> 
> HeightAndMargin FormattingContext::Geometry::floatingReplacedHeightAndMargin(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedHeight)
>@@ -757,10 +745,10 @@ WidthAndMargin FormattingContext::Geometry::floatingReplacedWidthAndMargin(const
>     //
>     // 1. If 'margin-left' or 'margin-right' are computed as 'auto', their used value is '0'.
>     // 2. The used value of 'width' is determined as for inline replaced elements.
>-    auto margin = computedNonCollapsedHorizontalMarginValue(layoutState, layoutBox);
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> floating replaced -> redirected to inline replaced");
>-    return inlineReplacedWidthAndMargin(layoutState, layoutBox, usedWidth, margin.start, margin.end);
>+    return inlineReplacedWidthAndMargin(layoutState, layoutBox, usedWidth, computedHorizontalMargin.start, computedHorizontalMargin.end);
> }
> 
> VerticalGeometry FormattingContext::Geometry::outOfFlowVerticalGeometry(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedHeight)
>@@ -871,19 +859,14 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(const L
>     auto& style = layoutBox.style();
>     auto& containingBlockDisplayBox = layoutState.displayBoxForLayoutBox(*layoutBox.containingBlock());
>     auto containingBlockWidth = containingBlockDisplayBox.width();
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
> 
>     auto usedMarginStart = [&] {
>-        if (precomputedMarginStart)
>-            return precomputedMarginStart.value();
>-        auto marginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
>-        return marginStart.valueOr(0_lu);
>+        return precomputedMarginStart.valueOr(computedHorizontalMargin.start.valueOr(0_lu));
>     };
> 
>     auto usedMarginEnd = [&] {
>-        if (precomputedMarginEnd)
>-            return precomputedMarginEnd.value();
>-        auto marginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
>-        return marginEnd.valueOr(0_lu);
>+        return precomputedMarginEnd.valueOr(computedHorizontalMargin.end.valueOr(0_lu));
>     };
> 
>     auto replaced = layoutBox.replaced();
>@@ -915,9 +898,7 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(const L
>     ASSERT(width);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> inflow replaced -> width(" << *width << "px) margin(" << usedMarginStart() << "px, " << usedMarginEnd() << "px) -> layoutBox(" << &layoutBox << ")");
>-    auto computedMarginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
>-    auto computedMarginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
>-    return { *width, { usedMarginStart(), usedMarginEnd() }, { computedMarginStart.valueOr(0), computedMarginEnd.valueOr(0) } };
>+    return { *width, { usedMarginStart(), usedMarginEnd() }, computedHorizontalMargin };
> }
> 
> LayoutSize FormattingContext::Geometry::inFlowPositionedPositionOffset(const LayoutState& layoutState, const Box& layoutBox)
>@@ -1020,16 +1001,11 @@ Optional<Edges> FormattingContext::Geometry::computedPadding(const LayoutState&
>     };
> }
> 
>-HorizontalMargin FormattingContext::Geometry::computedNonCollapsedHorizontalMarginValue(const LayoutState& layoutState, const Box& layoutBox)
>+ComputedHorizontalMargin FormattingContext::Geometry::computedHorizontalMargin(const LayoutState& layoutState, const Box& layoutBox)
> {
>     auto& style = layoutBox.style();
>     auto containingBlockWidth = layoutState.displayBoxForLayoutBox(*layoutBox.containingBlock()).contentBoxWidth();
>-
>-    auto marginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth).valueOr(0_lu);
>-    auto marginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth).valueOr(0_lu);
>-
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Margin] -> non collapsed horizontal -> margin(" << marginStart << "px, " << marginEnd << "px) -> layoutBox: " << &layoutBox);
>-    return { marginStart, marginEnd };
>+    return { computedValueIfNotAuto(style.marginStart(), containingBlockWidth), computedValueIfNotAuto(style.marginEnd(), containingBlockWidth) };
> }
> 
> VerticalMargin::ComputedValues FormattingContext::Geometry::computedNonCollapsedVerticalMarginValue(const LayoutState& layoutState, const Box& layoutBox)
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index 70ae2bc4112..3b4829afe70 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -103,8 +103,8 @@ struct Edges {
> 
> struct WidthAndMargin {
>     LayoutUnit width;
>-    HorizontalMargin usedMargin;
>-    HorizontalMargin computedMargin;
>+    UsedHorizontalMargin usedMargin;
>+    ComputedHorizontalMargin computedMargin;
> };
> 
> struct HeightAndMargin {
>diff --git a/Source/WebCore/layout/MarginTypes.h b/Source/WebCore/layout/MarginTypes.h
>index 85ffed65968..fe1fc18fa69 100644
>--- a/Source/WebCore/layout/MarginTypes.h
>+++ b/Source/WebCore/layout/MarginTypes.h
>@@ -56,7 +56,12 @@ private:
>     Optional<CollapsedValues> m_collapsed;
> };
> 
>-struct HorizontalMargin {
>+struct ComputedHorizontalMargin {
>+    Optional<LayoutUnit> start;
>+    Optional<LayoutUnit> end;
>+};
>+
>+struct UsedHorizontalMargin {
>     LayoutUnit start;
>     LayoutUnit end;
> };
>diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
>index 494157a9ee7..64f7a84f340 100644
>--- a/Source/WebCore/layout/Verification.cpp
>+++ b/Source/WebCore/layout/Verification.cpp
>@@ -247,8 +247,8 @@ static bool outputMismatchingBlockBoxInformationIfNeeded(TextStream& stream, con
> 
>         return Display::Box::Rect {
>             borderBox.top() - displayBox.nonCollapsedMarginBefore(),
>-            borderBox.left() - displayBox.computedMarginStart(),
>-            displayBox.computedMarginStart() + borderBox.width() + displayBox.computedMarginEnd(),
>+            borderBox.left() - displayBox.computedMarginStart().valueOr(0),
>+            displayBox.computedMarginStart().valueOr(0) + borderBox.width() + displayBox.computedMarginEnd().valueOr(0),
>             displayBox.nonCollapsedMarginBefore() + borderBox.height() + displayBox.nonCollapsedMarginAfter()
>         };
>     };
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 64a77584960..63e0682d5e9 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -141,10 +141,8 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
>         auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
> 
>         auto width = computedValueIfNotAuto(usedWidth ? Length { usedWidth.value(), Fixed } : style.logicalWidth(), containingBlockWidth);
>-        auto computedMarginStart = computedValueIfNotAuto(style.marginStart(), containingBlockWidth);
>-        auto computedMarginEnd = computedValueIfNotAuto(style.marginEnd(), containingBlockWidth);
>-        auto usedMarginStart = computedMarginStart;
>-        auto usedMarginEnd = computedMarginEnd;
>+        auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
>+        UsedHorizontalMargin usedHorizontalMargin;
>         auto borderLeft = displayBox.borderLeft();
>         auto borderRight = displayBox.borderRight();
>         auto paddingLeft = displayBox.paddingLeft().valueOr(0);
>@@ -152,47 +150,49 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
> 
>         // #1
>         if (width) {
>-            auto horizontalSpaceForMargin = containingBlockWidth - (usedMarginStart.valueOr(0) + borderLeft + paddingLeft + *width + paddingRight + borderRight + usedMarginEnd.valueOr(0));
>-            if (horizontalSpaceForMargin < 0) {
>-                usedMarginStart = usedMarginStart.valueOr(0);
>-                usedMarginEnd = usedMarginEnd.valueOr(0);
>-            }
>+            auto horizontalSpaceForMargin = containingBlockWidth - (computedHorizontalMargin.start.valueOr(0) + borderLeft + paddingLeft + *width + paddingRight + borderRight + computedHorizontalMargin.end.valueOr(0));
>+            if (horizontalSpaceForMargin < 0)
>+                usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
>         }
> 
>         // #2
>-        if (width && usedMarginStart && usedMarginEnd) {
>-            if (containingBlock->style().isLeftToRightDirection())
>-                usedMarginEnd = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft  + *width + paddingRight + borderRight);
>-            else
>-                usedMarginStart = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + *usedMarginEnd);
>+        if (width && computedHorizontalMargin.start && computedHorizontalMargin.end) {
>+            if (containingBlock->style().isLeftToRightDirection()) {
>+                usedHorizontalMargin.start = *computedHorizontalMargin.start;
>+                usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft  + *width + paddingRight + borderRight);
>+            } else {
>+                usedHorizontalMargin.end = *computedHorizontalMargin.end;
>+                usedHorizontalMargin.start = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>+            }
>         }
> 
>         // #3
>-        if (!usedMarginStart && width && usedMarginEnd)
>-            usedMarginStart = containingBlockWidth - (borderLeft + paddingLeft  + *width + paddingRight + borderRight + *usedMarginEnd);
>-        else if (usedMarginStart && !width && usedMarginEnd)
>-            width = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + paddingRight + borderRight + *usedMarginEnd);
>-        else if (usedMarginStart && width && !usedMarginEnd)
>-            usedMarginEnd = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + *width + paddingRight + borderRight);
>+        if (!computedHorizontalMargin.start && width && computedHorizontalMargin.end) {
>+            usedHorizontalMargin.end = *computedHorizontalMargin.end;
>+            usedHorizontalMargin.start = containingBlockWidth - (borderLeft + paddingLeft  + *width + paddingRight + borderRight + usedHorizontalMargin.end);
>+        } else if (computedHorizontalMargin.start && !width && computedHorizontalMargin.end) {
>+            usedHorizontalMargin = { *computedHorizontalMargin.start, *computedHorizontalMargin.end };
>+            width = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + paddingRight + borderRight + usedHorizontalMargin.end);
>+        } else if (computedHorizontalMargin.start && width && !computedHorizontalMargin.end) {
>+            usedHorizontalMargin.start = *computedHorizontalMargin.start;
>+            usedHorizontalMargin.end = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + *width + paddingRight + borderRight);
>+        }
> 
>         // #4
>         if (!width) {
>-            usedMarginStart = usedMarginStart.valueOr(0);
>-            usedMarginEnd = usedMarginEnd.valueOr(0);
>-            width = containingBlockWidth - (*usedMarginStart + borderLeft + paddingLeft + paddingRight + borderRight + *usedMarginEnd);
>+            usedHorizontalMargin = { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
>+            width = containingBlockWidth - (usedHorizontalMargin.start + borderLeft + paddingLeft + paddingRight + borderRight + usedHorizontalMargin.end);
>         }
> 
>         // #5
>-        if (!usedMarginStart && !usedMarginEnd) {
>+        if (!computedHorizontalMargin.start && !computedHorizontalMargin.end) {
>             auto horizontalSpaceForMargin = containingBlockWidth - (borderLeft + paddingLeft  + *width + paddingRight + borderRight);
>-            usedMarginStart = usedMarginEnd = horizontalSpaceForMargin / 2;
>+            usedHorizontalMargin = { horizontalSpaceForMargin / 2, horizontalSpaceForMargin / 2 };
>         }
> 
>         ASSERT(width);
>-        ASSERT(usedMarginStart);
>-        ASSERT(usedMarginEnd);
> 
>-        return WidthAndMargin { *width, { *usedMarginStart, *usedMarginEnd }, { computedMarginStart.valueOr(0), computedMarginEnd.valueOr(0) } };
>+        return WidthAndMargin { *width, usedHorizontalMargin, computedHorizontalMargin };
>     };
> 
>     auto widthAndMargin = compute();
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 75ec77187a4..9bc1b9feb0c 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -146,8 +146,8 @@ public:
> 
>     LayoutUnit nonCollapsedMarginBefore() const;
>     LayoutUnit nonCollapsedMarginAfter() const;
>-    LayoutUnit computedMarginStart() const;
>-    LayoutUnit computedMarginEnd() const;
>+    Optional<LayoutUnit> computedMarginStart() const;
>+    Optional<LayoutUnit> computedMarginEnd() const;
> 
>     Optional<LayoutUnit> estimatedMarginBefore() const { return m_estimatedMarginBefore; }
> 
>@@ -191,9 +191,9 @@ private:
>     void setContentBoxHeight(LayoutUnit);
>     void setContentBoxWidth(LayoutUnit);
> 
>-    void setHorizontalMargin(Layout::HorizontalMargin);
>+    void setHorizontalMargin(Layout::UsedHorizontalMargin);
>     void setVerticalMargin(Layout::VerticalMargin);
>-    void setHorizontalComputedMargin(Layout::HorizontalMargin);
>+    void setHorizontalComputedMargin(Layout::ComputedHorizontalMargin);
>     void setEstimatedMarginBefore(LayoutUnit marginBefore) { m_estimatedMarginBefore = marginBefore; }
> 
>     void setBorder(Layout::Edges);
>@@ -224,9 +224,9 @@ private:
>     LayoutUnit m_contentWidth;
>     LayoutUnit m_contentHeight;
> 
>-    Layout::HorizontalMargin m_horizontalMargin;
>+    Layout::UsedHorizontalMargin m_horizontalMargin;
>     Layout::VerticalMargin m_verticalMargin;
>-    Layout::HorizontalMargin m_horizontalComputedMargin;
>+    Layout::ComputedHorizontalMargin m_horizontalComputedMargin;
>     Optional<LayoutUnit> m_estimatedMarginBefore;
> 
>     Layout::Edges m_border;
>@@ -507,7 +507,7 @@ inline LayoutUnit Box::contentBoxWidth() const
>     return m_contentWidth;
> }
> 
>-inline void Box::setHorizontalMargin(Layout::HorizontalMargin margin)
>+inline void Box::setHorizontalMargin(Layout::UsedHorizontalMargin margin)
> {
> #if !ASSERT_DISABLED
>     setHasValidHorizontalMargin();
>@@ -525,7 +525,7 @@ inline void Box::setVerticalMargin(Layout::VerticalMargin margin)
>     m_verticalMargin = margin;
> }
> 
>-inline void Box::setHorizontalComputedMargin(Layout::HorizontalMargin margin)
>+inline void Box::setHorizontalComputedMargin(Layout::ComputedHorizontalMargin margin)
> {
> #if !ASSERT_DISABLED
>     setHasValidHorizontalComputedMargin();
>@@ -591,13 +591,13 @@ inline LayoutUnit Box::nonCollapsedMarginAfter() const
>     return m_verticalMargin.nonCollapsedValues().after;
> }
> 
>-inline LayoutUnit Box::computedMarginStart() const
>+inline Optional<LayoutUnit> Box::computedMarginStart() const
> {
>     ASSERT(m_hasValidHorizontalComputedMargin);
>     return m_horizontalComputedMargin.start;
> }
> 
>-inline LayoutUnit Box::computedMarginEnd() const
>+inline Optional<LayoutUnit> Box::computedMarginEnd() const
> {
>     ASSERT(m_hasValidHorizontalComputedMargin);
>     return m_horizontalComputedMargin.end;
>diff --git a/Source/WebCore/layout/floats/FloatAvoider.h b/Source/WebCore/layout/floats/FloatAvoider.h
>index 8b5cd884058..67c94296021 100644
>--- a/Source/WebCore/layout/floats/FloatAvoider.h
>+++ b/Source/WebCore/layout/floats/FloatAvoider.h
>@@ -73,8 +73,8 @@ protected:
>     LayoutUnit marginBefore() const { return displayBox().marginBefore(); }
>     LayoutUnit marginAfter() const { return displayBox().marginAfter(); }
>     // Do not use the used values here because they computed as if this box was not a float avoider.
>-    LayoutUnit marginStart() const { return displayBox().computedMarginStart(); }
>-    LayoutUnit marginEnd() const { return displayBox().computedMarginEnd(); }
>+    LayoutUnit marginStart() const { return displayBox().computedMarginStart().valueOr(0); }
>+    LayoutUnit marginEnd() const { return displayBox().computedMarginEnd().valueOr(0); }
> 
>     LayoutUnit marginBoxWidth() const { return marginStart() + displayBox().width() + marginEnd(); }
> 
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>index 5c5f3edb723..b36e68e8a7d 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>@@ -442,10 +442,10 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
>         createAndAppendInlineItem();
>         auto& inlineRun = *inlineFormattingState.inlineContent().last();
> 
>-        auto horizontalMargin = Geometry::computedNonCollapsedHorizontalMarginValue(layoutState(), root);
>+        auto horizontalMargin = Geometry::computedHorizontalMargin(layoutState(), root);
>         inlineRun.addDetachingRule({ InlineItem::DetachingRule::BreakAtStart, InlineItem::DetachingRule::BreakAtEnd });
>-        inlineRun.addNonBreakableStart(horizontalMargin.start);
>-        inlineRun.addNonBreakableEnd(horizontalMargin.end);
>+        inlineRun.addNonBreakableStart(horizontalMargin.start.valueOr(0));
>+        inlineRun.addNonBreakableEnd(horizontalMargin.end.valueOr(0));
>         // Skip formatting root subtree. They are not part of this inline formatting context.
>         return;
>     }
>@@ -465,7 +465,7 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
>     // FIXME: Revisit this when we figured out how inline boxes fit the display tree.
>     auto padding = Geometry::computedPadding(layoutState(), root);
>     auto border = Geometry::computedBorder(layoutState(), root);
>-    auto horizontalMargin = Geometry::computedNonCollapsedHorizontalMarginValue(layoutState(), root);
>+    auto horizontalMargin = Geometry::computedHorizontalMargin(layoutState(), root);
>     // Setup breaking boundaries for this subtree.
>     auto* lastDescendantInlineBox = inlineFormattingState.lastInlineItem();
>     // Empty container?
>@@ -496,7 +496,7 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
> 
>         ASSERT(firstDescendantInlineBox);
>         firstDescendantInlineBox->addDetachingRule(InlineItem::DetachingRule::BreakAtStart);
>-        auto startOffset = border.horizontal.left + horizontalMargin.start;
>+        auto startOffset = border.horizontal.left + horizontalMargin.start.valueOr(0);
>         if (padding)
>             startOffset += padding->horizontal.left;
>         firstDescendantInlineBox->addNonBreakableStart(startOffset);
>@@ -504,7 +504,7 @@ void InlineFormattingContext::collectInlineContentForSubtree(const Box& root, In
> 
>     if (rootBreaksAtEnd()) {
>         lastDescendantInlineBox->addDetachingRule(InlineItem::DetachingRule::BreakAtEnd);
>-        auto endOffset = border.horizontal.right + horizontalMargin.end;
>+        auto endOffset = border.horizontal.right + horizontalMargin.end.valueOr(0);
>         if (padding)
>             endOffset += padding->horizontal.right;
>         lastDescendantInlineBox->addNonBreakableEnd(endOffset);
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>index b65c89fc55f..e25e5b2c235 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>@@ -60,9 +60,9 @@ WidthAndMargin InlineFormattingContext::Geometry::inlineBlockWidthAndMargin(Layo
>         width = shrinkToFitWidth(layoutState, formattingContextRoot);
> 
>     // #2
>-    auto margin = computedNonCollapsedHorizontalMarginValue(layoutState, formattingContextRoot);
>+    auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, formattingContextRoot);
> 
>-    return WidthAndMargin { *width, margin, margin };
>+    return WidthAndMargin { *width, { computedHorizontalMargin.start.valueOr(0_lu), computedHorizontalMargin.end.valueOr(0_lu) }, computedHorizontalMargin };
> }
> 
> HeightAndMargin InlineFormattingContext::Geometry::inlineBlockHeightAndMargin(const LayoutState& layoutState, const Box& layoutBox)

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193131: 358302 | 358309