[Multipaste] Get rid of the hacky code for selection update

Use the interface provided by MenuController to update the selection
instead of sending the synthetic key event.

Bug: 1134422
Change-Id: I30d2f05a609e6406aaaf6d4f983ac3d18fd8c498
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459127
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#815496}

ash/clipboard/clipboard_history_controller_impl.cc

@@ -218,11 +218,10 @@ void ClipboardHistoryControllerImpl::ShowMenu(
   DCHECK(IsMenuShowing());
   accelerator_target_->OnMenuShown();
 
-  // Send the synthetic key event to let the menu controller select the first
-  // menu item after showing the clipboard history menu. Note that calling
-  // `MenuItemView::SetSelected()` directly cannot update the menu controller
-  // so it does not work here.
-  SendSyntheticKeyEvent(ui::VKEY_DOWN, ui::EF_NONE);
+  // The first menu item should be selected as default after the clipboard
+  // history menu shows.
+  context_menu_->SelectMenuItemWithCommandId(
+      ClipboardHistoryUtil::kFirstItemCommandId);
 }
 
 bool ClipboardHistoryControllerImpl::CanShowMenu() const {
@@ -343,21 +342,11 @@ void ClipboardHistoryControllerImpl::DeleteSelectedMenuItemIfAny() {
     return;
   }
 
-  using SelectionMoveDirection =
-      ClipboardHistoryMenuModelAdapter::SelectionMoveDirection;
-  SelectionMoveDirection move_direction =
-      context_menu_->CalculateSelectionMoveAfterDeletion(*selected_command);
-
+  base::Optional<int> new_selected_command_id =
+      context_menu_->CalculateSelectedCommandIdAfterDeletion();
   context_menu_->RemoveMenuItemWithCommandId(*selected_command);
-
-  // Select a new menu item.
-  switch (move_direction) {
-    case SelectionMoveDirection::kPrevious:
-      SendSyntheticKeyEvent(ui::VKEY_UP, ui::EF_NONE);
-      break;
-    case SelectionMoveDirection::kNext:
-      SendSyntheticKeyEvent(ui::VKEY_DOWN, ui::EF_NONE);
-  }
+  if (new_selected_command_id.has_value())
+    context_menu_->SelectMenuItemWithCommandId(*new_selected_command_id);
 }
 
 gfx::Rect ClipboardHistoryControllerImpl::CalculateAnchorRect() const {

ash/clipboard/clipboard_history_menu_model_adapter.cc

@@ -122,6 +122,15 @@ int ClipboardHistoryMenuModelAdapter::GetMenuItemsCount() const {
   return root_view_->GetSubmenu()->GetRowCount();
 }
 
+void ClipboardHistoryMenuModelAdapter::SelectMenuItemWithCommandId(
+    int command_id) {
+  views::MenuItemView* selected_menu_item =
+      root_view_->GetMenuItemByID(command_id);
+  DCHECK(IsRunning());
+  views::MenuController::GetActiveInstance()->SelectItemAndOpenSubmenu(
+      selected_menu_item);
+}
+
 void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
     int command_id) {
   model_->RemoveItemAt(model_->GetIndexOfCommandId(command_id));
@@ -133,23 +142,37 @@ void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
   item_snapshots_.erase(item_to_delete);
 }
 
-ClipboardHistoryMenuModelAdapter::SelectionMoveDirection
-ClipboardHistoryMenuModelAdapter::CalculateSelectionMoveAfterDeletion(
-    int command_id) const {
-  auto item_to_delete = item_snapshots_.find(command_id);
-  DCHECK(item_to_delete != item_snapshots_.end());
-
-  // The menu item to be deleted should be selected.
-  DCHECK(root_view_->GetMenuItemByID(command_id)->IsSelected());
+base::Optional<int>
+ClipboardHistoryMenuModelAdapter::CalculateSelectedCommandIdAfterDeletion()
+    const {
+  base::Optional<int> command_id = GetSelectedMenuItemCommand();
+  DCHECK(command_id.has_value());
 
   // If the menu item view to be deleted is the last one, Cancel()
   // should be called so this function should not be hit.
   DCHECK_GT(item_snapshots_.size(), 1u);
 
-  // select the next menu item if any or the previous one.
-  return std::next(item_to_delete, 1) == item_snapshots_.end()
-             ? SelectionMoveDirection::kPrevious
-             : SelectionMoveDirection::kNext;
+  auto start_item = item_snapshots_.find(*command_id);
+  DCHECK(start_item != item_snapshots_.cend());
+
+  // Search in the forward direction.
+  auto check_function = [this](const auto& key_value) -> bool {
+    return model_->IsEnabledAt(model_->GetIndexOfCommandId(key_value.first));
+  };
+  auto selectable_iter_forward = std::find_if(
+      std::next(start_item, 1), item_snapshots_.cend(), check_function);
+  if (selectable_iter_forward != item_snapshots_.cend())
+    return selectable_iter_forward->first;
+
+  // If no selectable item is found, then search in the reverse direction.
+  auto selectable_iter_reverse =
+      std::find_if(std::make_reverse_iterator(start_item),
+                   item_snapshots_.crend(), check_function);
+  if (selectable_iter_reverse != item_snapshots_.crend())
+    return selectable_iter_reverse->first;
+
+  // No other selectable item, then returns the invalid id.
+  return base::nullopt;
 }
 
 gfx::Rect ClipboardHistoryMenuModelAdapter::GetMenuBoundsInScreenForTest()

ash/clipboard/clipboard_history_menu_model_adapter.h

@@ -33,15 +33,6 @@ class ClipboardHistoryResourceManager;
 // copied.
 class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
  public:
-  // Indicates the direction of selection movement.
-  enum class SelectionMoveDirection {
-    // Selection moves up to the previous menu item.
-    kPrevious,
-
-    // Selection moves down to the next menu item.
-    kNext
-  };
-
   static std::unique_ptr<ClipboardHistoryMenuModelAdapter> Create(
       ui::SimpleMenuModel::Delegate* delegate,
       base::RepeatingClosure menu_closed_callback,
@@ -76,13 +67,16 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
   // Returns the count of menu items.
   int GetMenuItemsCount() const;
 
-  // Remove the menu item specified by `command_id`.
+  // Selects the menu item specified by `command_id`.
+  void SelectMenuItemWithCommandId(int command_id);
+
+  // Removes the menu item specified by `command_id`.
   void RemoveMenuItemWithCommandId(int command_id);
 
-  // Returns the direction in which the selection state should move if the menu
-  // item corresponding to `command_id` is deleted.
-  SelectionMoveDirection CalculateSelectionMoveAfterDeletion(
-      int command_id) const;
+  // Returns the command id of the menu item to be selected if any after the
+  // current selected item is deleted. If no menu item is selectable
+  // after deletion, an absent value is returned.
+  base::Optional<int> CalculateSelectedCommandIdAfterDeletion() const;
 
   // Returns menu bounds in screen coordinates.
   gfx::Rect GetMenuBoundsInScreenForTest() const;