[FlexNG] Introduce FlexItem::physical_margins.

Previously we'd store intermediate calculations of margins directly on
a FlexItem's box.

This instead stores the intermediate margin calculations on the
FlexItem directly.

This is primarily just a refactor / cleanup. (The existing code was
fine, not causing any bugs/issues), but was somewhat confusing from a
LayoutNG p.o.v. as transient calculations shouldn't be stored directly
on the LayoutBox.

Change-Id: Ia37b3837ecbbfbd3f61c19571536fc8484d85d54
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1977196
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729572}

third_party/blink/renderer/core/layout/flexible_box_algorithm.h

@@ -116,21 +116,24 @@ class FlexItem {
   // - |min_max_sizes| is the resolved min and max size properties in the
   //   main axis direction (not intrinsic widths). It does not include
   //   border/scrollbar/padding.
-  FlexItem(LayoutBox*,
+  FlexItem(const FlexLayoutAlgorithm*,
+           LayoutBox*,
+           const ComputedStyle& style,
            LayoutUnit flex_base_content_size,
            MinMaxSize min_max_main_axis_sizes,
            // Ignored for legacy, required for NG:
            base::Optional<MinMaxSize> min_max_cross_axis_sizes,
            LayoutUnit main_axis_border_padding,
-           LayoutUnit main_axis_margin);
+           NGPhysicalBoxStrut physical_margins);
 
   LayoutUnit HypotheticalMainAxisMarginBoxSize() const {
     return hypothetical_main_content_size + main_axis_border_padding +
-           main_axis_margin;
+           MainAxisMarginExtent();
   }
 
   LayoutUnit FlexBaseMarginBoxSize() const {
-    return flex_base_content_size + main_axis_border_padding + main_axis_margin;
+    return flex_base_content_size + main_axis_border_padding +
+           MainAxisMarginExtent();
   }
 
   LayoutUnit FlexedBorderBoxSize() const {
@@ -138,7 +141,8 @@ class FlexItem {
   }
 
   LayoutUnit FlexedMarginBoxSize() const {
-    return flexed_content_size + main_axis_border_padding + main_axis_margin;
+    return flexed_content_size + main_axis_border_padding +
+           MainAxisMarginExtent();
   }
 
   LayoutUnit ClampSizeToMinAndMax(LayoutUnit size) const {
@@ -152,6 +156,8 @@ class FlexItem {
   LayoutUnit FlowAwareMarginStart() const;
   LayoutUnit FlowAwareMarginEnd() const;
   LayoutUnit FlowAwareMarginBefore() const;
+
+  LayoutUnit MainAxisMarginExtent() const;
   LayoutUnit CrossAxisMarginExtent() const;
 
   LayoutUnit MarginBoxAscent() const;
@@ -178,15 +184,17 @@ class FlexItem {
                                     bool is_wrap_reverse,
                                     bool is_deprecated_webkit_box);
 
-  FlexLayoutAlgorithm* algorithm;
+  const FlexLayoutAlgorithm* algorithm;
   wtf_size_t line_number;
   LayoutBox* box;
+  const ComputedStyle& style;
   const LayoutUnit flex_base_content_size;
   const MinMaxSize min_max_sizes;
   const base::Optional<MinMaxSize> min_max_cross_sizes;
   const LayoutUnit hypothetical_main_content_size;
   const LayoutUnit main_axis_border_padding;
-  const LayoutUnit main_axis_margin;
+  NGPhysicalBoxStrut physical_margins;
+
   LayoutUnit flexed_content_size;
 
   // When set by the caller, this should be the size pre-stretching.
@@ -352,14 +360,13 @@ class FlexLayoutAlgorithm {
 
   template <typename... Args>
   FlexItem& emplace_back(Args&&... args) {
-    FlexItem& item = all_items_.emplace_back(std::forward<Args>(args)...);
-    item.algorithm = this;
-    return item;
+    return all_items_.emplace_back(this, std::forward<Args>(args)...);
   }
 
   const ComputedStyle* Style() const { return style_; }
   const ComputedStyle& StyleRef() const { return *style_; }
 
+  const Vector<FlexLine>& FlexLines() const { return flex_lines_; }
   Vector<FlexLine>& FlexLines() { return flex_lines_; }
 
   // Computes the next flex line, stores it in FlexLines(), and returns a

third_party/blink/renderer/core/layout/layout_flexible_box.cc

@@ -1018,19 +1018,11 @@ void LayoutFlexibleBox::PrepareOrderIteratorAndMargins() {
       continue;
 
     // Before running the flex algorithm, 'auto' has a margin of 0.
-    // Also, if we're not auto sizing, we don't do a layout that computes the
-    // start/end margins.
-    if (IsHorizontalFlow()) {
-      child->SetMarginLeft(
-          ComputeChildMarginValue(child->StyleRef().MarginLeft()));
-      child->SetMarginRight(
-          ComputeChildMarginValue(child->StyleRef().MarginRight()));
-    } else {
-      child->SetMarginTop(
-          ComputeChildMarginValue(child->StyleRef().MarginTop()));
-      child->SetMarginBottom(
-          ComputeChildMarginValue(child->StyleRef().MarginBottom()));
-    }
+    const ComputedStyle& style = child->StyleRef();
+    child->SetMarginTop(ComputeChildMarginValue(style.MarginTop()));
+    child->SetMarginRight(ComputeChildMarginValue(style.MarginRight()));
+    child->SetMarginBottom(ComputeChildMarginValue(style.MarginBottom()));
+    child->SetMarginLeft(ComputeChildMarginValue(style.MarginLeft()));
   }
 }
 
@@ -1213,11 +1205,12 @@ void LayoutFlexibleBox::ConstructAndAppendFlexItem(
   MinMaxSize sizes =
       ComputeMinAndMaxSizesForChild(*algorithm, child, border_and_padding);
 
-  LayoutUnit margin =
-      IsHorizontalFlow() ? child.MarginWidth() : child.MarginHeight();
-  algorithm->emplace_back(&child, child_inner_flex_base_size, sizes,
+  NGPhysicalBoxStrut physical_margins(child.MarginTop(), child.MarginRight(),
+                                      child.MarginBottom(), child.MarginLeft());
+  algorithm->emplace_back(&child, child.StyleRef(), child_inner_flex_base_size,
+                          sizes,
                           /* cross axis min max sizes */ base::nullopt,
-                          border_and_padding, margin);
+                          border_and_padding, physical_margins);
 }
 
 void LayoutFlexibleBox::SetOverrideMainAxisContentSizeForChild(FlexItem& item) {
@@ -1454,6 +1447,7 @@ void LayoutFlexibleBox::ApplyLineItemsPosition(FlexLine* current_line) {
     const FlexItem& flex_item = current_line->line_items[i];
     LayoutBox* child = flex_item.box;
     SetFlowAwareLocationForChild(*child, flex_item.desired_location);
+    child->SetMargin(flex_item.physical_margins);
 
     if (is_paginated)
       UpdateFragmentationInfoForChild(*child);
@@ -1557,6 +1551,7 @@ void LayoutFlexibleBox::AlignChildren(FlexLayoutAlgorithm& algorithm) {
         flex_item.needs_relayout_for_stretch = false;
       }
       ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
+      flex_item.box->SetMargin(flex_item.physical_margins);
     }
   }
 }

third_party/blink/renderer/core/layout/ng/ng_block_node.cc

@@ -1386,4 +1386,8 @@ void NGBlockNode::StoreMargins(const NGConstraintSpace& constraint_space,
   box_->SetMargin(physical_margins);
 }
 
+void NGBlockNode::StoreMargins(const NGPhysicalBoxStrut& physical_margins) {
+  box_->SetMargin(physical_margins);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/ng_block_node.h

@@ -136,6 +136,7 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
 
   // Write back resolved margins to legacy.
   void StoreMargins(const NGConstraintSpace&, const NGBoxStrut& margins);
+  void StoreMargins(const NGPhysicalBoxStrut& margins);
 
   static bool CanUseNewLayout(const LayoutBox&);
   bool CanUseNewLayout() const;

third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

@@ -404,15 +404,6 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
     LayoutUnit flex_base_content_size =
         flex_base_border_box - main_axis_border_padding;
 
-    NGPhysicalBoxStrut physical_child_margins =
-        ComputePhysicalMargins(child_space, child_style);
-    // Set margin because FlexibleBoxAlgorithm reads it from legacy.
-    child.GetLayoutBox()->SetMargin(physical_child_margins);
-
-    LayoutUnit main_axis_margin = is_horizontal_flow_
-                                      ? physical_child_margins.HorizontalSum()
-                                      : physical_child_margins.VerticalSum();
-
     MinMaxSize min_max_sizes_in_main_axis_direction{LayoutUnit(),
                                                     LayoutUnit::Max()};
     MinMaxSize min_max_sizes_in_cross_axis_direction{LayoutUnit(),
@@ -522,11 +513,15 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
     }
     // TODO(dgrogan): Should this include scrollbar?
     min_max_sizes_in_main_axis_direction -= main_axis_border_scrollbar_padding;
+
+    NGPhysicalBoxStrut physical_child_margins =
+        ComputePhysicalMargins(child_space, child_style);
     algorithm_
-        ->emplace_back(child.GetLayoutBox(), flex_base_content_size,
+        ->emplace_back(child.GetLayoutBox(), child.Style(),
+                       flex_base_content_size,
                        min_max_sizes_in_main_axis_direction,
                        min_max_sizes_in_cross_axis_direction,
-                       main_axis_border_padding, main_axis_margin)
+                       main_axis_border_padding, physical_child_margins)
         .ng_input_node = child;
   }
 }
@@ -767,6 +762,7 @@ void NGFlexLayoutAlgorithm::GiveLinesAndItemsFinalPositionAndSize() {
                                  : flex_item.desired_location;
       container_builder_.AddChild(flex_item.layout_result->PhysicalFragment(),
                                   {location.X(), location.Y()});
+      flex_item.ng_input_node.StoreMargins(flex_item.physical_margins);
     }
   }
 }