FlexLayout: configure how host view insets and interior margins are used

This CL adds two new ways to specify edge case behavior for FlexLayout
that will make some use cases much easier.

1. SetTargetArea(LayoutTargetArea) - specifies whether the host view's
insets are treated as inviolable (default and older behavior) or whether
they're considered part of the internal margins of the host view for
layout purposes. The new (optional) behavior allows certain cases where
a child view with internal padding cannot currently get close enough to
a host view border, inkdrop, or focus ring.

2. Set[Main|Cross]AxisMarginsPolicy(LayoutMarginPolicy) - specifies
whether child margins apply when they abut the edge of the host view.
The new (optional) behavior allows much simpler code when spacing between
views needs to be different than spacing at the edges of the host view.

OPEN QUESTIONS:
 - Should we just make the new behavior for (1) the default?

Bug: 898632
Change-Id: I5d336ed19b32a791f944f8ced615a099a0729802
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1676736
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683293}

ui/views/layout/flex_layout.cc

@@ -50,6 +50,7 @@ struct FlexChildData {
   NormalizedSize preferred_size;
   NormalizedSize current_size;
   NormalizedInsets margins;
+  bool using_default_margins = true;
   NormalizedInsets internal_padding;
   NormalizedRect actual_bounds;
   FlexSpecification flex;
@@ -63,10 +64,16 @@ struct FlexChildData {
 template <typename T>
 T GetViewProperty(const View* view,
                   const ui::PropertyHandler& defaults,
-                  const ui::ClassProperty<T*>* property) {
+                  const ui::ClassProperty<T*>* property,
+                  bool* is_default = nullptr) {
   T* found_value = view->GetProperty(property);
-  if (found_value)
+  if (found_value) {
+    if (is_default)
+      *is_default = false;
     return *found_value;
+  }
+  if (is_default)
+    *is_default = true;
   found_value = defaults.GetProperty(property);
   if (found_value)
     return *found_value;
@@ -257,6 +264,15 @@ FlexLayout& FlexLayout::SetOrientation(LayoutOrientation orientation) {
   return *this;
 }
 
+FlexLayout& FlexLayout::SetIncludeHostInsetsInLayout(
+    bool include_host_insets_in_layout) {
+  if (include_host_insets_in_layout != include_host_insets_in_layout_) {
+    include_host_insets_in_layout_ = include_host_insets_in_layout;
+    InvalidateLayout();
+  }
+  return *this;
+}
+
 FlexLayout& FlexLayout::SetCollapseMargins(bool collapse_margins) {
   if (collapse_margins != collapse_margins_) {
     collapse_margins_ = collapse_margins;
@@ -293,6 +309,15 @@ FlexLayout& FlexLayout::SetInteriorMargin(const gfx::Insets& interior_margin) {
   return *this;
 }
 
+FlexLayout& FlexLayout::SetIgnoreDefaultMainAxisMargins(
+    bool ignore_default_main_axis_margins) {
+  if (ignore_default_main_axis_margins_ != ignore_default_main_axis_margins) {
+    ignore_default_main_axis_margins_ = ignore_default_main_axis_margins;
+    InvalidateLayout();
+  }
+  return *this;
+}
+
 FlexLayout& FlexLayout::SetMinimumCrossAxisSize(int size) {
   if (minimum_cross_axis_size_ != size) {
     minimum_cross_axis_size_ = size;
@@ -305,8 +330,15 @@ LayoutManagerBase::ProposedLayout FlexLayout::CalculateProposedLayout(
     const SizeBounds& size_bounds) const {
   FlexLayoutData data;
 
-  data.host_insets = Normalize(orientation(), host_view()->GetInsets());
-  data.interior_margin = Normalize(orientation(), interior_margin());
+  if (include_host_insets_in_layout()) {
+    // Combining the interior margin and host insets means we only have to set
+    // the margin value; we'll leave the insets at zero.
+    data.interior_margin =
+        Normalize(orientation(), interior_margin() + host_view()->GetInsets());
+  } else {
+    data.host_insets = Normalize(orientation(), host_view()->GetInsets());
+    data.interior_margin = Normalize(orientation(), interior_margin());
+  }
   NormalizedSizeBounds bounds = Normalize(orientation(), size_bounds);
   bounds.Inset(data.host_insets);
   if (bounds.cross() && *bounds.cross() < minimum_cross_axis_size())
@@ -366,7 +398,8 @@ void FlexLayout::InitializeChildData(
 
     flex_child.margins =
         Normalize(orientation(),
-                  GetViewProperty(child, layout_defaults_, views::kMarginsKey));
+                  GetViewProperty(child, layout_defaults_, views::kMarginsKey,
+                                  &flex_child.using_default_margins));
     flex_child.internal_padding = Normalize(
         orientation(),
         GetViewProperty(child, layout_defaults_, views::kInternalPaddingKey));
@@ -443,6 +476,20 @@ void FlexLayout::CalculateChildBounds(const SizeBounds& size_bounds,
   }
 }
 
+Inset1D FlexLayout::GetCrossAxisMargins(const FlexLayoutData& layout,
+                                        size_t child_index) const {
+  const FlexChildData& child_data = layout.child_data[child_index];
+  const int leading_margin =
+      CalculateMargin(layout.interior_margin.cross_leading(),
+                      child_data.margins.cross_leading(),
+                      child_data.internal_padding.cross_leading());
+  const int trailing_margin =
+      CalculateMargin(layout.interior_margin.cross_trailing(),
+                      child_data.margins.cross_trailing(),
+                      child_data.internal_padding.cross_trailing());
+  return Inset1D(leading_margin, trailing_margin);
+}
+
 int FlexLayout::CalculateMargin(int margin1,
                                 int margin2,
                                 int internal_padding) const {
@@ -457,37 +504,39 @@ base::Optional<int> FlexLayout::GetAvailableCrossAxisSize(
     const NormalizedSizeBounds& bounds) const {
   if (!bounds.cross())
     return base::nullopt;
-
-  const FlexChildData& child_layout = layout.child_data[child_index];
-  const int leading_margin =
-      CalculateMargin(layout.interior_margin.cross_leading(),
-                      child_layout.margins.cross_leading(),
-                      child_layout.internal_padding.cross_leading());
-  const int trailing_margin =
-      CalculateMargin(layout.interior_margin.cross_trailing(),
-                      child_layout.margins.cross_trailing(),
-                      child_layout.internal_padding.cross_trailing());
-  return std::max(0, *bounds.cross() - (leading_margin + trailing_margin));
+  const Inset1D cross_margins = GetCrossAxisMargins(layout, child_index);
+  return std::max(0, *bounds.cross() - cross_margins.size());
 }
 
 int FlexLayout::CalculateChildSpacing(
     const FlexLayoutData& layout,
     base::Optional<size_t> child1_index,
     base::Optional<size_t> child2_index) const {
+  const FlexChildData* const child1 =
+      child1_index ? &layout.child_data[*child1_index] : nullptr;
+  const FlexChildData* const child2 =
+      child2_index ? &layout.child_data[*child2_index] : nullptr;
+
+  const int child1_trailing =
+      child1 && (child2 || !ignore_default_main_axis_margins() ||
+                 !child1->using_default_margins)
+          ? child1->margins.main_trailing()
+          : 0;
+  const int child2_leading =
+      child2 && (child1 || !ignore_default_main_axis_margins() ||
+                 !child2->using_default_margins)
+          ? child2->margins.main_leading()
+          : 0;
+
   const int left_margin =
-      child1_index ? layout.child_data[*child1_index].margins.main_trailing()
-                   : layout.interior_margin.main_leading();
+      child1 ? child1_trailing : layout.interior_margin.main_leading();
   const int right_margin =
-      child2_index ? layout.child_data[*child2_index].margins.main_leading()
-                   : layout.interior_margin.main_trailing();
+      child2 ? child2_leading : layout.interior_margin.main_trailing();
+
   const int left_padding =
-      child1_index
-          ? layout.child_data[*child1_index].internal_padding.main_trailing()
-          : 0;
+      child1 ? child1->internal_padding.main_trailing() : 0;
   const int right_padding =
-      child2_index
-          ? layout.child_data[*child2_index].internal_padding.main_leading()
-          : 0;
+      child2 ? child2->internal_padding.main_leading() : 0;
 
   return CalculateMargin(left_margin, right_margin,
                          left_padding + right_padding);
@@ -521,20 +570,12 @@ void FlexLayout::UpdateLayoutFromChildren(
       continue;
 
     // Update the cross-axis margins and if necessary, the size.
-    Inset1D& cross_spacing = cross_spacings[i];
-    cross_spacing.set_leading(
-        CalculateMargin(data->interior_margin.cross_leading(),
-                        flex_child.margins.cross_leading(),
-                        flex_child.internal_padding.cross_leading()));
-    cross_spacing.set_trailing(
-        CalculateMargin(data->interior_margin.cross_trailing(),
-                        flex_child.margins.cross_trailing(),
-                        flex_child.internal_padding.cross_trailing()));
+    cross_spacings[i] = GetCrossAxisMargins(*data, i);
 
     if (!force_cross_size) {
       const int cross_size = std::min(flex_child.current_size.cross(),
                                       flex_child.preferred_size.cross());
-      data->total_size.SetToMax(0, cross_spacing.size() + cross_size);
+      data->total_size.SetToMax(0, cross_spacings[i].size() + cross_size);
     }
 
     // Calculate main-axis size and upper-left main axis coordinate.

ui/views/layout/flex_layout.h

@@ -81,11 +81,14 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   //       .SetCrossAxisAlignment()
   //       .SetDefaultFlex(...);
   FlexLayout& SetOrientation(LayoutOrientation orientation);
-  FlexLayout& SetCollapseMargins(bool collapse_margins);
   FlexLayout& SetMainAxisAlignment(LayoutAlignment main_axis_alignment);
   FlexLayout& SetCrossAxisAlignment(LayoutAlignment cross_axis_alignment);
   FlexLayout& SetInteriorMargin(const gfx::Insets& interior_margin);
   FlexLayout& SetMinimumCrossAxisSize(int size);
+  FlexLayout& SetCollapseMargins(bool collapse_margins);
+  FlexLayout& SetIncludeHostInsetsInLayout(bool include_host_insets_in_layout);
+  FlexLayout& SetIgnoreDefaultMainAxisMargins(
+      bool ignore_default_main_axis_margins);
 
   LayoutOrientation orientation() const { return orientation_; }
   bool collapse_margins() const { return collapse_margins_; }
@@ -93,6 +96,12 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   LayoutAlignment cross_axis_alignment() const { return cross_axis_alignment_; }
   const gfx::Insets& interior_margin() const { return interior_margin_; }
   int minimum_cross_axis_size() const { return minimum_cross_axis_size_; }
+  bool include_host_insets_in_layout() const {
+    return include_host_insets_in_layout_;
+  }
+  bool ignore_default_main_axis_margins() const {
+    return ignore_default_main_axis_margins_;
+  }
 
   // Moves and uses |value| as the default value for layout property |key|.
   template <class T, class U>
@@ -135,6 +144,11 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   // See FlexSpecification::order().
   using FlexOrderToViewIndexMap = std::map<int, std::vector<size_t>>;
 
+  // Returns the combined margins across the cross axis of the host view, for a
+  // particular child view.
+  Inset1D GetCrossAxisMargins(const FlexLayoutData& layout,
+                              size_t child_index) const;
+
   // Calculates a margin between two child views based on each's margin and any
   // internal padding present in one or both elements. Uses properties of the
   // layout, like whether adjacent margins should be collapsed.
@@ -222,6 +236,33 @@ class VIEWS_EXPORT FlexLayout : public LayoutManagerBase {
   // The minimum cross axis size for the layout.
   int minimum_cross_axis_size_ = 0;
 
+  // Whether to include host insets in the layout. Use when e.g. the host has an
+  // empty border and you want to treat that empty space as part of the interior
+  // margin of the host view.
+  //
+  // Most useful in conjunction with |collapse_margins| so child margins can
+  // overlap with the host's insets.
+  //
+  // In the future, we might consider putting this as metadata on the host's
+  // border - e.g. an EmptyBorder would be included in host insets but a thick
+  // frame would not be.
+  bool include_host_insets_in_layout_ = false;
+
+  // Whether host |interior_margin| overrides default child margins at the
+  // leading and trailing edge of the host view.
+  //
+  // Example:
+  // layout->SetIgnoreDefaultMainAxisMargins(true)
+  //        .SetCollapseMargins(true)
+  //        .SetDefault(kMarginsKey, {5, 10})
+  //        .SetInteriorMargin({5, 5});
+  //
+  // This produces a margin of 5 DIP on all edges of the host view, with 10 DIP
+  // between child views. If SetIgnoreDefaultMainAxisMargins(true) was not
+  // called, the default child margin of 10 would also apply on the leading and
+  // trailing edge of the host view.
+  bool ignore_default_main_axis_margins_ = false;
+
   // Default properties for any views that don't have them explicitly set for
   // this layout.
   PropertyHandler layout_defaults_{this};

ui/views/layout/flex_layout_unittest.cc

@@ -821,6 +821,129 @@ TEST_F(FlexLayoutTest, Layout_HostInsets_Vertical_End) {
   EXPECT_EQ(Rect(6, expected_y, 12, 10), child->bounds());
 }
 
+// Include Host Insets Tests ---------------------------------------------------
+
+TEST_F(FlexLayoutTest, SetIncludeHostInsetsInLayout_NoChange) {
+  host_->SetBorder(views::CreateEmptyBorder(2, 2, 2, 2));
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetCollapseMargins(false);
+  layout_->SetInteriorMargin(kLayoutInsets);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kEnd);
+  layout_->SetDefault(views::kMarginsKey, gfx::Insets(4));
+  constexpr Size kChildSize(10, 10);
+  AddChild(kChildSize);
+  View* const child2 = AddChild(kChildSize);
+  AddChild(kChildSize);
+  child2->SetProperty(views::kMarginsKey, gfx::Insets(10));
+
+  const Size expected_preferred_size = host_->GetPreferredSize();
+  host_->SetSize(expected_preferred_size);
+  const std::vector<Rect> expected_bounds = GetChildBounds();
+
+  layout_->SetIncludeHostInsetsInLayout(true);
+  const Size preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(expected_preferred_size, preferred_size);
+  host_->Layout();
+  EXPECT_EQ(expected_bounds, GetChildBounds());
+}
+
+TEST_F(FlexLayoutTest, SetIncludeHostInsetsInLayout_CollapseIntoInsets) {
+  host_->SetBorder(views::CreateEmptyBorder(2, 2, 2, 2));
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetCollapseMargins(true);
+  layout_->SetInteriorMargin(kLayoutInsets);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kEnd);
+  layout_->SetDefault(views::kMarginsKey, gfx::Insets(4));
+  constexpr Size kChildSize(10, 10);
+  AddChild(kChildSize);
+  View* const child2 = AddChild(kChildSize);
+  AddChild(kChildSize);
+  child2->SetProperty(views::kMarginsKey, gfx::Insets(15));
+
+  layout_->SetIncludeHostInsetsInLayout(true);
+  const Size preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(40, 76), preferred_size);
+  host_->SetSize(preferred_size);
+  const std::vector<Rect> expected{Rect(19, 7, 10, 10), Rect(15, 32, 10, 10),
+                                   Rect(19, 57, 10, 10)};
+  EXPECT_EQ(expected, GetChildBounds());
+}
+
+TEST_F(FlexLayoutTest, SetIncludeHostInsetsInLayout_OverlapInsets) {
+  host_->SetBorder(views::CreateEmptyBorder(4, 5, 5, 5));
+  layout_->SetOrientation(LayoutOrientation::kHorizontal);
+  layout_->SetCollapseMargins(true);
+  layout_->SetInteriorMargin(kLayoutInsets);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  View* const child = AddChild(Size(10, 10));
+  child->SetProperty(views::kInternalPaddingKey, Insets(10, 10, 10, 10));
+
+  layout_->SetIncludeHostInsetsInLayout(true);
+  const Size preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(15, 12), preferred_size);
+  host_->SetSize(preferred_size);
+  EXPECT_EQ(Rect(1, 0, 10, 10), child->bounds());
+}
+
+// Default Main Axis Margins Tests ---------------------------------------------
+
+TEST_F(FlexLayoutTest, SetIgnoreDefaultMainAxisMargins_IgnoresDefaultMargins) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetCollapseMargins(false);
+  layout_->SetInteriorMargin(kLayoutInsets);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kEnd);
+  layout_->SetDefault(views::kMarginsKey, gfx::Insets(4));
+  constexpr Size kChildSize(10, 10);
+  AddChild(kChildSize);
+  AddChild(kChildSize);
+  AddChild(kChildSize);
+
+  Size preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(33, 66), preferred_size);
+
+  layout_->SetIgnoreDefaultMainAxisMargins(true);
+  preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(33, 58), preferred_size);
+
+  host_->SetSize(preferred_size);
+  const std::vector<Rect> expected{Rect(10, 5, 10, 10), Rect(10, 23, 10, 10),
+                                   Rect(10, 41, 10, 10)};
+  EXPECT_EQ(expected, GetChildBounds());
+}
+
+TEST_F(FlexLayoutTest,
+       SetIgnoreDefaultMainAxisMargins_IncludesExplicitMargins) {
+  layout_->SetOrientation(LayoutOrientation::kVertical);
+  layout_->SetCollapseMargins(true);
+  layout_->SetInteriorMargin(kLayoutInsets);
+  layout_->SetMainAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetCrossAxisAlignment(LayoutAlignment::kStart);
+  layout_->SetDefault(views::kMarginsKey, gfx::Insets(4));
+  constexpr Size kChildSize(10, 10);
+  View* const child1 = AddChild(kChildSize);
+  AddChild(kChildSize);
+  View* const child3 = AddChild(kChildSize);
+
+  child1->SetProperty(views::kMarginsKey, gfx::Insets(11));
+  child3->SetProperty(views::kMarginsKey, gfx::Insets(12));
+
+  Size preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(34, 76), preferred_size);
+
+  layout_->SetIgnoreDefaultMainAxisMargins(true);
+  preferred_size = host_->GetPreferredSize();
+  EXPECT_EQ(Size(34, 76), preferred_size);
+
+  host_->SetSize(preferred_size);
+  const std::vector<Rect> expected{Rect(11, 11, 10, 10), Rect(6, 32, 10, 10),
+                                   Rect(12, 54, 10, 10)};
+  EXPECT_EQ(expected, GetChildBounds());
+}
+
 // Alignment Tests -------------------------------------------------------------
 
 TEST_F(FlexLayoutTest, Layout_CrossStart) {