BoxLayout now suports per-view margins.

ui/views/layout/box_layout.cc

Index: ui/views/layout/box_layout.cc
diff --git a/ui/views/layout/box_layout.cc b/ui/views/layout/box_layout.cc
index 7e5abff4880af33a0b34583c3d0c7f127b7480c5..d08a92a81c59a90a066ad6828da222f8f580286b 100644
--- a/ui/views/layout/box_layout.cc
+++ b/ui/views/layout/box_layout.cc
@@ -11,6 +11,35 @@
 
 namespace views {
 
+BoxLayout::ViewWrapper::ViewWrapper()
+    : view_(nullptr), layout_(nullptr), margins_(0, 0, 0, 0){};

[sky, 2017/04/25 17:28:58]

(0,0,0,0) is the default, so need to specify margins_ here.

[kylix_rd, 2017/04/26 17:41:33]

Done.

+
+BoxLayout::ViewWrapper::ViewWrapper(const BoxLayout* layout, const View* view)
+    : view_(view), layout_(layout), margins_(layout_->GetViewMargins(view)) {}
+
+BoxLayout::ViewWrapper::ViewWrapper(const ViewWrapper& wrapper)

[sky, 2017/04/25 17:28:58]

= default?

[kylix_rd, 2017/04/26 17:41:33]

Done.

+    : view_(wrapper.view_),
+      layout_(wrapper.layout_),
+      margins_(wrapper.margins_) {}
+
+BoxLayout::ViewWrapper::~ViewWrapper() {}
+
+gfx::Size BoxLayout::ViewWrapper::GetPreferredSize() const {
+  gfx::Size preferred_size = view_->GetPreferredSize();
+  preferred_size.Enlarge(margins_.width(), margins_.height());
+  return preferred_size;
+}
+
+void BoxLayout::ViewWrapper::SetBoundsRect(const gfx::Rect& bounds) {
+  const_cast<View*>(view_)->SetBounds(

[sky, 2017/04/25 17:28:58]

Why the const_cast here?

[kylix_rd, 2017/04/26 17:41:33]

The view_ field is declared as "const View* view_;" Most references to the view
are const. So I elected to cast the "const" away only at the one point that
modifies the view.

[sky, 2017/04/26 19:57:07]

If you're going to const_cast, why make the member const? Otherwise, what's the
point?

+      bounds.x() + margins_.left(), bounds.y() + margins_.top(),
+      bounds.width() - margins_.width(), bounds.height() - margins_.height());
+}
+
+bool BoxLayout::ViewWrapper::visible() const {
+  return view_->visible();
+}
+
 BoxLayout::BoxLayout(BoxLayout::Orientation orientation,
                      int inside_border_horizontal_spacing,
                      int inside_border_vertical_spacing,
@@ -47,10 +76,17 @@ void BoxLayout::SetDefaultFlex(int default_flex) {
   default_flex_ = default_flex;
 }
 
+void BoxLayout::ClearMarginsForView(const View* view) {
+  margin_map_.erase(view);
+}
+
+void BoxLayout::SetViewMargins(const View* view, const gfx::Insets& margins) {
+  margin_map_[view] = margins;
+}
+
 void BoxLayout::Layout(View* host) {
   DCHECK_EQ(host_, host);
-  gfx::Rect child_area(host->GetLocalBounds());
-  child_area.Inset(host->GetInsets());
+  gfx::Rect child_area(host->GetContentsBounds());
   child_area.Inset(inside_border_insets_);
 
   int total_main_axis_size = 0;
@@ -58,11 +94,12 @@ void BoxLayout::Layout(View* host) {
   int flex_sum = 0;
   // Calculate the total size of children in the main axis.
   for (int i = 0; i < host->child_count(); ++i) {
-    View* child = host->child_at(i);
-    if (!child->visible())
+    ViewWrapper child = ViewWrapper(this, host->child_at(i));
+    if (!child.visible())
       continue;
-    int flex = GetFlexForView(child);
-    int child_main_axis_size = MainAxisSizeForView(child, child_area.width());
+    int flex = GetFlexForView(child.view());
+    int child_main_axis_size =
+        MainAxisSizeForView(child.view(), child_area.width());
     if (child_main_axis_size == 0 && flex == 0)
       continue;
     total_main_axis_size += child_main_axis_size + between_child_spacing_;
@@ -106,8 +143,8 @@ void BoxLayout::Layout(View* host) {
   int total_padding = 0;
   int current_flex = 0;
   for (int i = 0; i < host->child_count(); ++i) {
-    View* child = host->child_at(i);
-    if (!child->visible())
+    ViewWrapper child = ViewWrapper(this, host->child_at(i));
+    if (!child.visible())
       continue;
 
     // TODO(bruthig): Fix this. The main axis should be calculated before
@@ -118,7 +155,8 @@ void BoxLayout::Layout(View* host) {
     gfx::Rect bounds(child_area);
     SetMainAxisPosition(main_position, &bounds);
     if (cross_axis_alignment_ != CROSS_AXIS_ALIGNMENT_STRETCH) {
-      int free_space = CrossAxisSize(bounds) - CrossAxisSizeForView(child);
+      int free_space =
+          CrossAxisSize(bounds) - CrossAxisSizeForView(child.view());
       int position = CrossAxisPosition(bounds);
       if (cross_axis_alignment_ == CROSS_AXIS_ALIGNMENT_CENTER) {
         position += free_space / 2;
@@ -126,13 +164,13 @@ void BoxLayout::Layout(View* host) {
         position += free_space;
       }
       SetCrossAxisPosition(position, &bounds);
-      SetCrossAxisSize(CrossAxisSizeForView(child), &bounds);
+      SetCrossAxisSize(CrossAxisSizeForView(child.view()), &bounds);
     }
 
     // Calculate flex padding.
     int current_padding = 0;
-    if (GetFlexForView(child) > 0) {
-      current_flex += GetFlexForView(child);
+    if (GetFlexForView(child.view()) > 0) {
+      current_flex += GetFlexForView(child.view());
       int quot = (main_free_space * current_flex) / flex_sum;
       int rem = (main_free_space * current_flex) % flex_sum;
       current_padding = quot - total_padding;
@@ -145,14 +183,15 @@ void BoxLayout::Layout(View* host) {
     // Set main axis size.
     // TODO(bruthig): Use the allocated width to determine the cross axis size.
     // See https://crbug.com/682266.
-    int child_main_axis_size = MainAxisSizeForView(child, child_area.width());
+    int child_main_axis_size =
+        MainAxisSizeForView(child.view(), child_area.width());
     SetMainAxisSize(child_main_axis_size + current_padding, &bounds);
-    if (MainAxisSize(bounds) > 0 || GetFlexForView(child) > 0)
+    if (MainAxisSize(bounds) > 0 || GetFlexForView(child.view()) > 0)
       main_position += MainAxisSize(bounds) + between_child_spacing_;
 
     // Clamp child view bounds to |child_area|.
     bounds.Intersect(child_area);
-    child->SetBoundsRect(bounds);
+    child.SetBoundsRect(bounds);
   }
 
   // Flex views should have grown/shrunk to consume all free space.
@@ -166,11 +205,11 @@ gfx::Size BoxLayout::GetPreferredSize(const View* host) const {
   int width = 0;
   if (orientation_ == kVertical) {
     for (int i = 0; i < host->child_count(); ++i) {
-      const View* child = host->child_at(i);
-      if (!child->visible())
+      const ViewWrapper child = ViewWrapper(this, host->child_at(i));
+      if (!child.visible())
         continue;
 
-      width = std::max(width, child->GetPreferredSize().width());
+      width = std::max(width, child.GetPreferredSize().width());
     }
     width = std::max(width, minimum_cross_axis_size_);
   }
@@ -191,10 +230,11 @@ void BoxLayout::Installed(View* host) {
 
 void BoxLayout::ViewRemoved(View* host, View* view) {
   ClearFlexForView(view);
+  ClearMarginsForView(view);
 }
 
 int BoxLayout::GetFlexForView(const View* view) const {
-  std::map<const View*, int>::const_iterator it = flex_map_.find(view);
+  FlexMap::const_iterator it = flex_map_.find(view);
   if (it == flex_map_.end())
     return default_flex_;
 
@@ -273,11 +313,11 @@ gfx::Size BoxLayout::GetPreferredSizeForChildWidth(const View* host,
     // TODO(estade|bruthig): Fix this See // https://crbug.com/682266.
     int position = 0;
     for (int i = 0; i < host->child_count(); ++i) {
-      const View* child = host->child_at(i);
-      if (!child->visible())
+      const ViewWrapper child = ViewWrapper(this, host->child_at(i));
+      if (!child.visible())
         continue;
 
-      gfx::Size size(child->GetPreferredSize());
+      gfx::Size size(child.GetPreferredSize());
       if (size.IsEmpty())
         continue;
 
@@ -290,13 +330,13 @@ gfx::Size BoxLayout::GetPreferredSizeForChildWidth(const View* host,
   } else {
     int height = 0;
     for (int i = 0; i < host->child_count(); ++i) {
-      const View* child = host->child_at(i);
-      if (!child->visible())
+      const ViewWrapper child = ViewWrapper(this, host->child_at(i));
+      if (!child.visible())
         continue;
 
       // Use the child area width for getting the height if the child is
       // supposed to stretch. Use its preferred size otherwise.
-      int extra_height = MainAxisSizeForView(child, child_area_width);
+      int extra_height = MainAxisSizeForView(child.view(), child_area_width);
       // Only add |between_child_spacing_| if this is not the only child.
       if (height != 0 && extra_height > 0)
         height += between_child_spacing_;
@@ -318,4 +358,11 @@ gfx::Size BoxLayout::NonChildSize(const View* host) const {
                    insets.height() + inside_border_insets_.height());
 }
 
+gfx::Insets BoxLayout::GetViewMargins(const View* view) const {
+  MarginMap::const_iterator pos = margin_map_.find(view);
+  if (pos != margin_map_.cend())
+    return pos->second;
+  return gfx::Insets();
+}
+
 }  // namespace views