Desktop PWAs: Move minimal-ui buttons to top left corner

The minimal-ui buttons (Back and Reload) now appear to the left of the window title,

The origin and three dot menu appear to the far right (as before).

Box layout is used to layout the buttons on the left and the buttons on the right.

Flex layout is used to distribute space between the left and right button areas
and the window title area in the center.

Platform-specific adjustment of the spacing around the minimal-ui buttons
will occur in a subsequent CL.

Desktop web applications can request the minimal-ui buttons by specifying
display minimal-ui in their web app manifest:
https://www.w3.org/TR/appmanifest/#dom-displaymodetype-minimal-ui


Bug: 1016663
Change-Id: I449f85b2a8d5912811245e40e084411629e8b1db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900801
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715310}

chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_controller_views.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/layout_constants.h"
+#include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -74,12 +75,13 @@ BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac(
               frame, browser_view,
               GetCaptionColor(BrowserFrameActiveState::kActive),
               GetCaptionColor(BrowserFrameActiveState::kInactive),
-              kHostedAppMenuMargin)));
+              kHostedAppMenuMargin, kHostedAppMenuMargin)));
     }
 
     DCHECK(browser_view->ShouldShowWindowTitle());
     window_title_ = AddChildView(
         std::make_unique<views::Label>(browser_view->GetWindowTitle()));
+    window_title_->SetID(VIEW_ID_WINDOW_TITLE);
   }
 }
 
