[LayoutNG] Revert some flex patches because they regressed legacy

32758180 regressed but
84c273ae depends on it so they both have
to go.

I will fix after the 79 branch.


Revert "[LayoutNG] Implement flex-wrap:wrap-reverse"

This reverts commit 84c273ae.

Revert "[LayoutNG] Implement flexbox item alignment"

This reverts commit 32758180.

Bug: 845235, 1015475
Change-Id: I24a20d54e79608650426130e2ec106e35bdfde63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866434Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707089}

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

@@ -261,54 +261,6 @@ void FlexItem::ComputeStretchedSize() {
   }
 }
 
-// static
-LayoutUnit FlexItem::AlignmentOffset(LayoutUnit available_free_space,
-                                     ItemPosition position,
-                                     LayoutUnit ascent,
-                                     LayoutUnit max_ascent,
-                                     bool is_wrap_reverse,
-                                     bool is_deprecated_webkit_box) {
-  switch (position) {
-    case ItemPosition::kLegacy:
-    case ItemPosition::kAuto:
-    case ItemPosition::kNormal:
-      NOTREACHED();
-      break;
-    case ItemPosition::kStretch:
-      // Actual stretching must be handled by the caller. Since wrap-reverse
-      // flips cross start and cross end, stretch children should be aligned
-      // with the cross end. This matters because applyStretchAlignment
-      // doesn't always stretch or stretch fully (explicit cross size given, or
-      // stretching constrained by max-height/max-width). For flex-start and
-      // flex-end this is handled by alignmentForChild().
-      if (is_wrap_reverse)
-        return available_free_space;
-      break;
-    case ItemPosition::kFlexStart:
-      break;
-    case ItemPosition::kFlexEnd:
-      return available_free_space;
-    case ItemPosition::kCenter: {
-      const LayoutUnit result = (available_free_space / 2);
-      return is_deprecated_webkit_box ? result.ClampNegativeToZero() : result;
-    }
-    case ItemPosition::kBaseline:
-      // FIXME: If we get here in columns, we want the use the descent, except
-      // we currently can't get the ascent/descent of orthogonal children.
-      // https://bugs.webkit.org/show_bug.cgi?id=98076
-      return max_ascent - ascent;
-    case ItemPosition::kLastBaseline:
-    case ItemPosition::kSelfStart:
-    case ItemPosition::kSelfEnd:
-    case ItemPosition::kStart:
-    case ItemPosition::kEnd:
-    case ItemPosition::kLeft:
-    case ItemPosition::kRight:
-      // TODO(jferanndez): Implement these (https://crbug.com/722287).
-      break;
-  }
-  return LayoutUnit();
-}
 void FlexLine::FreezeViolations(ViolationsVector& violations) {
   const ComputedStyle& flex_box_style = algorithm->StyleRef();
   for (size_t i = 0; i < violations.size(); ++i) {
@@ -702,77 +654,6 @@ void FlexLayoutAlgorithm::AlignFlexLines(LayoutUnit cross_axis_content_extent) {
   }
 }
 
-void FlexLayoutAlgorithm::AlignChildren() {
-  // Keep track of the space between the baseline edge and the after edge of
-  // the box for each line.
-  Vector<LayoutUnit> min_margin_after_baselines;
-
-  for (FlexLine& line_context : flex_lines_) {
-    LayoutUnit min_margin_after_baseline = LayoutUnit::Max();
-    LayoutUnit max_ascent = line_context.max_ascent;
-
-    for (FlexItem& flex_item : line_context.line_items) {
-      DCHECK(!flex_item.box->IsOutOfFlowPositioned());
-
-      if (flex_item.UpdateAutoMarginsInCrossAxis(
-              std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace()))) {
-        continue;
-      }
-
-      ItemPosition position = flex_item.Alignment();
-      if (position == ItemPosition::kStretch)
-        flex_item.ComputeStretchedSize();
-      LayoutUnit available_space = flex_item.AvailableAlignmentSpace();
-      LayoutUnit offset = FlexItem::AlignmentOffset(
-          available_space, position, flex_item.MarginBoxAscent(), max_ascent,
-          StyleRef().FlexWrap() == EFlexWrap::kWrapReverse,
-          StyleRef().IsDeprecatedWebkitBox());
-      flex_item.desired_location.Move(LayoutUnit(), offset);
-      if (position == ItemPosition::kBaseline &&
-          StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
-        min_margin_after_baseline =
-            std::min(min_margin_after_baseline,
-                     flex_item.AvailableAlignmentSpace() - offset);
-      }
-    }
-    min_margin_after_baselines.push_back(min_margin_after_baseline);
-  }
-
-  if (StyleRef().FlexWrap() != EFlexWrap::kWrapReverse)
-    return;
-
-  // wrap-reverse flips the cross axis start and end. For baseline alignment,
-  // this means we need to align the after edge of baseline elements with the
-  // after edge of the flex line.
-  wtf_size_t line_number = 0;
-  for (FlexLine& line_context : flex_lines_) {
-    LayoutUnit min_margin_after_baseline =
-        min_margin_after_baselines[line_number++];
-    for (FlexItem& flex_item : line_context.line_items) {
-      if (flex_item.Alignment() == ItemPosition::kBaseline &&
-          !flex_item.HasAutoMarginsInCrossAxis() && min_margin_after_baseline) {
-        flex_item.desired_location.Move(LayoutUnit(),
-                                        min_margin_after_baseline);
-      }
-    }
-  }
-}
-
-void FlexLayoutAlgorithm::FlipForWrapReverse(
-    LayoutUnit cross_axis_start_edge,
-    LayoutUnit cross_axis_content_size) {
-  DCHECK_EQ(Style()->FlexWrap(), EFlexWrap::kWrapReverse);
-  for (FlexLine& line_context : flex_lines_) {
-    LayoutUnit original_offset =
-        line_context.cross_axis_offset - cross_axis_start_edge;
-    LayoutUnit new_offset = cross_axis_content_size - original_offset -
-                            line_context.cross_axis_extent;
-    LayoutUnit wrap_reverse_difference = new_offset - original_offset;
-    for (FlexItem& flex_item : line_context.line_items)
-      flex_item.desired_location.Move(LayoutUnit(), wrap_reverse_difference);
-  }
-}
-
 TransformedWritingMode FlexLayoutAlgorithm::GetTransformedWritingMode() const {
   return GetTransformedWritingMode(*style_);
 }

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

