Fix hosted app button layout when maximized in Windows 10

Hosted app windows using GlassBrowserFrameView (Windows 10 only)
had their controls and title misplaced when maximized.
This CL adds missing padding values to the titlebar layout.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=354512&signed_aid=Tmrgizah9A_QhgrbRHXHFQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=354513&signed_aid=h_aEa4bnb7fIu3UutTuNrA==&inline=1

Bug: 876168, 872203
Change-Id: I9e4d3e1ff0776cfba09981cad30a0cfa39469e7c
Reviewed-on: https://chromium-review.googlesource.com/1183019
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585681}

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

@@ -547,7 +547,7 @@ void BrowserNonClientFrameViewAsh::Layout() {
               ash::kCaptionButtonBoundsKey);
       if (inverted_caption_button_bounds) {
         hosted_app_button_container_->LayoutInContainer(
-            0, inverted_caption_button_bounds->x() + width(),
+            0, inverted_caption_button_bounds->x() + width(), 0,
             inverted_caption_button_bounds->height());
       }
     }
@@ -576,7 +576,7 @@ void BrowserNonClientFrameViewAsh::Layout() {
     LayoutIncognitoButton();
   if (hosted_app_button_container_) {
     hosted_app_button_container_->LayoutInContainer(
-        0, caption_button_container_->x(), painted_height);
+        0, caption_button_container_->x(), 0, painted_height);
   }
 
   BrowserNonClientFrameView::Layout();

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

