Fixes possible crash in showing menus. This is less likely now that

main menu is using the bookmark menu, but still worth trying to
fix. We run a nested message loop when showing a menu. When a click
occurs outside the menu gtk closes the menu and we exit the nested
message loop. The problem is that click is dispatched from the nested
message loop, meaning it's possible to attempt to spawn another menu
or do other things from the nested message loop. To fix it I'm
manually closing the popup and not dispatching the event.

BUG=40492
TEST=see bug, but in general make sure chrome specific menus (time,
network ...) continue to work.

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

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

2 files changed, 67 insertions, 11 deletions

diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index a10e130..43718f3 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -26,6 +26,10 @@ namespace {
 const char kPositionString[] = "position";
 const char kAccelGroupString[] = "accel_group";
 
+// Key for the property set on the gtk menu that gives the handle to the hosting
+// NativeMenuGtk.
+const char kNativeMenuGtkString[] = "native_menu_gtk";
+
 // Data passed to the MenuPositionFunc from gtk_menu_popup
 struct Position {
   // The point to run the menu at.
@@ -71,7 +75,7 @@ NativeMenuGtk::NativeMenuGtk(Menu2* menu)
     : parent_(NULL),
       model_(menu->model()),
       menu_(NULL),
-      menu_shown_(false),
+      menu_hidden_(true),
       suppress_activate_signal_(false),
       activated_menu_(NULL),
       activated_index_(-1),
@@ -101,9 +105,8 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
   // TODO(beng): value of '1' will not work for context menus!
   gtk_menu_popup(GTK_MENU(menu_), NULL, NULL, MenuPositionFunc, &position, 1,
                  gtk_get_current_event_time());
-
-  DCHECK(!menu_shown_);
-  menu_shown_ = true;
+  DCHECK(menu_hidden_);
+  menu_hidden_ = false;
 
   for (unsigned int i = 0; i < listeners_.size(); ++i) {
     listeners_[i]->OnMenuOpened();
@@ -119,11 +122,16 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
                        G_CALLBACK(OnMenuMoveCurrentThunk), this);
 
   // Block until menu is no longer shown by running a nested message loop.
-  MessageLoopForUI::current()->Run(NULL);
+  MessageLoopForUI::current()->Run(this);
+  if (!menu_hidden_) {
+    // If this happens it means we haven't yet gotten the hide signal and
+    // someone else quit the message loop on us.
+    NOTREACHED();
+    menu_hidden_ = true;
+  }
 
   g_signal_handler_disconnect(G_OBJECT(menu_), hide_handle_id);
   g_signal_handler_disconnect(G_OBJECT(menu_), move_handle_id);
-  menu_shown_ = false;
 
   if (activated_menu_) {
     MessageLoop::current()->PostTask(FROM_HERE,
@@ -205,17 +213,54 @@ void NativeMenuGtk::RemoveMenuListener(MenuListener* listener) {
   }
 }
 
+bool NativeMenuGtk::Dispatch(GdkEvent* event) {
+  if (menu_hidden_) {
+    // The menu has been closed but the message loop is still nested. Don't
+    // dispatch a message, otherwise we might spawn another message loop.
+    return false;  // Exits the nested message loop.
+  }
+  switch (event->type) {
+    case GDK_BUTTON_PRESS:
+    case GDK_2BUTTON_PRESS:
+    case GDK_3BUTTON_PRESS: {
+      gpointer data = NULL;
+      gdk_window_get_user_data(((GdkEventAny*)event)->window, &data);
+      GtkWidget* widget = reinterpret_cast<GtkWidget*>(data);
+      if (widget) {
+        GtkWidget* root = gtk_widget_get_toplevel(widget);
+        if (!g_object_get_data(G_OBJECT(root), kNativeMenuGtkString)) {
+          // The button event is not targeted at a menu, hide the menu and eat
+          // the event. If we didn't do this the button press is dispatched from
+          // the nested message loop and bad things can happen (like trying to
+          // spawn another menu.
+          gtk_menu_popdown(GTK_MENU(menu_));
+          // In some cases we may not have gotten the hide event, but the menu
+          // will be in the process of hiding so set menu_hidden_ anyway.
+          menu_hidden_ = true;
+          return false;  // Exits the nested message loop.
+        }
+      }
+      break;
+    }
+    default:
+      break;
+  }
+  gtk_main_do_event(event);
+  return true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeMenuGtk, private:
 
 void NativeMenuGtk::OnMenuHidden(GtkWidget* widget) {
-  if (!menu_shown_) {
-    // This indicates we don't have a menu open, and should never happen.
-    NOTREACHED();
+  if (menu_hidden_) {
+    // The menu has been already hidden by us and we're in the process of
+    // quiting the message loop..
     return;
   }
   // Quit the nested message loop we spawned in RunMenuAt.
   MessageLoop::current()->Quit();
+  menu_hidden_ = true;
 }
 
 void NativeMenuGtk::OnMenuMoveCurrent(GtkWidget* menu_widget,
@@ -330,6 +375,8 @@ void NativeMenuGtk::ResetMenu() {
     gtk_widget_destroy(menu_);
   }
   menu_ = gtk_menu_new();
+  g_object_set_data(
+      G_OBJECT(GTK_MENU(menu_)->toplevel), kNativeMenuGtkString, this);
   destroy_handler_id_ = g_signal_connect(
       menu_, "destroy", G_CALLBACK(NativeMenuGtk::MenuDestroyed), host_menu_);
 }
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index a8ba37c..5f33b8b 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "app/gtk_signal.h"
+#include "base/message_loop.h"
 #include "base/task.h"
 #include "views/controls/menu/menu_wrapper.h"
 
@@ -28,7 +29,8 @@ namespace views {
 // then notify.
 //
 // TODO(beng): rename to MenuGtk once the old class is dead.
-class NativeMenuGtk : public MenuWrapper {
+class NativeMenuGtk : public MenuWrapper,
+                      public MessageLoopForUI::Dispatcher {
  public:
   explicit NativeMenuGtk(Menu2* menu);
   virtual ~NativeMenuGtk();
@@ -43,6 +45,9 @@ class NativeMenuGtk : public MenuWrapper {
   virtual void AddMenuListener(MenuListener* listener);
   virtual void RemoveMenuListener(MenuListener* listener);
 
+  // Overriden from MessageLoopForUI::Dispatcher:
+  virtual bool Dispatch(GdkEvent* event);
+
  private:
   CHROMEGTK_CALLBACK_0(NativeMenuGtk, void, OnMenuHidden);
   CHROMEGTK_CALLBACK_1(NativeMenuGtk, void, OnMenuMoveCurrent,
@@ -93,7 +98,11 @@ class NativeMenuGtk : public MenuWrapper {
 
   GtkWidget* menu_;
 
-  bool menu_shown_;
+  // Has the menu been hidden?
+  // NOTE: this is maintained by us and do to the asynchronous nature of X may
+  // be out of sync with whether the window is actually hidden. None-the-less if
+  // true the menu is either truly hidden or in the process of hiding.
+  bool menu_hidden_;
 
   // A flag used to avoid misfiring ActivateAt call on the menu model.
   // This is necessary as GTK menu fires an activate signal even when the