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

ui/app_list/views/app_list_main_view.cc

 // Copyright 2013 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/app_list_main_view.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 21 matching lines @@
 namespace app_list {
 
 namespace {
 
 // Inner padding space in pixels of bubble contents.
 const int kInnerPadding = 1;
 
 // The maximum allowed time to wait for icon loading in milliseconds.
 const int kMaxIconLoadingWaitTimeInMs = 50;
 
-const int kContentsViewIndex = 1;
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // AppListMainView::IconLoader
 
 class AppListMainView::IconLoader : public AppListItemObserver {
  public:
   IconLoader(AppListMainView* owner,
              AppListItem* item,
              float scale)
@@ skipping 29 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // AppListMainView:
 
 AppListMainView::AppListMainView(AppListViewDelegate* delegate,
                                  int initial_apps_page,
                                  gfx::NativeView parent)
     : delegate_(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,
                                         kInnerPadding,
                                         kInnerPadding));
 
   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();
   if (pagination_model->is_valid_page(initial_apps_page))
     pagination_model->SelectPage(initial_apps_page, false);
 
   // Starts icon loading early.
   PreloadIcons(parent);
 }
 
 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_);
 
   contents_view_->SetPaintToLayer(true);
   contents_view_->SetFillsBoundsOpaquely(false);
   contents_view_->layer()->SetMasksToBounds(true);
 }
 
 AppListMainView::~AppListMainView() {
   pending_icon_loaders_.clear();
@@ skipping 30 matching lines @@
 void AppListMainView::Prerender() {
   contents_view_->Prerender();
 }
 
 void AppListMainView::ModelChanged() {
   pending_icon_loaders_.clear();
   model_ = delegate_->GetModel();
   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();
 }
 
 void AppListMainView::UpdateSearchBoxVisibility() {
   search_box_view_->SetVisible(
       !contents_view_->IsNamedPageActive(ContentsView::NAMED_PAGE_START) ||
       contents_view_->IsShowingSearchResults());
 }
 
@@ skipping 108 matching lines @@
   // Resubmit the query via a posted task so that all observers for the
   // uninstall notification are notified.
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&AppListMainView::QueryChanged,
                  weak_ptr_factory_.GetWeakPtr(),
                  search_box_view_));
 }
 
 }  // namespace app_list