Fixes possible crash in menus on views/gtk. The crash would happen if

between the time we called into the model when the user selected an
item and the time MessageLoop::Quit is processed the menu was
destroyed, we would crash. As MessageLoop::Quit isn't immediate, and
we really don't want to call out from a nested message loop, we wait
until the quit is processed, then notify the model.

CROS_BUG=1108
TEST=see bug

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

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

2 files changed, 21 insertions, 5 deletions

diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index 0bafffd..8b3ce33 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -66,7 +66,9 @@ NativeMenuGtk::NativeMenuGtk(menus::MenuModel* model)
     : model_(model),
       menu_(NULL),
       menu_shown_(false),
-      suppress_activate_signal_(false) {
+      suppress_activate_signal_(false),
+      menu_activated_(false),
+      activated_index_(-1) {
 }
 
 NativeMenuGtk::~NativeMenuGtk() {
@@ -77,6 +79,8 @@ NativeMenuGtk::~NativeMenuGtk() {
 // NativeMenuGtk, MenuWrapper implementation:
 
 void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
+  menu_activated_ = false;
+
   UpdateStates();
   Position position = { point, static_cast<Menu2::Alignment>(alignment) };
   // TODO(beng): value of '1' will not work for context menus!
@@ -95,6 +99,14 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
 
   g_signal_handler_disconnect(G_OBJECT(menu_), handle_id);
   menu_shown_ = false;
+
+  // Call into the model after the nested message loop quits. This way if the
+  // model ends up deleting us, or MessageLoop::Quit takes a while, there aren't
+  // any problems.
+  if (menu_activated_ && model_->IsEnabledAt(activated_index_) &&
+      MenuTypeCanExecute(model_->GetTypeAt(activated_index_))) {
+    model_->ActivatedAt(activated_index_);
+  }
 }
 
 void NativeMenuGtk::CancelMenu() {
@@ -293,10 +305,8 @@ void NativeMenuGtk::OnActivate(GtkMenuItem* menu_item) {
     return;
   }
 
-  if (model_->IsEnabledAt(position) &&
-      MenuTypeCanExecute(model_->GetTypeAt(position))) {
-    model_->ActivatedAt(position);
-  }
+  menu_activated_ = true;
+  activated_index_ = position;
 }
 
 // static
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index e83995a..8167954 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -63,6 +63,12 @@ class NativeMenuGtk : public MenuWrapper {
   // state is changed by |UpdateStates()| API.
   bool suppress_activate_signal_;
 
+  // Did the user select something from the menu?
+  bool menu_activated_;
+
+  // If menu_activated_ is true, this is the index of the item.
+  int activated_index_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeMenuGtk);
 };