Fix the Crash in the launcher's start page on Chrome OS.

ui/app_list/views/start_page_view.cc

Index: ui/app_list/views/start_page_view.cc
diff --git a/ui/app_list/views/start_page_view.cc b/ui/app_list/views/start_page_view.cc
index 0bac15b7b2ab6f8edd0963a7683af447347acb52..b0d2c5dfbad8e29af73eb6ff846c1dcbb52e188b 100644
--- a/ui/app_list/views/start_page_view.cc
+++ b/ui/app_list/views/start_page_view.cc
@@ -172,25 +172,28 @@ TileItemView* StartPageView::StartPageTilesContainer::GetTileItemView(
 }
 
 int StartPageView::StartPageTilesContainer::Update() {
+  std::vector<SearchResult*> display_results =
+      AppListModel::FilterSearchResultsByDisplayType(
+          results(), SearchResult::DISPLAY_RECOMMENDATION, kNumStartPageTiles);
+
   // Ignore updates and disable buttons when transitioning to a different
   // state.
   if (contents_view_->GetActiveState() != AppListModel::STATE_START) {
     for (auto* view : search_result_tile_views_)
       view->SetEnabled(false);
 
-    return num_results();
+    // We still need to update the number: add 1 to the results size to account
+    // for the all apps button.
+    return display_results.size() + 1;

[xiyuan, 2017/01/03 17:05:13]

Not sure whether this is the right thing to do. We will update num_results()
when this returns. However, |search_result_tile_views_| is not changed and
GetTileItemView probably will not work properly because of that, e.g.
display_results.size() + 1 is greater than old num_results and GetTileItemView
tries to access beyond the old num_results.

[xdai1, 2017/01/03 19:15:41]

Emmm... In this case the approach in the first patch might be better?
StartPageTilesContainer::Update() is not only called by
SearchResultContainerView::DoUpdate() but also can be called by
StartPageView::OnShown() (the state is AppListModel::STATE_START), which I think
is the case here. From the bug's crash stack, the crash is caused by the
keyboard navigation gesture StartPageView::OnKeyPressed() which is guaranteed to
be called after StartPageView::OnShown(). Thus if we call ScheduleUpdate() in
StartPageTilesContainer::Update(), num_results() and |search_result_tile_views_|
are forced to be updated to prevent the crash. Or maybe we can expose a
set_num_results() function and call it in StartPageView::OnShown()? Suggestions?

   }
 
-  std::vector<SearchResult*> display_results =
-      AppListModel::FilterSearchResultsByDisplayType(
-          results(), SearchResult::DISPLAY_RECOMMENDATION, kNumStartPageTiles);
   if (display_results.size() != search_result_tile_views_.size()) {
     // We should recreate the grid layout in this case.
     for (size_t i = 0; i < search_result_tile_views_.size(); ++i)
       delete search_result_tile_views_[i];
     search_result_tile_views_.clear();
     RemoveChildView(all_apps_button_);
-    CreateAppsGrid(std::min(kNumStartPageTiles, display_results.size()));

[xiyuan, 2017/01/03 17:05:14]

Would this change have side effect that we might display too many items on start
page?

[xdai1, 2017/01/03 19:15:41]

The maximum number of |display_results| is kNumStartPageTiles, so we should be
fine here. See line 184-186 in this patch.

+    CreateAppsGrid(display_results.size());
   }
 
   // Update the tile item results.
@@ -211,10 +214,10 @@ int StartPageView::StartPageTilesContainer::Update() {
 void StartPageView::StartPageTilesContainer::UpdateSelectedIndex(
     int old_selected,
     int new_selected) {
-  if (old_selected >= 0)
+  if (old_selected >= 0 && old_selected < num_results())
     GetTileItemView(old_selected)->SetSelected(false);
 
-  if (new_selected >= 0)
+  if (new_selected >= 0 && new_selected < num_results())
     GetTileItemView(new_selected)->SetSelected(true);
 }