BoxLayout now suports per-view margins.

ui/views/layout/box_layout.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_VIEWS_LAYOUT_BOX_LAYOUT_H_
 #define UI_VIEWS_LAYOUT_BOX_LAYOUT_H_
 
 #include <map>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/layout/layout_manager.h"
 
 namespace gfx {
 class Rect;
 class Size;
 }
 
 namespace views {
 
 class View;
 
 // A Layout manager that arranges child views vertically or horizontally in a
 // side-by-side fashion with spacing around and between the child views. The
 // child views are always sized according to their preferred size. If the
 // host's bounds provide insufficient space, child views will be clamped.
 // Excess space will not be distributed.
 class VIEWS_EXPORT BoxLayout : public LayoutManager {

[sky, 2017/05/09 19:26:03]

You should make this class a ViewObserver on the views so that if the margins
change you trigger a layout. You can do this in a separate patch as you'll need
to change ViewObserver.

  public:
   enum Orientation {
     kHorizontal,
     kVertical,
   };
 
   // This specifies where along the main axis the children should be laid out.
   // e.g. a horizontal layout of MAIN_AXIS_ALIGNMENT_END will result in the
   // child views being right-aligned.
   enum MainAxisAlignment {
@@ skipping 16 matching lines @@
   };
 
   // Use |inside_border_horizontal_spacing| and
   // |inside_border_vertical_spacing| to add additional space between the child
   // view area and the host view border. |between_child_spacing| controls the
   // space in between child views.
   BoxLayout(Orientation orientation,
             int inside_border_horizontal_spacing,
             int inside_border_vertical_spacing,
             int between_child_spacing);
+  // As above but with added |collapse_margins_spacing| paramter which controls
+  // whether or not spacing adjacent to view margins are collapsed base on the
+  // max of the two values.
+  BoxLayout(Orientation orientation,
+            int inside_border_horizontal_spacing,
+            int inside_border_vertical_spacing,
+            int between_child_spacing,
+            bool collapse_margins_spacing);

[sky, 2017/05/09 19:26:03]

Merge this with other constructor and make collapse_margins_spacing have a
default value.

[kylix_rd, 2017/05/09 20:07:06]

I'm wondering if the default should be "true". It has a zero net-effect if there
are no margins set on the individual views.

[sky, 2017/05/09 23:55:29]

I think existing consumers should have to opt into the new behavior. I say that
based on a feeling that harmony would want to set default margins at view
creation time, but I could certainly be wrong there.

[kylix_rd, 2017/05/10 15:30:37]

Done.

   ~BoxLayout() override;
 
   void set_main_axis_alignment(MainAxisAlignment main_axis_alignment) {
     main_axis_alignment_ = main_axis_alignment;
   }
 
   void set_cross_axis_alignment(CrossAxisAlignment cross_axis_alignment) {
     cross_axis_alignment_ = cross_axis_alignment;
   }
 
@@ skipping 22 matching lines @@
   void SetDefaultFlex(int default_flex);
 
   // Overridden from views::LayoutManager:
   void Installed(View* host) override;
   void ViewRemoved(View* host, View* view) override;
   void Layout(View* host) override;
   gfx::Size GetPreferredSize(const View* host) const override;
   int GetPreferredHeightForWidth(const View* host, int width) const override;
 
  private:
+  // This struct is used internally to "wrap" a child view in order to obviate

[sky, 2017/05/09 19:26:03]

Thanks for the detailed comments!

+  // the need for the main layout logic to be fully aware of the per-view
+  // margins when |collapse_margin_spacing_| is false. Since each view is a
+  // rectangle of a certain size, this wrapper, coupled with any margins set
+  // will increase the apparent size of the view. All necessary view size/
+  // position methods required for the layout logic add or subtract the margins
+  // where appropriate to ensure the actual visible size of the view doesn't
+  // include the margins.
+  // When |collapse_margin_spacing_| is true, this wrapper provides quick access
+  // to the view's margins for use by the layout to collapse adjacent spacing
+  // to the largest of the several values.
+  struct ViewWrapper {

[sky, 2017/05/09 19:26:03]

Style guide says, "Use a struct only for passive objects that carry data;
everything else is a class." Given you have a bunch of functions use a class
here.

[kylix_rd, 2017/05/09 20:07:06]

Sure thing.

This comes from my notion that "class" is for dynamically allocated objects (ie.
new & delete) and "struct" (or "record" in Delphi) is for stack-based or
inplace-allocated instances. Old habits and all :).

[kylix_rd, 2017/05/10 15:30:37]

Done.

+    ViewWrapper();
+    ViewWrapper(const BoxLayout* layout, View* view);
+    ViewWrapper(const ViewWrapper& wrapper) = delete;
+    ~ViewWrapper();
+    int GetHeightForWidth(int width) const;
+    const gfx::Insets& margins() const { return margins_; }
+    gfx::Size GetPreferredSize() const;
+    void SetBoundsRect(const gfx::Rect& bounds);
+    View* view() const { return view_; }
+    bool visible() const;
+
+   private:
+    View* view_;
+    const BoxLayout* layout_;
+    gfx::Insets margins_;
+  };
+
+  using FlexMap = std::map<const View*, int>;
+
   // Returns the flex for the specified |view|.
   int GetFlexForView(const View* view) const;
 
   // Returns the size and position along the main axis of |rect|.
   int MainAxisSize(const gfx::Rect& rect) const;
   int MainAxisPosition(const gfx::Rect& rect) const;
 
   // Sets the size and position along the main axis of |rect|.
   void SetMainAxisSize(int size, gfx::Rect* rect) const;
   void SetMainAxisPosition(int position, gfx::Rect* rect) const;
 
   // Returns the size and position along the cross axis of |rect|.
   int CrossAxisSize(const gfx::Rect& rect) const;
   int CrossAxisPosition(const gfx::Rect& rect) const;
 
   // Sets the size and position along the cross axis of |rect|.
   void SetCrossAxisSize(int size, gfx::Rect* rect) const;
   void SetCrossAxisPosition(int size, gfx::Rect* rect) const;
 
   // Returns the main axis size for the given view. |child_area_width| is needed
   // to calculate the height of the view when the orientation is vertical.
-  int MainAxisSizeForView(const View* view, int child_area_width) const;
+  int MainAxisSizeForView(const ViewWrapper& view, int child_area_width) const;
+
+  // Returns the main axis margin spacing between the two views which is the max
+  // of the right margin from the |left| view, the left margin of the |right|
+  // view and |between_child_spacing_|.
+  int MainAxisMarginBetweenViews(const ViewWrapper& left,
+                                 const ViewWrapper& right) const;
+
+  // Adjusts the main axis for |rect| by optionally collapsing the
+  // left or top margin of the first view with corresponding side of
+  // |inside_border_insets_| and the right or bottom margin of the last view
+  // with the corresponding side of |inside_border_insets_|.
+  void AdjustMainAxisForMargin(gfx::Rect* rect) const;
+
+  // Adjust the cross axis of |rect| by optionally collapsing the closest margin
+  // of |control| with the corresponding side of |inside_border_insets_|
+  void AdjustCrossAxisForMargin(const ViewWrapper& child,
+                                const gfx::Rect& client,
+                                gfx::Rect* rect) const;
 
   // Returns the cross axis size for the given view.
-  int CrossAxisSizeForView(const View* view) const;
+  int CrossAxisSizeForView(const ViewWrapper& view) const;
 
   // The preferred size for the dialog given the width of the child area.
   gfx::Size GetPreferredSizeForChildWidth(const View* host,
                                           int child_area_width) const;
 
   // The amount of space the layout requires in addition to any space for the
   // child views.
   gfx::Size NonChildSize(const View* host) const;
 
+  // The next visible view at > index. If no other views are visible, return
+  // nullptr.
+  View* NextVisibleView(View* host, int index) const;
+
+  // Return the first visible view in the host or nullptr if none are visible.
+  View* FirstVisibleView(View* host) const;
+
+  // Return the last visible view in the host or nullptr if none are visible.
+  View* LastVisibleView(View* host) const;
+
   const Orientation orientation_;
 
   // Spacing between child views and host view border.
   gfx::Insets inside_border_insets_;
 
   // Spacing to put in between child views.
   const int between_child_spacing_;
 
   // The alignment of children in the main axis. This is
   // MAIN_AXIS_ALIGNMENT_START by default.
   MainAxisAlignment main_axis_alignment_;
 
   // The alignment of children in the cross axis. This is
   // CROSS_AXIS_ALIGNMENT_STRETCH by default.
   CrossAxisAlignment cross_axis_alignment_;
 
   // A map of views to their flex weights.
-  std::map<const View*, int> flex_map_;
+  FlexMap flex_map_;
 
   // The flex weight for views if none is set. Defaults to 0.
   int default_flex_;
 
   // The minimum cross axis size for the layout.
   int minimum_cross_axis_size_;
 
+  // Adjacent view margins and spacing should be collapsed.
+  bool collapse_margins_spacing_;

[sky, 2017/05/09 19:26:03]

const

[kylix_rd, 2017/05/10 15:30:37]

Done.

+
   // The view that this BoxLayout is managing the layout for.
   views::View* host_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(BoxLayout);
 };
 
 } // namespace views
 
 #endif  // UI_VIEWS_LAYOUT_BOX_LAYOUT_H_