Fixes crash in disabling browser action from context menu on

views/gtk. The crash occurred because on views we dispatch the menu
action immediately (well, after the nested message loop exits), but on
windows the action is posted after a delay. The code wasn't written to
deal with immediate dispatch, so I changed the gtk code to post a task
to dispatch activation. I've also added a warning that menu2 won't
work on the stack, which has always been the case, just not stated.

BUG=33619
TEST=see bug report.

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

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

3 files changed, 33 insertions, 5 deletions

diff --git a/views/controls/menu/menu_2.h b/views/controls/menu/menu_2.h
index 416c949..1000d86 100644
--- a/views/controls/menu/menu_2.h
+++ b/views/controls/menu/menu_2.h
@@ -18,6 +18,10 @@ namespace views {
 class NativeMenuGtk;
 
 // A menu. Populated from a model, and relies on a delegate to execute commands.
+//
+// WARNING: do NOT create and use Menu2 on the stack. Menu2 notifies the model
+// of selection AFTER a delay. This means that if use a Menu2 on the stack
+// ActivatedAt is never invoked.
 class Menu2 {
  public:
   // Creates a new menu populated with the contents of |model|.
diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index 2bc1fbc..c1c2d22 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -70,6 +70,7 @@ NativeMenuGtk::NativeMenuGtk(Menu2* menu)
       suppress_activate_signal_(false),
       activated_menu_(NULL),
       activated_index_(-1),
+      activate_factory_(this),
       host_menu_(menu) {
 }
 
@@ -107,11 +108,11 @@ 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 (activated_menu_)
-    activated_menu_->Activate();
+  if (activated_menu_) {
+    MessageLoop::current()->PostTask(FROM_HERE,
+                                     activate_factory_.NewRunnableMethod(
+                                         &NativeMenuGtk::ProcessActivate));
+  }
 }
 
 void NativeMenuGtk::CancelMenu() {
@@ -119,6 +120,8 @@ void NativeMenuGtk::CancelMenu() {
 }
 
 void NativeMenuGtk::Rebuild() {
+  activated_menu_ = NULL;
+
   ResetMenu();
 
   std::map<int, GtkRadioMenuItem*> radio_groups_;
@@ -335,6 +338,11 @@ NativeMenuGtk* NativeMenuGtk::GetAncestor() {
   return ancestor;
 }
 
+void NativeMenuGtk::ProcessActivate() {
+  if (activated_menu_)
+    activated_menu_->Activate();
+}
+
 void NativeMenuGtk::Activate() {
   if (model_->IsEnabledAt(activated_index_) &&
       MenuTypeCanExecute(model_->GetTypeAt(activated_index_))) {
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index af7e9b61..4062bc0 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -7,6 +7,7 @@
 
 #include <gtk/gtk.h>
 
+#include "base/task.h"
 #include "views/controls/menu/menu_wrapper.h"
 
 namespace menus {
@@ -16,6 +17,13 @@ class MenuModel;
 namespace views {
 
 // A Gtk implementation of MenuWrapper.
+//
+// NOTE: On windows the activate message is not sent immediately when an item
+// is selected. Instead a message is added to the queue that is processed later.
+// To simulate that (and avoid having to deal with different behavior between
+// the platforms) we mimick that by posting a task after a menu item is selected
+// then notify.
+//
 // TODO(beng): rename to MenuGtk once the old class is dead.
 class NativeMenuGtk : public MenuWrapper {
  public:
@@ -58,6 +66,10 @@ class NativeMenuGtk : public MenuWrapper {
   // Returns the root of the menu tree.
   NativeMenuGtk* GetAncestor();
 
+  // Callback that we should really process the menu activation.
+  // See description above class for why we delay processing activation.
+  void ProcessActivate();
+
   // Notifies the model the user selected an item.
   void Activate();
 
@@ -89,6 +101,10 @@ class NativeMenuGtk : public MenuWrapper {
   // user selects and not the root.
   int activated_index_;
 
+  // Used when a menu item is selected. See description above class as to why
+  // we do this.
+  ScopedRunnableMethodFactory<NativeMenuGtk> activate_factory_;
+
   // A eference to the hosting menu2 object and signal handler id
   // used to delete the menu2 when its native menu gtk is destroyed first.
   Menu2* host_menu_;