Remove SearchResultViewDelegate from the app list.

This CL removes the SearchResultViewDelegate in favor of more directly
calling the code that was being delegated to. It also removes some
methods from SearchResultListViewDelegate.

This is a pure refactor without any functionality changes.

BUG=None

Review URL: https://codereview.chromium.org/179013003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255557 0039d316-1c4b-4281-b951-d872f2087c98

ui/app_list/app_list.gyp

@@ -130,7 +130,6 @@
         'views/search_result_list_view_delegate.h',
         'views/search_result_view.cc',
         'views/search_result_view.h',
-        'views/search_result_view_delegate.h',
         'views/signin_view.cc',
         'views/signin_view.h',
         'views/speech_view.cc',

ui/app_list/test/app_list_test_view_delegate.cc

@@ -91,6 +91,13 @@ void AppListTestViewDelegate::GetShortcutPathForApp(
 void AppListTestViewDelegate::OpenSearchResult(SearchResult* result,
                                                bool auto_launch,
                                                int event_flags) {
+  const AppListModel::SearchResults* results = model_->results();
+  for (size_t i = 0; i < results->item_count(); ++i) {
+    if (results->GetItemAt(i) == result) {
+      open_search_result_counts_[i]++;
+      break;
+    }
+  }
   ++open_search_result_count_;
 }
 

ui/app_list/test/app_list_test_view_delegate.h

@@ -5,6 +5,7 @@
 #ifndef UI_APP_LIST_TEST_APP_LIST_TEST_VIEW_DELEGATE_H_
 #define UI_APP_LIST_TEST_APP_LIST_TEST_VIEW_DELEGATE_H_
 
+#include <map>
 #include <string>
 
 #include "base/callback_forward.h"
@@ -31,6 +32,9 @@ class AppListTestViewDelegate : public AppListViewDelegate {
   void SetUsers(const Users& users) {
     users_ = users;
   }
+  std::map<size_t, int> open_search_result_counts() {
+    return open_search_result_counts_;
+  }
 
   void set_auto_launch_timeout(const base::TimeDelta& timeout) {
     auto_launch_timeout_ = timeout;
@@ -84,6 +88,7 @@ class AppListTestViewDelegate : public AppListViewDelegate {
  private:
   int dismiss_count_;
   int open_search_result_count_;
+  std::map<size_t, int> open_search_result_counts_;
   Users users_;
   scoped_ptr<TestSigninDelegate> test_signin_delegate_;
   scoped_ptr<AppListTestModel> model_;

ui/app_list/views/app_list_main_view.cc

@@ -230,18 +230,6 @@ void AppListMainView::QueryChanged(SearchBoxView* sender) {
     delegate_->StopSearch();
 }
 
-void AppListMainView::OpenResult(SearchResult* result,
-                                 bool auto_launch,
-                                 int event_flags) {
-  delegate_->OpenSearchResult(result, auto_launch, event_flags);
-}
-
-void AppListMainView::InvokeResultAction(SearchResult* result,
-                                         int action_index,
-                                         int event_flags) {
-  delegate_->InvokeSearchResultAction(result, action_index, event_flags);
-}
-
 void AppListMainView::OnResultInstalled(SearchResult* result) {
   // Clears the search to show the apps grid. The last installed app
   // should be highlighted and made visible already.

ui/app_list/views/app_list_main_view.h

@@ -85,12 +85,6 @@ class APP_LIST_EXPORT AppListMainView : public views::View,
   virtual void QueryChanged(SearchBoxView* sender) OVERRIDE;
 
   // Overridden from SearchResultListViewDelegate:
-  virtual void OpenResult(SearchResult* result,
-                          bool auto_launch,
-                          int event_flags) OVERRIDE;
-  virtual void InvokeResultAction(SearchResult* result,
-                                  int action_index,
-                                  int event_flags) OVERRIDE;
   virtual void OnResultInstalled(SearchResult* result) OVERRIDE;
   virtual void OnResultUninstalled(SearchResult* result) OVERRIDE;
 

ui/app_list/views/search_result_list_view.cc

@@ -44,7 +44,7 @@ SearchResultListView::SearchResultListView(
       new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
 
   for (int i = 0; i < kMaxResults; ++i)
-    results_container_->AddChildView(new SearchResultView(this, this));
+    results_container_->AddChildView(new SearchResultView(this));
   AddChildView(results_container_);
 
   auto_launch_indicator_->set_background(
@@ -216,7 +216,7 @@ void SearchResultListView::VisibilityChanged(views::View* starting_from,
 
 void SearchResultListView::AnimationEnded(const gfx::Animation* animation) {
   DCHECK_EQ(auto_launch_animation_.get(), animation);
-  delegate_->OpenResult(results_->GetItemAt(0), true, ui::EF_NONE);
+  view_delegate_->OpenSearchResult(results_->GetItemAt(0), true, ui::EF_NONE);
 
   // The auto-launch has to be canceled explicitly. Think that one of searcher
   // is extremely slow. Sometimes the events would happen in the following
@@ -260,15 +260,15 @@ void SearchResultListView::ListItemsChanged(size_t start, size_t count) {
 
 void SearchResultListView::SearchResultActivated(SearchResultView* view,
                                                  int event_flags) {
-  if (delegate_ && view->result())
-    delegate_->OpenResult(view->result(), false, event_flags);
+  if (view_delegate_ && view->result())
+    view_delegate_->OpenSearchResult(view->result(), false, event_flags);
 }
 
 void SearchResultListView::SearchResultActionActivated(SearchResultView* view,
                                                        size_t action_index,
                                                        int event_flags) {
-  if (delegate_ && view->result()) {
-    delegate_->InvokeResultAction(
+  if (view_delegate_ && view->result()) {
+    view_delegate_->InvokeSearchResultAction(
         view->result(), action_index, event_flags);
   }
 }

ui/app_list/views/search_result_list_view.h

@@ -8,7 +8,6 @@
 #include "base/basictypes.h"
 #include "base/memory/weak_ptr.h"
 #include "ui/app_list/app_list_model.h"
-#include "ui/app_list/views/search_result_view_delegate.h"
 #include "ui/base/models/list_model_observer.h"
 #include "ui/gfx/animation/animation_delegate.h"
 #include "ui/views/view.h"
@@ -30,8 +29,7 @@ class SearchResultView;
 // SearchResultView.
 class APP_LIST_EXPORT SearchResultListView : public views::View,
                                              public gfx::AnimationDelegate,
-                                             public ui::ListModelObserver,
-                                             public SearchResultViewDelegate {
+                                             public ui::ListModelObserver {
  public:
   SearchResultListView(SearchResultListViewDelegate* delegate,
                        AppListViewDelegate* view_delegate);
@@ -45,6 +43,16 @@ class APP_LIST_EXPORT SearchResultListView : public views::View,
 
   bool IsResultViewSelected(const SearchResultView* result_view) const;
 
+  void SearchResultActivated(SearchResultView* view, int event_flags);
+
+  void SearchResultActionActivated(SearchResultView* view,
+                                   size_t action_index,
+                                   int event_flags);
+
+  void OnSearchResultInstalled(SearchResultView* view);
+
+  void OnSearchResultUninstalled(SearchResultView* view);
+
   // Overridden from views::View:
   virtual bool OnKeyPressed(const ui::KeyEvent& event) OVERRIDE;
   virtual gfx::Size GetPreferredSize() OVERRIDE;
@@ -85,15 +93,6 @@ class APP_LIST_EXPORT SearchResultListView : public views::View,
   virtual void ListItemMoved(size_t index, size_t target_index) OVERRIDE;
   virtual void ListItemsChanged(size_t start, size_t count) OVERRIDE;
 
-  // Overridden from SearchResultViewDelegate:
-  virtual void SearchResultActivated(SearchResultView* view,
-                                     int event_flags) OVERRIDE;
-  virtual void SearchResultActionActivated(SearchResultView* view,
-                                           size_t action_index,
-                                           int event_flags) OVERRIDE;
-  virtual void OnSearchResultInstalled(SearchResultView* view) OVERRIDE;
-  virtual void OnSearchResultUninstalled(SearchResultView* view) OVERRIDE;
-
   SearchResultListViewDelegate* delegate_;  // Not owned.
   AppListViewDelegate* view_delegate_;  // Not owned.
   AppListModel::SearchResults* results_;  // Owned by AppListModel.

ui/app_list/views/search_result_list_view_delegate.h

@@ -13,21 +13,6 @@ class SearchResult;
 
 class APP_LIST_EXPORT SearchResultListViewDelegate {
  public:
-  // Invoked to open given |result|. |event_flags| contains the flags of the
-  // keyboard/mouse event that triggers the "open" request. |auto_launch| is
-  // true when the result is opened automatically through voice-search. Delegate
-  // could use the |event_flags| information to choose different ways to open
-  // the result.
-  virtual void OpenResult(SearchResult* result,
-                          bool auto_launch,
-                          int event_flags) = 0;
-
-  // Called to invoke a custom action on |result|.  |action_index| corresponds
-  // to the index of the icon in |result.action_icons()| that was activated.
-  virtual void InvokeResultAction(SearchResult* result,
-                                  int action_index,
-                                  int event_flags) = 0;
-
   // Called when the app represented by |result| is installed.
   virtual void OnResultInstalled(SearchResult* result) = 0;
 

ui/app_list/views/search_result_list_view_unittest.cc

@@ -6,10 +6,12 @@
 
 #include <map>
 
+#include "base/strings/utf_string_conversions.h"
 #include "ui/app_list/app_list_model.h"
 #include "ui/app_list/search_result.h"
 #include "ui/app_list/test/app_list_test_view_delegate.h"
 #include "ui/app_list/views/search_result_list_view_delegate.h"
+#include "ui/app_list/views/search_result_view.h"
 #include "ui/views/test/views_test_base.h"
 
 namespace app_list {
@@ -55,8 +57,8 @@ class SearchResultListViewTest : public views::ViewsTestBase,
   }
 
   int GetOpenResultCountAndReset(int ranking) {
-    int result = open_result_counts_[ranking];
-    open_result_counts_.clear();
+    int result = view_delegate_.open_search_result_counts()[ranking];
+    view_delegate_.open_search_result_counts().clear();
     return result;
   }
 
@@ -72,6 +74,14 @@ class SearchResultListViewTest : public views::ViewsTestBase,
     view_->SetSelectedIndex(0);
   }
 
+  void AddTestResultAtIndex(int index) {
+    view_delegate_.GetModel()->results()->Add(new SearchResult());
+  }
+
+  void DeleteResultAt(int index) {
+    view_delegate_.GetModel()->results()->DeleteAt(index);
+  }
+
   bool KeyPress(ui::KeyboardCode key_code) {
     ui::KeyEvent event(ui::ET_KEY_PRESSED, key_code, ui::EF_NONE, true);
     return view_->OnKeyPressed(event);
@@ -85,29 +95,22 @@ class SearchResultListViewTest : public views::ViewsTestBase,
     view_->ForceAutoLaunchForTest();
   }
 
- private:
-  // Overridden from SearchResultListViewDelegate:
-  virtual void OpenResult(SearchResult* result,
-                          bool auto_launch,
-                          int event_flags) OVERRIDE {
-    const AppListModel::SearchResults* results =
-        view_delegate_.GetModel()->results();
+  void ExpectConsistent() {
+    // Adding results will schedule Update().
+    RunPendingMessages();
+
+    AppListModel::SearchResults* results = view_delegate_.GetModel()->results();
     for (size_t i = 0; i < results->item_count(); ++i) {
-      if (results->GetItemAt(i) == result) {
-        open_result_counts_[i]++;
-        break;
-      }
+      EXPECT_EQ(results->GetItemAt(i), view_->GetResultViewAt(i)->result());
     }
   }
-  virtual void InvokeResultAction(SearchResult* result,
-                                  int action_index,
-                                  int event_flags) OVERRIDE {}
+
+ private:
   virtual void OnResultInstalled(SearchResult* result) OVERRIDE {}
   virtual void OnResultUninstalled(SearchResult* result) OVERRIDE {}
 
   AppListTestViewDelegate view_delegate_;
   scoped_ptr<SearchResultListView> view_;
-  std::map<int, int> open_result_counts_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchResultListViewTest);
 };
@@ -185,5 +188,26 @@ TEST_F(SearchResultListViewTest, CancelAutoLaunch) {
   EXPECT_TRUE(IsAutoLaunching());
 }
 
+TEST_F(SearchResultListViewTest, ModelObservers) {
+  SetUpSearchResults();
+  ExpectConsistent();
+
+  // Insert at start.
+  AddTestResultAtIndex(0);
+  ExpectConsistent();
+
+  // Remove from end.
+  DeleteResultAt(kDefaultSearchItems);
+  ExpectConsistent();
+
+  // Insert at end.
+  AddTestResultAtIndex(kDefaultSearchItems);
+  ExpectConsistent();
+
+  // Delete from start.
+  DeleteResultAt(0);
+  ExpectConsistent();
+}
+
 }  // namespace test
 }  // namespace app_list

ui/app_list/views/search_result_view.cc

@@ -66,12 +66,10 @@ gfx::RenderText* CreateRenderText(const base::string16& text,
 // static
 const char SearchResultView::kViewClassName[] = "ui/app_list/SearchResultView";
 
-SearchResultView::SearchResultView(SearchResultListView* list_view,
-                                   SearchResultViewDelegate* delegate)
+SearchResultView::SearchResultView(SearchResultListView* list_view)
     : views::CustomButton(this),
       result_(NULL),
       list_view_(list_view),
-      delegate_(delegate),
       icon_(new views::ImageView),
       actions_view_(new SearchResultActionsView(this)),
       progress_bar_(new ProgressBarView) {
@@ -188,7 +186,7 @@ bool SearchResultView::OnKeyPressed(const ui::KeyEvent& event) {
       if (actions_view_->IsValidActionIndex(selected)) {
         OnSearchResultActionActivated(selected, event.flags());
       } else {
-        delegate_->SearchResultActivated(this, event.flags());
+        list_view_->SearchResultActivated(this, event.flags());
       }
       return true;
     }
@@ -268,7 +266,7 @@ void SearchResultView::ButtonPressed(views::Button* sender,
                                      const ui::Event& event) {
   DCHECK(sender == this);
 
-  delegate_->SearchResultActivated(this, event.flags());
+  list_view_->SearchResultActivated(this, event.flags());
 }
 
 void SearchResultView::OnIconChanged() {
@@ -315,11 +313,11 @@ void SearchResultView::OnPercentDownloadedChanged() {
 }
 
 void SearchResultView::OnItemInstalled() {
-  delegate_->OnSearchResultInstalled(this);
+  list_view_->OnSearchResultInstalled(this);
 }
 
 void SearchResultView::OnItemUninstalled() {
-  delegate_->OnSearchResultUninstalled(this);
+  list_view_->OnSearchResultUninstalled(this);
 }
 
 void SearchResultView::OnSearchResultActionActivated(size_t index,
@@ -330,7 +328,7 @@ void SearchResultView::OnSearchResultActionActivated(size_t index,
 
   DCHECK_LT(index, result_->actions().size());
 
-  delegate_->SearchResultActionActivated(this, index, event_flags);
+  list_view_->SearchResultActionActivated(this, index, event_flags);
 }
 
 void SearchResultView::ShowContextMenuForView(views::View* source,

ui/app_list/views/search_result_view.h

@@ -43,8 +43,7 @@ class SearchResultView : public views::CustomButton,
   // Internal class name.
   static const char kViewClassName[];
 
-  SearchResultView(SearchResultListView* list_view,
-                   SearchResultViewDelegate* delegate);
+  SearchResultView(SearchResultListView* list_view);
   virtual ~SearchResultView();
 
   // Sets/gets SearchResult displayed by this view.
@@ -95,9 +94,6 @@ class SearchResultView : public views::CustomButton,
   // Parent list view. Owned by views hierarchy.
   SearchResultListView* list_view_;
 
-  // Not owned by us.
-  SearchResultViewDelegate* delegate_;
-
   views::ImageView* icon_;  // Owned by views hierarchy.
   scoped_ptr<gfx::RenderText> title_text_;
   scoped_ptr<gfx::RenderText> details_text_;

ui/app_list/views/search_result_view_delegate.h

-// Copyright 2013 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 UI_APP_LIST_VIEWS_SEARCH_RESULT_VIEW_DELEGATE_H_
-#define UI_APP_LIST_VIEWS_SEARCH_RESULT_VIEW_DELEGATE_H_
-
-#include "ui/app_list/app_list_export.h"
-
-namespace app_list {
-
-class SearchResultView;
-
-class APP_LIST_EXPORT SearchResultViewDelegate {
- public:
-  // Called when the search result is activated.
-  virtual void SearchResultActivated(SearchResultView* view,
-                                     int event_flags) = 0;
-
-  // Called when one of the search result's optional action icons is activated.
-  // |action_index| contains the 0-based index of the action.
-  virtual void SearchResultActionActivated(SearchResultView* view,
-                                           size_t action_index,
-                                           int event_flags) = 0;
-
-  // Called when the app represented by the search result is installed.
-  virtual void OnSearchResultInstalled(SearchResultView* view) = 0;
-
-  // Called when the app represented by the search result is uninstalled.
-  virtual void OnSearchResultUninstalled(SearchResultView* view) = 0;
-
- protected:
-  virtual ~SearchResultViewDelegate() {}
-};
-
-}  // namespace app_list
-
-#endif  // UI_APP_LIST_VIEWS_SEARCH_RESULT_VIEW_DELEGATE_H_