Fix bug in flex layout that did not correctly handle host insets.

Update to code was using non-normalized insets to calculate positioning
when justifying elements left, right, or center along the main axis.

Change-Id: I9892572ab37174fe14c20b2b0a1431377092098e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696453Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676262}

ui/views/layout/flex_layout.cc

@@ -229,7 +229,7 @@ struct FlexLayout::FlexLayoutData {
   // The total size of the layout (minus parent insets).
   NormalizedSize total_size;
   NormalizedInsets interior_margin;
-  gfx::Insets host_insets;
+  NormalizedInsets host_insets;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FlexLayoutData);
@@ -305,10 +305,10 @@ LayoutManagerBase::ProposedLayout FlexLayout::CalculateProposedLayout(
     const SizeBounds& size_bounds) const {
   FlexLayoutData data;
 
-  data.host_insets = host_view()->GetInsets();
+  data.host_insets = Normalize(orientation(), host_view()->GetInsets());
   data.interior_margin = Normalize(orientation(), interior_margin());
   NormalizedSizeBounds bounds = Normalize(orientation(), size_bounds);
-  bounds.Inset(Normalize(orientation(), data.host_insets));
+  bounds.Inset(data.host_insets);
   if (bounds.cross() && *bounds.cross() < minimum_cross_axis_size())
     bounds.set_cross(minimum_cross_axis_size());
 
@@ -334,9 +334,10 @@ LayoutManagerBase::ProposedLayout FlexLayout::CalculateProposedLayout(
   }
 
   // Calculate the size of the host view.
-  data.layout.host_size = Denormalize(orientation(), data.total_size);
-  data.layout.host_size.Enlarge(data.host_insets.width(),
-                                data.host_insets.height());
+  NormalizedSize host_size = data.total_size;
+  host_size.Enlarge(data.host_insets.main_size(),
+                    data.host_insets.cross_size());
+  data.layout.host_size = Denormalize(orientation(), host_size);
 
   // Size and position the children in screen space.
   CalculateChildBounds(size_bounds, &data);
@@ -408,11 +409,10 @@ void FlexLayout::CalculateChildBounds(const SizeBounds& size_bounds,
   // Apply main axis alignment (we've already done cross-axis alignment above).
   int available_main = (size_bounds.width() ? *size_bounds.width()
                                             : data->layout.host_size.width());
-  available_main = std::max(0, available_main - data->host_insets.width());
+  available_main = std::max(0, available_main - data->host_insets.main_size());
   const int excess_main = available_main - data->total_size.main();
-  NormalizedPoint start =
-      Normalize(orientation(),
-                gfx::Point(data->host_insets.left(), data->host_insets.top()));
+  NormalizedPoint start(data->host_insets.main_leading(),
+                        data->host_insets.cross_leading());
   switch (main_axis_alignment()) {
     case LayoutAlignment::kStart:
       break;

ui/views/layout/flex_layout_unittest.cc

@@ -117,6 +117,7 @@ class FlexLayoutTest : public testing::Test {
 
  protected:
   // Constants re-used in many tests.
+  static const Insets kSmallInsets;
   static const Insets kLayoutInsets;
   static const Insets kLargeInsets;
   static const Size kChild1Size;
@@ -148,6 +149,7 @@ class FlexLayoutTest : public testing::Test {
   FlexLayout* layout_;
 };
 
+const Insets FlexLayoutTest::kSmallInsets{1, 2, 3, 4};
 const Insets FlexLayoutTest::kLayoutInsets{5, 6, 7, 9};
 const Insets FlexLayoutTest::kLargeInsets{10, 11, 12, 13};
 const Size FlexLayoutTest::kChild1Size{12, 10};
@@ -210,13 +212,20 @@ TEST_F(FlexLayoutTest, GetMinimumSize_Empty) {
   EXPECT_EQ(Size(0, 0), host_->GetMinimumSize());
 }
 
-TEST_F(FlexLayoutTest, GetMinimumSize_Empty_ViewInsets) {
+TEST_F(FlexLayoutTest, GetMinimumSize_Empty_ViewInsets_Horizontal) {
   layout_->SetOrientation(LayoutOrientation::kHorizontal);
   layout_->SetCollapseMargins(false);
   host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
   EXPECT_EQ(Size(15, 12), host_->GetMinimumSize());
 }
 
+TEST_F(FlexLayoutTest, GetMinimumSize_Empty_ViewInsets_Vertical) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetCollapseMargins(false);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  EXPECT_EQ(Size(15, 12), host_->GetMinimumSize());
+}
+
 TEST_F(FlexLayoutTest, GetMinimumSize_Empty_InternalMargin_Collapsed) {
   layout_->SetOrientation(LayoutOrientation::kHorizontal);
   layout_->SetCollapseMargins(true);
@@ -248,8 +257,8 @@ TEST_F(FlexLayoutTest, GetMinimumSize_MinimumCross_Horizontal) {
   EXPECT_EQ(Size(9, 7), host_->GetMinimumSize());
   layout_->SetMinimumCrossAxisSize(10);
   EXPECT_EQ(Size(9, 10), host_->GetMinimumSize());
-  host_->SetBorder(CreateEmptyBorder(2, 2, 2, 2));
-  EXPECT_EQ(Size(13, 14), host_->GetMinimumSize());
+  host_->SetBorder(CreateEmptyBorder(kSmallInsets));
+  EXPECT_EQ(Size(15, 14), host_->GetMinimumSize());
 }
 
 TEST_F(FlexLayoutTest, GetMinimumSize_MinimumCross_Vertical) {
@@ -260,8 +269,8 @@ TEST_F(FlexLayoutTest, GetMinimumSize_MinimumCross_Vertical) {
   EXPECT_EQ(Size(9, 7), host_->GetMinimumSize());
   layout_->SetMinimumCrossAxisSize(10);
   EXPECT_EQ(Size(10, 7), host_->GetMinimumSize());
-  host_->SetBorder(CreateEmptyBorder(2, 2, 2, 2));
-  EXPECT_EQ(Size(14, 11), host_->GetMinimumSize());
+  host_->SetBorder(CreateEmptyBorder(kSmallInsets));
+  EXPECT_EQ(Size(16, 11), host_->GetMinimumSize());
 }
 
 // Visibility and Inclusion Tests ----------------------------------------------
@@ -721,6 +730,97 @@ TEST_F(FlexLayoutTest, LayoutMultipleViews_InteriorPadding_Additive) {
   EXPECT_EQ(Size(70, 50), host_->GetPreferredSize());
 }
 
+// Host insets tests -----------------------------------------------------------
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Horizontal) {
+  layout_->SetOrientation(LayoutOrientation::kHorizontal);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->Layout();
+  EXPECT_EQ(Rect(6, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Vertical) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->Layout();
+  EXPECT_EQ(Rect(6, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Horizontal_Leading) {
+  layout_->SetOrientation(LayoutOrientation::kHorizontal);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  EXPECT_EQ(Rect(6, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Vertical_Leading) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  EXPECT_EQ(Rect(6, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Horizontal_Center) {
+  layout_->SetOrientation(LayoutOrientation::kHorizontal);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kCenter);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  const int expected_x =
+      kLayoutInsets.left() +
+      (host_->size().width() - kChild1Size.width() - kLayoutInsets.width()) / 2;
+  EXPECT_EQ(Rect(expected_x, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Vertical_Center) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kCenter);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  const int expected_y =
+      kLayoutInsets.top() +
+      (host_->size().height() - kChild1Size.height() - kLayoutInsets.height()) /
+          2;
+  EXPECT_EQ(Rect(6, expected_y, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Horizontal_End) {
+  layout_->SetOrientation(LayoutOrientation::kHorizontal);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kEnd);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  const int expected_x =
+      kLayoutInsets.left() +
+      (host_->size().width() - kChild1Size.width() - kLayoutInsets.width());
+  EXPECT_EQ(Rect(expected_x, 5, 12, 10), child->bounds());
+}
+
+TEST_F(FlexLayoutTest, Layout_HostInsets_Vertical_End) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kEnd);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  host_->SetBorder(CreateEmptyBorder(kLayoutInsets));
+  View* child = AddChild(kChild1Size);
+  host_->SetSize({100, 100});
+  const int expected_y =
+      kLayoutInsets.top() +
+      (host_->size().height() - kChild1Size.height() - kLayoutInsets.height());
+  EXPECT_EQ(Rect(6, expected_y, 12, 10), child->bounds());
+}
+
 // Alignment Tests -------------------------------------------------------------
 
 TEST_F(FlexLayoutTest, Layout_CrossStart) {