[image search] Remove TODOs

All Launcher Image Search feature should be done at this point. Clean
up all TODOs and make sure we have everything finished.

This cl removes the context menu from the image search view as this is
not included in our launch plan. However, the global delegate is kept
in case we are adding the context menu or selection in the near future.

Bug: 1352636
Change-Id: Icf03c6a115e3e6e51fbcfbf2cdd0e6ecc8d942f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4898337
Reviewed-by: Yulun Wu <yulunwu@chromium.org>
Commit-Queue: Wen-Chien Wang <wcwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1204136}
diff --git a/ash/app_list/app_list_controller_impl.cc b/ash/app_list/app_list_controller_impl.cc
index 5c6ed2f..03068a4 100644
--- a/ash/app_list/app_list_controller_impl.cc
+++ b/ash/app_list/app_list_controller_impl.cc
@@ -329,8 +329,6 @@
   registry->RegisterBooleanPref(prefs::kLauncherUseLongContinueDelay, false);
 
   // The prefs for launcher search controls.
-  // TODO(crbug.com/1352636): Consider merging this to
-  // `search_notifier_controller` and rename it.
   registry->RegisterDictionaryPref(prefs::kLauncherSearchCategoryControlStatus);
 }
 
diff --git a/ash/app_list/model/search/search_result.h b/ash/app_list/model/search/search_result.h
index e816fa0..4265324 100644
--- a/ash/app_list/model/search/search_result.h
+++ b/ash/app_list/model/search/search_result.h
@@ -238,9 +238,6 @@
 
  private:
   friend class SearchController;
-  // TODO(crbug.com/1352636) Remove this friend class. Currently used to mock
-  // results for SearchResultImageView prototyping.
-  friend class SearchResultImageView;
 
   // Opens the result. Clients should use AppListViewDelegate::OpenSearchResult.
   virtual void Open(int event_flags);
diff --git a/ash/app_list/views/app_list_search_view_unittest.cc b/ash/app_list/views/app_list_search_view_unittest.cc
index d4bbb427..dc3b7ff 100644
--- a/ash/app_list/views/app_list_search_view_unittest.cc
+++ b/ash/app_list/views/app_list_search_view_unittest.cc
@@ -19,7 +19,6 @@
 #include "ash/app_list/views/search_box_view.h"
 #include "ash/app_list/views/search_notifier_controller.h"
 #include "ash/app_list/views/search_result_image_list_view.h"
-#include "ash/app_list/views/search_result_image_view_delegate.h"
 #include "ash/app_list/views/search_result_list_view.h"
 #include "ash/app_list/views/search_result_page_view.h"
 #include "ash/constants/ash_features.h"
@@ -416,47 +415,6 @@
   client->set_search_callback(TestAppListClient::SearchCallback());
 }
 
