Changeset 240467 in webkit

Timestamp:

Jan 24, 2019 8:14:32 PM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][BFC][MarginCollapsing] Refactor MarginCollapse::updateCollapsedMarginAfter
​https://bugs.webkit.org/show_bug.cgi?id=193807

Reviewed by Simon Fraser.

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/blockformatting/BlockFormattingContext.cpp (modified) (1 diff)
• layout/blockformatting/BlockFormattingContext.h (modified) (1 diff)
• layout/blockformatting/BlockMarginCollapse.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Simon Fraser.
+
+        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  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp

@@ -430 +430 @@
     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);
 }
 

trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h

@@ -97 +97 @@
         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&);

trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp

@@ -482 +482 @@
 }
 
-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.
@@ -490 +490 @@
     // 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))
-        return;
-
-    auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
-    auto verticalMargin = displayBox.verticalMargin();
-    auto verticalMarginAfter = nextSiblingVerticalMargin.before();
-    Optional<LayoutUnit> verticalMarginBefore;
-
-    auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, layoutBox);
-    if (marginsCollapseThrough)
-        verticalMarginBefore = verticalMarginAfter; 
-    else if (verticalMargin.hasCollapsedValues())
-        verticalMarginBefore = verticalMargin.collapsedValues().before;
-
-    // Update vertical margin values.
-    verticalMargin.setCollapsedValues({ verticalMarginBefore, verticalMarginAfter });
-    displayBox.setVerticalMargin(verticalMargin);
-
-    // Update positive/negative cache.
-    auto& blockFormattingState = downcast<BlockFormattingState>(layoutState.formattingStateForBox(layoutBox));
-    auto positiveNegativeMargin = blockFormattingState.positiveAndNegativeVerticalMargin(layoutBox);
-    auto nextSiblingPositiveNegativeMarginBefore = blockFormattingState.positiveAndNegativeVerticalMargin(*layoutBox.nextInFlowSibling()).before;
-
-    positiveNegativeMargin.after = computedPositiveAndNegativeMargin(nextSiblingPositiveNegativeMarginBefore, positiveNegativeMargin.after);
-    if (marginsCollapseThrough) {
-        positiveNegativeMargin.before = computedPositiveAndNegativeMargin(positiveNegativeMargin.before, positiveNegativeMargin.after);
-        positiveNegativeMargin.after = positiveNegativeMargin.before; 
-    }
-    blockFormattingState.setPositiveAndNegativeVerticalMargin(layoutBox, positiveNegativeMargin);
-
-    if (!marginsCollapseThrough || !marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, layoutBox))
-        return;
-
-    updateCollapsedMarginAfter(layoutState, *layoutBox.previousInFlowSibling(), verticalMargin);
+    // 7. No need to propagate to parent because its margin is not computed yet (estimated at most).
+    auto* currentBox = &layoutBox;
+    while (marginBeforeCollapsesWithPreviousSiblingMarginAfter(layoutState, *currentBox)) {
+        auto& previousSibling = *currentBox->previousInFlowSibling();
+        auto& previousSiblingDisplayBox = layoutState.displayBoxForLayoutBox(previousSibling);
+        auto previousSiblingVerticalMargin = previousSiblingDisplayBox.verticalMargin();
+
+        auto collapsedVerticalMarginBefore = previousSiblingVerticalMargin.collapsedValues().before;
+        auto collapsedVerticalMarginAfter = layoutState.displayBoxForLayoutBox(*currentBox).verticalMargin().before();
+
+        auto marginsCollapseThrough = MarginCollapse::marginsCollapseThrough(layoutState, previousSibling);
+        if (marginsCollapseThrough)
+            collapsedVerticalMarginBefore = collapsedVerticalMarginAfter;
+
+        // Update collapsed vertical margin values.
+        previousSiblingVerticalMargin.setCollapsedValues({ collapsedVerticalMarginBefore, collapsedVerticalMarginAfter });
+        previousSiblingDisplayBox.setVerticalMargin(previousSiblingVerticalMargin);
+
+        // Update positive/negative cache.
+        auto& blockFormattingState = downcast<BlockFormattingState>(layoutState.formattingStateForBox(previousSibling));
+        auto previousSiblingPositiveNegativeMargin = blockFormattingState.positiveAndNegativeVerticalMargin(previousSibling);
+        auto positiveNegativeMarginBefore = blockFormattingState.positiveAndNegativeVerticalMargin(*currentBox).before;
+
+        previousSiblingPositiveNegativeMargin.after = computedPositiveAndNegativeMargin(positiveNegativeMarginBefore, previousSiblingPositiveNegativeMargin.after);
+        if (marginsCollapseThrough) {
+            previousSiblingPositiveNegativeMargin.before = computedPositiveAndNegativeMargin(previousSiblingPositiveNegativeMargin.before, previousSiblingPositiveNegativeMargin.after);
+            previousSiblingPositiveNegativeMargin.after = previousSiblingPositiveNegativeMargin.before;
+        }
+        blockFormattingState.setPositiveAndNegativeVerticalMargin(previousSibling, previousSiblingPositiveNegativeMargin);
+
+        if (!marginsCollapseThrough)
+            break;
+
+        currentBox = &previousSibling;
+    }
 }