[Multipaste] Accord the selection behavior to specs

This CL ensures that the multipaste menu's selection behavior follows
the specs:
(1) When the menu item is selected as default or by the arrow key,
the menu item's delete button should not show.
(2) When the menu item is selected by mouse hovering, the menu item's
delete button should show.

Bug: 1134422
Change-Id: I4553077aa343cd5f740035e85f9dc1cef538f6f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446459Reviewed-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@{#814761}

ash/clipboard/views/clipboard_history_item_view.cc

@@ -50,10 +50,12 @@ ClipboardHistoryItemView::ContentsView::ContentsView(
 ClipboardHistoryItemView::ContentsView::~ContentsView() = default;
 
 void ClipboardHistoryItemView::ContentsView::OnSelectionChanged() {
-  // Update `delete_button_`'s visibility if the selection state switches.
-  const bool is_selected = container_->IsSelected();
-  if (is_selected != delete_button_->GetVisible())
-    delete_button_->SetVisible(is_selected);
+  // The delete button should be visible when it is under selection.
+  const bool target_visibility =
+      container_->selection_flags_ & SelectionFlag::kDeleteButtonSelected;
+
+  if (target_visibility != delete_button_->GetVisible())
+    delete_button_->SetVisible(target_visibility);
 }
 
 void ClipboardHistoryItemView::ContentsView::InstallDeleteButton() {
@@ -90,7 +92,10 @@ class ash::ClipboardHistoryItemView::MainButton : public views::Button {
   const char* GetClassName() const override { return "MainButton"; }
 
   void PaintButtonContents(gfx::Canvas* canvas) override {
-    if (!container_->IsSelected())
+    // Highlight the main button's background when it is under selection.
+    const bool should_highlight =
+        container_->selection_flags_ & SelectionFlag::kMainButtonSelected;
+    if (!should_highlight)
       return;
 
     // Highlight the background when the menu item is selected or pressed.
@@ -157,7 +162,8 @@ ClipboardHistoryItemView::~ClipboardHistoryItemView() = default;
 ClipboardHistoryItemView::ClipboardHistoryItemView(
     const ClipboardHistoryItem* clipboard_history_item,
     views::MenuItemView* container)
-    : clipboard_history_item_(clipboard_history_item), container_(container) {}
+    : clipboard_history_item_(clipboard_history_item),
+      container_(container) {}
 
 void ClipboardHistoryItemView::Init() {
   SetLayoutManager(std::make_unique<views::FillLayout>());
@@ -177,13 +183,14 @@ void ClipboardHistoryItemView::Init() {
       &ClipboardHistoryItemView::OnSelectionChanged, base::Unretained(this)));
 }
 
-bool ClipboardHistoryItemView::IsSelected() const {
-  return container_->IsSelected();
-}
-
 void ClipboardHistoryItemView::OnSelectionChanged() {
-  contents_view_->OnSelectionChanged();
-  main_button_->SchedulePaint();
+  int target_flags = 0;
+  if (container_->IsSelected()) {
+    target_flags |= SelectionFlag::kMainButtonSelected;
+    if (main_button_->IsMouseHovered())
+      target_flags |= SelectionFlag::kDeleteButtonSelected;
+  }
+  SetSelectionFlags(target_flags);
 }
 
 void ClipboardHistoryItemView::RecordButtonPressedHistogram(
@@ -217,4 +224,13 @@ void ClipboardHistoryItemView::ExecuteCommand(int command_id,
   container_->GetDelegate()->ExecuteCommand(command_id, event.flags());
 }
 
+void ClipboardHistoryItemView::SetSelectionFlags(int selection_flags) {
+  if (selection_flags_ == selection_flags)
+    return;
+
+  selection_flags_ = selection_flags;
+  contents_view_->OnSelectionChanged();
+  main_button_->SchedulePaint();
+}
+
 }  // namespace ash

ash/clipboard/views/clipboard_history_item_view.h

@@ -19,6 +19,14 @@ class ClipboardHistoryResourceManager;
 // The base class for menu items of the clipboard history menu.
 class ClipboardHistoryItemView : public views::View {
  public:
+  enum SelectionFlag {
+    // The main button is selected.
+    kMainButtonSelected = 1 << 0,
+
+    // The delete button is selected.
+    kDeleteButtonSelected = 1 << 1
+  };
+
   static std::unique_ptr<ClipboardHistoryItemView>
   CreateFromClipboardHistoryItem(
       const ClipboardHistoryItem& item,
@@ -33,12 +41,15 @@ class ClipboardHistoryItemView : public views::View {
   // Initializes the menu item.
   void Init();
 
-  // Returns whether the menu item is under selection.
-  bool IsSelected() const;
-
   // Called when the selection state has changed.
   void OnSelectionChanged();
 
+  int selection_flags() const { return selection_flags_; }
+
+  const views::View* delete_button_for_test() const {
+    return contents_view_->delete_button();
+  }
+
  protected:
   class MainButton;
 
@@ -110,6 +121,9 @@ class ClipboardHistoryItemView : public views::View {
   // Executes |command_id| on the delegate.
   void ExecuteCommand(int command_id, const ui::Event& event);
 
+  // Updates the child views selection bitset.
+  void SetSelectionFlags(int selection_flag);
+
   // Owned by ClipboardHistoryMenuModelAdapter.
   const ClipboardHistoryItem* const clipboard_history_item_;
 
@@ -119,6 +133,9 @@ class ClipboardHistoryItemView : public views::View {
 
   MainButton* main_button_ = nullptr;
 
+  // The bitset indicating the child view(s) under selection.
+  int selection_flags_ = 0;
+
   views::PropertyChangedSubscription subscription_;
 };
 

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_item_view.h"
 #include "ash/shell.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -223,6 +224,64 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
   }
 }
 
+// Verifies the history menu's ui interaction with the menu item selection.
+IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
+                       VerifySelectionBehavior) {
+  LoginUser(account_id1_);
+
+  SetClipboardText("A");
+  SetClipboardText("B");
+  SetClipboardText("C");
+
+  ShowContextMenuViaAccelerator();
+  ASSERT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
+  ASSERT_EQ(3, GetContextMenu()->GetMenuItemsCount());
+
+  // The history menu's first item should be selected as default after the menu
+  // shows. Meanwhile, its delete button should not show.
+  const views::MenuItemView* first_menu_item_view =
+      GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/0);
+  EXPECT_TRUE(first_menu_item_view->IsSelected());
+  ASSERT_EQ(1u, first_menu_item_view->children().size());
+  const ash::ClipboardHistoryItemView* first_history_item_view =
+      static_cast<const ash::ClipboardHistoryItemView*>(
+          first_menu_item_view->children()[0]);
+  EXPECT_FALSE(first_history_item_view->delete_button_for_test()->GetVisible());
+
+  // Move the mouse to the second menu item.
+  const views::MenuItemView* second_menu_item_view =
+      GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/1);
+  EXPECT_FALSE(second_menu_item_view->IsSelected());
+  GetEventGenerator()->MoveMouseTo(
+      second_menu_item_view->GetBoundsInScreen().CenterPoint());
+
+  // The first menu item should not be selected while the second one should be.
+  EXPECT_FALSE(first_menu_item_view->IsSelected());
+  EXPECT_TRUE(second_menu_item_view->IsSelected());
+
+  // Under mouse hovering, the second item's delete button should show.
+  ASSERT_EQ(1u, second_menu_item_view->children().size());
+  const ash::ClipboardHistoryItemView* second_history_item_view =
+      static_cast<const ash::ClipboardHistoryItemView*>(
+          second_menu_item_view->children()[0]);
+  EXPECT_TRUE(second_history_item_view->delete_button_for_test()->GetVisible());
+
+  const views::MenuItemView* third_menu_item_view =
+      GetContextMenu()->GetMenuItemViewAtForTest(/*index=*/2);
+  EXPECT_FALSE(third_menu_item_view->IsSelected());
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN, ui::EF_NONE);
+
+  // Move the selection to the third item by pressing the arrow key. The third
+  // item should be selected and its delete button should not show.
+  EXPECT_FALSE(second_menu_item_view->IsSelected());
+  EXPECT_TRUE(third_menu_item_view->IsSelected());
+  ASSERT_EQ(1u, third_menu_item_view->children().size());
+  const ash::ClipboardHistoryItemView* third_history_item_view =
+      static_cast<const ash::ClipboardHistoryItemView*>(
+          third_menu_item_view->children()[0]);
+  EXPECT_FALSE(third_history_item_view->delete_button_for_test()->GetVisible());
+}
+
 // Verifies that the history menu is anchored at the cursor's location when
 // not having any textfield.
 IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,