Attachment 360069 Details for Bug 193807 – Patch

[patch] Patch

bug-193807-20190124191804.patch (text/plain), 9.42 KB, created by zalan on 2019-01-24 19:18:12 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-24 19:18:12 PST

Size: 9.42 KB

patch

obsolete

>Subversion Revision: 240436
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 674fa72dd77dcce79c0a8f4282574e2c4040610c..2c031d1da6103ae6b44548fffc8a07b1ca6147e7 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-01-24  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC][MarginCollapsing] Refactor MarginCollapse::updateCollapsedMarginAfter
>+        https://bugs.webkit.org/show_bug.cgi?id=193807
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rename updateCollapsedMarginAfter to updateMarginAfterForPreviousSibling and make the margin updating logic more explicit. 
>+
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
>+        * layout/blockformatting/BlockFormattingContext.h:
>+        * layout/blockformatting/BlockMarginCollapse.cpp:
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::updateMarginAfterForPreviousSibling):
>+        (WebCore::Layout::BlockFormattingContext::MarginCollapse::updateCollapsedMarginAfter): Deleted.
>+
> 2019-01-24  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] MarginCollapse::collapsedVerticalValues should not return computed non-collapsed values.
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index b7fae7213e8b81809716ee78540f807c33d55667..341f0509d59b65e66470b721260330e434e9af8e 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -429,12 +429,10 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
>     ASSERT(!hasEstimatedMarginBefore(layoutBox) || estimatedMarginBefore(layoutBox).usedValue() == verticalMargin.before());
>     removeEstimatedMarginBefore(layoutBox);
>     displayBox.setTop(adjustedVerticalPositionAfterMarginCollapsing(layoutBox, verticalMargin));
>-    // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed.
>-    if (MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
>-        MarginCollapse::updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin);
>-
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>     displayBox.setVerticalMargin(verticalMargin);
>+    // Adjust the previous sibling's margin bottom now that this box's vertical margin is computed.
>+    MarginCollapse::updateMarginAfterForPreviousSibling(layoutState, layoutBox);
> }
> 
> FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>index c3e58f32a9365951cc72401eefc9c3009c36dc7d..e0543d14891e8db4b7c67849e01d70af2c2880c8 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.h
>@@ -96,7 +96,7 @@ private:
> 
>         static EstimatedMarginBefore estimatedMarginBefore(const LayoutState&, const Box&);
>         static LayoutUnit marginBeforeIgnoringCollapsingThrough(const LayoutState&, const Box&, const UsedVerticalMargin::NonCollapsedValues&);
>-        static void updateCollapsedMarginAfter(const LayoutState&, const Box&, const UsedVerticalMargin& nextSiblingVerticalMargin);
>+        static void updateMarginAfterForPreviousSibling(const LayoutState&, const Box&);
> 
>         static bool marginBeforeCollapsesWithParentMarginBefore(const LayoutState&, const Box&);
>         static bool marginBeforeCollapsesWithFirstInFlowChildMarginBefore(const LayoutState&, const Box&);
>diff --git a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>index aeb12342960bc5833f19bcf0d08f648c8a55ea7d..035a76426e557997438a6b4f437991545b86e948 100644
>--- a/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp
>@@ -481,7 +481,7 @@ static Optional<LayoutUnit> marginValue(PositiveAndNegativeVerticalMargin::Value
>     return *marginValues.positive + *marginValues.negative;
> }
> 
>-void BlockFormattingContext::MarginCollapse::updateCollapsedMarginAfter(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin& nextSiblingVerticalMargin)
>+void BlockFormattingContext::MarginCollapse::updateMarginAfterForPreviousSibling(const LayoutState& layoutState, const Box& layoutBox)
> {
>     // 1. Get the margin before value from the next in-flow sibling. This is the same as this box's margin after value now since they are collapsed.
>     // 2. Update the collapsed margin after value as well as the positive/negative cache.
>@@ -489,40 +489,41 @@ void BlockFormattingContext::MarginCollapse::updateCollapsedMarginAfter(const La
>     // 4. If so, update the collapsed margin before value as well as the positive/negative cache.
>     // 5. In case of collapsed through margins check if the before margin collapes with the previous inflow sibling's after margin.
>     // 6. If so, jump to #2.
>-    if (!marginAfterCollapsesWithNextSiblingMarginBefore(layoutState, layoutBox))
>+    // 7. No need to propagate to parent because its margin is not computed yet (estimated at most) 
>+    if (!marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
>         return;
> 
>-    auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>-    auto verticalMargin = displayBox.verticalMargin();
>-    auto verticalMarginAfter = nextSiblingVerticalMargin.before();
>-    Optional<LayoutUnit> verticalMarginBefore;
>+    auto& previousSibling = *layoutBox.previousInFlowSibling();
>+    auto& previousSiblingDisplayBox = layoutState.displayBoxForLayoutBox(previousSibling);
>+    auto previousSiblingVerticalMargin = previousSiblingDisplayBox.verticalMargin();
>+    
>+    auto collapsedVerticalMarginBefore = previousSiblingVerticalMargin.collapsedValues().before;
>+    auto collapsedVerticalMarginAfter = layoutState.displayBoxForLayoutBox(layoutBox).verticalMargin().before();
> 
>-    auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox);
>+    auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, previousSibling);
>     if (marginsCollapseThrough)
>-        verticalMarginBefore = verticalMarginAfter; 
>-    else if (verticalMargin.hasCollapsedValues())
>-        verticalMarginBefore = verticalMargin.collapsedValues().before;
>+        collapsedVerticalMarginBefore = collapsedVerticalMarginAfter; 
> 
>-    // Update vertical margin values.
>-    verticalMargin.setCollapsedValues({ verticalMarginBefore, verticalMarginAfter });
>-    displayBox.setVerticalMargin(verticalMargin);
>+    // Update collapsed vertical margin values.
>+    previousSiblingVerticalMargin.setCollapsedValues({ collapsedVerticalMarginBefore, collapsedVerticalMarginAfter });
>+    previousSiblingDisplayBox.setVerticalMargin(previousSiblingVerticalMargin);
> 
>     // Update positive/negative cache.
>-    auto& blockFormattingState = downcast<BlockFormattingState>(layoutState.formattingStateForBox(layoutBox));
>-    auto positiveNegativeMargin = blockFormattingState.positiveAndNegativeVerticalMargin(layoutBox);
>-    auto nextSiblingPositiveNegativeMarginBefore = blockFormattingState.positiveAndNegativeVerticalMargin(*layoutBox.nextInFlowSibling()).before;
>+    auto& blockFormattingState = downcast<BlockFormattingState>(layoutState.formattingStateForBox(previousSibling));
>+    auto previousSiblingPositiveNegativeMargin = blockFormattingState.positiveAndNegativeVerticalMargin(previousSibling);
>+    auto positiveNegativeMarginBefore = blockFormattingState.positiveAndNegativeVerticalMargin(layoutBox).before;
> 
>-    positiveNegativeMargin.after = computedPositiveAndNegativeMargin(nextSiblingPositiveNegativeMarginBefore, positiveNegativeMargin.after);
>+    previousSiblingPositiveNegativeMargin.after = computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, previousSiblingPositiveNegativeMargin.after);
>     if (marginsCollapseThrough) {
>-        positiveNegativeMargin.before = computedPositiveAndNegativeMargin(positiveNegativeMargin.before, positiveNegativeMargin.after);
>-        positiveNegativeMargin.after = positiveNegativeMargin.before; 
>+        previousSiblingPositiveNegativeMargin.before = computedPositiveAndNegativeMargin(previousSiblingPositiveNegativeMargin.before, previousSiblingPositiveNegativeMargin.after);
>+        previousSiblingPositiveNegativeMargin.after = previousSiblingPositiveNegativeMargin.before; 
>     }
>-    blockFormattingState.setPositiveAndNegativeVerticalMargin(layoutBox, positiveNegativeMargin);
>+    blockFormattingState.setPositiveAndNegativeVerticalMargin(previousSibling, previousSiblingPositiveNegativeMargin);
> 
>-    if (!marginsCollapseThrough || !marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
>+    if (!marginsCollapseThrough || !marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, previousSibling))
>         return;
> 
>-    updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin);
>+    updateMarginAfterForPreviousSibling(layoutState, previousSibling);
> }
> 
> PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse::positiveNegativeValues(const LayoutState& layoutState, const Box& layoutBox, MarginType marginType)

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193807: 360069