Attachment 347241 Details for Bug 188630 – Patch

[patch] Patch

bug-188630-20180815213829.patch (text/plain), 14.03 KB, created by zalan on 2018-08-15 21:38:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-15 21:38:31 PDT

Size: 14.03 KB

patch

obsolete

>Subversion Revision: 234903
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 45f39ffa7cb52927e6309e931190956946750dd3..517adf29cac6f0e6a29d751a9a83d6b5737f72fe 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-08-15  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC] Display::Box interface should reflect that padding is optional.
>+        https://bugs.webkit.org/show_bug.cgi?id=188630
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Certain type of boxes can't have paddings (see Layout::Box::isPaddingApplicable).
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeBorderAndPadding const):
>+        (WebCore::Layout::FormattingContext::validateGeometryConstraintsAfterLayout const):
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::width const):
>+        (WebCore::Display::Box::height const):
>+        (WebCore::Display::Box::contentBoxTop const):
>+        (WebCore::Display::Box::contentBoxLeft const):
>+        (WebCore::Display::Box::setPadding):
>+        (WebCore::Display::Box::paddingTop const):
>+        (WebCore::Display::Box::paddingLeft const):
>+        (WebCore::Display::Box::paddingBottom const):
>+        (WebCore::Display::Box::paddingRight const):
>+
> 2018-08-15  Christopher Reid  <chris.reid@sony.com>
> 
>         [Curl] Implement default cookie path handling correctly as outlined in RFC6265.
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 4163e76193fb1b2d9b5b74d9c49dae5adbb8485b..de4800940226a02e102f725a4dfab49ef2e6699e 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -90,8 +90,7 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(LayoutContext& layoutCo
> void FormattingContext::computeBorderAndPadding(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
> {
>     displayBox.setBorder(Geometry::computedBorder(layoutContext, layoutBox));
>-    if (auto padding = Geometry::computedPadding(layoutContext, layoutBox))
>-        displayBox.setPadding(*padding);
>+    displayBox.setPadding(Geometry::computedPadding(layoutContext, layoutBox));
> }
> 
> void FormattingContext::placeInFlowPositionedChildren(LayoutContext& layoutContext, const Container& container) const
>@@ -204,16 +203,16 @@ void FormattingContext::validateGeometryConstraintsAfterLayout(const LayoutConte
>         if ((layoutBox.isBlockLevelBox() || layoutBox.isOutOfFlowPositioned()) && !layoutBox.replaced()) {
>             // margin-left + border-left-width + padding-left + width + padding-right + border-right-width + margin-right = width of containing block
>             auto containingBlockWidth = containingBlockDisplayBox.contentBoxWidth();
>-            ASSERT(displayBox->marginLeft() + displayBox->borderLeft() + displayBox->paddingLeft() + displayBox->contentBoxWidth()
>-                + displayBox->paddingRight() + displayBox->borderRight() + displayBox->marginRight() == containingBlockWidth);
>+            ASSERT(displayBox->marginLeft() + displayBox->borderLeft() + displayBox->paddingLeft().value_or(0) + displayBox->contentBoxWidth()
>+                + displayBox->paddingRight().value_or(0) + displayBox->borderRight() + displayBox->marginRight() == containingBlockWidth);
>         }
> 
>         // 10.6.4 Absolutely positioned, non-replaced elements
>         if (layoutBox.isOutOfFlowPositioned() && !layoutBox.replaced()) {
>             // top + margin-top + border-top-width + padding-top + height + padding-bottom + border-bottom-width + margin-bottom + bottom = height of containing block
>             auto containingBlockHeight = containingBlockDisplayBox.contentBoxHeight();
>-            ASSERT(displayBox->top() + displayBox->marginTop() + displayBox->borderTop() + displayBox->paddingTop() + displayBox->contentBoxHeight()
>-                + displayBox->paddingBottom() + displayBox->borderBottom() + displayBox->marginBottom() == containingBlockHeight);
>+            ASSERT(displayBox->top() + displayBox->marginTop() + displayBox->borderTop() + displayBox->paddingTop().value_or(0) + displayBox->contentBoxHeight()
>+                + displayBox->paddingBottom().value_or(0) + displayBox->borderBottom() + displayBox->marginBottom() == containingBlockHeight);
>         }
>     }
> }
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index d90be215553d1e27a6427cccbe05f35a72969f63..c833b1813d27456ae12071dd0359c62bebb65bf4 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -196,8 +196,8 @@ VerticalGeometry FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeomet
>     auto height = computedValueIfNotAuto(style.logicalHeight(), containingBlockHeight);
>     auto marginTop = computedValueIfNotAuto(style.marginTop(), containingBlockWidth);
>     auto marginBottom = computedValueIfNotAuto(style.marginBottom(), containingBlockWidth);
>-    auto paddingTop = displayBox.paddingTop();
>-    auto paddingBottom = displayBox.paddingBottom();
>+    auto paddingTop = displayBox.paddingTop().value_or(0);
>+    auto paddingBottom = displayBox.paddingBottom().value_or(0);
>     auto borderTop = displayBox.borderTop();
>     auto borderBottom = displayBox.borderBottom();
> 
>@@ -314,8 +314,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>     auto width = computedValueIfNotAuto(style.logicalWidth(), containingBlockWidth);
>     auto marginLeft = computedValueIfNotAuto(style.marginLeft(), containingBlockWidth);
>     auto marginRight = computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>-    auto paddingLeft = displayBox.paddingLeft();
>-    auto paddingRight = displayBox.paddingRight();
>+    auto paddingLeft = displayBox.paddingLeft().value_or(0);
>+    auto paddingRight = displayBox.paddingRight().value_or(0);
>     auto borderLeft = displayBox.borderLeft();
>     auto borderRight = displayBox.borderRight();
> 
>@@ -442,8 +442,8 @@ VerticalGeometry FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry(
>     auto height = inlineReplacedHeightAndMargin(layoutContext, layoutBox).height;
>     auto marginTop = computedValueIfNotAuto(style.marginTop(), containingBlockWidth);
>     auto marginBottom = computedValueIfNotAuto(style.marginBottom(), containingBlockWidth);
>-    auto paddingTop = displayBox.paddingTop();
>-    auto paddingBottom = displayBox.paddingBottom();
>+    auto paddingTop = displayBox.paddingTop().value_or(0);
>+    auto paddingBottom = displayBox.paddingBottom().value_or(0);
>     auto borderTop = displayBox.borderTop();
>     auto borderBottom = displayBox.borderBottom();
> 
>@@ -515,8 +515,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
>     auto marginLeft = computedValueIfNotAuto(style.marginLeft(), containingBlockWidth);
>     auto marginRight = computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>     auto width = inlineReplacedWidthAndMargin(layoutContext, layoutBox).width;
>-    auto paddingLeft = displayBox.paddingLeft();
>-    auto paddingRight = displayBox.paddingRight();
>+    auto paddingLeft = displayBox.paddingLeft().value_or(0);
>+    auto paddingRight = displayBox.paddingRight().value_or(0);
>     auto borderLeft = displayBox.borderLeft();
>     auto borderRight = displayBox.borderRight();
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 6dffc0ba9d9ba2cd2ed0de237a14c72299947907..b8142d8d8cd3ae6961e7c677ca4f363057b941d5 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -104,7 +104,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMarg
>         VerticalEdges nonCollapsedMargin = { FormattingContext::Geometry::computedValueIfNotAuto(style.marginTop(), containingBlockWidth).value_or(0),
>             FormattingContext::Geometry::computedValueIfNotAuto(style.marginBottom(), containingBlockWidth).value_or(0) }; 
>         VerticalEdges collapsedMargin = { MarginCollapse::marginTop(layoutContext, layoutBox), MarginCollapse::marginBottom(layoutContext, layoutBox) };
>-        auto borderAndPaddingTop = displayBox.borderTop() + displayBox.paddingTop();
>+        auto borderAndPaddingTop = displayBox.borderTop() + displayBox.paddingTop().value_or(0);
>         
>         auto height = style.logicalHeight();
>         if (!height.isAuto()) {
>@@ -191,8 +191,8 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
>         auto marginRight = FormattingContext::Geometry::computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>         auto borderLeft = displayBox.borderLeft();
>         auto borderRight = displayBox.borderRight();
>-        auto paddingLeft = displayBox.paddingLeft();
>-        auto paddingRight = displayBox.paddingRight();
>+        auto paddingLeft = displayBox.paddingLeft().value_or(0);
>+        auto paddingRight = displayBox.paddingRight().value_or(0);
> 
>         // #1
>         if (width) {
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index fd096d3f33214df1632670dd205fff094378a14e..3e624f09529730ed119651354abcaaf2bf91d8bf 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -126,8 +126,8 @@ public:
>     LayoutPoint bottomRight() const { return { right(), bottom() }; }
> 
>     LayoutSize size() const { return { width(), height() }; }
>-    LayoutUnit width() const { return borderLeft() + paddingLeft() + contentBoxWidth() + paddingRight() + borderRight(); }
>-    LayoutUnit height() const { return borderTop() + paddingTop() + contentBoxHeight() + paddingBottom() + borderBottom(); }
>+    LayoutUnit width() const { return borderLeft() + paddingLeft().value_or(0) + contentBoxWidth() + paddingRight().value_or(0) + borderRight(); }
>+    LayoutUnit height() const { return borderTop() + paddingTop().value_or(0) + contentBoxHeight() + paddingBottom().value_or(0) + borderBottom(); }
>     Rect rect() const { return { top(), left(), width(), height() }; }
>     Rect rectWithMargin() const { return { top() - marginTop(), left() - marginLeft(), marginLeft() + width() + marginRight(), marginTop() + height() + marginBottom() }; }
> 
>@@ -144,13 +144,13 @@ public:
>     LayoutUnit borderBottom() const;
>     LayoutUnit borderRight() const;
> 
>-    LayoutUnit paddingTop() const;
>-    LayoutUnit paddingLeft() const;
>-    LayoutUnit paddingBottom() const;
>-    LayoutUnit paddingRight() const;
>+    std::optional<LayoutUnit> paddingTop() const;
>+    std::optional<LayoutUnit> paddingLeft() const;
>+    std::optional<LayoutUnit> paddingBottom() const;
>+    std::optional<LayoutUnit> paddingRight() const;
> 
>-    LayoutUnit contentBoxTop() const { return borderTop() + paddingTop(); }
>-    LayoutUnit contentBoxLeft() const { return borderLeft() + paddingLeft(); }
>+    LayoutUnit contentBoxTop() const { return borderTop() + paddingTop().value_or(0); }
>+    LayoutUnit contentBoxLeft() const { return borderLeft() + paddingLeft().value_or(0); }
>     LayoutUnit contentBoxBottom() const { return contentBoxTop() + contentBoxHeight(); }
>     LayoutUnit contentBoxRight() const { return contentBoxLeft() + contentBoxWidth(); }
>     LayoutUnit contentBoxHeight() const;
>@@ -186,7 +186,7 @@ private:
>     void setVerticalNonCollapsedMargin(Layout::VerticalEdges);
> 
>     void setBorder(Layout::Edges);
>-    void setPadding(Layout::Edges);
>+    void setPadding(std::optional<Layout::Edges>);
> 
> #if !ASSERT_DISABLED
>     void invalidateMargin();
>@@ -214,7 +214,7 @@ private:
>     Layout::VerticalEdges m_verticalNonCollapsedMargin;
> 
>     Layout::Edges m_border;
>-    Layout::Edges m_padding;
>+    std::optional<Layout::Edges> m_padding;
> 
> #if !ASSERT_DISABLED
>     bool m_hasValidHorizontalMargin { false };
>@@ -476,7 +476,7 @@ inline void Box::setBorder(Layout::Edges border)
>     m_border = border;
> }
> 
>-inline void Box::setPadding(Layout::Edges padding)
>+inline void Box::setPadding(std::optional<Layout::Edges> padding)
> {
> #if !ASSERT_DISABLED
>     setHasValidPadding();
>@@ -520,28 +520,36 @@ inline LayoutUnit Box::nonCollapsedMarginBottom() const
>     return m_verticalNonCollapsedMargin.bottom;
> }
> 
>-inline LayoutUnit Box::paddingTop() const
>+inline std::optional<LayoutUnit> Box::paddingTop() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.vertical.top;
>+    if (!m_padding)
>+        return { };
>+    return m_padding->vertical.top;
> }
> 
>-inline LayoutUnit Box::paddingLeft() const
>+inline std::optional<LayoutUnit> Box::paddingLeft() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.horizontal.left;
>+    if (!m_padding)
>+        return { };
>+    return m_padding->horizontal.left;
> }
> 
>-inline LayoutUnit Box::paddingBottom() const
>+inline std::optional<LayoutUnit> Box::paddingBottom() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.vertical.bottom;
>+    if (!m_padding)
>+        return { };
>+    return m_padding->vertical.bottom;
> }
> 
>-inline LayoutUnit Box::paddingRight() const
>+inline std::optional<LayoutUnit> Box::paddingRight() const
> {
>     ASSERT(m_hasValidPadding);
>-    return m_padding.horizontal.right;
>+    if (!m_padding)
>+        return { };
>+    return m_padding->horizontal.right;
> }
> 
> inline LayoutUnit Box::borderTop() const

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188630: 347241