-TEST_P(SearchResultImageViewTest, ShowContextMenu) {
-  auto* test_helper = GetAppListTestHelper();
-  test_helper->ShowAppList();
-
-  // Press a key to start a search.
-  PressAndReleaseKey(ui::VKEY_A);
-
-  SearchModel::SearchResults* results = test_helper->GetSearchResults();
-  SetUpImageSearchResults(
-      results, 1, SharedAppListConfig::instance().image_search_max_results());
-
-  // Check result container visibility.
-  std::vector<SearchResultContainerView*> result_containers =
-      GetSearchView()->result_container_views_for_test();
-  ASSERT_EQ(static_cast<int>(result_containers.size()), kResultContainersCount);
-  for (auto* container : result_containers) {
-    EXPECT_TRUE(container->RunScheduledUpdateForTest());
-  }
-
-  // SearchResultImageListView container should be visible.
-  ASSERT_TRUE(
-      views::IsViewClass<SearchResultImageListView>(result_containers[2]));
-  EXPECT_TRUE(result_containers[2]->GetVisible());
-  auto* search_result_image_view = result_containers[2]->GetResultViewAt(2);
-  ASSERT_TRUE(search_result_image_view->GetVisible());
-  ASSERT_TRUE(
-      views::IsViewClass<SearchResultImageView>(search_result_image_view));
-
-  // Perform a long tap on `search_result_image_view`.
-  auto image_view_center_point =
-      search_result_image_view->GetBoundsInScreen().CenterPoint();
-  auto* event_generator = GetEventGenerator();
-  ui::GestureEvent long_tap(image_view_center_point.x(),
-                            image_view_center_point.y(), 0, base::TimeTicks(),
-                            ui::GestureEventDetails(ui::ET_GESTURE_LONG_TAP));
-  event_generator->Dispatch(&long_tap);
-
-  // The `SearchResultImageViewDelegate` should be showing a context menu.
-  EXPECT_TRUE(SearchResultImageViewDelegate::Get()->HasActiveContextMenu());
-}
-
 TEST_P(SearchResultImageViewTest, ActivateImageResult) {
   auto* test_helper = GetAppListTestHelper();
   test_helper->ShowAppList();
@@ -593,7 +551,7 @@
   auto* notifier_controller = GetSearchView()->search_notifier_controller();
   EXPECT_EQ(notifier_controller->GetPrivacyNoticeShownCount(prefs), 0);
   EXPECT_TRUE(notifier_controller->ShouldShowPrivacyNotice());
-  // TODO(crbug.com/1352636): Check that the image search is not enabled.
+  EXPECT_FALSE(IsImageSearchEnabled(prefs));
 
   // Press a character key to open the search.
   PressAndReleaseKey(ui::VKEY_A);
@@ -646,7 +604,9 @@
   auto* search_notifier_controller =
       GetSearchView()->search_notifier_controller();
   EXPECT_TRUE(search_notifier_controller->ShouldShowPrivacyNotice());
-  // TODO(crbug.com/1352636): Check that the image search is not enabled.
+  PrefService* prefs =
+      Shell::Get()->session_controller()->GetLastActiveUserPrefService();
+  EXPECT_FALSE(IsImageSearchEnabled(prefs));
 
   // Press a character key to open the search.
   PressAndReleaseKey(ui::VKEY_A);
@@ -661,8 +621,6 @@
   // The privacy notice should not be shown again after accepted.
   EXPECT_FALSE(GetSearchView()->search_notifier_view());
   EXPECT_FALSE(search_notifier_controller->ShouldShowPrivacyNotice());
-  PrefService* prefs =
-      Shell::Get()->session_controller()->GetLastActiveUserPrefService();
   EXPECT_TRUE(IsImageSearchEnabled(prefs));
 }
 
diff --git a/ash/app_list/views/search_box_view.cc b/ash/app_list/views/search_box_view.cc
index b9009568..ccbb174 100644
--- a/ash/app_list/views/search_box_view.cc
+++ b/ash/app_list/views/search_box_view.cc
@@ -1452,8 +1452,6 @@
       break;
     case ResultSelectionController::MoveResult::
         kSelectionCycleBeforeFirstResult:
-      // TODO(crbug.com/1352636): Handle the case where the search notifier
-      // exists.
       // If the result selection was about to cycle, clear the selection and
       // move the focus to the next element in the SearchBoxView -
       // close_button().
