Make BoxLayout main axis clipping relative to main axis alignment

This CL updates BoxLayout to avoid clipping the main axis alignment
point when there isn't enough main axis space.

This means for:
 - left aligned layouts the right most children will be clipped first
   (existing behaviour).
 - center aligned layouts the left and right most children will be
   clipped first.
 - right aligned layouts the left most children will be clipped first.
Where previously the right most children were always clipped first
regardless of the main axis alignment position.

This fixes hosted app titlebar icons such that they clip from the
left instead of clipping out the app menu button first.

Analysis of existing alignment use cases here:
https://bugs.chromium.org/p/chromium/issues/detail?id=862045#c3

Before:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=347613&signed_aid=Nst_-68AwDajlir_GX00cQ==&inline=1
After:
https://bugs.chromium.org/p/chromium/issues/attachment?aid=349265&signed_aid=RgUQgRr8Udaf6_WsZG31ig==&inline=1

Bug: 862045
Change-Id: I073cadced4d9b9172f9c803506c2faadd11683ca
Reviewed-on: https://chromium-review.googlesource.com/1143115
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577463}

chrome/browser/ui/views/frame/glass_browser_frame_view_browsertest_win.cc

@@ -8,8 +8,11 @@
 #include "chrome/browser/extensions/browsertest_util.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/views/frame/app_menu_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
+#include "chrome/browser/ui/views/page_action/page_action_icon_container_view.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -80,7 +83,7 @@ IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, ThemeColor) {
   if (!InstallAndLaunchHostedApp())
     return;
 
-  DCHECK_EQ(glass_frame_view_->GetTitlebarColor(), *theme_color_);
+  EXPECT_EQ(glass_frame_view_->GetTitlebarColor(), *theme_color_);
 }
 
 IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, NoThemeColor) {
@@ -88,7 +91,48 @@ IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, NoThemeColor) {
   if (!InstallAndLaunchHostedApp())
     return;
 
-  DCHECK_EQ(
+  EXPECT_EQ(
       glass_frame_view_->GetTitlebarColor(),
       ThemeProperties::GetDefaultColor(ThemeProperties::COLOR_FRAME, false));
 }
+
+IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, SpaceConstrained) {
+  theme_color_ = base::nullopt;
+  if (!InstallAndLaunchHostedApp())
+    return;
+
+  views::View* page_action_icon_container =
+      browser_view_->toolbar_button_provider()
+          ->GetPageActionIconContainerView();
+  EXPECT_EQ(page_action_icon_container->parent(), hosted_app_button_container_);
+
+  views::View* menu_button =
+      browser_view_->toolbar_button_provider()->GetAppMenuButton();
+  EXPECT_EQ(menu_button->parent(), hosted_app_button_container_);
+
+  // Initially the page action icons are not visible, just the menu button has
+  // width.
+  EXPECT_EQ(page_action_icon_container->width(), 0);
+  int original_menu_button_width = menu_button->width();
+  EXPECT_GT(original_menu_button_width, 0);
+
+  // Cause the zoom page action icon to be visible.
+  chrome::Zoom(app_browser_, content::PAGE_ZOOM_IN);
+
+  // The page action icons should now take up width.
+  EXPECT_GT(page_action_icon_container->width(), 0);
+  EXPECT_EQ(menu_button->width(), original_menu_button_width);
+
+  // Resize the HostedAppButtonContainer just enough to clip out the page action
+  // icons.
+  hosted_app_button_container_->SetSize(
+      gfx::Size(hosted_app_button_container_->width() -
+                    page_action_icon_container->bounds().right(),
+                hosted_app_button_container_->height()));
+  hosted_app_button_container_->Layout();
+
+  // The page action icons should be clipped to 0 width while the app menu
+  // button retains its full width.
+  EXPECT_EQ(page_action_icon_container->width(), 0);
+  EXPECT_EQ(menu_button->width(), original_menu_button_width);
+}

chrome/browser/ui/views/frame/hosted_app_button_container.cc

@@ -162,10 +162,13 @@ HostedAppButtonContainer::ContentSettingsContainer::ContentSettingsContainer(
       extensions::HostedAppBrowserController::IsForExperimentalHostedAppBrowser(
           browser_view->browser()));
 
-  SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::kHorizontal, gfx::Insets(),
-      views::LayoutProvider::Get()->GetDistanceMetric(
-          views::DISTANCE_RELATED_CONTROL_HORIZONTAL)));
+  views::BoxLayout& layout =
+      *SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::kHorizontal, gfx::Insets(),
+          views::LayoutProvider::Get()->GetDistanceMetric(
+              views::DISTANCE_RELATED_CONTROL_HORIZONTAL)));
+  // Right align to clip the leftmost items first when not enough space.
+  layout.set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
 
   std::vector<std::unique_ptr<ContentSettingImageModel>> models =
       ContentSettingImageModel::GenerateContentSettingImageModels();
