Implement PWA title bar UI for Mac

This CL updates hosted app windows on Mac to paint the site's theme colour and include the hosted app menu button and permission icons. This change is hidden behind the kDesktopPWAWindowing flag. Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=363793&signed_aid=_Z4yIZ_JlJC81IRgR-cUrA==&inline=1 After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=363794&signed_aid=hHrKili6tqk3XTdQPfWLiQ==&inline=1 Bug: 895690 Change-Id: I7753b5119f165dab9573cc35f6d56cafb1ccfdb8 Reviewed-on: https://chromium-review.googlesource.com/c/1290057 Commit-Queue: Alan Cutter <alancutter@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#601443}
hylschwy · Oct 21, 2018 · 2f84adc · 2f84adc
1 parent 06696fa
commit 2f84adc
Show file tree

Hide file tree

Showing 7 changed files with 168 additions and 15 deletions.
diff --git a/chrome/browser/ui/views/frame/browser_frame_mac.mm b/chrome/browser/ui/views/frame/browser_frame_mac.mm
@@ -127,7 +127,8 @@ - (NSTouchBar*)makeTouchBar API_AVAILABLE(macos(10.12.2)) {
                           NSMiniaturizableWindowMask | NSResizableWindowMask;
 
   base::scoped_nsobject<NativeWidgetMacNSWindow> ns_window;
-  if (browser_view_->IsBrowserTypeNormal()) {
+  if (browser_view_->IsBrowserTypeNormal() ||
+      browser_view_->IsBrowserTypeHostedApp()) {
     if (@available(macOS 10.10, *))
       style_mask |= NSFullSizeContentViewWindowMask;
     ns_window.reset([[BrowserNativeWidgetWindow alloc]
@@ -136,8 +137,12 @@ - (NSTouchBar*)makeTouchBar API_AVAILABLE(macos(10.12.2)) {
                     backing:NSBackingStoreBuffered
                       defer:NO]);
     // Ensure tabstrip/profile button are visible.
-    if (@available(macOS 10.10, *))
+    if (@available(macOS 10.10, *)) {
       [ns_window setTitlebarAppearsTransparent:YES];
+      // Hosted apps draw their own window title.
+      if (browser_view_->IsBrowserTypeHostedApp())
+        [ns_window setTitleVisibility:NSWindowTitleHidden];
+    }
   } else {
     ns_window.reset([[NativeWidgetMacNSWindow alloc]
         initWithContentRect:ui::kWindowSizeDeterminedLater

diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h
@@ -12,6 +12,10 @@
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "components/prefs/pref_change_registrar.h"
 
+namespace views {
+class Label;
+}
+
 @class FullscreenToolbarControllerViews;
 
 class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
@@ -36,7 +40,6 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
       const gfx::Rect& client_bounds) const override;
   int NonClientHitTest(const gfx::Point& point) override;
   void GetWindowMask(const gfx::Size& size, gfx::Path* window_mask) override;
-  void ResetWindowControls() override;
   void UpdateWindowIcon() override;
   void UpdateWindowTitle() override;
   void SizeConstraintsChanged() override;
@@ -47,10 +50,24 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
  protected:
   // views::View:
   void OnPaint(gfx::Canvas* canvas) override;
+  void Layout() override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(BrowserNonClientFrameViewMacTest,
+                           GetCenteredTitleBounds);
+
+  static gfx::Rect GetCenteredTitleBounds(int frame_width,
+                                          int frame_height,
+                                          int left_inset_x,
+                                          int right_inset_x,
+                                          int title_width);
+
   void PaintThemedFrame(gfx::Canvas* canvas);
 
+  // Returns the color to use for text and other title bar elements given the
+  // frame background color for |active_state|.
+  SkColor GetReadableFrameForegroundColor(ActiveState active_state) const;
+
   CGFloat FullscreenBackingBarHeight() const;
 
   // Calculate the y offset the top UI needs to shift down due to showing the
@@ -60,6 +77,8 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
   // Used to keep track of the update of kShowFullscreenToolbar preference.
   PrefChangeRegistrar pref_registrar_;
 
+  views::Label* window_title_ = nullptr;
+
   base::scoped_nsobject<FullscreenToolbarControllerViews>
       fullscreen_toolbar_controller_;
 

diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm
@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/browser_view_layout.h"
+#include "chrome/browser/ui/views/frame/hosted_app_button_container.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
@@ -24,13 +25,18 @@
 #include "ui/gfx/canvas.h"
 
 namespace {
+
+constexpr int kHostedAppMenuMargin = 7;
+constexpr int kFramePaddingLeft = 75;
+
 FullscreenToolbarStyle GetUserPreferredToolbarStyle(
     const PrefService* pref_service) {
   return pref_service->GetBoolean(prefs::kShowFullscreenToolbar)
              ? FullscreenToolbarStyle::TOOLBAR_PRESENT
              : FullscreenToolbarStyle::TOOLBAR_HIDDEN;
 }
-}
+
+}  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewMac, public:
@@ -50,6 +56,20 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
       prefs::kShowFullscreenToolbar,
       base::BindRepeating(&BrowserNonClientFrameViewMac::UpdateFullscreenTopUI,
                           base::Unretained(this), true));
+
+  if (browser_view->IsBrowserTypeHostedApp()) {
+    set_hosted_app_button_container(new HostedAppButtonContainer(
+        frame, browser_view, GetReadableFrameForegroundColor(kActive),
+        GetReadableFrameForegroundColor(kInactive), kHostedAppMenuMargin));
+    AddChildView(hosted_app_button_container());
+
+    DCHECK(browser_view->ShouldShowWindowTitle());
+    window_title_ = new views::Label(browser_view->GetWindowTitle());
+    // view::Label's readability algorithm conflicts with the one used by
+    // |GetReadableFrameForegroundColor|.
+    window_title_->SetAutoColorReadabilityEnabled(false);
+    AddChildView(window_title_);
+  }
 }
 
 BrowserNonClientFrameViewMac::~BrowserNonClientFrameViewMac() {
@@ -94,6 +114,10 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
 }
 
 int BrowserNonClientFrameViewMac::GetTopInset(bool restored) const {
+  if (browser_view()->IsBrowserTypeHostedApp())
+    return hosted_app_button_container()->GetPreferredSize().height() +
+           kHostedAppMenuMargin * 2;
+
   if (!browser_view()->IsTabStripVisible())
     return 0;
 
@@ -188,6 +212,10 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
 }
 
 int BrowserNonClientFrameViewMac::NonClientHitTest(const gfx::Point& point) {
+  int super_component = BrowserNonClientFrameView::NonClientHitTest(point);
+  if (super_component != HTNOWHERE)
+    return super_component;
+
   // BrowserView::NonClientHitTest will return HTNOWHERE for points that hit
   // the native title bar. On Mac, we need to explicitly return HTCAPTION for
   // those points.
@@ -200,13 +228,14 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
                                                  gfx::Path* window_mask) {
 }
 
-void BrowserNonClientFrameViewMac::ResetWindowControls() {
-}
-
 void BrowserNonClientFrameViewMac::UpdateWindowIcon() {
 }
 
 void BrowserNonClientFrameViewMac::UpdateWindowTitle() {
+  if (window_title_ && !frame()->IsFullscreen()) {
+    window_title_->SetText(browser_view()->GetWindowTitle());
+    Layout();
+  }
 }
 
 void BrowserNonClientFrameViewMac::SizeConstraintsChanged() {
@@ -231,20 +260,64 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
 // views::View:
 
 void BrowserNonClientFrameViewMac::OnPaint(gfx::Canvas* canvas) {
-  if (!browser_view()->IsBrowserTypeNormal())
+  if (!browser_view()->IsBrowserTypeNormal() &&
+      !browser_view()->IsBrowserTypeHostedApp()) {
     return;
+  }
 
-  canvas->DrawColor(GetFrameColor());
+  SkColor frame_color = GetFrameColor();
+  canvas->DrawColor(frame_color);
+
+  if (window_title_) {
+    window_title_->SetBackgroundColor(frame_color);
+    window_title_->SetEnabledColor(
+        GetReadableFrameForegroundColor(kUseCurrent));
+  }
 
   auto* theme_service =
       ThemeServiceFactory::GetForProfile(browser_view()->browser()->profile());
   if (!theme_service->UsingSystemTheme())
     PaintThemedFrame(canvas);
 }
 
+void BrowserNonClientFrameViewMac::Layout() {
+  const int available_height = GetTopInset(true);
+  int leading_x = kFramePaddingLeft;
+  int trailing_x = width();
+
+  if (hosted_app_button_container()) {
+    trailing_x = hosted_app_button_container()->LayoutInContainer(
+        leading_x, trailing_x, 0, available_height);
+    window_title_->SetBoundsRect(GetCenteredTitleBounds(
+        width(), available_height, leading_x, trailing_x,
+        window_title_->CalculatePreferredSize().width()));
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewMac, private:
 
+gfx::Rect BrowserNonClientFrameViewMac::GetCenteredTitleBounds(
+    int frame_width,
+    int frame_height,
+    int left_inset_x,
+    int right_inset_x,
+    int title_width) {
+  // Center in container.
+  int title_x = (frame_width - title_width) / 2;
+
+  // Align right side to right inset if overlapping.
+  title_x = std::min(title_x, right_inset_x - title_width);
+
+  // Align left side to left inset if overlapping.
+  title_x = std::max(title_x, left_inset_x);
+
+  // Clip width to right inset if overlapping.
+  title_width = std::min(title_width, right_inset_x - title_x);
+
+  return gfx::Rect(title_x, 0, title_width, frame_height);
+}
+
 void BrowserNonClientFrameViewMac::PaintThemedFrame(gfx::Canvas* canvas) {
   gfx::ImageSkia image = GetFrameImage();
   canvas->TileImageInt(image, 0, TopUIFullscreenYOffset(), width(),
@@ -253,6 +326,11 @@ FullscreenToolbarStyle GetUserPreferredToolbarStyle(
   canvas->DrawImageInt(overlay, 0, 0);
 }
 
+SkColor BrowserNonClientFrameViewMac::GetReadableFrameForegroundColor(
+    ActiveState active_state) const {
+  return color_utils::GetThemedAssetColor(GetFrameColor(active_state));
+}
+
 CGFloat BrowserNonClientFrameViewMac::FullscreenBackingBarHeight() const {
   BrowserView* browser_view = this->browser_view();
   DCHECK(browser_view->IsFullscreen());

diff --git a/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac_unittest.mm b/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac_unittest.mm
@@ -0,0 +1,42 @@
+// Copyright 2018 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_mac.h"
+
+#include "base/strings/stringprintf.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(BrowserNonClientFrameViewMacTest, GetCenteredTitleBounds) {
+  struct {
+    int frame_width;
+    int frame_height;
+    int left_inset_x;
+    int right_inset_x;
+    int title_width;
+    int expected_title_x;
+    int expected_title_width;
+  } test_cases[] = {
+      {800, 40, 0, 800, 400, 200, 400},   {800, 40, 100, 700, 400, 200, 400},
+      {800, 40, 250, 550, 400, 250, 300}, {800, 40, 350, 450, 400, 350, 100},
+      {800, 40, 100, 700, 400, 200, 400}, {800, 40, 250, 700, 400, 250, 400},
+      {800, 40, 350, 700, 400, 350, 350}, {800, 40, 650, 700, 400, 650, 50},
+      {800, 40, 100, 700, 400, 200, 400}, {800, 40, 100, 550, 400, 150, 400},
+      {800, 40, 100, 450, 400, 100, 350}, {800, 40, 100, 150, 400, 100, 50},
+  };
+
+  int index = 0;
+  for (const auto& test_case : test_cases) {
+    SCOPED_TRACE(base::StringPrintf("\nTest case index: %d", index));
+    gfx::Rect title_bounds =
+        BrowserNonClientFrameViewMac::GetCenteredTitleBounds(
+            test_case.frame_width, test_case.frame_height,
+            test_case.left_inset_x, test_case.right_inset_x,
+            test_case.title_width);
+    gfx::Rect expected_title_bounds =
+        gfx::Rect(test_case.expected_title_x, 0, test_case.expected_title_width,
+                  test_case.frame_height);
+    EXPECT_EQ(title_bounds, expected_title_bounds);
+    index++;
+  }
+}
diff --git a/chrome/browser/ui/views/frame/hosted_app_button_container.cc b/chrome/browser/ui/views/frame/hosted_app_button_container.cc
@@ -164,10 +164,12 @@ HostedAppButtonContainer::ContentSettingsContainer::ContentSettingsContainer(
   }
 }
 
-HostedAppButtonContainer::HostedAppButtonContainer(views::Widget* widget,
-                                                   BrowserView* browser_view,
-                                                   SkColor active_color,
-                                                   SkColor inactive_color)
+HostedAppButtonContainer::HostedAppButtonContainer(
+    views::Widget* widget,
+    BrowserView* browser_view,
+    SkColor active_color,
+    SkColor inactive_color,
+    base::Optional<int> right_margin)
     : scoped_widget_observer_(this),
       browser_view_(browser_view),
       active_color_(active_color),
@@ -192,7 +194,8 @@ HostedAppButtonContainer::HostedAppButtonContainer(views::Widget* widget,
   views::BoxLayout& layout =
       *SetLayoutManager(std::make_unique<views::BoxLayout>(
           views::BoxLayout::kHorizontal,
-          gfx::Insets(0, HorizontalPaddingBetweenItems()),
+          gfx::Insets(0,
+                      right_margin.value_or(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);

diff --git a/chrome/browser/ui/views/frame/hosted_app_button_container.h b/chrome/browser/ui/views/frame/hosted_app_button_container.h
@@ -65,14 +65,17 @@ class HostedAppButtonContainer : public views::AccessiblePaneView,
   HostedAppButtonContainer(views::Widget* widget,
                            BrowserView* browser_view,
                            SkColor active_color,
-                           SkColor inactive_color);
+                           SkColor inactive_color,
+                           base::Optional<int> right_margin = base::nullopt);
   ~HostedAppButtonContainer() override;
 
   void UpdateContentSettingViewsVisibility();
 
   // 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,

diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
@@ -4337,6 +4337,9 @@ test("unit_tests") {
       "../browser/ui/views/translate/translate_bubble_view_unittest.cc",
       "../browser/ui/views/webshare/webshare_target_picker_view_unittest.cc",
     ]
+    if (is_mac) {
+      sources += [ "../browser/ui/views/frame/browser_non_client_frame_view_mac_unittest.mm" ]
+    }
     if (is_chromeos) {
       sources += [ "../browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc" ]
     }