[Multipaste] Correct multipaste menu items' heights

The bug was introduced by https://crrev.com/c/2542925 The problem is:
the contents view's insets are ignored when setting a menu item's
height. To solve this bug, all constants related to multipaste menu
views are grouped in one file. It increases the code readability.

Bug: 1154070
Change-Id: Ie44ecf40afd1796ce6d18ccd8be921c29ee79199
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566448Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832513}

ash/BUILD.gn

@@ -335,6 +335,7 @@ component("ash") {
     "clipboard/views/clipboard_history_main_button.h",
     "clipboard/views/clipboard_history_text_item_view.cc",
     "clipboard/views/clipboard_history_text_item_view.h",
+    "clipboard/views/clipboard_history_view_constants.h",
     "dbus/ash_dbus_services.cc",
     "dbus/ash_dbus_services.h",
     "dbus/display_service_provider.cc",

ash/clipboard/views/clipboard_history_bitmap_item_view.cc

@@ -8,6 +8,7 @@
 #include "ash/clipboard/clipboard_history_resource_manager.h"
 #include "ash/clipboard/clipboard_history_util.h"
 #include "ash/clipboard/views/clipboard_history_delete_button.h"
+#include "ash/clipboard/views/clipboard_history_view_constants.h"
 #include "ash/public/cpp/rounded_image_view.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/style/scoped_light_mode_as_default.h"
@@ -29,13 +30,6 @@ namespace ash {
 
 namespace {
 
-// The preferred height for the bitmap.
-constexpr int kBitmapHeight = 80;
-
-// The margins of the delete button.
-constexpr gfx::Insets kDeleteButtonMargins =
-    gfx::Insets(/*top=*/4, /*left=*/0, /*bottom=*/0, /*right=*/4);
-
 // The duration of the fade out animation for transitioning the placeholder
 // image to rendered HTML.
 constexpr base::TimeDelta kFadeOutDurationMs =
@@ -46,15 +40,6 @@ constexpr base::TimeDelta kFadeOutDurationMs =
 constexpr base::TimeDelta kFadeInDurationMs =
     base::TimeDelta::FromMilliseconds(200);
 
-// The radius of the image's rounded corners.
-constexpr int kRoundedCornerRadius = 4;
-
-// The thickness of the image border.
-constexpr int kBorderThickness = 1;
-
-// The opacity of the image shown in a disabled item view.
-constexpr float kDisabledAlpha = 0.38f;
-
 ////////////////////////////////////////////////////////////////////////////////
 // FadeImageView
 // An ImageView which reacts to updates from ClipboardHistoryResourceManager by
@@ -68,7 +53,7 @@ class FadeImageView : public RoundedImageView,
                 const ClipboardHistoryResourceManager* resource_manager,
                 float opacity,
                 base::RepeatingClosure update_callback)
-      : RoundedImageView(kRoundedCornerRadius,
+      : RoundedImageView(ClipboardHistoryViews::kImageRoundedCornerRadius,
                          RoundedImageView::Alignment::kCenter),
         resource_manager_(resource_manager),
         clipboard_history_item_(*clipboard_history_item),
@@ -184,9 +169,12 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
     SetLayoutManager(std::make_unique<views::FillLayout>());
 
     auto image_view = BuildImageView();
-    image_view->SetPreferredSize(gfx::Size(INT_MAX, kBitmapHeight));
+    image_view->SetPreferredSize(
+        gfx::Size(INT_MAX, ClipboardHistoryViews::kImageViewPreferredHeight));
     image_view->SetBorder(views::CreateRoundedRectBorder(
-        kBorderThickness, kRoundedCornerRadius, gfx::kPlaceholderColor));
+        ClipboardHistoryViews::kImageBorderThickness,
+        ClipboardHistoryViews::kImageRoundedCornerRadius,
+        gfx::kPlaceholderColor));
     image_view_ = AddChildView(std::move(image_view));
 
     InstallDeleteButton();
@@ -210,7 +198,9 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
     auto delete_button =
         std::make_unique<ClipboardHistoryDeleteButton>(container_);
     delete_button->SetVisible(false);
-    delete_button->SetProperty(views::kMarginsKey, kDeleteButtonMargins);
+    delete_button->SetProperty(
+        views::kMarginsKey,
+        ClipboardHistoryViews::kBitmapItemDeleteButtonMargins);
     ClipboardHistoryDeleteButton* delete_button_ptr =
         delete_button_container->AddChildView(std::move(delete_button));
     AddChildView(std::move(delete_button_container));
@@ -243,7 +233,9 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
     // bounds is still visible when the context menu is in overflow.
 
     const float image_opacity =
-        container_->IsItemEnabled() ? 1.f : kDisabledAlpha;
+        container_->IsItemEnabled()
+            ? 1.f
+            : ClipboardHistoryViews::kDisabledImageAlpha;
     const auto* clipboard_history_item = container_->clipboard_history_item();
     switch (container_->data_format_) {
       case ui::ClipboardInternalFormat::kHtml:
@@ -254,7 +246,8 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
                                 weak_ptr_factory_.GetWeakPtr()));
       case ui::ClipboardInternalFormat::kBitmap: {
         auto image_view = std::make_unique<RoundedImageView>(
-            kRoundedCornerRadius, RoundedImageView::Alignment::kCenter);
+            ClipboardHistoryViews::kImageRoundedCornerRadius,
+            RoundedImageView::Alignment::kCenter);
         gfx::ImageSkia bitmap_image = gfx::ImageSkia::CreateFrom1xBitmap(
             clipboard_history_item->data().bitmap());
         if (image_opacity != 1.f) {

ash/clipboard/views/clipboard_history_delete_button.cc

@@ -5,18 +5,12 @@
 #include "ash/clipboard/views/clipboard_history_delete_button.h"
 
 #include "ash/clipboard/views/clipboard_history_item_view.h"
+#include "ash/clipboard/views/clipboard_history_view_constants.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/style/scoped_light_mode_as_default.h"
 
 namespace ash {
-namespace {
-
-// The size of the `DeleteButton`.
-constexpr int kDeleteButtonSizeDip = 16;
-
-}  // namespace
-
 ClipboardHistoryDeleteButton::ClipboardHistoryDeleteButton(
     ClipboardHistoryItemView* listener)
     : views::ImageButton(base::BindRepeating(
@@ -28,7 +22,8 @@ ClipboardHistoryDeleteButton::ClipboardHistoryDeleteButton(
   SetAccessibleName(base::ASCIIToUTF16(std::string(GetClassName())));
   SetImageHorizontalAlignment(views::ImageButton::ALIGN_CENTER);
   SetImageVerticalAlignment(views::ImageButton::ALIGN_MIDDLE);
-  SetPreferredSize(gfx::Size(kDeleteButtonSizeDip, kDeleteButtonSizeDip));
+  SetPreferredSize(gfx::Size(ClipboardHistoryViews::kDeleteButtonSizeDip,
+                             ClipboardHistoryViews::kDeleteButtonSizeDip));
 }
 
 ClipboardHistoryDeleteButton::~ClipboardHistoryDeleteButton() = default;
@@ -44,7 +39,7 @@ void ClipboardHistoryDeleteButton::OnThemeChanged() {
   ScopedLightModeAsDefault scoped_light_mode_as_default;
 
   views::ImageButton::OnThemeChanged();
-  AshColorProvider::Get()->DecorateCloseButton(this, kDeleteButtonSizeDip,
-                                               kCloseButtonIcon);
+  AshColorProvider::Get()->DecorateCloseButton(
+      this, ClipboardHistoryViews::kDeleteButtonSizeDip, kCloseButtonIcon);
 }
 }  // namespace ash

ash/clipboard/views/clipboard_history_item_view.cc

@@ -10,6 +10,7 @@
 #include "ash/clipboard/views/clipboard_history_file_item_view.h"
 #include "ash/clipboard/views/clipboard_history_main_button.h"
 #include "ash/clipboard/views/clipboard_history_text_item_view.h"
+#include "ash/clipboard/views/clipboard_history_view_constants.h"
 #include "base/auto_reset.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ -20,21 +21,16 @@
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/layout/fill_layout.h"
 
+namespace ash {
 namespace {
-using Action = ash::ClipboardHistoryUtil::Action;
-
-// The insets within the contents view.
-constexpr gfx::Insets kContentsInsets(/*vertical=*/4, /*horizontal=*/16);
-
+using Action = ClipboardHistoryUtil::Action;
 }  // namespace
 
-namespace ash {
-
 ClipboardHistoryItemView::ContentsView::ContentsView(
     ClipboardHistoryItemView* container)
     : container_(container) {
   SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
-  SetBorder(views::CreateEmptyBorder(kContentsInsets));
+  SetBorder(views::CreateEmptyBorder(ClipboardHistoryViews::kContentsInsets));
 }
 
 ClipboardHistoryItemView::ContentsView::~ContentsView() = default;

ash/clipboard/views/clipboard_history_label.cc

@@ -4,20 +4,15 @@
 
 #include "ash/clipboard/views/clipboard_history_label.h"
 
+#include "ash/clipboard/views/clipboard_history_view_constants.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/style/scoped_light_mode_as_default.h"
 
-namespace {
-
-// The preferred height for the label.
-constexpr int kLabelPreferredHeight = 32;
-
-}  // namespace
-
 namespace ash {
 ClipboardHistoryLabel::ClipboardHistoryLabel(const base::string16& text)
     : views::Label(text) {
-  SetPreferredSize(gfx::Size(INT_MAX, kLabelPreferredHeight));
+  SetPreferredSize(
+      gfx::Size(INT_MAX, ClipboardHistoryViews::kLabelPreferredHeight));
   SetFontList(views::style::GetFont(views::style::CONTEXT_TOUCH_MENU,
                                     views::style::STYLE_PRIMARY));
   SetMultiLine(false);

ash/clipboard/views/clipboard_history_text_item_view.cc

@@ -9,18 +9,12 @@
 #include "ash/clipboard/clipboard_history_util.h"
 #include "ash/clipboard/views/clipboard_history_delete_button.h"
 #include "ash/clipboard/views/clipboard_history_label.h"
+#include "ash/clipboard/views/clipboard_history_view_constants.h"
 #include "ash/shell.h"
 #include "base/metrics/histogram_macros.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/view_class_properties.h"
 
-namespace {
-
-// The margins of the delete button.
-constexpr gfx::Insets kDeleteButtonMargins =
-    gfx::Insets(/*top=*/0, /*left=*/8, /*bottom=*/0, /*right=*/4);
-}  // namespace
-
 namespace ash {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -53,7 +47,9 @@ class ClipboardHistoryTextItemView::TextContentsView
     auto delete_button =
         std::make_unique<ClipboardHistoryDeleteButton>(container());
     delete_button->SetVisible(false);
-    delete_button->SetProperty(views::kMarginsKey, kDeleteButtonMargins);
+    delete_button->SetProperty(
+        views::kMarginsKey,
+        ClipboardHistoryViews::kDefaultItemDeleteButtonMargins);
     return AddChildView(std::move(delete_button));
   }
 

ash/clipboard/views/clipboard_history_view_constants.h

+// Copyright 2020 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.
+
+#ifndef ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_VIEW_CONSTANTS_H_
+#define ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_VIEW_CONSTANTS_H_
+
+#include "ui/gfx/geometry/insets.h"
+
+namespace ash {
+namespace ClipboardHistoryViews {
+
+// The insets within the contents view.
+constexpr int kContentsVerticalInset = 4;
+constexpr gfx::Insets kContentsInsets(kContentsVerticalInset,
+                                      /*horizontal=*/16);
+
+// The size of the `DeleteButton`.
+constexpr int kDeleteButtonSizeDip = 16;
+
+// The margins of the `DeleteButton` instance showing on
+// `ClipboardHistoryTextItemView` or `ClipboardHistoryFileItemView`.
+constexpr gfx::Insets kDefaultItemDeleteButtonMargins =
+    gfx::Insets(/*top=*/0, /*left=*/8, /*bottom=*/0, /*right=*/4);
+
+// The margins of the `DeleteButton` instance showing on
+// `ClipboardHistoryBitmapItemView`.
+constexpr gfx::Insets kBitmapItemDeleteButtonMargins =
+    gfx::Insets(/*top=*/4, /*left=*/0, /*bottom=*/0, /*right=*/4);
+
+// The preferred height of `ClipboardHistoryLabel`.
+constexpr int kLabelPreferredHeight = 28;
+
+// The preferred height of the image view showing on
+// `ClipboardHistoryBitmapItemView`.
+constexpr int kImageViewPreferredHeight = 72;
+
+// The radius of the image's rounded corners.
+constexpr int kImageRoundedCornerRadius = 4;
+
+// The thickness of the image border.
+constexpr int kImageBorderThickness = 1;
+
+// The opacity of the image shown in a disabled item view.
+constexpr float kDisabledImageAlpha = 0.38f;
+
+}  // namespace ClipboardHistoryViews
+}  // namespace ash
+
+#endif  // ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_VIEW_CONSTANTS_H_

chrome/browser/ui/ash/clipboard_history_browsertest.cc

@@ -279,6 +279,7 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
   EXPECT_FALSE(GetHistoryItemViewForIndex(/*index=*/0)
                    ->delete_button_for_test()
                    ->GetVisible());
+  EXPECT_EQ(gfx::Size(256, 36), first_menu_item_view->size());
 
   // Move the mouse to the second menu item.
   const views::MenuItemView* second_menu_item_view =