app_list: Make search results scroll box work with keyboard selection

With kEnableSearchBoxSelection feature, the focus stays within the
search box while the selected result changes (instead of individual
search results getting focused as selection changes) - this means that
scroll view used by search results page does not handle selection
changes automatically.

This cl adds a callback to ResultSelectionController that
SearchResultPageView can use to detect selection changes, and update the
scroll bar to ensure the selected result is visible in the scroll view.

Change-Id: Icf24b85174a12cac0b62592a080898cff6375da5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819329Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699026}

ash/app_list/views/result_selection_controller.cc

@@ -37,8 +37,10 @@ bool ResultLocationDetails::operator!=(
 }
 
 ResultSelectionController::ResultSelectionController(
-    const ResultSelectionModel* result_container_views)
-    : result_selection_model_(result_container_views) {}
+    const ResultSelectionModel* result_container_views,
+    const base::RepeatingClosure& selection_change_callback)
+    : result_selection_model_(result_container_views),
+      selection_change_callback_(selection_change_callback) {}
 
 ResultSelectionController::~ResultSelectionController() = default;
 
@@ -99,6 +101,8 @@ void ResultSelectionController::ResetSelection(const ui::KeyEvent* key_event) {
 
   if (selected_result_)
     selected_result_->SetSelected(true, is_shift_tab);
+
+  selection_change_callback_.Run();
 }
 
 void ResultSelectionController::ClearSelection() {
@@ -230,6 +234,7 @@ void ResultSelectionController::SetSelection(
   selected_location_details_ =
       std::make_unique<ResultLocationDetails>(location);
   selected_result_->SetSelected(true, reverse_tab_order);
+  selection_change_callback_.Run();
 }
 
 SearchResultBaseView* ResultSelectionController::GetResultAtLocation(

ash/app_list/views/result_selection_controller.h

@@ -11,6 +11,7 @@
 #include "ash/app_list/app_list_export.h"
 #include "ash/app_list/views/search_result_base_view.h"
 #include "ash/app_list/views/search_result_container_view.h"
+#include "base/callback.h"
 #include "base/macros.h"
 
 namespace app_list {
@@ -77,8 +78,9 @@ class APP_LIST_EXPORT ResultSelectionController {
     kResultChanged,
   };
 
-  explicit ResultSelectionController(
-      const ResultSelectionModel* result_container_views);
+  ResultSelectionController(
+      const ResultSelectionModel* result_container_views,
+      const base::RepeatingClosure& selection_change_callback);
   ~ResultSelectionController();
 
   // Returns the currently selected result.
@@ -147,6 +149,10 @@ class APP_LIST_EXPORT ResultSelectionController {
   // |SearchResultContainerView|->|IsHorizontallyTraversable|.
   bool IsContainerAtIndexHorizontallyTraversable(int index) const;
 
+  // The callback run when the selected result changes (including when the
+  // selected result is cleared).
+  base::RepeatingClosure selection_change_callback_;
+
   // The currently selected result view
   SearchResultBaseView* selected_result_ = nullptr;
 

ash/app_list/views/search_result_page_view.cc

@@ -21,6 +21,7 @@
 #include "ash/public/cpp/app_list/app_list_config.h"
 #include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/view_shadow.h"
+#include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "ui/chromeos/search_box/search_box_constants.h"
 #include "ui/compositor/layer.h"
@@ -170,7 +171,7 @@ SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate)
   // set transparent so that the rounded corner is not overwritten.
   SetBackground(std::make_unique<SearchResultPageBackground>(
       AppListConfig::instance().card_background_color()));
-  views::ScrollView* const scroller = new views::ScrollView;
+  auto scroller = std::make_unique<views::ScrollView>();
   // Leaves a placeholder area for the search box and the separator below it.
   scroller->SetBorder(views::CreateEmptyBorder(gfx::Insets(
       kSearchBoxHeight + kSearchBoxBottomSpacing + kSeparatorThickness, 0, 0,
@@ -181,14 +182,16 @@ SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate)
   // contents' size. Using zeroes doesn't prevent it from scrolling and sizing
   // correctly.
   scroller->ClipHeightTo(0, 0);
-  scroller->SetVerticalScrollBar(new ZeroWidthVerticalScrollBar);
+  scroller->SetVerticalScrollBar(new ZeroWidthVerticalScrollBar());
   scroller->SetBackgroundColor(SK_ColorTRANSPARENT);
-  AddChildView(scroller);
+  AddChildView(std::move(scroller));
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
-  result_selection_controller_ =
-      std::make_unique<ResultSelectionController>(&result_container_views_);
+  result_selection_controller_ = std::make_unique<ResultSelectionController>(
+      &result_container_views_,
+      base::BindRepeating(&SearchResultPageView::SelectedResultChanged,
+                          base::Unretained(this)));
 }
 
 SearchResultPageView::~SearchResultPageView() = default;
@@ -307,6 +310,26 @@ void SearchResultPageView::ReorderSearchResultContainers() {
   Layout();
 }
 
+void SearchResultPageView::SelectedResultChanged() {
+  if (!result_selection_controller_->selected_location_details() ||
+      !result_selection_controller_->selected_result()) {
+    return;
+  }
+
+  const ResultLocationDetails* selection_details =
+      result_selection_controller_->selected_location_details();
+  views::View* selected_row = nullptr;
+  // For horizontal containers ensure that the whole container fits in the
+  // scroll view, to account for vertical padding within the container.
+  if (selection_details->container_is_horizontal) {
+    selected_row = result_container_views_[selection_details->container_index];
+  } else {
+    selected_row = result_selection_controller_->selected_result();
+  }
+
+  selected_row->ScrollViewToVisible();
+}
+
 void SearchResultPageView::OnSearchResultContainerResultsChanging() {
   // Block any result selection changes while result updates are in flight.
   // The selection will be reset once the results are all updated.

ash/app_list/views/search_result_page_view.h

@@ -89,6 +89,11 @@ class APP_LIST_EXPORT SearchResultPageView
   // Sort the result container views.
   void ReorderSearchResultContainers();
 
+  // Passed to |result_selection_controller_| as a callback that gets called
+  // when the currently selected result changes.
+  // Ensures that |scroller_| visible rect contains the newly selected result.
+  void SelectedResultChanged();
+
   AppListViewDelegate* view_delegate_;
 
   // The SearchResultContainerViews that compose the search page. All owned by