[desktop-pwas] Fix browser actions in touch optimized mode.

This CL forces the hosted app browser actions to be 32x32, even in
touch optimized mode. This fixes an issue where the hosted app title bar
was too tall.

Bug: 830285
Change-Id: I1bc3539f8e43ca332783511dbb937389f09eb20b
Reviewed-on: https://chromium-review.googlesource.com/1006135
Commit-Queue: calamity <calamity@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550060}

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

@@ -422,7 +422,7 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
 
 - (void)addViewForAction:(ToolbarActionViewController*)action
                withIndex:(NSUInteger)index {
-  const gfx::Size size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size size = toolbarActionsBar_->GetViewSize();
   NSRect buttonFrame = NSMakeRect(NSMaxX([containerView_ bounds]), 0,
                                   size.width(), size.height());
   BrowserActionButton* newButton =
@@ -700,7 +700,7 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
   // Calculate the row index and the index in the row. We bound the latter
   // because the view can go farther right than the right-most icon in the last
   // row of the overflow menu.
-  const gfx::Size size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size size = toolbarActionsBar_->GetViewSize();
   NSInteger rowIndex = midPoint.y / size.height();
   int icons_per_row = isOverflow_ ?
       toolbarActionsBar_->platform_settings().icons_per_overflow_menu_row :
@@ -733,7 +733,7 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
 
 - (NSRect)frameForIndex:(NSUInteger)index {
   gfx::Rect frameRect = toolbarActionsBar_->GetFrameForIndex(index);
-  const gfx::Size size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size size = toolbarActionsBar_->GetViewSize();
   int iconWidth = size.width();
   // The toolbar actions bar will return an empty rect if the index is for an
   // action that is before range we show (i.e., is for a button that's on the

chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc

@@ -27,7 +27,7 @@ TEST_P(ToolbarActionsBarUnitTest, ExtensionActionWantsToRunAppearance) {
 
   AddTab(browser(), GURL("chrome://newtab"));
 
-  const gfx::Size size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size size = toolbar_actions_bar()->GetViewSize();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ExtensionActionViewController* action =
@@ -79,7 +79,7 @@ TEST_P(ToolbarActionsBarUnitTest, ExtensionActionBlockedActions) {
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(web_contents);
-  const gfx::Size size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size size = toolbar_actions_bar()->GetViewSize();
   std::unique_ptr<IconWithBadgeImageSource> image_source =
       browser_action->GetIconImageSourceForTesting(web_contents, size);
   EXPECT_FALSE(image_source->grayscale());

chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc

@@ -638,7 +638,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionsBarIncognitoTest, IncognitoMode) {
   ASSERT_EQ(1u, actions.size());
   gfx::Image icon = actions[0]->GetIcon(
       second_browser->tab_strip_model()->GetActiveWebContents(),
-      ToolbarActionsBar::GetViewSize());
+      second_browser->window()->GetToolbarActionsBar()->GetViewSize());
   const gfx::ImageSkia* skia = icon.ToImageSkia();
   ASSERT_TRUE(skia);
   // Force the image to try and load a representation.

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

@@ -125,7 +125,16 @@ ToolbarActionsBar::~ToolbarActionsBar() {
 }
 
 // static
-gfx::Size ToolbarActionsBar::GetViewSize() {
+void ToolbarActionsBar::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterBooleanPref(
+      prefs::kToolbarIconSurfacingBubbleAcknowledged, false,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  registry->RegisterInt64Pref(prefs::kToolbarIconSurfacingBubbleLastShowTime,
+                              0);
+}
+
+gfx::Size ToolbarActionsBar::GetViewSize() const {
 #if defined(OS_MACOSX)
   // On the Mac, the spec is a 24x24 button in a 28x28 space.
   constexpr gfx::Size kSize(24, 24);
@@ -137,17 +146,6 @@ gfx::Size ToolbarActionsBar::GetViewSize() {
   return rect.size();
 }
 
-// static
-void ToolbarActionsBar::RegisterProfilePrefs(
-    user_prefs::PrefRegistrySyncable* registry) {
-  registry->RegisterBooleanPref(
-      prefs::kToolbarIconSurfacingBubbleAcknowledged,
-      false,
-      user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
-  registry->RegisterInt64Pref(prefs::kToolbarIconSurfacingBubbleLastShowTime,
-                              0);
-}
-
 gfx::Size ToolbarActionsBar::GetFullSize() const {
   // If there are no actions to show (and this isn't an overflow container),
   // then don't show the container at all.

chrome/browser/ui/toolbar/toolbar_actions_bar.h

@@ -82,12 +82,12 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer,
                     ToolbarActionsBar* main_bar);
   ~ToolbarActionsBar() override;
 
-  // Returns the size of ToolbarActionView.
-  static gfx::Size GetViewSize();
-
   // Registers profile preferences.
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
+  // Returns the size of ToolbarActionView.
+  virtual gfx::Size GetViewSize() const;
+
   // Returns the default/full size for the toolbar; this does *not* reflect any
   // animations that may be running.
   gfx::Size GetFullSize() const;

chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc

@@ -254,7 +254,7 @@ TEST_P(ToolbarActionsBarUnitTest, BasicToolbarActionsBarTest) {
 
   // By default, all three actions should be visible.
   EXPECT_EQ(3u, toolbar_actions_bar()->GetIconCount());
-  const gfx::Size view_size = ToolbarActionsBar::GetViewSize();
+  const gfx::Size view_size = toolbar_actions_bar()->GetViewSize();
   // Check the widths.
   int expected_width = 3 * view_size.width();
   EXPECT_EQ(expected_width, toolbar_actions_bar()->GetFullSize().width());
@@ -442,8 +442,8 @@ TEST_P(ToolbarActionsBarUnitTest, TestHighlightMode) {
 
 // Test the bounds calculation for different indices.
 TEST_P(ToolbarActionsBarUnitTest, TestActionFrameBounds) {
-  const auto icon_rect = [](int x, int y) {
-    const auto size = ToolbarActionsBar::GetViewSize();
+  const auto size = toolbar_actions_bar()->GetViewSize();
+  const auto icon_rect = [size](int x, int y) {
     return gfx::Rect(gfx::Point(x * size.width(), y * size.height()), size);
   };
 
@@ -457,7 +457,7 @@ TEST_P(ToolbarActionsBarUnitTest, TestActionFrameBounds) {
                           ActionType::BROWSER_ACTION);
   }
   toolbar_model()->SetVisibleIconCount(kNumExtensions);
-  const int icon_width = ToolbarActionsBar::GetViewSize().width();
+  const int icon_width = toolbar_actions_bar()->GetViewSize().width();
   overflow_bar()->SetOverflowRowWidth(icon_width * kIconsPerOverflowRow);
   EXPECT_EQ(kIconsPerOverflowRow,
             overflow_bar()->platform_settings().icons_per_overflow_menu_row);
@@ -493,7 +493,7 @@ TEST_P(ToolbarActionsBarUnitTest, TestActionFrameBounds) {
 }
 
 TEST_P(ToolbarActionsBarUnitTest, TestStartAndEndIndexes) {
-  const int icon_width = ToolbarActionsBar::GetViewSize().width();
+  const int icon_width = toolbar_actions_bar()->GetViewSize().width();
 
   for (int i = 0; i < 3; ++i) {
     CreateAndAddExtension(base::StringPrintf("extension %d", i),

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

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "ash/ash_constants.h"
+#include "ash/ash_layout_constants.h"
 #include "ash/frame/caption_buttons/frame_caption_button.h"
 #include "ash/frame/caption_buttons/frame_caption_button_container_view.h"
 #include "ash/frame/default_frame_header.h"
@@ -693,6 +694,17 @@ IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest, ThemeColor) {
   EXPECT_EQ(SK_ColorWHITE, GetActiveIconColor(hosted_app_button_container_));
 }
 
+// Make sure that for hosted apps, the height of the frame header and its
+// contents don't exceed the height of the caption buttons.
+IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest, FrameSize) {
+  EXPECT_EQ(frame_header_->GetHeaderHeight(),
+            GetAshLayoutSize(AshLayoutSize::kNonBrowserCaption).height());
+  EXPECT_LE(app_menu_button_->size().height(),
+            frame_header_->GetHeaderHeight());
+  EXPECT_LE(hosted_app_button_container_->size().height(),
+            frame_header_->GetHeaderHeight());
+}
+
 // Tests that the focus toolbar command focuses the app menu button in web app
 // windows.
 IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest,

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

@@ -27,6 +27,8 @@ namespace {
 
 bool g_animation_disabled_for_testing = false;
 
+constexpr int kBrowserActionSize = 32;
+
 constexpr base::TimeDelta kContentSettingsFadeInDuration =
     base::TimeDelta::FromMilliseconds(500);
 
@@ -34,6 +36,12 @@ class HostedAppToolbarActionsBar : public ToolbarActionsBar {
  public:
   using ToolbarActionsBar::ToolbarActionsBar;
 
+  gfx::Size GetViewSize() const override {
+    // TODO(calamity): Unify this toolbar action size with other clients once
+    // all toolbar button sizings are consolidated. https://crbug.com/822967.
+    return gfx::Size(kBrowserActionSize, kBrowserActionSize);
+  }
+
   size_t GetIconCount() const override {
     // Only show an icon when an extension action is popped out due to
     // activation, and none otherwise.

chrome/browser/ui/views/toolbar/browser_actions_container.cc

@@ -151,6 +151,10 @@ views::MenuButton* BrowserActionsContainer::GetOverflowReferenceView() {
   return delegate_->GetOverflowReferenceView();
 }
 
+gfx::Size BrowserActionsContainer::GetToolbarActionSize() {
+  return toolbar_actions_bar_->GetViewSize();
+}
+
 void BrowserActionsContainer::AddViewForAction(
    ToolbarActionViewController* view_controller,
    size_t index) {
@@ -348,7 +352,8 @@ gfx::Size BrowserActionsContainer::CalculatePreferredSize() const {
   preferred_width = base::ClampToRange(preferred_width, GetResizeAreaWidth(),
                                        GetWidthWithAllActionsVisible());
 
-  return gfx::Size(preferred_width, ToolbarActionsBar::GetViewSize().height());
+  return gfx::Size(preferred_width,
+                   toolbar_actions_bar_->GetViewSize().height());
 }
 
 int BrowserActionsContainer::GetHeightForWidth(int width) const {
@@ -360,7 +365,7 @@ int BrowserActionsContainer::GetHeightForWidth(int width) const {
 gfx::Size BrowserActionsContainer::GetMinimumSize() const {
   DCHECK(interactive_);
   return gfx::Size(GetResizeAreaWidth(),
-                   ToolbarActionsBar::GetViewSize().height());
+                   toolbar_actions_bar_->GetViewSize().height());
 }
 
 void BrowserActionsContainer::Layout() {
@@ -442,7 +447,7 @@ int BrowserActionsContainer::OnDragUpdated(
     // event coordinate into the index of the icon we want to display the
     // indicator before. We also mirror the event.x() so that our calculations
     // are consistent with left-to-right.
-    const auto size = ToolbarActionsBar::GetViewSize();
+    const auto size = toolbar_actions_bar_->GetViewSize();
     const int offset_into_icon_area =
         GetMirroredXInView(event.x()) + (size.width() / 2);
     const int before_icon_unclamped = offset_into_icon_area / size.width();
@@ -545,7 +550,8 @@ void BrowserActionsContainer::WriteDragDataForView(View* sender,
   ToolbarActionViewController* view_controller = (*it)->view_controller();
   data->provider().SetDragImage(
       view_controller
-          ->GetIcon(GetCurrentWebContents(), ToolbarActionsBar::GetViewSize())
+          ->GetIcon(GetCurrentWebContents(),
+                    toolbar_actions_bar_->GetViewSize())
           .AsImageSkia(),
       press_pt.OffsetFromOrigin());
   // Fill in the remaining info.
@@ -636,7 +642,7 @@ void BrowserActionsContainer::OnPaint(gfx::Canvas* canvas) {
 
     // Convert back to a pixel offset into the container.  First find the X
     // coordinate of the drop icon.
-    const auto size = ToolbarActionsBar::GetViewSize();
+    const auto size = toolbar_actions_bar_->GetViewSize();
     const int drop_icon_x =
         drop_position_->icon_in_row * size.width() - (kDropIndicatorWidth / 2);
 
@@ -697,7 +703,7 @@ int BrowserActionsContainer::GetWidthForIconCount(size_t num_icons) const {
   if (num_icons == 0)
     return 0;
   return GetSeparatorAreaWidth() +
-         num_icons * ToolbarActionsBar::GetViewSize().width();
+         num_icons * toolbar_actions_bar_->GetViewSize().width();
 }
 
 int BrowserActionsContainer::GetWidthWithAllActionsVisible() const {

chrome/browser/ui/views/toolbar/browser_actions_container.h

@@ -225,6 +225,7 @@ class BrowserActionsContainer : public views::View,
   bool ShownInsideMenu() const override;
   void OnToolbarActionViewDragDone() override;
   views::MenuButton* GetOverflowReferenceView() override;
+  gfx::Size GetToolbarActionSize() override;
 
   // ToolbarActionsBarDelegate:
   void AddViewForAction(ToolbarActionViewController* action,

chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc

@@ -51,7 +51,7 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(
   // overflow won't be excessively tall (at 8 icons per row, this allows for
   // 104 total extensions).
   constexpr int kMaxOverflowRows = 13;
-  max_height_ = ToolbarActionsBar::GetViewSize().height() * kMaxOverflowRows;
+  max_height_ = container_->GetToolbarActionSize().height() * kMaxOverflowRows;
   ClipHeightTo(0, max_height_);
 }
 

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

@@ -208,7 +208,7 @@ gfx::ImageSkia ToolbarActionView::GetIconForTest() {
 }
 
 gfx::Size ToolbarActionView::CalculatePreferredSize() const {
-  return ToolbarActionsBar::GetViewSize();
+  return delegate_->GetToolbarActionSize();
 }
 
 bool ToolbarActionView::OnMousePressed(const ui::MouseEvent& event) {

chrome/browser/ui/views/toolbar/toolbar_action_view.h

@@ -47,6 +47,9 @@ class ToolbarActionView : public views::MenuButton,
     // reference point for a popup when this view isn't visible.
     virtual views::MenuButton* GetOverflowReferenceView() = 0;
 
+    // Returns the preferred size of the ToolbarActionView.
+    virtual gfx::Size GetToolbarActionSize() = 0;
+
    protected:
     ~Delegate() override {}
   };

chrome/browser/ui/views/toolbar/toolbar_action_view_unittest.cc

@@ -38,6 +38,7 @@ class TestToolbarActionViewDelegate : public ToolbarActionView::Delegate {
   views::MenuButton* GetOverflowReferenceView() override {
     return overflow_reference_view_;
   }
+  gfx::Size GetToolbarActionSize() override { return gfx::Size(32, 32); }
   void WriteDragDataForView(views::View* sender,
                             const gfx::Point& press_pt,
                             ui::OSExchangeData* data) override {}