Fix use-after-free when switching profiles in the experimental app list.

This CL fixes an issue where the ContentsSwitcherView would dereference a
pointer to a freed ContentsView.

BUG=384957

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

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

ui/app_list/views/app_list_main_view.cc

@@ -39,8 +39,6 @@ const int kInnerPadding = 1;
 // The maximum allowed time to wait for icon loading in milliseconds.
 const int kMaxIconLoadingWaitTimeInMs = 50;
 
-const int kContentsViewIndex = 1;
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -90,6 +88,7 @@ AppListMainView::AppListMainView(AppListViewDelegate* delegate,
       model_(delegate->GetModel()),
       search_box_view_(NULL),
       contents_view_(NULL),
+      contents_switcher_view_(NULL),
       weak_ptr_factory_(this) {
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical,
                                         kInnerPadding,
@@ -98,9 +97,7 @@ AppListMainView::AppListMainView(AppListViewDelegate* delegate,
 
   search_box_view_ = new SearchBoxView(this, delegate);
   AddChildView(search_box_view_);
-  AddContentsView();
-  if (app_list::switches::IsExperimentalAppListEnabled())
-    AddChildView(new ContentsSwitcherView(contents_view_));
+  AddContentsViews();
 
   // Switch the apps grid view to the specified page.
   app_list::PaginationModel* pagination_model = GetAppsPaginationModel();
@@ -111,9 +108,13 @@ AppListMainView::AppListMainView(AppListViewDelegate* delegate,
   PreloadIcons(parent);
 }
 
-void AppListMainView::AddContentsView() {
+void AppListMainView::AddContentsViews() {
   contents_view_ = new ContentsView(this, model_, delegate_);
-  AddChildViewAt(contents_view_, kContentsViewIndex);
+  AddChildView(contents_view_);
+  if (app_list::switches::IsExperimentalAppListEnabled()) {
+    contents_switcher_view_ = new ContentsSwitcherView(contents_view_);
+    AddChildView(contents_switcher_view_);
+  }
 
   search_box_view_->set_contents_view(contents_view_);
 
@@ -164,7 +165,11 @@ void AppListMainView::ModelChanged() {
   search_box_view_->ModelChanged();
   delete contents_view_;
   contents_view_ = NULL;
-  AddContentsView();
+  if (contents_switcher_view_) {
+    delete contents_switcher_view_;
+    contents_switcher_view_ = NULL;
+  }
+  AddContentsViews();
   Layout();
 }
 

ui/app_list/views/app_list_main_view.h

@@ -26,6 +26,7 @@ class AppListItem;
 class AppListModel;
 class AppListViewDelegate;
 class ApplicationDragAndDropHost;
+class ContentsSwitcherView;
 class ContentsView;
 class PaginationModel;
 class SearchBoxView;
@@ -65,6 +66,9 @@ class APP_LIST_EXPORT AppListMainView : public views::View,
       ApplicationDragAndDropHost* drag_and_drop_host);
 
   ContentsView* contents_view() const { return contents_view_; }
+  ContentsSwitcherView* contents_switcher_view() const {
+    return contents_switcher_view_;
+  }
   AppListModel* model() { return model_; }
 
   // Returns true if the app list should be centered and in landscape mode.
@@ -73,7 +77,8 @@ class APP_LIST_EXPORT AppListMainView : public views::View,
  private:
   class IconLoader;
 
-  void AddContentsView();
+  // Adds the ContentsView and the ContentsSwitcherView.
+  void AddContentsViews();
 
   // Gets the PaginationModel owned by the AppsGridView.
   PaginationModel* GetAppsPaginationModel();
@@ -110,6 +115,9 @@ class APP_LIST_EXPORT AppListMainView : public views::View,
   SearchBoxView* search_box_view_;  // Owned by views hierarchy.
   ContentsView* contents_view_;  // Owned by views hierarchy.
 
+  // Owned by views hierarchy. NULL in the non-experimental app list.
+  ContentsSwitcherView* contents_switcher_view_;
+
   // A timer that fires when maximum allowed time to wait for icon loading has
   // passed.
   base::OneShotTimer<AppListMainView> icon_loading_wait_timer_;

ui/app_list/views/app_list_view_unittest.cc

@@ -16,6 +16,7 @@
 #include "ui/app_list/views/app_list_main_view.h"
 #include "ui/app_list/views/apps_container_view.h"
 #include "ui/app_list/views/apps_grid_view.h"
+#include "ui/app_list/views/contents_switcher_view.h"
 #include "ui/app_list/views/contents_view.h"
 #include "ui/app_list/views/search_box_view.h"
 #include "ui/app_list/views/search_result_list_view.h"
@@ -339,10 +340,16 @@ void AppListViewTestContext::RunProfileChangeTest() {
 
   StartPageView* start_page_view =
       view_->app_list_main_view()->contents_view()->start_page_view();
+  ContentsSwitcherView* contents_switcher_view =
+      view_->app_list_main_view()->contents_switcher_view();
   if (test_type_ == EXPERIMENTAL) {
+    EXPECT_NO_FATAL_FAILURE(CheckView(contents_switcher_view));
+    EXPECT_EQ(view_->app_list_main_view()->contents_view(),
+              contents_switcher_view->contents_view());
     EXPECT_NO_FATAL_FAILURE(CheckView(start_page_view));
     EXPECT_EQ(1u, GetVisibleTileItemViews(start_page_view->tile_views()));
   } else {
+    EXPECT_EQ(NULL, contents_switcher_view);
     EXPECT_EQ(NULL, start_page_view);
   }
 

ui/app_list/views/contents_switcher_view.h

@@ -20,6 +20,8 @@ class ContentsSwitcherView : public views::View, public views::ButtonListener {
   explicit ContentsSwitcherView(ContentsView* contents_view);
   virtual ~ContentsSwitcherView();
 
+  ContentsView* contents_view() const { return contents_view_; }
+
  private:
   // Adds a switcher button using |resource_id| as the button's image and |tag|
   // as the button's id.