@@ -171,13 +171,6 @@ class FlexItem {
 
   inline const FlexLine* Line() const;
 
-  static LayoutUnit AlignmentOffset(LayoutUnit available_free_space,
-                                    ItemPosition position,
-                                    LayoutUnit ascent,
-                                    LayoutUnit max_ascent,
-                                    bool is_wrap_reverse,
-                                    bool is_deprecated_webkit_box);
-
   FlexLayoutAlgorithm* algorithm;
   wtf_size_t line_number;
   LayoutBox* box;
@@ -385,13 +378,6 @@ class FlexLayoutAlgorithm {
   // FlexLine::cross_axis_extent.
   void AlignFlexLines(LayoutUnit cross_axis_content_extent);
 
-  // Positions flex items by modifying FlexItem::desired_location.
-  // When lines stretch, also modifies FlexItem::cross_axis_size.
-  void AlignChildren();
-
-  void FlipForWrapReverse(LayoutUnit cross_axis_start_edge,
-                          LayoutUnit cross_axis_content_size);
-
   static TransformedWritingMode GetTransformedWritingMode(const ComputedStyle&);
 
   static const StyleContentAlignmentData& ContentAlignmentNormalBehavior();

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

@@ -416,16 +416,10 @@ void LayoutFlexibleBox::RepositionLogicalHeightDependentFlexItems(
 
   AlignFlexLines(algorithm);
 
-  AlignChildren(algorithm);
+  AlignChildren(line_contexts);
 
-  if (StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
-    algorithm.FlipForWrapReverse(cross_axis_start_edge,
-                                 CrossAxisContentExtent());
-    for (FlexLine& line_context : line_contexts) {
-      for (FlexItem& flex_item : line_context.line_items)
-        ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
-    }
-  }
+  if (StyleRef().FlexWrap() == EFlexWrap::kWrapReverse)
+    FlipForWrapReverse(line_contexts, cross_axis_start_edge);
 
   // direction:rtl + flex-direction:column means the cross-axis direction is
   // flipped.
@@ -1204,6 +1198,54 @@ void LayoutFlexibleBox::ConstructAndAppendFlexItem(
                           border_and_padding, margin);
 }
 
+static LayoutUnit AlignmentOffset(LayoutUnit available_free_space,
+                                  ItemPosition position,
+                                  LayoutUnit ascent,
+                                  LayoutUnit max_ascent,
+                                  bool is_wrap_reverse,
+                                  bool is_deprecated_webkit_box) {
+  switch (position) {
+    case ItemPosition::kLegacy:
+    case ItemPosition::kAuto:
+    case ItemPosition::kNormal:
+      NOTREACHED();
+      break;
+    case ItemPosition::kStretch:
+      // Actual stretching must be handled by the caller. Since wrap-reverse
+      // flips cross start and cross end, stretch children should be aligned
+      // with the cross end. This matters because applyStretchAlignment
+      // doesn't always stretch or stretch fully (explicit cross size given, or
+      // stretching constrained by max-height/max-width). For flex-start and
+      // flex-end this is handled by alignmentForChild().
+      if (is_wrap_reverse)
+        return available_free_space;
+      break;
+    case ItemPosition::kFlexStart:
+      break;
+    case ItemPosition::kFlexEnd:
+      return available_free_space;
+    case ItemPosition::kCenter: {
+      const LayoutUnit result = (available_free_space / 2);
+      return is_deprecated_webkit_box ? result.ClampNegativeToZero() : result;
+    }
+    case ItemPosition::kBaseline:
+      // FIXME: If we get here in columns, we want the use the descent, except
+      // we currently can't get the ascent/descent of orthogonal children.
+      // https://bugs.webkit.org/show_bug.cgi?id=98076
+      return max_ascent - ascent;
+    case ItemPosition::kLastBaseline:
+    case ItemPosition::kSelfStart:
+    case ItemPosition::kSelfEnd:
+    case ItemPosition::kStart:
+    case ItemPosition::kEnd:
+    case ItemPosition::kLeft:
+    case ItemPosition::kRight:
+      // TODO(jferanndez): Implement these (https://crbug.com/722287).
+      break;
+  }
+  return LayoutUnit();
+}
+
 void LayoutFlexibleBox::SetOverrideMainAxisContentSizeForChild(FlexItem& item) {
   if (MainAxisIsInlineAxis(*item.box)) {
     item.box->SetOverrideLogicalWidth(item.FlexedBorderBoxSize());
@@ -1231,7 +1273,7 @@ LayoutUnit LayoutFlexibleBox::StaticCrossAxisPositionForPositionedChild(
     const LayoutBox& child) {
   LayoutUnit available_space =
       CrossAxisContentExtent() - CrossAxisExtentForChild(child);
-  return FlexItem::AlignmentOffset(
+  return AlignmentOffset(
       available_space,
       FlexLayoutAlgorithm::AlignmentForChild(StyleRef(), child.StyleRef()),
       LayoutUnit(), LayoutUnit(),
@@ -1535,17 +1577,60 @@ void LayoutFlexibleBox::ResetAlignmentForChild(
       child, {FlowAwareLocationForChild(child).X(), new_cross_axis_position});
 }
 
-void LayoutFlexibleBox::AlignChildren(FlexLayoutAlgorithm& algorithm) {
-  Vector<FlexLine>& line_contexts = algorithm.FlexLines();
+void LayoutFlexibleBox::AlignChildren(Vector<FlexLine>& line_contexts) {
+  // Keep track of the space between the baseline edge and the after edge of
+  // the box for each line.
+  // TODO(cbiesinger): This should be stored in FlexLine
+  Vector<LayoutUnit> min_margin_after_baselines;
+
+  for (FlexLine& line_context : line_contexts) {
+    LayoutUnit min_margin_after_baseline = LayoutUnit::Max();
+    LayoutUnit max_ascent = line_context.max_ascent;
 
-  algorithm.AlignChildren();
-  for (unsigned line_number = 0; line_number < line_contexts.size();
-       ++line_number) {
-    FlexLine& line_context = line_contexts[line_number];
     for (FlexItem& flex_item : line_context.line_items) {
-      if (flex_item.Alignment() == ItemPosition::kStretch)
+      DCHECK(!flex_item.box->IsOutOfFlowPositioned());
+
+      if (flex_item.UpdateAutoMarginsInCrossAxis(
+              std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace()))) {
+        ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
+        continue;
+      }
+
+      ItemPosition position = flex_item.Alignment();
+      if (position == ItemPosition::kStretch) {
+        flex_item.ComputeStretchedSize();
         ApplyStretchAlignmentToChild(flex_item);
-      ResetAlignmentForChild(*flex_item.box, flex_item.desired_location.Y());
+      }
+      LayoutUnit available_space = flex_item.AvailableAlignmentSpace();
+      LayoutUnit offset = AlignmentOffset(
+          available_space, position, flex_item.MarginBoxAscent(), max_ascent,
+          StyleRef().FlexWrap() == EFlexWrap::kWrapReverse,
+          StyleRef().IsDeprecatedWebkitBox());
+      AdjustAlignmentForChild(*flex_item.box, offset);
+      if (position == ItemPosition::kBaseline &&
+          StyleRef().FlexWrap() == EFlexWrap::kWrapReverse) {
+        min_margin_after_baseline =
+            std::min(min_margin_after_baseline,
+                     flex_item.AvailableAlignmentSpace() - offset);
+      }
+    }
+    min_margin_after_baselines.push_back(min_margin_after_baseline);
+  }
+
+  if (StyleRef().FlexWrap() != EFlexWrap::kWrapReverse)
+    return;
+
+  // wrap-reverse flips the cross axis start and end. For baseline alignment,
+  // this means we need to align the after edge of baseline elements with the
+  // after edge of the flex line.
+  wtf_size_t line_number = 0;
+  for (FlexLine& line_context : line_contexts) {
+    LayoutUnit min_margin_after_baseline =
+        min_margin_after_baselines[line_number++];
+    for (FlexItem& flex_item : line_context.line_items) {
+      if (flex_item.Alignment() == ItemPosition::kBaseline &&
+          !flex_item.HasAutoMarginsInCrossAxis() && min_margin_after_baseline)
+        AdjustAlignmentForChild(*flex_item.box, min_margin_after_baseline);
     }
   }
 }
@@ -1599,4 +1684,20 @@ void LayoutFlexibleBox::FlipForRightToLeftColumn(
   }
 }
 
+void LayoutFlexibleBox::FlipForWrapReverse(
+    const Vector<FlexLine>& line_contexts,
+    LayoutUnit cross_axis_start_edge) {
+  LayoutUnit content_extent = CrossAxisContentExtent();
+  for (const FlexLine& line_context : line_contexts) {
+    for (const FlexItem& flex_item : line_context.line_items) {
+      LayoutUnit line_cross_axis_extent = line_context.cross_axis_extent;
+      LayoutUnit original_offset =
+          line_context.cross_axis_offset - cross_axis_start_edge;
+      LayoutUnit new_offset =
+          content_extent - original_offset - line_cross_axis_extent;
+      AdjustAlignmentForChild(*flex_item.box, new_offset - original_offset);
+    }
+  }
+}
+
 }  // namespace blink

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

@@ -201,9 +201,11 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock {
                            LayoutUnit cross_axis_offset,
                            LayoutUnit available_free_space);
   void AlignFlexLines(FlexLayoutAlgorithm&);
-  void AlignChildren(FlexLayoutAlgorithm&);
+  void AlignChildren(Vector<FlexLine>&);
   void ApplyStretchAlignmentToChild(FlexItem& child);
   void FlipForRightToLeftColumn(const Vector<FlexLine>& line_contexts);
+  void FlipForWrapReverse(const Vector<FlexLine>&,
+                          LayoutUnit cross_axis_start_edge);
 
   float CountIntrinsicSizeForAlgorithmChange(
       LayoutUnit max_preferred_width,

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

@@ -527,72 +527,60 @@ scoped_refptr<const NGLayoutResult> NGFlexLayoutAlgorithm::Layout() {
   return container_builder_.ToBoxFragment();
 }
 
-void NGFlexLayoutAlgorithm::ApplyStretchAlignmentToChild(FlexItem& flex_item) {
-  WritingMode child_writing_mode =
-      flex_item.ng_input_node.Style().GetWritingMode();
-  NGConstraintSpaceBuilder space_builder(ConstraintSpace(), child_writing_mode,
-                                         /* is_new_fc */ true);
-  SetOrthogonalFallbackInlineSizeIfNeeded(Style(), flex_item.ng_input_node,
-                                          &space_builder);
-
-  LogicalSize available_size(
-      flex_item.flexed_content_size + flex_item.main_axis_border_padding,
-      flex_item.cross_axis_size);
-  if (is_column_) {
-    available_size.Transpose();
-    if (!IsColumnContainerMainSizeDefinite() &&
-        !IsItemMainSizeDefinite(flex_item.ng_input_node)) {
-      space_builder.SetIsFixedBlockSizeIndefinite(true);
-    }
-  }
-  space_builder.SetAvailableSize(available_size);
-  space_builder.SetPercentageResolutionSize(content_box_size_);
-  space_builder.SetIsFixedInlineSize(true);
-  space_builder.SetIsFixedBlockSize(true);
-  NGConstraintSpace child_space = space_builder.ToConstraintSpace();
-  flex_item.layout_result =
-      flex_item.ng_input_node.Layout(child_space, /* break_token */ nullptr);
-}
-
 void NGFlexLayoutAlgorithm::GiveLinesAndItemsFinalPositionAndSize() {
-  Vector<FlexLine>& line_contexts = algorithm_->FlexLines();
-  const LayoutUnit cross_axis_start_edge =
-      line_contexts.IsEmpty() ? LayoutUnit()
-                              : line_contexts[0].cross_axis_offset;
-  const LayoutUnit final_content_cross_size =
+  // TODO(dgrogan): This needs to eventually encompass all of the behavior in
+  // LayoutFlexibleBox::RepositionLogicalHeightDependentFlexItems. It currently
+  // does AlignFlexLines and the stretch part of AlignChildren.
+  LayoutUnit final_content_cross_size =
       is_column_ ? container_builder_.InlineSize() -
                        border_scrollbar_padding_.InlineSum()
                  : container_builder_.BlockSize() -
                        border_scrollbar_padding_.BlockSum();
-
-  // TODO(dgrogan): Implement the behavior from
-  // LayoutFlexibleBox::LayoutColumnReverse here.
-
-  if (!algorithm_->IsMultiline() && !line_contexts.IsEmpty())
-    line_contexts[0].cross_axis_extent = final_content_cross_size;
+  if (!algorithm_->IsMultiline() && !algorithm_->FlexLines().IsEmpty())
+    algorithm_->FlexLines()[0].cross_axis_extent = final_content_cross_size;
 
   algorithm_->AlignFlexLines(final_content_cross_size);
 
-  algorithm_->AlignChildren();
-
-  if (Style().FlexWrap() == EFlexWrap::kWrapReverse) {
-    // flex-wrap: wrap-reverse reverses the order of the lines in the container;
-    // FlipForWrapReverse recalculates each item's cross axis position. We have
-    // to do that after AlignChildren sets an initial cross axis position.
-    algorithm_->FlipForWrapReverse(cross_axis_start_edge,
-                                   final_content_cross_size);
-  }
-
-  for (FlexLine& line_context : line_contexts) {
+  for (FlexLine& line_context : algorithm_->FlexLines()) {
     for (wtf_size_t child_number = 0;
          child_number < line_context.line_items.size(); ++child_number) {
       FlexItem& flex_item = line_context.line_items[child_number];
 
-      if (DoesItemStretch(flex_item.ng_input_node))
-        ApplyStretchAlignmentToChild(flex_item);
-
-      // TODO(dgrogan): Implement behavior from legacy's
-      // FlipForRightToLeftColumn here.
+      // UpdateAutoMarginsInCrossAxis updates the flex_item's desired_location
+      // if the auto margins have an effect.
+      if (!flex_item.UpdateAutoMarginsInCrossAxis(
+              std::max(LayoutUnit(), flex_item.AvailableAlignmentSpace())) &&
+          flex_item.Alignment() == ItemPosition::kStretch) {
+        flex_item.ComputeStretchedSize();
+
+        WritingMode child_writing_mode =
+            flex_item.ng_input_node.Style().GetWritingMode();
+        NGConstraintSpaceBuilder space_builder(ConstraintSpace(),
+                                               child_writing_mode,
+                                               /* is_new_fc */ true);
+        SetOrthogonalFallbackInlineSizeIfNeeded(
+            Style(), flex_item.ng_input_node, &space_builder);
+
+        LogicalSize available_size(
+            flex_item.flexed_content_size + flex_item.main_axis_border_padding,
+            flex_item.cross_axis_size);
+        if (is_column_) {
+          available_size.Transpose();
+          if (!IsColumnContainerMainSizeDefinite() &&
+              !IsItemMainSizeDefinite(flex_item.ng_input_node)) {
+            space_builder.SetIsFixedBlockSizeIndefinite(true);
+          }
+        }
+        space_builder.SetAvailableSize(available_size);
+        space_builder.SetPercentageResolutionSize(content_box_size_);
+        space_builder.SetIsFixedInlineSize(true);
+        space_builder.SetIsFixedBlockSize(true);
+        NGConstraintSpace child_space = space_builder.ToConstraintSpace();
+        flex_item.layout_result = flex_item.ng_input_node.Layout(
+            child_space, /* break_token */ nullptr);
+      }
+      // TODO(dgrogan): Add an extra pass for kColumnReverse containers like
+      // legacy does in LayoutColumnReverse.
 
       // flex_item.desired_location stores the main axis offset in X and the
       // cross axis offset in Y. But AddChild wants offset from parent

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

@@ -43,7 +43,6 @@ class CORE_EXPORT NGFlexLayoutAlgorithm
   NGConstraintSpace BuildConstraintSpaceForDeterminingFlexBasis(
       const NGBlockNode& flex_item) const;
   void ConstructAndAppendFlexItems();
-  void ApplyStretchAlignmentToChild(FlexItem& flex_item);
   void GiveLinesAndItemsFinalPositionAndSize();
   // This is same method as FlexItem but we need that logic before FlexItem is
   // constructed.