[css-flexbox] Merge align-content and justify-content handling

third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp

Index: third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
index ae3f5d5196e7e557e76a443cab7ebb0c62449414..23ab0273af242f27c2fd0448d330e473d3b03a06 100644
--- a/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
@@ -1501,33 +1501,36 @@ bool LayoutFlexibleBox::ResolveFlexibleLengths(
   return !total_violation;
 }
 
-static LayoutUnit InitialJustifyContentOffset(
+static LayoutUnit InitialContentPositionOffset(
     LayoutUnit available_free_space,
-    ContentPosition justify_content,
-    ContentDistributionType justify_content_distribution,
-    unsigned number_of_children) {
-  if (justify_content == kContentPositionFlexEnd)
+    ContentPosition content_position,
+    ContentDistributionType content_distribution,
+    unsigned number_of_items) {
+  if (content_position == kContentPositionFlexEnd)
     return available_free_space;
-  if (justify_content == kContentPositionCenter)
+  if (content_position == kContentPositionCenter)
     return available_free_space / 2;
-  if (justify_content_distribution == kContentDistributionSpaceAround) {
-    if (available_free_space > 0 && number_of_children)
-      return available_free_space / (2 * number_of_children);
+  if (content_distribution == kContentDistributionSpaceAround) {
+    if (available_free_space > 0 && number_of_items)
+      return available_free_space / (2 * number_of_items);
 
     return available_free_space / 2;
   }
   return LayoutUnit();
 }
 
-static LayoutUnit JustifyContentSpaceBetweenChildren(
+static LayoutUnit ContentDistributionSpaceBetweenChildren(
+    bool is_align_content,

[jfernandez, 2017/04/21 20:43:21]

I don't like too much the bool argument to change the behavior of the function.
Also, I don't understand why this function should be dependent on the specific
property it handles. 

It's not that justify-content has a different behavior for 'stretch', but it's
an invalid value for that property. We can simply define the behavior of
'stretch' here and let the caller pass the appropriated value. If
justify-content can't have 'stretch' we shouldn't need this conditional clause.

     LayoutUnit available_free_space,
-    ContentDistributionType justify_content_distribution,
-    unsigned number_of_children) {
-  if (available_free_space > 0 && number_of_children > 1) {
-    if (justify_content_distribution == kContentDistributionSpaceBetween)
-      return available_free_space / (number_of_children - 1);
-    if (justify_content_distribution == kContentDistributionSpaceAround)
-      return available_free_space / number_of_children;
+    ContentDistributionType content_distribution,
+    unsigned number_of_items) {
+  if (available_free_space > 0 && number_of_items > 1) {
+    if (content_distribution == kContentDistributionSpaceBetween)
+      return available_free_space / (number_of_items - 1);
+    if (content_distribution == kContentDistributionSpaceAround ||
+        (is_align_content &&

[jfernandez, 2017/04/21 20:43:21]

ditto.

+         content_distribution == kContentDistributionStretch))
+      return available_free_space / number_of_items;
   }
   return LayoutUnit();
 }
@@ -1600,7 +1603,7 @@ LayoutUnit LayoutFlexibleBox::StaticMainAxisPositionForPositionedChild(
       StyleRef().ResolvedJustifyContentDistribution(
           ContentAlignmentNormalBehavior());
   LayoutUnit offset =
-      InitialJustifyContentOffset(available_space, position, distribution, 1);
+      InitialContentPositionOffset(available_space, position, distribution, 1);
   if (StyleRef().FlexDirection() == kFlowRowReverse ||
       StyleRef().FlexDirection() == kFlowColumnReverse)
     offset = available_space - offset;
@@ -1782,7 +1785,7 @@ void LayoutFlexibleBox::LayoutAndPlaceChildren(
       AutoMarginOffsetInMainAxis(children, available_free_space);
   LayoutUnit main_axis_offset =
       FlowAwareBorderStart() + FlowAwarePaddingStart();
-  main_axis_offset += InitialJustifyContentOffset(
+  main_axis_offset += InitialContentPositionOffset(
       available_free_space, position, distribution, children.size());
   if (Style()->FlexDirection() == kFlowRowReverse &&
       ShouldPlaceBlockDirectionScrollbarOnLogicalLeft())
@@ -1880,8 +1883,8 @@ void LayoutFlexibleBox::LayoutAndPlaceChildren(
 
     if (i != children.size() - 1) {
       // The last item does not get extra space added.
-      main_axis_offset += JustifyContentSpaceBetweenChildren(
-          available_free_space, distribution, children.size());
+      main_axis_offset += ContentDistributionSpaceBetweenChildren(
+          false, available_free_space, distribution, children.size());

[jfernandez, 2017/04/21 20:43:21]

Ditto.

     }
 
     if (is_paginated)
@@ -1923,7 +1926,7 @@ void LayoutFlexibleBox::LayoutColumnReverse(const Vector<FlexItem>& children,
   // layout anything since we're just moving the children to a new position.
   LayoutUnit main_axis_offset =
       LogicalHeight() - FlowAwareBorderEnd() - FlowAwarePaddingEnd();
-  main_axis_offset -= InitialJustifyContentOffset(
+  main_axis_offset -= InitialContentPositionOffset(
       available_free_space, position, distribution, children.size());
   main_axis_offset -= IsHorizontalFlow() ? VerticalScrollbarWidth()
                                          : HorizontalScrollbarHeight();
@@ -1943,45 +1946,11 @@ void LayoutFlexibleBox::LayoutColumnReverse(const Vector<FlexItem>& children,
 
     main_axis_offset -= FlowAwareMarginStartForChild(*child);
 
-    main_axis_offset -= JustifyContentSpaceBetweenChildren(
-        available_free_space, distribution, children.size());
+    main_axis_offset -= ContentDistributionSpaceBetweenChildren(
+        false, available_free_space, distribution, children.size());

[jfernandez, 2017/04/21 20:43:21]

Ditto.

   }
 }
 
-static LayoutUnit InitialAlignContentOffset(
-    LayoutUnit available_free_space,
-    ContentPosition align_content,
-    ContentDistributionType align_content_distribution,
-    unsigned number_of_lines) {
-  if (number_of_lines <= 1)
-    return LayoutUnit();
-  if (align_content == kContentPositionFlexEnd)
-    return available_free_space;
-  if (align_content == kContentPositionCenter)
-    return available_free_space / 2;
-  if (align_content_distribution == kContentDistributionSpaceAround) {
-    if (available_free_space > 0 && number_of_lines)
-      return available_free_space / (2 * number_of_lines);
-    if (available_free_space < 0)
-      return available_free_space / 2;
-  }
-  return LayoutUnit();
-}
-
-static LayoutUnit AlignContentSpaceBetweenChildren(
-    LayoutUnit available_free_space,
-    ContentDistributionType align_content_distribution,
-    unsigned number_of_lines) {
-  if (available_free_space > 0 && number_of_lines > 1) {
-    if (align_content_distribution == kContentDistributionSpaceBetween)
-      return available_free_space / (number_of_lines - 1);
-    if (align_content_distribution == kContentDistributionSpaceAround ||
-        align_content_distribution == kContentDistributionStretch)
-      return available_free_space / number_of_lines;
-  }
-  return LayoutUnit();
-}
-
 void LayoutFlexibleBox::AlignFlexLines(Vector<LineContext>& line_contexts) {
   ContentPosition position =
       StyleRef().ResolvedAlignContentPosition(ContentAlignmentNormalBehavior());
@@ -2005,8 +1974,12 @@ void LayoutFlexibleBox::AlignFlexLines(Vector<LineContext>& line_contexts) {
   for (size_t i = 0; i < line_contexts.size(); ++i)
     available_cross_axis_space -= line_contexts[i].cross_axis_extent;
 
-  LayoutUnit line_offset = InitialAlignContentOffset(
-      available_cross_axis_space, position, distribution, line_contexts.size());
+  LayoutUnit line_offset;
+  if (line_contexts.size() > 1) {
+    line_offset =
+        InitialContentPositionOffset(available_cross_axis_space, position,
+                                     distribution, line_contexts.size());
+  }
   for (unsigned line_number = 0; line_number < line_contexts.size();
        ++line_number) {
     LineContext& line_context = line_contexts[line_number];
@@ -2023,8 +1996,8 @@ void LayoutFlexibleBox::AlignFlexLines(Vector<LineContext>& line_contexts) {
           available_cross_axis_space /
           static_cast<unsigned>(line_contexts.size());
 
-    line_offset += AlignContentSpaceBetweenChildren(
-        available_cross_axis_space, distribution, line_contexts.size());
+    line_offset += ContentDistributionSpaceBetweenChildren(
+        true, available_cross_axis_space, distribution, line_contexts.size());

[jfernandez, 2017/04/21 20:43:21]

Ditto.

   }
 }