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

ui/app_list/views/app_list_main_view.cc

Index: ui/app_list/views/app_list_main_view.cc
diff --git a/ui/app_list/views/app_list_main_view.cc b/ui/app_list/views/app_list_main_view.cc
index 8874bbeccc6d7db126e3298b2dd4f527848d391b..2ecb29e51b0b0bfbf549ab59d15da5af6be50699 100644
--- a/ui/app_list/views/app_list_main_view.cc
+++ b/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,
@@ -99,8 +98,6 @@ 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_));
 
   // Switch the apps grid view to the specified page.
   app_list::PaginationModel* pagination_model = GetAppsPaginationModel();
@@ -113,7 +110,12 @@ AppListMainView::AppListMainView(AppListViewDelegate* delegate,
 
 void AppListMainView::AddContentsView() {
   contents_view_ = new ContentsView(this, model_, delegate_);
-  AddChildViewAt(contents_view_, kContentsViewIndex);
+  DCHECK_EQ(1, child_count());

[Matt Giuca, 2014/06/17 05:49:26]

This doesn't seem robust. Is this assuming that there is always exactly one
child view of AppListMainView at the time the contents view is added?

Why is it needed?

[calamity, 2014/06/17 09:15:39]

It's not. AddChildViewAt was there to fix a bug that is now resolved. Removed.

[Matt Giuca, 2014/06/18 00:06:06]

I'm a little unsure what you mean here. Does "It's not" mean "the DCHECK is not
needed", and "Removed" refer to the DCHECK? If so, you didn't remove it.

Otherwise, I don't know what you mean.

[calamity, 2014/06/18 01:00:02]

Oops. Actually removed.

+  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,6 +166,9 @@ void AppListMainView::ModelChanged() {
   search_box_view_->ModelChanged();
   delete contents_view_;
   contents_view_ = NULL;
+  if (contents_switcher_view_)
+    delete contents_switcher_view_;
+  contents_switcher_view_ = NULL;

[Matt Giuca, 2014/06/17 05:49:25]

nit: May as well put this line inside the if statement.

[calamity, 2014/06/17 09:15:39]

Done.

   AddContentsView();
   Layout();
 }