Improve app list search accessibility

Makes search result page view a list box, whose value indicates the
number of available results, and the query for which the results are
shown. The search result view roles are changed to list box options
(which should give the user feedback of the relative position of the
currently selected result).

When the result list is updated, the accessibility framework is notified
of the results page view's value change (so the new value gets announced
by spoken feedback). The notification is sent with a delay, once the set
of results, and the query itself stabilize to avoid spamming accessibility
framework with interim values.

The result selection changes are still announced by sending kSelection
events on the selected result, but the timing of the notification is
now controlled by the search results page view. The notification is not
sent from the views when the selection state changes - instead,
SearchResultsPageView will request the selected result view to send
notification when required. With this, selection change notifications are
throttled while the result list is still changing.


Bug: 939111, 1021270
Change-Id: I0db6f1263e0bea0dddd0f278436bfc025bc6f450
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1925769
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Jenny Zhang <jennyz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718405}

ash/app_list/views/contents_view.cc

@@ -103,17 +103,15 @@ void ContentsView::Init(AppListModel* model) {
   AddLauncherPage(horizontal_page_container_, ash::AppListState::kStateApps);
 
   // Search results UI.
-  search_results_page_view_ = new SearchResultPageView(view_delegate);
-
-  // Search result containers.
-  SearchModel::SearchResults* results =
-      view_delegate->GetSearchModel()->results();
+  search_results_page_view_ =
+      new SearchResultPageView(view_delegate, view_delegate->GetSearchModel());
 
+  // Search result containers:
   if (app_list_features::IsAnswerCardEnabled()) {
     search_result_answer_card_view_ =
         new SearchResultAnswerCardView(view_delegate);
     search_results_page_view_->AddSearchResultContainerView(
-        results, search_result_answer_card_view_);
+        search_result_answer_card_view_);
   }
 
   expand_arrow_view_ = new ExpandArrowView(this, app_list_view_);
@@ -123,12 +121,12 @@ void ContentsView::Init(AppListModel* model) {
       search_results_page_view_, GetSearchBoxView()->search_box(),
       view_delegate);
   search_results_page_view_->AddSearchResultContainerView(
-      results, search_result_tile_item_list_view_);
+      search_result_tile_item_list_view_);
 
   search_result_list_view_ =
       new SearchResultListView(GetAppListMainView(), view_delegate);
   search_results_page_view_->AddSearchResultContainerView(
-      results, search_result_list_view_);
+      search_result_list_view_);
 
   AddLauncherPage(search_results_page_view_,
                   ash::AppListState::kStateSearchResults);

ash/app_list/views/result_selection_controller.cc

