[Multipaste] Refactor: Move buttons to independent files

In this CL, the code of MainButton and DeleteButton is moved to
independent files instead of living in clipboard_history_item_view.cc.
It improves the code readability.

This CL should not introduce any visual differences.

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

ash/BUILD.gn

@@ -323,12 +323,16 @@ component("ash") {
     "clipboard/scoped_clipboard_history_pause_impl.h",
     "clipboard/views/clipboard_history_bitmap_item_view.cc",
     "clipboard/views/clipboard_history_bitmap_item_view.h",
+    "clipboard/views/clipboard_history_delete_button.cc",
+    "clipboard/views/clipboard_history_delete_button.h",
     "clipboard/views/clipboard_history_file_item_view.cc",
     "clipboard/views/clipboard_history_file_item_view.h",
     "clipboard/views/clipboard_history_item_view.cc",
     "clipboard/views/clipboard_history_item_view.h",
     "clipboard/views/clipboard_history_label.cc",
     "clipboard/views/clipboard_history_label.h",
+    "clipboard/views/clipboard_history_main_button.cc",
+    "clipboard/views/clipboard_history_main_button.h",
     "clipboard/views/clipboard_history_text_item_view.cc",
     "clipboard/views/clipboard_history_text_item_view.h",
     "dbus/ash_dbus_services.cc",

ash/clipboard/views/clipboard_history_bitmap_item_view.cc

@@ -7,6 +7,7 @@
 #include "ash/clipboard/clipboard_history_item.h"
 #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/public/cpp/rounded_image_view.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/style/scoped_light_mode_as_default.h"
@@ -196,7 +197,7 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
 
  private:
   // ContentsView:
-  DeleteButton* CreateDeleteButton() override {
+  ClipboardHistoryDeleteButton* CreateDeleteButton() override {
     auto delete_button_container = std::make_unique<views::View>();
     auto* layout_manager = delete_button_container->SetLayoutManager(
         std::make_unique<views::BoxLayout>(
@@ -206,10 +207,11 @@ class ClipboardHistoryBitmapItemView::BitmapContentsView
     layout_manager->set_cross_axis_alignment(
         views::BoxLayout::CrossAxisAlignment::kStart);
 
-    auto delete_button = std::make_unique<DeleteButton>(container_);
+    auto delete_button =
+        std::make_unique<ClipboardHistoryDeleteButton>(container_);
     delete_button->SetVisible(false);
     delete_button->SetProperty(views::kMarginsKey, kDeleteButtonMargins);
-    DeleteButton* delete_button_ptr =
+    ClipboardHistoryDeleteButton* delete_button_ptr =
         delete_button_container->AddChildView(std::move(delete_button));
     AddChildView(std::move(delete_button_container));
 

ash/clipboard/views/clipboard_history_delete_button.cc

+// 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.
+
+#include "ash/clipboard/views/clipboard_history_delete_button.h"
+
+#include "ash/clipboard/views/clipboard_history_item_view.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(
+          [](ClipboardHistoryItemView* item_view, const ui::Event& event) {
+            item_view->HandleDeleteButtonPressEvent(event);
+          },
+          base::Unretained(listener))) {
+  SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
+  SetAccessibleName(base::ASCIIToUTF16(std::string(GetClassName())));
+  SetImageHorizontalAlignment(views::ImageButton::ALIGN_CENTER);
+  SetImageVerticalAlignment(views::ImageButton::ALIGN_MIDDLE);
+  SetPreferredSize(gfx::Size(kDeleteButtonSizeDip, kDeleteButtonSizeDip));
+}
+
+ClipboardHistoryDeleteButton::~ClipboardHistoryDeleteButton() = default;
+
+const char* ClipboardHistoryDeleteButton::GetClassName() const {
+  return "DeleteButton";
+}
+
+void ClipboardHistoryDeleteButton::OnThemeChanged() {
+  // Use the light mode as default because the light mode is the default mode of
+  // the native theme which decides the context menu's background color.
+  // TODO(andrewxu): remove this line after https://crbug.com/1143009 is fixed.
+  ScopedLightModeAsDefault scoped_light_mode_as_default;
+
+  views::ImageButton::OnThemeChanged();
+  AshColorProvider::Get()->DecorateCloseButton(this, kDeleteButtonSizeDip,
+                                               kCloseButtonIcon);
+}
+}  // namespace ash

ash/clipboard/views/clipboard_history_delete_button.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_DELETE_BUTTON_H_
+#define ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_DELETE_BUTTON_H_
+
+#include "ui/views/controls/button/image_button.h"
+
+namespace ash {
+class ClipboardHistoryItemView;
+
+// The button to delete the menu item and its corresponding clipboard data.
+class ClipboardHistoryDeleteButton : public views::ImageButton {
+ public:
+  explicit ClipboardHistoryDeleteButton(ClipboardHistoryItemView* listener);
+  ClipboardHistoryDeleteButton(const ClipboardHistoryDeleteButton& rhs) =
+      delete;
+  ClipboardHistoryDeleteButton& operator=(
+      const ClipboardHistoryDeleteButton& rhs) = delete;
+  ~ClipboardHistoryDeleteButton() override;
+
+ private:
+  // views::ImageButton:
+  const char* GetClassName() const override;
+  void OnThemeChanged() override;
+};
+}  // namespace ash
+
+#endif  // ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_DELETE_BUTTON_H_

ash/clipboard/views/clipboard_history_item_view.h

@@ -6,7 +6,7 @@
 #define ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_ITEM_VIEW_H_
 
 #include "ash/clipboard/clipboard_history_util.h"
-#include "ui/views/controls/button/image_button.h"
+#include "ui/views/view.h"
 #include "ui/views/view_targeter_delegate.h"
 
 namespace views {
@@ -14,7 +14,9 @@ class MenuItemView;
 }  // namespace views
 
 namespace ash {
+class ClipboardHistoryDeleteButton;
 class ClipboardHistoryItem;
+class ClipboardHistoryMainButton;
 class ClipboardHistoryResourceManager;
 
 // The base class for menu items of the clipboard history menu.
@@ -31,43 +33,34 @@ class ClipboardHistoryItemView : public views::View {
       delete;
   ~ClipboardHistoryItemView() override;
 
+  // Advances the pseudo focus (backward if reverse is true). Returns whether
+  // the view still keeps the pseudo focus.
+  bool AdvancePseudoFocus(bool reverse);
+
+  void HandleDeleteButtonPressEvent(const ui::Event& event);
+
+  void HandleMainButtonPressEvent(const ui::Event& event);
+
   // Initializes the menu item.
   void Init();
 
+  // Attempts to handle the gesture event redirected from `main_button_`.
+  void MaybeHandleGestureEventFromMainButton(ui::GestureEvent* event);
+
   // Called when the selection state has changed.
   void OnSelectionChanged();
 
-  // Advances the pseudo focus (backward if reverse is true). Returns whether
-  // the view still keeps the pseudo focus.
-  bool AdvancePseudoFocus(bool reverse);
-
   ClipboardHistoryUtil::Action action() const { return action_; }
 
-  views::View* delete_button_for_test() {
+  ClipboardHistoryDeleteButton* delete_button_for_test() {
     return contents_view_->delete_button();
   }
 
-  const views::View* delete_button_for_test() const {
+  const ClipboardHistoryDeleteButton* delete_button_for_test() const {
     return contents_view_->delete_button();
   }
 
  protected:
-  class MainButton;
-
-  // The button to delete the menu item and its corresponding clipboard data.
-  class DeleteButton : public views::ImageButton {
-   public:
-    explicit DeleteButton(ClipboardHistoryItemView* listener);
-    DeleteButton(const DeleteButton& rhs) = delete;
-    DeleteButton& operator=(const DeleteButton& rhs) = delete;
-    ~DeleteButton() override;
-
-   private:
-    // views::ImageButton:
-    const char* GetClassName() const override;
-    void OnThemeChanged() override;
-  };
-
   // Used by subclasses to draw contents, such as text or bitmaps.
   class ContentsView : public views::View, public views::ViewTargeterDelegate {
    public:
@@ -79,11 +72,13 @@ class ClipboardHistoryItemView : public views::View {
     // Install DeleteButton on the contents view.
     void InstallDeleteButton();
 
-    views::View* delete_button() { return delete_button_; }
-    const views::View* delete_button() const { return delete_button_; }
+    ClipboardHistoryDeleteButton* delete_button() { return delete_button_; }
+    const ClipboardHistoryDeleteButton* delete_button() const {
+      return delete_button_;
+    }
 
    protected:
-    virtual DeleteButton* CreateDeleteButton() = 0;
+    virtual ClipboardHistoryDeleteButton* CreateDeleteButton() = 0;
 
     ClipboardHistoryItemView* container() { return container_; }
 
@@ -96,7 +91,7 @@ class ClipboardHistoryItemView : public views::View {
                            const gfx::Rect& rect) const override;
 
     // Owned by the view hierarchy.
-    DeleteButton* delete_button_ = nullptr;
+    ClipboardHistoryDeleteButton* delete_button_ = nullptr;
 
     // The parent of ContentsView.
     ClipboardHistoryItemView* const container_;
@@ -163,9 +158,6 @@ class ClipboardHistoryItemView : public views::View {
   // Updates `pseudo_focus_` and children visibility.
   void SetPseudoFocus(PseudoFocus new_pseudo_focus);
 
-  // Attempts to handle the gesture event redirected from `main_button_`.
-  void MaybeHandleGestureEventFromMainButton(ui::GestureEvent* event);
-
   // Owned by ClipboardHistoryMenuModelAdapter.
   const ClipboardHistoryItem* const clipboard_history_item_;
 
@@ -173,7 +165,7 @@ class ClipboardHistoryItemView : public views::View {
 
   ContentsView* contents_view_ = nullptr;
 
-  MainButton* main_button_ = nullptr;
+  ClipboardHistoryMainButton* main_button_ = nullptr;
 
   PseudoFocus pseudo_focus_ = PseudoFocus::kEmpty;
 

ash/clipboard/views/clipboard_history_main_button.cc

+// 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.
+
+#include "ash/clipboard/views/clipboard_history_main_button.h"
+
+#include "ash/clipboard/views/clipboard_history_item_view.h"
+#include "ash/style/ash_color_provider.h"
+#include "ash/style/scoped_light_mode_as_default.h"
+#include "ui/gfx/canvas.h"
+#include "ui/views/accessibility/view_accessibility.h"
+
+namespace ash {
+namespace {
+
+// The menu background's color type.
+constexpr ash::AshColorProvider::BaseLayerType kMenuBackgroundColorType =
+    ash::AshColorProvider::BaseLayerType::kOpaque;
+
+}  // namespace
+
+ClipboardHistoryMainButton::ClipboardHistoryMainButton(
+    ClipboardHistoryItemView* container)
+    : Button(base::BindRepeating(
+          [](ClipboardHistoryItemView* item, const ui::Event& event) {
+            item->HandleMainButtonPressEvent(event);
+          },
+          base::Unretained(container))),
+      container_(container) {
+  SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+
+  // Let the parent handle accessibility features.
+  GetViewAccessibility().OverrideIsIgnored(/*value=*/true);
+}
+
+ClipboardHistoryMainButton::~ClipboardHistoryMainButton() = default;
+
+void ClipboardHistoryMainButton::SetShouldHighlight(bool should_highlight) {
+  if (should_highlight_ == should_highlight)
+    return;
+
+  should_highlight_ = should_highlight;
+  SchedulePaint();
+}
+
+const char* ClipboardHistoryMainButton::GetClassName() const {
+  return "ClipboardHistoryMainButton";
+}
+
+void ClipboardHistoryMainButton::OnThemeChanged() {
+  views::Button::OnThemeChanged();
+  SchedulePaint();
+}
+
+void ClipboardHistoryMainButton::OnGestureEvent(ui::GestureEvent* event) {
+  // Give `container_` a chance to handle `event`.
+  container_->MaybeHandleGestureEventFromMainButton(event);
+  if (event->handled())
+    return;
+
+  views::Button::OnGestureEvent(event);
+
+  // Prevent the menu controller from handling gesture events. The menu
+  // controller may bring side-effects such as canceling the item selection.
+  event->SetHandled();
+}
+
+void ClipboardHistoryMainButton::PaintButtonContents(gfx::Canvas* canvas) {
+  if (!should_highlight_)
+    return;
+
+  // Use the light mode as default because the light mode is the default mode
+  // of the native theme which decides the context menu's background color.
+  // TODO(andrewxu): remove this line after https://crbug.com/1143009 is
+  // fixed.
+  ScopedLightModeAsDefault scoped_light_mode_as_default;
+
+  // Highlight the background when the menu item is selected or pressed.
+  cc::PaintFlags flags;
+  flags.setAntiAlias(true);
+
+  const auto* color_provider = AshColorProvider::Get();
+  const AshColorProvider::RippleAttributes ripple_attributes =
+      color_provider->GetRippleAttributes(
+          color_provider->GetBaseLayerColor(kMenuBackgroundColorType));
+  flags.setColor(SkColorSetA(ripple_attributes.base_color,
+                             ripple_attributes.highlight_opacity * 0xFF));
+  flags.setStyle(cc::PaintFlags::kFill_Style);
+  canvas->DrawRect(GetLocalBounds(), flags);
+}
+
+}  // namespace ash

ash/clipboard/views/clipboard_history_main_button.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_MAIN_BUTTON_H_
+#define ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_MAIN_BUTTON_H_
+
+#include "ui/views/controls/button/button.h"
+
+namespace ash {
+class ClipboardHistoryItemView;
+
+// The view responding to mouse click or gesture tap events.
+class ClipboardHistoryMainButton : public views::Button {
+ public:
+  explicit ClipboardHistoryMainButton(ClipboardHistoryItemView* container);
+  ClipboardHistoryMainButton(const ClipboardHistoryMainButton& rhs) = delete;
+  ClipboardHistoryMainButton& operator=(const ClipboardHistoryMainButton& rhs) =
+      delete;
+  ~ClipboardHistoryMainButton() override;
+
+  void SetShouldHighlight(bool should_highlight);
+
+ private:
+  // views::Button:
+  const char* GetClassName() const override;
+  void OnThemeChanged() override;
+  void OnGestureEvent(ui::GestureEvent* event) override;
+  void PaintButtonContents(gfx::Canvas* canvas) override;
+
+  // The parent view.
+  ClipboardHistoryItemView* const container_;
+
+  // Indicates whether the view should be highlighted.
+  bool should_highlight_ = false;
+};
+
+}  // namespace ash
+
+#endif  // ASH_CLIPBOARD_VIEWS_CLIPBOARD_HISTORY_MAIN_BUTTON_H_

ash/clipboard/views/clipboard_history_text_item_view.cc

@@ -7,6 +7,7 @@
 #include "ash/clipboard/clipboard_history_controller_impl.h"
 #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_label.h"
 #include "ash/shell.h"
 #include "base/metrics/histogram_macros.h"
@@ -48,8 +49,9 @@ class ClipboardHistoryTextItemView::TextContentsView
 
  private:
   // ContentsView:
-  DeleteButton* CreateDeleteButton() override {
-    auto delete_button = std::make_unique<DeleteButton>(container());
+  ClipboardHistoryDeleteButton* CreateDeleteButton() override {
+    auto delete_button =
+        std::make_unique<ClipboardHistoryDeleteButton>(container());
     delete_button->SetVisible(false);
     delete_button->SetProperty(views::kMarginsKey, kDeleteButtonMargins);
     return AddChildView(std::move(delete_button));

chrome/browser/ui/ash/clipboard_history_browsertest.cc

@@ -9,6 +9,7 @@
 #include "ash/clipboard/clipboard_history_controller_impl.h"
 #include "ash/clipboard/clipboard_history_item.h"
 #include "ash/clipboard/clipboard_history_menu_model_adapter.h"
+#include "ash/clipboard/views/clipboard_history_delete_button.h"
 #include "ash/clipboard/views/clipboard_history_item_view.h"
 #include "ash/shell.h"
 #include "base/test/bind.h"