@@ -600,7 +600,18 @@ int GlassBrowserFrameView::TopAreaHeight(bool restored) const {
 }
 
 int GlassBrowserFrameView::TitlebarMaximizedVisualHeight() const {
-  return display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYCAPTION);
+  int maximized_height =
+      display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYCAPTION);
+  if (hosted_app_button_container_) {
+    // Adding 2px of vertical padding puts at least 1 px of space on the top and
+    // bottom of the element.
+    constexpr int kVerticalPadding = 2;
+    maximized_height =
+        std::max(maximized_height,
+                 hosted_app_button_container_->GetPreferredSize().height() +
+                     kVerticalPadding);
+  }
+  return maximized_height;
 }
 
 int GlassBrowserFrameView::TitlebarHeight(bool restored) const {
@@ -710,11 +721,6 @@ SkColor GlassBrowserFrameView::GetTitlebarColor() const {
   return GetFrameColor();
 }
 
-HostedAppButtonContainer*
-GlassBrowserFrameView::GetHostedAppButtonContainerForTesting() const {
-  return hosted_app_button_container_;
-}
-
 Windows10CaptionButton* GlassBrowserFrameView::CreateCaptionButton(
     ViewID button_type,
     int accessible_name_resource_id) {
@@ -916,16 +922,16 @@ void GlassBrowserFrameView::LayoutTitleBar() {
   gfx::Rect window_icon_bounds;
   const int icon_size =
       display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYSMICON);
-  constexpr int kIconMaximizedLeftMargin = 2;
   const int titlebar_visual_height =
       IsMaximized() ? TitlebarMaximizedVisualHeight() : TitlebarHeight(false);
   // Don't include the area above the screen when maximized. However it only
   // looks centered if we start from y=0 when restored.
   const int window_top = IsMaximized() ? WindowTopY() : 0;
   int next_leading_x =
-      IsMaximized()
-          ? kIconMaximizedLeftMargin
-          : display::win::ScreenWin::GetSystemMetricsInDIP(SM_CXSIZEFRAME);
+      display::win::ScreenWin::GetSystemMetricsInDIP(SM_CXSIZEFRAME);
+  constexpr int kMaximizedLeftMargin = 2;
+  if (IsMaximized())
+    next_leading_x += kMaximizedLeftMargin;
   int next_trailing_x = MinimizeButtonX();
 
   const int y = window_top + (titlebar_visual_height - icon_size) / 2;
@@ -943,7 +949,7 @@ void GlassBrowserFrameView::LayoutTitleBar() {
   if (hosted_app_button_container_) {
     DCHECK(!GetProfileSwitcherButton());
     next_trailing_x = hosted_app_button_container_->LayoutInContainer(
-        next_leading_x, next_trailing_x, titlebar_visual_height);
+        next_leading_x, next_trailing_x, window_top, titlebar_visual_height);
   }
 
   if (ShowCustomTitle()) {

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

@@ -74,7 +74,11 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView,
 
   SkColor GetTitlebarColor() const;
 
-  HostedAppButtonContainer* GetHostedAppButtonContainerForTesting() const;
+  HostedAppButtonContainer* hosted_app_button_container_for_testing() {
+    return hosted_app_button_container_;
+  }
+
+  views::Label* window_title_for_testing() { return window_title_; }
 
  protected:
   // views::View:

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

@@ -61,7 +61,7 @@ class HostedAppGlassBrowserFrameViewTest : public InProcessBrowserTest {
     glass_frame_view_ = static_cast<GlassBrowserFrameView*>(frame_view);
 
     hosted_app_button_container_ =
-        glass_frame_view_->GetHostedAppButtonContainerForTesting();
+        glass_frame_view_->hosted_app_button_container_for_testing();
     DCHECK(hosted_app_button_container_);
     DCHECK(hosted_app_button_container_->visible());
     return true;
@@ -136,3 +136,15 @@ IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, SpaceConstrained) {
   EXPECT_EQ(page_action_icon_container->width(), 0);
   EXPECT_EQ(menu_button->width(), original_menu_button_width);
 }
+
+IN_PROC_BROWSER_TEST_F(HostedAppGlassBrowserFrameViewTest, MaximizedLayout) {
+  if (!InstallAndLaunchHostedApp())
+    return;
+
+  glass_frame_view_->frame()->Maximize();
+  static_cast<views::View*>(glass_frame_view_)->Layout();
+
+  DCHECK_GT(glass_frame_view_->window_title_for_testing()->x(), 0);
+  DCHECK_GT(glass_frame_view_->hosted_app_button_container_for_testing()->y(),
+            0);
+}

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

@@ -268,6 +268,7 @@ void HostedAppButtonContainer::SetPaintAsActive(bool active) {
 
 int HostedAppButtonContainer::LayoutInContainer(int leading_x,
                                                 int trailing_x,
+                                                int y,
                                                 int available_height) {
   if (available_height == 0) {
     SetSize(gfx::Size());
@@ -279,7 +280,8 @@ int HostedAppButtonContainer::LayoutInContainer(int leading_x,
       std::min(preferred_size.width(), std::max(0, trailing_x - leading_x));
   const int height = preferred_size.height();
   DCHECK_LE(height, available_height);
-  SetBounds(trailing_x - width, (available_height - height) / 2, width, height);
+  SetBounds(trailing_x - width, y + (available_height - height) / 2, width,
+            height);
   Layout();
   return bounds().x();
 }

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

@@ -59,7 +59,10 @@ class HostedAppButtonContainer : public views::AccessiblePaneView,
   // Sets the container to paints its buttons the active/inactive color.
   void SetPaintAsActive(bool active);
 
-  int LayoutInContainer(int leading_x, int trailing_x, int available_height);
+  int LayoutInContainer(int leading_x,
+                        int trailing_x,
+                        int y,
+                        int available_height);
 
  private:
   friend class HostedAppNonClientFrameViewAshTest;

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

@@ -421,7 +421,7 @@ void OpaqueBrowserFrameViewLayout::LayoutTitleBar(views::View* host) {
     if (hosted_app_button_container_) {
       available_space_trailing_x_ =
           hosted_app_button_container_->LayoutInContainer(
-              available_space_leading_x_, available_space_trailing_x_,
+              available_space_leading_x_, available_space_trailing_x_, y,
               available_height);
     }
   }