Fixe use after free during destruction

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 2012 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 "chrome/browser/ui/views/frame/browser_view.h"
 
 #include <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 441 matching lines @@
   if (tabstrip_) {
     tabstrip_->parent()->RemoveChildView(tabstrip_);
     if (browser_view_layout)
       browser_view_layout->set_tab_strip(NULL);
     delete tabstrip_;
     tabstrip_ = NULL;
   }
   // Child views maintain PrefMember attributes that point to
   // OffTheRecordProfile's PrefService which gets deleted by ~Browser.
   RemoveAllChildViews(true);
+  toolbar_ = NULL;
 
   // It is possible that we were forced-closed by the native view system and
-  // that tabs remain in the browser. Close any such remaining tabs.
-  while (browser_->tab_strip_model()->count())
-    delete browser_->tab_strip_model()->GetWebContentsAt(0);
+  // that tabs remain in the browser. Close any such remaining tabs. Detach
+  // before destroying in hopes of avoiding less callbacks trying to access
+  // members since destroyed.
+  {
+    ScopedVector<content::WebContents> contents;
+    while (browser_->tab_strip_model()->count())
+      contents.push_back(browser_->tab_strip_model()->DetachWebContentsAt(0));
+  }

[Avi (use Gerrit), 2013/09/04 21:10:45]

Wow, this code is even sketchier than the code I wrote. But if it works...

 
   // Explicitly set browser_ to NULL.
   browser_.reset();
 }
 
 void BrowserView::Init(Browser* browser) {
   browser_.reset(browser);
   browser_->tab_strip_model()->AddObserver(this);
 }
 
@@ skipping 479 matching lines @@
     focus_manager->ClearFocus();
   }
 }
 
 void BrowserView::UpdateReloadStopState(bool is_loading, bool force) {
   toolbar_->reload_button()->ChangeMode(
       is_loading ? ReloadButton::MODE_STOP : ReloadButton::MODE_RELOAD, force);
 }
 
 void BrowserView::UpdateToolbar(content::WebContents* contents) {
-  toolbar_->Update(contents);
+  // We may end up here during destruction.
+  if (toolbar_)
+    toolbar_->Update(contents);
 }
 
 void BrowserView::FocusToolbar() {
   // Temporarily reveal the top-of-window views (if not already revealed) so
   // that the toolbar is visible and is considered focusable. If the
   // toolbar gains focus, |immersive_mode_controller_| will keep the
   // top-of-window views revealed.
   scoped_ptr<ImmersiveRevealedLock> focus_reveal_lock(
       immersive_mode_controller_->GetRevealedLock(
           ImmersiveModeController::ANIMATE_REVEAL_YES));
@@ skipping 1689 matching lines @@
     // The +1 in the next line creates a 1-px gap between icon and arrow tip.
     gfx::Point icon_bottom(0, location_icon_view->GetImageBounds().bottom() -
         LocationBarView::kIconInternalPadding + 1);
     ConvertPointToTarget(location_icon_view, this, &icon_bottom);
     gfx::Point infobar_top(0, infobar_container_->GetVerticalOverlap(NULL));
     ConvertPointToTarget(infobar_container_, this, &infobar_top);
     top_arrow_height = infobar_top.y() - icon_bottom.y();
   }
   return top_arrow_height;
 }