@@ -141,6 +141,7 @@ ResultSelectionController::GetNextResultLocationForLocation(
 
   if (selected_result_ && event.key_code() == ui::VKEY_TAB &&
       selected_result_->SelectNextResultAction(event.IsShiftDown())) {
+    selection_change_callback_.Run();
     return MoveResult::kNone;
   }
 

ash/app_list/views/result_selection_controller_unittest.cc

@@ -54,7 +54,6 @@ class TestResultViewWithActions : public TestResultView,
 
   // SearchResultActionsViewDelegate:
   void OnSearchResultActionActivated(size_t index, int event_flags) override {}
-  void OnSearchResultActionsUnSelected() override {}
   bool IsSearchResultHoveredOrSelected() override { return selected(); }
 
   SearchResultActionsView* GetActionsView() {
@@ -470,7 +469,7 @@ class ResultSelectionTest : public testing::Test,
     // expected to change.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -479,7 +478,7 @@ class ResultSelectionTest : public testing::Test,
     // expected to change.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(1));
@@ -516,7 +515,7 @@ class ResultSelectionTest : public testing::Test,
     // TAB - stay at the same result, but select next action.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -524,7 +523,7 @@ class ResultSelectionTest : public testing::Test,
     // Shift-TAB - same result, but deselects actions.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(shift_tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionNotSelected());
@@ -532,7 +531,7 @@ class ResultSelectionTest : public testing::Test,
     // TAB - reselect the first action.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -540,7 +539,7 @@ class ResultSelectionTest : public testing::Test,
     // TAB - select the next action.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(1));
@@ -568,7 +567,7 @@ class ResultSelectionTest : public testing::Test,
     // Shift-TAB - move to previous action.
     EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
               result_selection_controller_->MoveSelection(shift_tab_key_));
-    EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+    EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
     ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
     EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -873,7 +872,7 @@ TEST_F(ResultSelectionTest, TabCycleInContainerWithResultActions) {
   // expected to change.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
   ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -891,7 +890,7 @@ TEST_F(ResultSelectionTest, TabCycleInContainerWithResultActions) {
   // TAB - next action selected.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
   ASSERT_EQ(create_test_location(1, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -972,7 +971,7 @@ TEST_F(ResultSelectionTest, TabCycleInContainerSingleResultWithActionUsingTab) {
   // expected to change.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
 
   ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
@@ -1055,7 +1054,7 @@ TEST_P(ResultSelectionTest,
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1071,7 +1070,7 @@ TEST_P(ResultSelectionTest,
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1112,7 +1111,7 @@ TEST_F(ResultSelectionTest,
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1128,7 +1127,7 @@ TEST_F(ResultSelectionTest,
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1168,7 +1167,7 @@ TEST_F(ResultSelectionTest, ResetWhileFirstResultActionSelected) {
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 0), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1214,7 +1213,7 @@ TEST_F(ResultSelectionTest, ResetWhileResultActionSelected) {
   // TAB to select an action.
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(1, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(0));
 
@@ -1264,7 +1263,7 @@ TEST_F(ResultSelectionTest, ActionRemovedWhileSelected) {
             result_selection_controller_->MoveSelection(tab_key_));
   EXPECT_EQ(ResultSelectionController::MoveResult::kNone,
             result_selection_controller_->MoveSelection(tab_key_));
-  EXPECT_EQ(0, GetAndResetSelectionChangeCount());
+  EXPECT_EQ(3, GetAndResetSelectionChangeCount());
   ASSERT_EQ(create_test_location(0, 1), GetCurrentLocation());
   EXPECT_TRUE(CurrentResultActionSelected(2));
 

ash/app_list/views/search_result_actions_view.cc

@@ -80,7 +80,6 @@ class SearchResultImageButton : public views::ImageButton {
   SearchResultActionsView* parent_;
   const bool visible_on_hover_;
   bool to_be_activate_by_long_press_ = false;
-  bool selected_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(SearchResultImageButton);
 };
@@ -188,13 +187,6 @@ void SearchResultImageButton::UpdateOnStateChanged() {
     SetVisible(parent_->IsSearchResultHoveredOrSelected() ||
                parent()->Contains(GetFocusManager()->GetFocusedView()));
   }
-
-  const bool selected = parent_->GetSelectedAction() == tag();
-  if (selected_ != selected) {
-    selected_ = selected;
-    if (selected)
-      NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
-  }
 }
 
 void SearchResultImageButton::OnPaintBackground(gfx::Canvas* canvas) {
@@ -235,10 +227,8 @@ SearchResultActionsView::SearchResultActionsView(
 SearchResultActionsView::~SearchResultActionsView() {}
 
 void SearchResultActionsView::SetActions(const SearchResult::Actions& actions) {
-  if (selected_action_.has_value()) {
+  if (selected_action_.has_value())
     selected_action_.reset();
-    delegate_->OnSearchResultActionsUnSelected();
-  }
   buttons_.clear();
   RemoveAllChildViews(true);
 
@@ -306,9 +296,20 @@ bool SearchResultActionsView::SelectNextAction(bool reverse_tab_order) {
   return true;
 }
 
+void SearchResultActionsView::NotifyA11yResultSelected() {
+  DCHECK(HasSelectedAction());
+
+  int selected_action = GetSelectedAction();
+  for (views::Button* button : buttons_) {
+    if (button->tag() == selected_action) {
+      button->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
+      return;
+    }
+  }
+}
+
 void SearchResultActionsView::ClearSelectedAction() {
   selected_action_.reset();
-  delegate_->OnSearchResultActionsUnSelected();
   UpdateButtonsOnStateChanged();
 }
 

ash/app_list/views/search_result_actions_view.h

@@ -60,6 +60,9 @@ class APP_LIST_EXPORT SearchResultActionsView : public views::View,
   // getting cleared).
   bool SelectNextAction(bool reverse_tab_order);
 
+  // Sends kSelection a11y notification for the selected action button.
+  void NotifyA11yResultSelected();
+
   // Clears selected action state.
   void ClearSelectedAction();
 

ash/app_list/views/search_result_actions_view_delegate.h

@@ -15,11 +15,6 @@ class SearchResultActionsViewDelegate {
   // in SearchResultActionsView.
   virtual void OnSearchResultActionActivated(size_t index, int event_flags) = 0;
 
-  // Invoked when result action selection is cleared (it changes from the state
-  // where a result action is selected to the state where no actions are
-  // selected).
-  virtual void OnSearchResultActionsUnSelected() = 0;
-
   // Returns true if the associated search result is hovered by mouse, or
   // or selected by keyboard.
   virtual bool IsSearchResultHoveredOrSelected() = 0;

ash/app_list/views/search_result_base_view.cc

@@ -60,6 +60,14 @@ bool SearchResultBaseView::SelectNextResultAction(bool reverse_tab_order) {
   return true;
 }
 
+void SearchResultBaseView::NotifyA11yResultSelected() {
+  if (actions_view_ && actions_view_->HasSelectedAction()) {
+    actions_view_->NotifyA11yResultSelected();
+    return;
+  }
+  NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
+}
+
 void SearchResultBaseView::SetResult(SearchResult* result) {
   OnResultChanging(result);
   ClearResult();
@@ -97,6 +105,7 @@ void SearchResultBaseView::UpdateAccessibleName() {
 void SearchResultBaseView::ClearResult() {
   if (result_)
     result_->RemoveObserver(this);
+  SetSelected(false, base::nullopt);
   result_ = nullptr;
 }
 

ash/app_list/views/search_result_base_view.h

@@ -39,6 +39,12 @@ class APP_LIST_EXPORT SearchResultBaseView : public views::Button,
   // Returns whether the selected result action was changed.
   bool SelectNextResultAction(bool reverse_tab_order);
 
+  // If the search result is currently selected, sends the appropriate
+  // kSelection view accessibility event. For example, if a result action is
+  // selected, the notification will be sent for the selected action button
+  // view.
+  void NotifyA11yResultSelected();
+
   SearchResult* result() const { return result_; }
   void SetResult(SearchResult* result);
 

ash/app_list/views/search_result_page_view.cc

@@ -23,6 +23,8 @@
 #include "ash/public/cpp/view_shadow.h"
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/chromeos/search_box/search_box_constants.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
@@ -30,6 +32,8 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
+#include "ui/strings/grit/ui_strings.h"
+#include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/scrollbar/overlay_scroll_bar.h"
@@ -66,6 +70,11 @@ constexpr SkColor kSeparatorColor = SkColorSetA(gfx::kGoogleGrey900, 0x24);
 // The shadow elevation value for the shadow of the expanded search box.
 constexpr int kSearchBoxSearchResultShadowElevation = 12;
 
+// The amount of time by which notifications to accessibility framework about
+// result page changes are delayed.
+constexpr base::TimeDelta kNotifyA11yDelay =
+    base::TimeDelta::FromMilliseconds(500);
+
 // A container view that ensures the card background and the shadow are painted
 // in the correct order.
 class SearchCardView : public views::View {
@@ -152,8 +161,11 @@ class SearchResultPageView::HorizontalSeparator : public views::View {
   DISALLOW_COPY_AND_ASSIGN(HorizontalSeparator);
 };
 
-SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate)
-    : view_delegate_(view_delegate), contents_view_(new views::View) {
+SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate,
+                                           SearchModel* search_model)
+    : view_delegate_(view_delegate),
+      search_model_(search_model),
+      contents_view_(new views::View) {
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
   contents_view_->SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -196,12 +208,13 @@ SearchResultPageView::SearchResultPageView(AppListViewDelegate* view_delegate)
       &result_container_views_,
       base::BindRepeating(&SearchResultPageView::SelectedResultChanged,
                           base::Unretained(this)));
+
+  search_box_observer_.Add(search_model->search_box());
 }
 
 SearchResultPageView::~SearchResultPageView() = default;
 
 void SearchResultPageView::AddSearchResultContainerView(
-    SearchModel::SearchResults* results_model,
     SearchResultContainerView* result_container) {
   if (!result_container_views_.empty()) {
     HorizontalSeparator* separator = new HorizontalSeparator(bounds().width());
@@ -210,7 +223,7 @@ void SearchResultPageView::AddSearchResultContainerView(
   }
   contents_view_->AddChildView(new SearchCardView(result_container));
   result_container_views_.push_back(result_container);
-  result_container->SetResults(results_model);
+  result_container->SetResults(search_model_->results());
   result_container->set_delegate(this);
 }
 
@@ -278,6 +291,33 @@ void SearchResultPageView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
     layer()->SetClipRect(gfx::Rect());
 }
 
+void SearchResultPageView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  if (!GetVisible())
+    return;
+
+  node_data->role = ax::mojom::Role::kListBox;
+
+  base::string16 value;
+  base::string16 query = search_model_->search_box()->text();
+  if (!query.empty()) {
+    if (last_search_result_count_ == 1) {
+      value = l10n_util::GetStringFUTF16(
+          IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_SINGLE_RESULT,
+          query);
+    } else {
+      value = l10n_util::GetStringFUTF16(
+          IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT,
+          base::NumberToString16(last_search_result_count_), query);
+    }
+  } else {
+    value = l10n_util::GetStringFUTF16(
+        IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_ZERO_STATE,
+        base::NumberToString16(last_search_result_count_));
+  }
+
+  node_data->SetValue(value);
+}
+
 void SearchResultPageView::ReorderSearchResultContainers() {
   int view_offset = 0;
   if (assistant_privacy_info_view_) {
@@ -340,6 +380,47 @@ void SearchResultPageView::SelectedResultChanged() {
   }
 
   selected_row->ScrollViewToVisible();
+
+  NotifySelectedResultChanged();
+}
+
+void SearchResultPageView::SetIgnoreResultChangesForA11y(bool ignore) {
+  if (ignore_result_changes_for_a11y_ == ignore)
+    return;
+  ignore_result_changes_for_a11y_ = ignore;
+
+  GetViewAccessibility().OverrideIsLeaf(ignore);
+  NotifyAccessibilityEvent(ax::mojom::Event::kTreeChanged, true);
+}
+
+void SearchResultPageView::ScheduleResultsChangedA11yNotification() {
+  if (!ignore_result_changes_for_a11y_) {
+    NotifyA11yResultsChanged();
+    return;
+  }
+
+  notify_a11y_results_changed_timer_.Start(
+      FROM_HERE, kNotifyA11yDelay,
+      base::BindOnce(&SearchResultPageView::NotifyA11yResultsChanged,
+                     base::Unretained(this)));
+}
+
+void SearchResultPageView::NotifyA11yResultsChanged() {
+  SetIgnoreResultChangesForA11y(false);
+
+  NotifyAccessibilityEvent(ax::mojom::Event::kValueChanged, true);
+  NotifySelectedResultChanged();
+}
+
+void SearchResultPageView::NotifySelectedResultChanged() {
+  if (ignore_result_changes_for_a11y_ ||
+      !result_selection_controller_->selected_location_details() ||
+      !result_selection_controller_->selected_result()) {
+    return;
+  }
+
+  NotifyAccessibilityEvent(ax::mojom::Event::kSelectedChildrenChanged, true);
+  result_selection_controller_->selected_result()->NotifyA11yResultSelected();
 }
 
 void SearchResultPageView::OnSearchResultContainerResultsChanging() {
@@ -347,20 +428,29 @@ void SearchResultPageView::OnSearchResultContainerResultsChanging() {
   // The selection will be reset once the results are all updated.
   if (app_list_features::IsSearchBoxSelectionEnabled())
     result_selection_controller_->set_block_selection_changes(true);
+
+  notify_a11y_results_changed_timer_.Stop();
+  SetIgnoreResultChangesForA11y(true);
 }
 
 void SearchResultPageView::OnSearchResultContainerResultsChanged() {
   DCHECK(!result_container_views_.empty());
   DCHECK(result_container_views_.size() == separators_.size() + 1);
 
+  int result_count = 0;
   // Only sort and layout the containers when they have all updated.
   for (SearchResultContainerView* view : result_container_views_) {
     if (view->UpdateScheduled())
       return;
+    result_count += view->num_results();
   }
 
+  last_search_result_count_ = result_count;
+
   ReorderSearchResultContainers();
 
+  ScheduleResultsChangedA11yNotification();
+
   if (!app_list_features::IsSearchBoxSelectionEnabled()) {
     views::View* focused_view = GetFocusManager()->GetFocusedView();
 
@@ -370,8 +460,6 @@ void SearchResultPageView::OnSearchResultContainerResultsChanged() {
   }
 
   first_result_view_ = result_container_views_[0]->GetFirstResultView();
-  if (!first_result_view_)
-    return;
 
   if (!app_list_features::IsSearchBoxSelectionEnabled()) {
     views::View* focused_view = GetFocusManager()->GetFocusedView();
@@ -419,6 +507,16 @@ void SearchResultPageView::OnSearchResultContainerResultFocused(
   }
 }
 
+void SearchResultPageView::HintTextChanged() {}
+
+void SearchResultPageView::Update() {
+  notify_a11y_results_changed_timer_.Stop();
+}
+
+void SearchResultPageView::SearchEngineChanged() {}
+
+void SearchResultPageView::ShowAssistantChanged() {}
+
 void SearchResultPageView::OnAssistantPrivacyInfoViewCloseButtonPressed() {
   ReorderSearchResultContainers();
 }
@@ -427,6 +525,7 @@ void SearchResultPageView::OnHidden() {
   // Hide the search results page when it is behind search box to avoid focus
   // being moved onto suggested apps when zero state is enabled.
   AppListPage::OnHidden();
+  notify_a11y_results_changed_timer_.Stop();
   SetVisible(false);
   for (auto* container_view : result_container_views_) {
     container_view->SetShown(false);
@@ -438,6 +537,7 @@ void SearchResultPageView::OnShown() {
   for (auto* container_view : result_container_views_) {
     container_view->SetShown(true);
   }
+  ScheduleResultsChangedA11yNotification();
 }
 
 void SearchResultPageView::OnAnimationStarted(AppListState from_state,

ash/app_list/views/search_result_page_view.h

@@ -9,11 +9,14 @@
 
 #include "ash/app_list/app_list_export.h"
 #include "ash/app_list/model/app_list_model.h"
+#include "ash/app_list/model/search/search_box_model.h"
+#include "ash/app_list/model/search/search_box_model_observer.h"
 #include "ash/app_list/views/app_list_page.h"
 #include "ash/app_list/views/result_selection_controller.h"
 #include "ash/app_list/views/search_result_container_view.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
 
 namespace ash {
 
@@ -24,13 +27,14 @@ class ViewShadow;
 // The search results page for the app list.
 class APP_LIST_EXPORT SearchResultPageView
     : public AppListPage,
-      public SearchResultContainerView::Delegate {
+      public SearchResultContainerView::Delegate,
+      public SearchBoxModelObserver {
  public:
-  explicit SearchResultPageView(AppListViewDelegate* view_delegate);
+  SearchResultPageView(AppListViewDelegate* view_delegate,
+                       SearchModel* search_model);
   ~SearchResultPageView() override;
 
   void AddSearchResultContainerView(
-      SearchModel::SearchResults* result_model,
       SearchResultContainerView* result_container);
 
   const std::vector<SearchResultContainerView*>& result_container_views() {
@@ -45,6 +49,7 @@ class APP_LIST_EXPORT SearchResultPageView
   const char* GetClassName() const override;
   gfx::Size CalculatePreferredSize() const override;
   void OnBoundsChanged(const gfx::Rect& previous_bounds) override;
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
   // AppListPage overrides:
   void OnHidden() override;
@@ -65,12 +70,18 @@ class APP_LIST_EXPORT SearchResultPageView
   views::View* GetFirstFocusableView() override;
   views::View* GetLastFocusableView() override;
 
-  // Overridden from SearchResultContainerView::Delegate :
+  // Overridden from SearchResultContainerView::Delegate:
   void OnSearchResultContainerResultsChanging() override;
   void OnSearchResultContainerResultsChanged() override;
   void OnSearchResultContainerResultFocused(
       SearchResultBaseView* focused_result_view) override;
 
+  // Overridden from SearchBoxModelObserver:
+  void HintTextChanged() override;
+  void Update() override;
+  void SearchEngineChanged() override;
+  void ShowAssistantChanged() override;
+
   void OnAssistantPrivacyInfoViewCloseButtonPressed();
 
   views::View* contents_view() { return contents_view_; }
@@ -92,8 +103,38 @@ class APP_LIST_EXPORT SearchResultPageView
   // Ensures that |scroller_| visible rect contains the newly selected result.
   void SelectedResultChanged();
 
+  // Sets whether changes in search result containers should be hidden from the
+  // accessibility framework.
+  // This is set while search results are being updated to reduce noisy updates
+  // sent to the accessibility framework while the search result containers are
+  // being rebuilt.
+  // The |ignore| value is reset in NotifyA11yResultsChanged(), at which time
+  // accessibility framework is notified that the view value/selected children
+  // have changed.
+  void SetIgnoreResultChangesForA11y(bool ignore);
+
+  // Schedules a call to |NotifyA11yResultsChanged|. Called from
+  // OnSearchResultContainerResultsChanged() when all result containers have
+  // finished changing. The goal of the delay is to reduce the noise if the set
+  // of results for a query has not stabilized, or while the user is still
+  // changing the query.
+  void ScheduleResultsChangedA11yNotification();
+
+  // Notifies the accessibility framework that the set of search results has
+  // changed.
+  // Note: This ensures that results changes are not being hidden from a11y
+  // framework.
+  void NotifyA11yResultsChanged();
+
+  // If required, sends a kSelection a11y notification for the currently
+  // selected search result view.
+  void NotifySelectedResultChanged();
+
   AppListViewDelegate* view_delegate_;
 
+  // The search model for which the results are displayed.
+  SearchModel* const search_model_;
+
   // The SearchResultContainerViews that compose the search page. All owned by
   // the views hierarchy.
   std::vector<SearchResultContainerView*> result_container_views_;
@@ -110,10 +151,24 @@ class APP_LIST_EXPORT SearchResultPageView
   // The first search result's view or nullptr if there's no search result.
   SearchResultBaseView* first_result_view_ = nullptr;
 
+  // Timer used to delay calls to NotifyA11yResultsChanged().
+  base::OneShotTimer notify_a11y_results_changed_timer_;
+
+  // Whether the changes in search result containers are being hidden from the
+  // accessibility framework.
+  bool ignore_result_changes_for_a11y_ = false;
+
+  // The last reported number of search results shown within search result
+  // containers.
+  int last_search_result_count_ = 0;
+
   views::View* assistant_privacy_info_view_ = nullptr;
 
   std::unique_ptr<ash::ViewShadow> view_shadow_;
 
+  ScopedObserver<SearchBoxModel, SearchBoxModelObserver> search_box_observer_{
+      this};
+
   DISALLOW_COPY_AND_ASSIGN(SearchResultPageView);
 };
 

ash/app_list/views/search_result_tile_item_list_view_unittest.cc

@@ -281,7 +281,7 @@ TEST_P(SearchResultTileItemListViewTest, Basic) {
     ui::AXNodeData node_data;
     view()->children()[first_child + i * child_step]->GetAccessibleNodeData(
         &node_data);
-    EXPECT_EQ(ax::mojom::Role::kButton, node_data.role);
+    EXPECT_EQ(ax::mojom::Role::kListBoxOption, node_data.role);
     EXPECT_EQ("InstalledApp " + base::NumberToString(i) + ", " +
                   l10n_util::GetStringUTF8(
                       IDS_APP_ACCESSIBILITY_INSTALLED_APP_ANNOUNCEMENT),
@@ -297,7 +297,7 @@ TEST_P(SearchResultTileItemListViewTest, Basic) {
     view()
         ->children()[first_child + (i + kInstalledApps) * child_step]
         ->GetAccessibleNodeData(&node_data);
-    EXPECT_EQ(ax::mojom::Role::kButton, node_data.role);
+    EXPECT_EQ(ax::mojom::Role::kListBoxOption, node_data.role);
     EXPECT_EQ("PlayStoreApp " + base::NumberToString(i) + ", " +
                   l10n_util::GetStringUTF8(
                       IDS_APP_ACCESSIBILITY_ARC_APP_ANNOUNCEMENT) +
@@ -313,7 +313,7 @@ TEST_P(SearchResultTileItemListViewTest, Basic) {
     view()
         ->children()[first_child + (i + start_index) * child_step]
         ->GetAccessibleNodeData(&node_data);
-    EXPECT_EQ(ax::mojom::Role::kButton, node_data.role);
+    EXPECT_EQ(ax::mojom::Role::kListBoxOption, node_data.role);
     EXPECT_EQ("RecommendedApp " + base::NumberToString(i) + ", Star rating " +
                   base::NumberToString(i + 1) + ".0, App recommendation",
               node_data.GetStringAttribute(ax::mojom::StringAttribute::kName));
@@ -354,7 +354,7 @@ TEST_P(SearchResultTileItemListViewTest, TestRecommendations) {
     ui::AXNodeData node_data;
     view()->children()[first_index + i * child_step]->GetAccessibleNodeData(
         &node_data);
-    EXPECT_EQ(ax::mojom::Role::kButton, node_data.role);
+    EXPECT_EQ(ax::mojom::Role::kListBoxOption, node_data.role);
     EXPECT_EQ("RecommendedApp " + base::NumberToString(i) + ", Star rating " +
                   base::NumberToString(i + 1) + ".0, App recommendation",
               node_data.GetStringAttribute(ax::mojom::StringAttribute::kName));

ash/app_list/views/search_result_tile_item_view.cc

@@ -29,6 +29,7 @@
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/strings/grit/ui_strings.h"
+#include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
@@ -89,6 +90,8 @@ SearchResultTileItemView::SearchResultTileItemView(
   // a non-null item makes the tile visible.
   SetVisible(false);
 
+  GetViewAccessibility().OverrideIsLeaf(true);
+
   // Prevent the icon view from interfering with our mouse events.
   icon_ = new views::ImageView;
   icon_->set_can_process_events_within_subtree(false);
@@ -269,6 +272,12 @@ void SearchResultTileItemView::ButtonPressed(views::Button* sender,
 void SearchResultTileItemView::GetAccessibleNodeData(
     ui::AXNodeData* node_data) {
   views::Button::GetAccessibleNodeData(node_data);
+
+  // The tile is a list item in the search result page's result list.
+  node_data->role = ax::mojom::Role::kListBoxOption;
+  node_data->AddBoolAttribute(ax::mojom::BoolAttribute::kSelected, selected());
+  node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kClick);
+
   // Specify |ax::mojom::StringAttribute::kDescription| with an empty string, so
   // that long truncated names are not read twice. Details of this issue: - The
   // Play Store app's name is shown in a label |title_|. - If the name is too
@@ -464,7 +473,6 @@ void SearchResultTileItemView::SetBadgeIcon(const gfx::ImageSkia& badge_icon) {
 
 void SearchResultTileItemView::SetTitle(const base::string16& title) {
   title_->SetText(title);
-  title_->NotifyAccessibilityEvent(ax::mojom::Event::kTextChanged, true);
 }
 
 void SearchResultTileItemView::SetRating(float rating) {

ash/app_list/views/search_result_view.cc

@@ -332,12 +332,12 @@ void SearchResultView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   if (!GetVisible())
     return;
 
-  // This is a work around to deal with the nested button case(append and remove
+  // Mark the result is a list item in the list of search results.
+  // Also avoids an issue with the nested button case(append and remove
   // button are child button of SearchResultView), which is not supported by
   // ChromeVox. see details in crbug.com/924776.
-  // We change the role of the parent view SearchResultView to kGenericContainer
-  // i.e., not a kButton anymore.
-  node_data->role = ax::mojom::Role::kGenericContainer;
+  node_data->role = ax::mojom::Role::kListBoxOption;
+  node_data->AddBoolAttribute(ax::mojom::BoolAttribute::kSelected, selected());
   node_data->AddState(ax::mojom::State::kFocusable);
   node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kClick);
   node_data->SetName(GetAccessibleName());
@@ -444,13 +444,6 @@ void SearchResultView::OnSearchResultActionActivated(size_t index,
   }
 }
 
-void SearchResultView::OnSearchResultActionsUnSelected() {
-  // If the selection has changed to default result action, announce the
-  // selection change to a11y stack.
-  if (selected())
-    NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
-}
-
 bool SearchResultView::IsSearchResultHoveredOrSelected() {
   return IsMouseHovered() || selected();
 }

ash/app_list/views/search_result_view.h

@@ -108,7 +108,6 @@ class APP_LIST_EXPORT SearchResultView
 
   // SearchResultActionsViewDelegate overrides:
   void OnSearchResultActionActivated(size_t index, int event_flags) override;
-  void OnSearchResultActionsUnSelected() override;
   bool IsSearchResultHoveredOrSelected() override;
 
   // Invoked when the context menu closes.

ui/strings/ui_strings.grd

@@ -879,6 +879,15 @@ need to be translated for each locale.-->
       <message name="IDS_APP_LIST_CLEAR_SEARCHBOX" desc="Tooltip for the button that clears all text from the search box in the app list.">
         Clear searchbox text
       </message>
+      <message name="IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_ZERO_STATE" desc="The accessibility announcement notifying the user that app list search is displaying a set of suggested items - i.e. items  shown as search results by default when the user has not entered any search query.">
+        Displaying suggestions
+      </message>
+      <message name="IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT" desc="The accessibility announcement notifying the user about the number of results shown in app list search, and the query for which the results are displayed.">
+          Displaying <ph name="result_count">$1<ex>10</ex></ph> results for <ph name="query">$2<ex>search query</ex></ph>
+      </message>
+      <message name="IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_SINGLE_RESULT"  desc="The accessibility announcement notifying the user about the number of results shown in app list search, and the query for which the results are displayed. Used if search results contain a single item.">
+          Displaying 1 result for <ph name="query">$1<ex>search query</ex></ph>
+      </message>
       <message name="IDS_SHELF_ALIGNMENT_BOTTOM" desc="Accessibility announcement notifying users that the shelf is now placed at the bottom of the screen">
         Shelf on bottom
       </message>

ui/strings/ui_strings_grd/IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT.png.sha1

+af2da80cac63e3cd50aec6720544ebf796fbcc43
\ No newline at end of file

ui/strings/ui_strings_grd/IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_SINGLE_RESULT.png.sha1

+ee7b27995572bdc4bcd4e1553f66489ecb6c3438
\ No newline at end of file

ui/strings/ui_strings_grd/IDS_APP_LIST_SEARCHBOX_RESULTS_ACCESSIBILITY_ANNOUNCEMENT_ZERO_STATE.png.sha1

+6f2a66931f5da990b57137ca32018323bcfd2008
\ No newline at end of file