diff --git a/ash/app_list/views/search_result_container_view.cc b/ash/app_list/views/search_result_container_view.cc
index f7a44d52..4d176c3 100644
--- a/ash/app_list/views/search_result_container_view.cc
+++ b/ash/app_list/views/search_result_container_view.cc
@@ -143,9 +143,7 @@
 }
 
 void SearchResultContainerView::AppendShownResultMetadata(
-    std::vector<SearchResultAimationMetadata>* result_metadata_) {
-  NOTREACHED();
-}
+    std::vector<SearchResultAimationMetadata>* result_metadata_) {}
 
 bool SearchResultContainerView::HasAnimatingChildView() {
   auto is_animating = [](views::View* view) {
diff --git a/ash/app_list/views/search_result_image_list_view.cc b/ash/app_list/views/search_result_image_list_view.cc
index 7d3d8c8f..2ad7519 100644
--- a/ash/app_list/views/search_result_image_list_view.cc
+++ b/ash/app_list/views/search_result_image_list_view.cc
@@ -193,12 +193,6 @@
   return image_views_[index];
 }
 
-void SearchResultImageListView::AppendShownResultMetadata(
-    std::vector<SearchResultAimationMetadata>* result_metadata_) {
-  // TODO(crbug.com/1352636) Update once animations are defined by UX.
-  return;
-}
-
 std::vector<SearchResultImageView*>
 SearchResultImageListView::GetSearchResultImageViews() {
   return image_views_;
@@ -244,11 +238,6 @@
   }
 }
 
-void SearchResultImageListView::OnSelectedResultChanged() {
-  // TODO(crbug.com/1352636) once result selection spec is available.
-  return;
-}
-
 int SearchResultImageListView::DoUpdate() {
   std::vector<SearchResult*> display_results =
       SearchModel::FilterSearchResultsByFunction(
diff --git a/ash/app_list/views/search_result_image_list_view.h b/ash/app_list/views/search_result_image_list_view.h
index ce386e66..13b6cb9 100644
--- a/ash/app_list/views/search_result_image_list_view.h
+++ b/ash/app_list/views/search_result_image_list_view.h
@@ -39,8 +39,6 @@
 
   // Overridden from SearchResultContainerView:
   SearchResultImageView* GetResultViewAt(size_t index) override;
-  void AppendShownResultMetadata(
-      std::vector<SearchResultAimationMetadata>* result_metadata_) override;
 
   // Returns all search result image views children of this view.
   std::vector<SearchResultImageView*> GetSearchResultImageViews();
@@ -62,7 +60,6 @@
 
  private:
   // Overridden from SearchResultContainerView:
-  void OnSelectedResultChanged() override;
   int DoUpdate() override;
   void UpdateResultsVisibility(bool force_hide) override;
   views::View* GetTitleLabel() override;
diff --git a/ash/app_list/views/search_result_image_view.cc b/ash/app_list/views/search_result_image_view.cc
index 3708ddf..be85184 100644
--- a/ash/app_list/views/search_result_image_view.cc
+++ b/ash/app_list/views/search_result_image_view.cc
@@ -99,7 +99,6 @@
   SetCallback(base::BindRepeating(&SearchResultImageView::OnImageViewPressed,
                                   base::Unretained(this)));
 
-  set_context_menu_controller(SearchResultImageViewDelegate::Get());
   set_drag_controller(SearchResultImageViewDelegate::Get());
 }
 
@@ -108,18 +107,6 @@
                                     true /* by_button_press */);
 }
 
-void SearchResultImageView::OnGestureEvent(ui::GestureEvent* event) {
-  SearchResultImageViewDelegate::Get()->HandleSearchResultImageViewGestureEvent(
-      this, *event);
-  SearchResultBaseView::OnGestureEvent(event);
-}
-
-void SearchResultImageView::OnMouseEvent(ui::MouseEvent* event) {
-  SearchResultImageViewDelegate::Get()->HandleSearchResultImageViewMouseEvent(
-      this, *event);
-  SearchResultBaseView::OnMouseEvent(event);
-}
-
 gfx::Size SearchResultImageView::CalculatePreferredSize() const {
   // Keep the ratio of the width and height be 3:2.
   return gfx::Size(preferred_width_, 2 * preferred_width_ / 3);
diff --git a/ash/app_list/views/search_result_image_view.h b/ash/app_list/views/search_result_image_view.h
index c3f7b97..b2ba443 100644
--- a/ash/app_list/views/search_result_image_view.h
+++ b/ash/app_list/views/search_result_image_view.h
@@ -30,8 +30,6 @@
   void OnImageViewPressed(const ui::Event& event);
 
   // Overridden from views::View:
-  void OnGestureEvent(ui::GestureEvent* event) override;
-  void OnMouseEvent(ui::MouseEvent* event) override;
   gfx::Size CalculatePreferredSize() const override;
 
   // Updates `preferred_width_`.
diff --git a/ash/app_list/views/search_result_image_view_delegate.cc b/ash/app_list/views/search_result_image_view_delegate.cc
index 0e0e88b..82816cf 100644
--- a/ash/app_list/views/search_result_image_view_delegate.cc
+++ b/ash/app_list/views/search_result_image_view_delegate.cc
@@ -33,74 +33,6 @@
   g_instance = nullptr;
 }
 
