Views: Add padding to the right hand side of the TabStripRegionView

Currently the TabStripRegionView controls are flush with the window's
caption buttons on Windows, Linux and ChromeOS and sitting flush with
the browser window itself on Mac.

This CL introduces an 8pt padding to the TabStripRegionView when tab
strip controls are present in keeping with UX guidance. Presently the
only applicable control is the TabSearchBubtton.

This introduces the desired spacing between window caption buttons
and right aligned controls. It also correctly aligns the rightmost
control over the 3 dot menu in the toolbar on Mac.

Bug: 1145277
Change-Id: Ic199b7f21f158723559d82dc37046ff463ae6c51
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522167Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826152}

chrome/browser/ui/layout_constants.cc

@@ -73,6 +73,8 @@ int GetLayoutConstant(LayoutConstant constant) {
       return 8;
     case TAB_STACK_DISTANCE:
       return touch_ui ? 4 : 6;
+    case TABSTRIP_REGION_VIEW_CONTROL_PADDING:
+      return 8;
     case TABSTRIP_TOOLBAR_OVERLAP:
       return 1;
     case TOOLBAR_BUTTON_HEIGHT:

chrome/browser/ui/layout_constants.h

@@ -91,6 +91,10 @@ enum LayoutConstant {
   // non-pixel-aligned drawing goes in.  See https://crbug.com/765723.
   TABSTRIP_TOOLBAR_OVERLAP,
 
+  // The horizontal padding between any right aligned controls and the end of
+  // the TabStripRegionView.
+  TABSTRIP_REGION_VIEW_CONTROL_PADDING,
+
   // The total height, including icons and insets, of buttons in the toolbar.
   TOOLBAR_BUTTON_HEIGHT,
 

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

@@ -61,9 +61,8 @@ class FrameGrabHandle : public views::View {
 }  // namespace
 
 TabStripRegionView::TabStripRegionView(std::unique_ptr<TabStrip> tab_strip) {
-  views::FlexLayout* layout_manager =
-      SetLayoutManager(std::make_unique<views::FlexLayout>());
-  layout_manager->SetOrientation(views::LayoutOrientation::kHorizontal);
+  layout_manager_ = SetLayoutManager(std::make_unique<views::FlexLayout>());
+  layout_manager_->SetOrientation(views::LayoutOrientation::kHorizontal);
 
   tab_strip_ = tab_strip.get();
   tab_strip->SetAvailableWidthCallback(
@@ -142,6 +141,11 @@ TabStripRegionView::TabStripRegionView(std::unique_ptr<TabStrip> tab_strip) {
     tab_search_button->SetProperty(views::kCrossAxisAlignmentKey,
                                    views::LayoutAlignment::kCenter);
     tab_search_button_ = AddChildView(std::move(tab_search_button));
+
+    // Add the margin necessary to ensure correct spacing between right-aligned
+    // controls and the end of the TabStripRegionView.
+    layout_manager_->SetInteriorMargin(gfx::Insets(
+        0, 0, 0, GetLayoutConstant(TABSTRIP_REGION_VIEW_CONTROL_PADDING)));
   }
 }
 
@@ -253,7 +257,8 @@ int TabStripRegionView::CalculateTabStripAvailableWidth() {
       reserved_width += child->GetMinimumSize().width();
   }
 
-  return size().width() - reserved_width;
+  return size().width() - reserved_width -
+         layout_manager_->interior_margin().width();
 }
 
 void TabStripRegionView::ScrollTowardsLeadingTab() {

chrome/browser/ui/views/frame/tab_strip_region_view.h

@@ -9,6 +9,10 @@
 #include "ui/base/pointer/touch_ui_controller.h"
 #include "ui/views/accessible_pane_view.h"
 
+namespace views {
+class FlexLayout;
+}
+
 class NewTabButton;
 class TabSearchButton;
 class TabStrip;
@@ -48,6 +52,8 @@ class TabStripRegionView final : public views::AccessiblePaneView,
   // views::ViewObserver:
   void OnViewPreferredSizeChanged(View* view) override;
 
+  views::FlexLayout* layout_manager_for_testing() { return layout_manager_; }
+
   // TODO(958173): Override OnBoundsChanged to cancel tabstrip animations.
 
  private:
@@ -65,6 +71,7 @@ class TabStripRegionView final : public views::AccessiblePaneView,
   // whenever any input of the computation of the border's sizing changes.
   void UpdateNewTabButtonBorder();
 
+  views::FlexLayout* layout_manager_ = nullptr;
   views::View* tab_strip_container_;
   views::View* reserved_grab_handle_space_;
   TabStrip* tab_strip_;

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

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
+#include "ui/views/layout/flex_layout.h"
 
 class TabStripRegionViewBrowserTest
     : public InProcessBrowserTest,
@@ -179,7 +180,11 @@ IN_PROC_BROWSER_TEST_P(TabStripRegionViewBrowserTest, TestBeginEndFocus) {
 IN_PROC_BROWSER_TEST_P(TabStripRegionViewBrowserTest,
                        TestSearchButtonIsEndAligned) {
   if (base::FeatureList::IsEnabled(features::kTabSearch)) {
-    EXPECT_EQ(tab_strip_region_view()->GetLocalBounds().right(),
+    const int kRightMargin = tab_strip_region_view()
+                                 ->layout_manager_for_testing()
+                                 ->interior_margin()
+                                 .right();
+    EXPECT_EQ(tab_strip_region_view()->GetLocalBounds().right() - kRightMargin,
               tab_search_button()->bounds().right());
   }
 }