@@ -209,13 +212,15 @@ HostedAppButtonContainer::HostedAppButtonContainer(
                                       false /* interactive */)),
       app_menu_button_(new HostedAppMenuButton(browser_view)) {
   DCHECK(browser_view_);
-  auto layout = std::make_unique<views::BoxLayout>(
-      views::BoxLayout::kHorizontal,
-      gfx::Insets(0, HorizontalPaddingBetweenItems()),
-      HorizontalPaddingBetweenItems());
-  layout->set_cross_axis_alignment(
+  views::BoxLayout& layout =
+      *SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::kHorizontal,
+          gfx::Insets(0, HorizontalPaddingBetweenItems()),
+          HorizontalPaddingBetweenItems()));
+  // Right align to clip the leftmost items first when not enough space.
+  layout.set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
+  layout.set_cross_axis_alignment(
       views::BoxLayout::CROSS_AXIS_ALIGNMENT_CENTER);
-  SetLayoutManager(std::move(layout));
 
   AddChildView(content_settings_container_);
   AddChildView(page_action_icon_container_view_);

chrome/browser/ui/views/page_action/page_action_icon_container_view.cc

@@ -18,8 +18,11 @@ PageActionIconContainerView::PageActionIconContainerView(
     PageActionIconView::Delegate* page_action_icon_delegate,
     LocationBarView::Delegate* location_bar_delegate)
     : zoom_observer_(this) {
-  SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::kHorizontal, gfx::Insets(), between_icon_spacing));
+  views::BoxLayout& layout =
+      *SetLayoutManager(std::make_unique<views::BoxLayout>(
+          views::BoxLayout::kHorizontal, gfx::Insets(), between_icon_spacing));
+  // Right align to clip the leftmost items first when not enough space.
+  layout.set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
 
   for (PageActionIconType type : types_enabled) {
     switch (type) {

ui/views/layout/box_layout.cc

@@ -165,19 +165,19 @@ void BoxLayout::Layout(View* host) {
   total_main_axis_size -= between_child_spacing_;
   // Free space can be negative indicating that the views want to overflow.
   int main_free_space = MainAxisSize(child_area) - total_main_axis_size;
+  int main_position = MainAxisPosition(child_area);
   {
-    int position = MainAxisPosition(child_area);
     int size = MainAxisSize(child_area);
     if (!flex_sum) {
       switch (main_axis_alignment_) {
         case MAIN_AXIS_ALIGNMENT_START:
           break;
         case MAIN_AXIS_ALIGNMENT_CENTER:
-          position += main_free_space / 2;
+          main_position += main_free_space / 2;
           size = total_main_axis_size;
           break;
         case MAIN_AXIS_ALIGNMENT_END:
-          position += main_free_space;
+          main_position += main_free_space;
           size = total_main_axis_size;
           break;
         default:
@@ -186,12 +186,11 @@ void BoxLayout::Layout(View* host) {
       }
     }
     gfx::Rect new_child_area(child_area);
-    SetMainAxisPosition(position, &new_child_area);
+    SetMainAxisPosition(main_position, &new_child_area);
     SetMainAxisSize(size, &new_child_area);
     child_area.Intersect(new_child_area);
   }
 
-  int main_position = MainAxisPosition(child_area);
   int total_padding = 0;
   int current_flex = 0;
   for (int i = 0; i < host->child_count(); ++i) {

ui/views/layout/box_layout.h

@@ -33,9 +33,10 @@ class VIEWS_EXPORT BoxLayout : public LayoutManager {
     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.
+  // This specifies that the start/center/end of the collective child views is
+  // aligned with the start/center/end of the host view. e.g. a horizontal
+  // layout of MAIN_AXIS_ALIGNMENT_END will result in the child views being
+  // right-aligned.
   enum MainAxisAlignment {
     MAIN_AXIS_ALIGNMENT_START,
     MAIN_AXIS_ALIGNMENT_CENTER,

ui/views/layout/box_layout_unittest.cc

@@ -102,29 +102,30 @@ TEST_F(BoxLayoutTest, Overflow) {
   host_->AddChildView(v1);
   View* v2 = new StaticSizedView(gfx::Size(10, 20));
   host_->AddChildView(v2);
-  host_->SetBounds(0, 0, 10, 10);
+  host_->SetBounds(0, 0, 15, 10);
 
   // Overflows by positioning views at the start and truncating anything that
   // doesn't fit.
   host_->Layout();
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), v1->bounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 15, 10), v1->bounds());
   EXPECT_EQ(gfx::Rect(0, 0, 0, 0), v2->bounds());
 
-  // All values of main axis alignment should overflow in the same manner.
+  // Clipping of children should occur at the opposite end(s) to the main axis
+  // alignment position.
   layout->set_main_axis_alignment(BoxLayout::MAIN_AXIS_ALIGNMENT_START);
   host_->Layout();
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), v1->bounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 15, 10), v1->bounds());
   EXPECT_EQ(gfx::Rect(0, 0, 0, 0), v2->bounds());
 
   layout->set_main_axis_alignment(BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
   host_->Layout();
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), v1->bounds());
-  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), v2->bounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 13, 10), v1->bounds());
+  EXPECT_EQ(gfx::Rect(13, 0, 2, 10), v2->bounds());
 
   layout->set_main_axis_alignment(BoxLayout::MAIN_AXIS_ALIGNMENT_END);
   host_->Layout();
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), v1->bounds());
-  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), v2->bounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 5, 10), v1->bounds());
+  EXPECT_EQ(gfx::Rect(5, 0, 10, 10), v2->bounds());
 }
 
 TEST_F(BoxLayoutTest, NoSpace) {