-// TODO(crbug.com/1352636)
-void SearchResultImageViewDelegate::HandleSearchResultImageViewGestureEvent(
-    SearchResultImageView* view,
-    const ui::GestureEvent& event) {}
-
-// TODO(crbug.com/1352636)
-void SearchResultImageViewDelegate::HandleSearchResultImageViewMouseEvent(
-    SearchResultImageView* view,
-    const ui::MouseEvent& event) {
-  switch (event.type()) {
-    case ui::ET_MOUSE_ENTERED:
-      // TODO(crbug.com/1352636) Paint background highlight to indicate hover.
-      break;
-    case ui::ET_MOUSE_EXITED:
-      // TODO(crbug.com/1352636) Remove hover background highlight.
-      break;
-    case ui::ET_MOUSE_PRESSED:
-      // TODO(crbug.com/1352636) implement multi-result selection.
-      break;
-    case ui::ET_MOUSE_RELEASED:
-      // TODO(crbug.com/1352636) implement multi-result selection.
-      break;
-    default:
-      break;
-  }
-}
-
-bool SearchResultImageViewDelegate::HasActiveContextMenu() const {
-  return context_menu_runner_ && context_menu_runner_->IsRunning();
-}
-
-ui::SimpleMenuModel* SearchResultImageViewDelegate::BuildMenuModel() {
-  context_menu_model_ = std::make_unique<ui::SimpleMenuModel>(this);
-  // TODO(crbug.com/1352636) update with internationalized accessible name if we
-  // launch this feature.
-  context_menu_model_->AddTitle(u"Search Result Image View Context Menu");
-  // TODO(crbug.com/1352636) AddItemWithStringId(command_id,string_id);
-  return context_menu_model_.get();
-}
-
-void SearchResultImageViewDelegate::OnMenuClosed() {
-  context_menu_model_.reset();
-  context_menu_runner_.reset();
-}
-
-// TODO(crbug.com/1352636) Add context menu support.
-void SearchResultImageViewDelegate::ShowContextMenuForViewImpl(
-    views::View* source,
-    const gfx::Point& point,
-    ui::MenuSourceType source_type) {
-  if (HasActiveContextMenu())
-    return;
-
-  int run_types = views::MenuRunner::USE_ASH_SYS_UI_LAYOUT |
-                  views::MenuRunner::CONTEXT_MENU |
-                  views::MenuRunner::FIXED_ANCHOR;
-
-  context_menu_runner_ = std::make_unique<views::MenuRunner>(
-      BuildMenuModel(), run_types,
-      base::BindRepeating(&SearchResultImageViewDelegate::OnMenuClosed,
-                          weak_ptr_factory_.GetWeakPtr()));
-
-  context_menu_runner_->RunMenuAt(
-      source->GetWidget(), /*button_controller=*/nullptr,
-      /*bounds=*/gfx::Rect(point, gfx::Size()),
-      views::MenuAnchorPosition::kBubbleBottomRight, source_type);
-}
-
 bool SearchResultImageViewDelegate::CanStartDragForView(
     views::View* sender,
     const gfx::Point& press_pt,
@@ -130,10 +62,4 @@
   return;
 }
 
