Fix a crash when closing an incognito window with a download

shelf visible.

We explicitly remove the download shelf view from the browser
view hierarchy during a window close operation. This avoids
calling back into the partially deleted view hierarchy with
download deleted observer notifications. Explicitly removing
the shelf allows the observer notifications to run first while
the views are still valid.


To reproduce:
1. Launch Chrome
2. Open an incognito window
3. Download something in the incognito window
4. The download shelf should become visible with one entry
5. Close the incognito window
6. Crash

BUG=13681 (http://crbug.com/13681)

Review URL: http://codereview.chromium.org/126082

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

2 files changed, 15 insertions, 8 deletions

diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 2c2d7a6..b9c8c40 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -274,7 +274,6 @@ BrowserView::BrowserView(Browser* browser)
       active_bookmark_bar_(NULL),
       tabstrip_(NULL),
       toolbar_(NULL),
-      download_shelf_(NULL),
       infobar_container_(NULL),
       find_bar_y_(0),
       contents_container_(NULL),
@@ -298,6 +297,12 @@ BrowserView::~BrowserView() {
   ticker_.UnregisterTickHandler(&hung_window_detector_);
 #endif
 
+  // We destroy the download shelf before |browser_| to remove its child
+  // download views from the set of download observers (since the observed
+  // downloads can be destroyed along with |browser_| and the observer
+  // notifications will call back into deleted objects).
+  download_shelf_.reset();
+
   // Explicitly set browser_ to NULL
   browser_.reset();
 }
@@ -787,7 +792,7 @@ gfx::Rect BrowserView::GetRootWindowResizerRect() const {
   // shelf, so we don't want others to do it for us in this case.
   // Currently, the only visible bottom shelf is the download shelf.
   // Other tests should be added here if we add more bottom shelves.
-  if (download_shelf_ && download_shelf_->IsShowing()) {
+  if (download_shelf_.get() && download_shelf_->IsShowing()) {
     return gfx::Rect();
   }
 
@@ -842,13 +847,15 @@ void BrowserView::SetDownloadShelfVisible(bool visible) {
 }
 
 bool BrowserView::IsDownloadShelfVisible() const {
-  return download_shelf_ && download_shelf_->IsShowing();
+  return download_shelf_.get() && download_shelf_->IsShowing();
 }
 
 DownloadShelf* BrowserView::GetDownloadShelf() {
-  if (!download_shelf_)
-    download_shelf_ = new DownloadShelfView(browser_.get(), this);
-  return download_shelf_;
+  if (!download_shelf_.get()) {
+    download_shelf_.reset(new DownloadShelfView(browser_.get(), this));
+    download_shelf_->SetParentOwned(false);
+  }
+  return download_shelf_.get();
 }
 
 void BrowserView::ShowReportBugDialog() {
@@ -1493,7 +1500,7 @@ int BrowserView::LayoutDownloadShelf(int bottom) {
   if (IsDownloadShelfVisible()) {
     bool visible = browser_->SupportsWindowFeature(
         Browser::FEATURE_DOWNLOADSHELF);
-    DCHECK(download_shelf_);
+    DCHECK(download_shelf_.get());
     int height = visible ? download_shelf_->GetPreferredSize().height() : 0;
     download_shelf_->SetVisible(visible);
     download_shelf_->SetBounds(0, bottom - height, width(), height);
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index eb1c3dc8..b7a552e 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -391,7 +391,7 @@ class BrowserView : public BrowserWindow,
   scoped_ptr<BookmarkBarView> bookmark_bar_view_;
 
   // The download shelf view (view at the bottom of the page).
-  DownloadShelfView* download_shelf_;
+  scoped_ptr<DownloadShelfView> download_shelf_;
 
   // The InfoBarContainer that contains InfoBars for the current tab.
   InfoBarContainer* infobar_container_;