Fix the Crash in the launcher's start page (a better approach).

ui/app_list/views/start_page_view.cc

 // Copyright 2014 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.
 
 #include "ui/app_list/views/start_page_view.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/i18n/rtl.h"
@@ skipping 108 matching lines @@
 
   TileItemView* GetTileItemView(int index);
 
   const std::vector<SearchResultTileItemView*>& tile_views() const {
     return search_result_tile_views_;
   }
 
   AllAppsTileItemView* all_apps_button() { return all_apps_button_; }
 
   // Overridden from SearchResultContainerView:
-  int Update() override;
-  void UpdateSelectedIndex(int old_selected, int new_selected) override;
   void OnContainerSelected(bool from_bottom,
                            bool directional_movement) override;
   void NotifyFirstResultYIndex(int y_index) override;
   int GetYSize() override;
 
  private:
   void CreateAppsGrid(int apps_num);
 
+  // Overridden from SearchResultContainerView:
+  int DoUpdate() override;
+  void UpdateSelectedIndex(int old_selected, int new_selected) override;

[xiyuan, 2017/01/04 00:12:09]

nit: I don't have a strong preference of whether to keep overridden method
private. But we should probably keep them together. That is, either move the
remaining ones here or put those two back.

[xdai1, 2017/01/04 00:30:20]

Just curious, why should we keep the overridden methods together?

[xiyuan, 2017/01/04 00:36:27]

Think that would make it easier to read. And style guide says to keep similar
declarations together.

https://google.github.io/styleguide/cppguide.html#Declaration_Order

[xdai1, 2017/01/04 23:18:41]

Done. Thanks for the reference!

+
   ContentsView* contents_view_;
   AppListViewDelegate* view_delegate_;
 
   std::vector<SearchResultTileItemView*> search_result_tile_views_;
   AllAppsTileItemView* all_apps_button_;
 
   DISALLOW_COPY_AND_ASSIGN(StartPageTilesContainer);
 };
 
 StartPageView::StartPageTilesContainer::StartPageTilesContainer(
@@ skipping 15 matching lines @@
 
 TileItemView* StartPageView::StartPageTilesContainer::GetTileItemView(
     int index) {
   DCHECK_GT(num_results(), index);
   if (index == num_results() - 1)
     return all_apps_button_;
 
   return search_result_tile_views_[index];
 }
 
-int StartPageView::StartPageTilesContainer::Update() {
-  // 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();
-  }
-
-  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()));
-  }
-
-  // Update the tile item results.
-  for (size_t i = 0; i < search_result_tile_views_.size(); ++i) {
-    SearchResult* item = nullptr;
-    if (i < display_results.size())
-      item = display_results[i];
-    search_result_tile_views_[i]->SetSearchResult(item);
-    search_result_tile_views_[i]->SetEnabled(true);
-  }
-
-  Layout();
-  parent()->Layout();
-  // Add 1 to the results size to account for the all apps button.
-  return display_results.size() + 1;
-}
-
-void StartPageView::StartPageTilesContainer::UpdateSelectedIndex(
-    int old_selected,
-    int new_selected) {
-  if (old_selected >= 0)
-    GetTileItemView(old_selected)->SetSelected(false);
-
-  if (new_selected >= 0)
-    GetTileItemView(new_selected)->SetSelected(true);
-}
-
 void StartPageView::StartPageTilesContainer::OnContainerSelected(
     bool /*from_bottom*/,
     bool /*directional_movement*/) {
   NOTREACHED();
 }
 
 void StartPageView::StartPageTilesContainer::NotifyFirstResultYIndex(
     int /*y_index*/) {
   NOTREACHED();
 }
@@ skipping 31 matching lines @@
   }
 
   // Also add a special "all apps" button to the end of the container.
   all_apps_button_->UpdateIcon();
   if (i % kNumStartPageTilesCols == 0)
     tiles_layout_manager->StartRow(0, 0);
   tiles_layout_manager->AddView(all_apps_button_);
   AddChildView(all_apps_button_);
 }
 
+int StartPageView::StartPageTilesContainer::DoUpdate() {
+  // 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();
+  }
+
+  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()));
+  }
+
+  // Update the tile item results.
+  for (size_t i = 0; i < search_result_tile_views_.size(); ++i) {
+    SearchResult* item = nullptr;
+    if (i < display_results.size())
+      item = display_results[i];
+    search_result_tile_views_[i]->SetSearchResult(item);
+    search_result_tile_views_[i]->SetEnabled(true);
+  }
+
+  Layout();
+  parent()->Layout();
+  // Add 1 to the results size to account for the all apps button.
+  return display_results.size() + 1;
+}
+
+void StartPageView::StartPageTilesContainer::UpdateSelectedIndex(
+    int old_selected,
+    int new_selected) {
+  if (old_selected >= 0 && old_selected < num_results())
+    GetTileItemView(old_selected)->SetSelected(false);
+
+  if (new_selected >= 0 && new_selected < num_results())
+    GetTileItemView(new_selected)->SetSelected(true);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // StartPageView implementation:
 StartPageView::StartPageView(AppListMainView* app_list_main_view,
                              AppListViewDelegate* view_delegate)
     : app_list_main_view_(app_list_main_view),
       view_delegate_(view_delegate),
       search_box_spacer_view_(new SearchBoxSpacerView(
           app_list_main_view->search_box_view()->GetPreferredSize())),
       instant_container_(new views::View),
       custom_launcher_page_background_(new CustomLauncherPageBackgroundView(
@@ skipping 71 matching lines @@
 
 void StartPageView::OnShown() {
   // When the start page is shown, show or hide the custom launcher page
   // based on whether it is enabled.
   CustomLauncherPageView* custom_page_view =
       app_list_main_view_->contents_view()->custom_page_view();
   if (custom_page_view) {
     custom_page_view->SetVisible(
         app_list_main_view_->ShouldShowCustomLauncherPage());
   }
+  tiles_container_->ClearSelectedIndex();
   tiles_container_->Update();
-  tiles_container_->ClearSelectedIndex();
   custom_launcher_page_background_->SetSelected(false);
 }
 
 gfx::Rect StartPageView::GetPageBoundsForState(
     AppListModel::State state) const {
   gfx::Rect onscreen_bounds(GetFullContentsBounds());
   if (state == AppListModel::STATE_START)
     return onscreen_bounds;
 
   return GetAboveContentsOffscreenBounds(onscreen_bounds.size());
@@ skipping 152 matching lines @@
   // is enabled).
   if (event->type() == ui::ET_SCROLL && event->y_offset() < 0)
     MaybeOpenCustomLauncherPage();
 }
 
 TileItemView* StartPageView::GetTileItemView(size_t index) {
   return tiles_container_->GetTileItemView(index);
 }
 
 }  // namespace app_list