-// TODO(crbug.com/1352636) Add context menu support.
-void SearchResultImageViewDelegate::ExecuteCommand(int command_id,
-                                                   int event_flags) {
-  return;
-}
-
 }  // namespace ash
diff --git a/ash/app_list/views/search_result_image_view_delegate.h b/ash/app_list/views/search_result_image_view_delegate.h
index 611c419..fd394f79 100644
--- a/ash/app_list/views/search_result_image_view_delegate.h
+++ b/ash/app_list/views/search_result_image_view_delegate.h
@@ -5,34 +5,19 @@
 #ifndef ASH_APP_LIST_VIEWS_SEARCH_RESULT_IMAGE_VIEW_DELEGATE_H_
 #define ASH_APP_LIST_VIEWS_SEARCH_RESULT_IMAGE_VIEW_DELEGATE_H_
 
-#include <memory>
-
 #include "ash/ash_export.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/drag_controller.h"
 
-namespace ui {
-class GestureEvent;
-class MouseEvent;
-}  // namespace ui
-
-namespace views {
-class MenuRunner;
-}  // namespace views
-
 namespace ash {
 class SearchResultImageView;
 
-// TODO(crbug.com/1352636) implement class functionality.
-// A delegate for `SearchResultImageView` which implements context menu, drag
-// and drop, and selection functionality. Only a single delegate instance
-// exists at a time and is shared by all existing search result image views in
-// order to support multiselection which requires a shared state.
-class ASH_EXPORT SearchResultImageViewDelegate
-    : public views::ContextMenuController,
-      public views::DragController,
-      public ui::SimpleMenuModel::Delegate {
+// A delegate for `SearchResultImageView` which implements drag and drop. Only a
+// single delegate instance exists at a time and is shared by all existing
+// search result image views in order to support multiselection which requires a
+// shared state.
+class ASH_EXPORT SearchResultImageViewDelegate : public views::DragController {
  public:
   // Returns the singleton instance.
   static SearchResultImageViewDelegate* Get();
@@ -43,30 +28,7 @@
       const SearchResultImageViewDelegate&) = delete;
   ~SearchResultImageViewDelegate() override;
 
-  // Invoked when `view` receives the specified gesture `event`.
-  void HandleSearchResultImageViewGestureEvent(SearchResultImageView* view,
-                                               const ui::GestureEvent& event);
-
-  // Invoked when `view` receives the specified mouse pressed `event`.
-  void HandleSearchResultImageViewMouseEvent(SearchResultImageView* view,
-                                             const ui::MouseEvent& event);
-
-  // Checks for an active `context_menu_runner_`.
-  bool HasActiveContextMenu() const;
-
  private:
-  // Builds and returns a raw pointer to `context_menu_model_`.
-  ui::SimpleMenuModel* BuildMenuModel();
-
-  // Called when the context menu is closed. Used as a callback for
-  // `context_menu_runner_`.
-  void OnMenuClosed();
-
-  // views::ContextMenuController:
-  void ShowContextMenuForViewImpl(views::View* source,
-                                  const gfx::Point& point,
-                                  ui::MenuSourceType source_type) override;
-
   // views::DragController:
   bool CanStartDragForView(views::View* sender,
                            const gfx::Point& press_pt,
@@ -76,14 +38,6 @@
   void WriteDragDataForView(views::View* sender,
                             const gfx::Point& press_pt,
                             ui::OSExchangeData* data) override;
-
-  // SimpleMenuModel::Delegate:
-  void ExecuteCommand(int command_id, int event_flags) override;
-
-  std::unique_ptr<ui::SimpleMenuModel> context_menu_model_;
-  std::unique_ptr<views::MenuRunner> context_menu_runner_;
-
-  base::WeakPtrFactory<SearchResultImageViewDelegate> weak_ptr_factory_{this};
 };
 
 }  // namespace ash