Fixes possible crash if the window hosting a menu was closed while the

menu was showing. When this happens the window the menu creates is
implicitly destroyed (because the parent is going away).

BUG=none
TEST=Use the test case in 25563, but instead of opening a bookmark
menu open either the page or wrench menus.

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

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

2 files changed, 25 insertions, 1 deletions

diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index b5ba4cc..43d4430 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -68,7 +68,8 @@ ToolbarView::ToolbarView(Browser* browser)
       profiles_menu_contents_(NULL),
       last_focused_view_storage_id_(-1),
       menu_bar_emulation_mode_(false),
-      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
+      destroyed_flag_(NULL) {
   SetID(VIEW_ID_TOOLBAR);
   browser_->command_updater()->AddCommandObserver(IDC_BACK, this);
   browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this);
@@ -86,6 +87,8 @@ ToolbarView::ToolbarView(Browser* browser)
 }
 
 ToolbarView::~ToolbarView() {
+  if (destroyed_flag_)
+    *destroyed_flag_ = true;
   if (menu_bar_emulation_mode_) {
     focus_manager_->UnregisterAccelerators(this);
     focus_manager_->RemoveFocusChangeListener(this);
@@ -721,12 +724,19 @@ void ToolbarView::LoadRightSideControlsImages() {
 }
 
 void ToolbarView::RunPageMenu(const gfx::Point& pt) {
+  bool destroyed_flag = false;
+  destroyed_flag_ = &destroyed_flag;
+
   page_menu_model_.reset(new PageMenuModel(this, browser_));
   page_menu_menu_.reset(new views::Menu2(page_menu_model_.get()));
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     page_menu_menu_->AddMenuListener(menu_listeners_[i]);
   }
   page_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
+
+  if (destroyed_flag)
+    return;
+
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     page_menu_menu_->RemoveMenuListener(menu_listeners_[i]);
   }
@@ -734,12 +744,19 @@ void ToolbarView::RunPageMenu(const gfx::Point& pt) {
 }
 
 void ToolbarView::RunAppMenu(const gfx::Point& pt) {
+  bool destroyed_flag = false;
+  destroyed_flag_ = &destroyed_flag;
+
   if (app_menu_model_->BuildProfileSubMenu())
     app_menu_menu_->Rebuild();
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     app_menu_menu_->AddMenuListener(menu_listeners_[i]);
   }
   app_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
+
+  if (destroyed_flag)
+    return;
+
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     app_menu_menu_->RemoveMenuListener(menu_listeners_[i]);
   }
diff --git a/chrome/browser/views/toolbar_view.h b/chrome/browser/views/toolbar_view.h
index 6e45799..734da7f 100644
--- a/chrome/browser/views/toolbar_view.h
+++ b/chrome/browser/views/toolbar_view.h
@@ -225,6 +225,13 @@ class ToolbarView : public AccessibleToolbarView,
 
   // Used to post tasks to switch to the next/previous menu.
   ScopedRunnableMethodFactory<ToolbarView> method_factory_;
+
+  // If non-null the destuctor sets this to true. This is set to a non-null
+  // while the menu is showing and used to detect if the menu was deleted while
+  // running.
+  bool* destroyed_flag_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ToolbarView);
 };
 
 #endif  // CHROME_BROWSER_VIEWS_TOOLBAR_VIEW_H_