@@ -325,8 +327,11 @@ void BrowserNonClientFrameViewMac::Layout() {
   int trailing_x = width();
 
   if (web_app_frame_toolbar()) {
-    trailing_x = web_app_frame_toolbar()->LayoutInContainer(
-        leading_x, trailing_x, 0, available_height);
+    std::pair<int, int> remaining_bounds =
+        web_app_frame_toolbar()->LayoutInContainer(leading_x, trailing_x, 0,
+                                                   available_height);
+    leading_x = remaining_bounds.first;
+    trailing_x = remaining_bounds.second;
 
     const int title_padding = base::checked_cast<int>(
         std::round(width() * kTitlePaddingWidthFraction));

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

@@ -676,17 +676,20 @@ void GlassBrowserFrameView::LayoutTitleBar() {
   }
 
   if (web_app_frame_toolbar()) {
-    next_trailing_x = web_app_frame_toolbar()->LayoutInContainer(
-        next_leading_x, next_trailing_x, window_top, titlebar_visual_height);
+    std::pair<int, int> remaining_bounds =
+        web_app_frame_toolbar()->LayoutInContainer(next_leading_x,
+                                                   next_trailing_x, window_top,
+                                                   titlebar_visual_height);
+    next_leading_x = remaining_bounds.first;
+    next_trailing_x = remaining_bounds.second;
   }
 
   if (ShowCustomTitle()) {
-    if (!ShowCustomIcon()) {
-      // This matches native Windows 10 UWP apps that don't have window icons.
-      constexpr int kMinimumTitleLeftBorderMargin = 11;
-      DCHECK_LE(next_leading_x, kMinimumTitleLeftBorderMargin);
-      next_leading_x = kMinimumTitleLeftBorderMargin;
-    }
+    // If nothing has been added to the left, match native Windows 10 UWP apps
+    // that don't have window icons.
+    constexpr int kMinimumTitleLeftBorderMargin = 11;
+    next_leading_x = std::max(next_leading_x, kMinimumTitleLeftBorderMargin);
+
     window_title_->SetText(browser_view()->GetWindowTitle());
     const int max_text_width = std::max(0, next_trailing_x - next_leading_x);
     window_title_->SetBounds(next_leading_x, window_icon_bounds.y(),

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

@@ -88,50 +88,6 @@ IN_PROC_BROWSER_TEST_F(WebAppGlassBrowserFrameViewTest, NoThemeColor) {
       ThemeProperties::GetDefaultColor(ThemeProperties::COLOR_FRAME, false));
 }
 
-IN_PROC_BROWSER_TEST_F(WebAppGlassBrowserFrameViewTest, SpaceConstrained) {
-  theme_color_ = base::nullopt;
-  if (!InstallAndLaunchWebApp())
-    return;
-
-  views::View* page_action_icon_container =
-      web_app_frame_toolbar_->GetPageActionIconContainerForTesting();
-  EXPECT_EQ(page_action_icon_container->parent(), web_app_frame_toolbar_);
-
-  views::View* menu_button =
-      browser_view_->toolbar_button_provider()->GetAppMenuButton();
-  EXPECT_EQ(menu_button->parent(), web_app_frame_toolbar_);
-
-  // 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 layout should be invalidated, but since we don't have the benefit of
-  // the compositor to immediately kick a layout off, we have to do it manually.
-  web_app_frame_toolbar_->Layout();
-
-  // 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 WebAppFrameToolbarView just enough to clip out the page action
-  // icons.
-  web_app_frame_toolbar_->SetSize(
-      gfx::Size(web_app_frame_toolbar_->width() -
-                    page_action_icon_container->bounds().right(),
-                web_app_frame_toolbar_->height()));
-  web_app_frame_toolbar_->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);
-}
-
 IN_PROC_BROWSER_TEST_F(WebAppGlassBrowserFrameViewTest, MaximizedLayout) {
   if (!InstallAndLaunchWebApp())
     return;

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

@@ -140,9 +140,9 @@ IN_PROC_BROWSER_TEST_F(WebAppOpaqueBrowserFrameViewTest, StaticTitleBarHeight) {
   const int title_bar_height = GetRestoredTitleBarHeight();
   EXPECT_GT(title_bar_height, 0);
 
-  // Add taller children to the web app frame toolbar.
+  // Add taller children to the web app frame toolbar RHS.
   const int container_height = web_app_frame_toolbar_->height();
-  web_app_frame_toolbar_->AddChildView(
+  web_app_frame_toolbar_->GetRightContainerForTesting()->AddChildView(
       new views::StaticSizedView(gfx::Size(1, title_bar_height * 2)));
   opaque_browser_frame_view_->Layout();
 

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

@@ -329,9 +329,12 @@ void OpaqueBrowserFrameViewLayout::LayoutTitleBar() {
     minimum_size_for_buttons_ += size + kIconLeftSpacing;
 
     if (web_app_frame_toolbar_) {
-      available_space_trailing_x_ = web_app_frame_toolbar_->LayoutInContainer(
-          available_space_leading_x_, available_space_trailing_x_, 0,
-          available_height);
+      std::pair<int, int> remaining_bounds =
+          web_app_frame_toolbar_->LayoutInContainer(available_space_leading_x_,
+                                                    available_space_trailing_x_,
+                                                    0, available_height);
+      available_space_leading_x_ = remaining_bounds.first;
+      available_space_trailing_x_ = remaining_bounds.second;
     }
   }
 

chrome/browser/ui/views/web_apps/web_app_frame_toolbar_browsertest.cc

+// Copyright 2019 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.
+
+#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
+#include "chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.h"
+
+#include "base/logging.h"
+#include "base/test/scoped_feature_list.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/web_applications/test/web_app_browsertest_util.h"
+#include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/common/web_application_info.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "ui/gfx/geometry/size.h"
+#include "ui/views/view.h"
+#include "ui/views/widget/widget.h"
+#include "ui/views/window/non_client_view.h"
+#include "url/gurl.h"
+
+class WebAppFrameToolbarBrowserTest : public InProcessBrowserTest {
+ public:
+  WebAppFrameToolbarBrowserTest() {
+    scoped_feature_list_.InitWithFeatures({features::kDesktopMinimalUI}, {});
+  }
+  ~WebAppFrameToolbarBrowserTest() override = default;
+
+  WebAppFrameToolbarBrowserTest(const WebAppFrameToolbarBrowserTest&) = delete;
+  WebAppFrameToolbarBrowserTest& operator=(
+      const WebAppFrameToolbarBrowserTest&) = delete;
+
+  GURL GetAppURL() { return GURL("https://test.org"); }
+
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+
+    WebAppFrameToolbarView::DisableAnimationForTesting();
+  }
+
+  void InstallAndLaunchWebApp() {
+    auto web_app_info = std::make_unique<WebApplicationInfo>();
+    web_app_info->app_url = GetAppURL();
+    web_app_info->scope = GetAppURL().GetWithoutFilename();
+    web_app_info->display_mode = web_app::DisplayMode::kMinimalUi;
+    web_app_info->open_as_window = true;
+
+    web_app::AppId app_id =
+        web_app::InstallWebApp(browser()->profile(), std::move(web_app_info));
+    content::TestNavigationObserver navigation_observer(GetAppURL());
+    navigation_observer.StartWatchingNewWebContents();
+    app_browser_ = web_app::LaunchWebAppBrowser(browser()->profile(), app_id);
+    navigation_observer.WaitForNavigationFinished();
+
+    browser_view_ = BrowserView::GetBrowserViewForBrowser(app_browser_);
+    views::NonClientFrameView* frame_view =
+        browser_view_->GetWidget()->non_client_view()->frame_view();
+    frame_view_ = static_cast<BrowserNonClientFrameView*>(frame_view);
+
+    web_app_frame_toolbar_ = frame_view_->web_app_frame_toolbar_for_testing();
+    DCHECK(web_app_frame_toolbar_);
+    DCHECK(web_app_frame_toolbar_->GetVisible());
+  }
+
+  Browser* app_browser_ = nullptr;
+  BrowserView* browser_view_ = nullptr;
+  BrowserNonClientFrameView* frame_view_ = nullptr;
+  WebAppFrameToolbarView* web_app_frame_toolbar_ = nullptr;
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest, SpaceConstrained) {
+  InstallAndLaunchWebApp();
+
+  views::View* toolbar_right_container =
+      web_app_frame_toolbar_->GetRightContainerForTesting();
+  EXPECT_EQ(toolbar_right_container->parent(), web_app_frame_toolbar_);
+
+  views::View* page_action_icon_container =
+      web_app_frame_toolbar_->GetPageActionIconContainerForTesting();
+  EXPECT_EQ(page_action_icon_container->parent(), toolbar_right_container);
+
+  views::View* menu_button =
+      browser_view_->toolbar_button_provider()->GetAppMenuButton();
+  EXPECT_EQ(menu_button->parent(), toolbar_right_container);
+
+  // Initially the page action icons are not visible, just the menu button has
+  // width.
+  EXPECT_EQ(page_action_icon_container->width(), 0);
+  const 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 layout should be invalidated, but since we don't have the benefit of
+  // the compositor to immediately kick a layout off, we have to do it manually.
+  web_app_frame_toolbar_->Layout();
+
+  // 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 WebAppFrameToolbarView just enough to clip out the page action
+  // icons.
+  web_app_frame_toolbar_->SetSize(
+      gfx::Size(toolbar_right_container->width() -
+                    page_action_icon_container->bounds().right(),
+                web_app_frame_toolbar_->height()));
+  web_app_frame_toolbar_->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/web_apps/web_app_frame_toolbar_view.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_WEB_APPS_WEB_APP_FRAME_TOOLBAR_VIEW_H_
 
 #include <memory>
+#include <utility>
 
 #include "base/macros.h"
 #include "base/scoped_observer.h"
@@ -20,6 +21,7 @@
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/base/material_design/material_design_controller_observer.h"
 #include "ui/gfx/color_palette.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/views/accessible_pane_view.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/menu_button.h"
@@ -71,6 +73,7 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
                          BrowserView* browser_view,
                          SkColor active_color,
                          SkColor inactive_color,
+                         base::Optional<int> left_margin = base::nullopt,
                          base::Optional<int> right_margin = base::nullopt);
   ~WebAppFrameToolbarView() override;
 
@@ -81,12 +84,12 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
   // Sets the container to paints its buttons the active/inactive color.
   void SetPaintAsActive(bool active);
 
-  // Sets own bounds to be right aligned and vertically centered in the given
-  // space, returns a new trailing_x value.
-  int LayoutInContainer(int leading_x,
-                        int trailing_x,
-                        int y,
-                        int available_height);
+  // Sets own bounds equal to the available space and returns the bounds of the
+  // remaining inner space as a pair of (leading x, trailing x).
+  std::pair<int, int> LayoutInContainer(int leading_x,
+                                        int trailing_x,
+                                        int y,
+                                        int available_height);
 
   SkColor active_color_for_testing() const { return active_color_; }
 
@@ -145,11 +148,11 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
   void OnTouchUiChanged() override;
 
   static void DisableAnimationForTesting();
+  views::View* GetRightContainerForTesting();
   views::View* GetPageActionIconContainerForTesting();
 
  protected:
   // views::AccessiblePaneView:
-  gfx::Size CalculatePreferredSize() const override;
   void ChildPreferredSizeChanged(views::View* child) override;
 
  private:
@@ -201,7 +204,19 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
   SkColor active_color_;
   SkColor inactive_color_;
 
-  // Owned by the views hierarchy.
+  class ToolbarButtonContainer;
+
+  // All remaining members are owned by the views hierarchy.
+
+  // These three fields are only created when the display mode is minimal-ui.
+  ToolbarButtonContainer* left_container_ = nullptr;
+  ToolbarButton* back_ = nullptr;
+  ReloadButton* reload_ = nullptr;
+
+  // Empty container used by the parent frame to layout additional elements.
+  views::View* center_container_ = nullptr;
+
+  ToolbarButtonContainer* right_container_ = nullptr;
   WebAppOriginText* web_app_origin_text_ = nullptr;
   ContentSettingsContainer* content_settings_container_ = nullptr;
   PageActionIconContainerView* page_action_icon_container_view_ = nullptr;
@@ -209,9 +224,6 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
   ExtensionsToolbarContainer* extensions_container_ = nullptr;
   WebAppMenuButton* web_app_menu_button_ = nullptr;
 
-  // The following buttons are only created when the display mode is minimal-ui.
-  ToolbarButton* back_ = nullptr;
-  ReloadButton* reload_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(WebAppFrameToolbarView);
 };

chrome/test/BUILD.gn

@@ -1299,6 +1299,7 @@ if (!is_android) {
       "../browser/ui/views/hats/hats_browsertest.cc",
       "../browser/ui/views/intent_picker_bubble_view_browsertest.cc",
       "../browser/ui/views/try_chrome_dialog_win/try_chrome_dialog_browsertest.cc",
+      "../browser/ui/views/web_apps/web_app_frame_toolbar_browsertest.cc",
       "../browser/ui/views/web_apps/web_app_minimal_ui_test.cc",
       "../browser/ui/views/webauthn/authenticator_dialog_view_browsertest.cc",
       "../browser/ui/views/webauthn/authenticator_qr_code_test.cc",