Menu crash fix for domui/gtk menu that is caused by multiple context menu requests.

Renderer may issue multiple ContextMenu request even if the menu is open.(this happenes when renderer receives mouse events before context menu is actually opened by previous request.)
 This is causing a crash in menu2 (both domiu/gtk impl) because menu2 has nested message loop, and the menu object gets deleted by 2nd request while it's handling message loop.

This CL addresses this issue by creating separate object to handle
the nested loop. Use only variables on stack and do not touch
instance variable if the menu is already deleted.

BUG=chromium-os:7929, chromium-os:7228
TEST=manual. on a page, continously click mouse to open/close context menu.
   It used to crash at some point, but shouldn't crash with this patch.

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

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

5 files changed, 114 insertions, 2 deletions

diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index 14c1f6d..31a8c3f 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -21,6 +21,7 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "views/accelerator.h"
 #include "views/controls/menu/menu_2.h"
+#include "views/controls/menu/nested_dispatcher_gtk.h"
 
 namespace {
 
@@ -63,10 +64,16 @@ NativeMenuGtk::NativeMenuGtk(Menu2* menu)
       activated_index_(-1),
       activate_factory_(this),
       host_menu_(menu),
-      menu_action_(MENU_ACTION_NONE) {
+      menu_action_(MENU_ACTION_NONE),
+      nested_dispatcher_(NULL) {
 }
 
 NativeMenuGtk::~NativeMenuGtk() {
+  if (nested_dispatcher_) {
+    // Menu is destroyed while its in message loop.
+    // Let nested dispatcher know the creator is deleted.
+    nested_dispatcher_->CreatorDestroyed();
+  }
   if (menu_) {
     // Don't call MenuDestroyed because menu2 has already been destroyed.
     g_signal_handler_disconnect(menu_, destroy_handler_id_);
@@ -104,7 +111,14 @@ 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(this);
+  nested_dispatcher_ = new NestedDispatcherGtk(this, false);
+  bool deleted = nested_dispatcher_->RunAndSelfDestruct();
+  if (deleted) {
+    // The menu was destryed while menu is shown, so return immediately.
+    // Don't touch the instance which is already deleted.
+    return;
+  }
+  nested_dispatcher_ = NULL;
   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.
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index 79a6060..6145925 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -21,6 +21,8 @@ class MenuModel;
 
 namespace views {
 
+class NestedDispatcherGtk;
+
 // A Gtk implementation of MenuWrapper.
 //
 // NOTE: On windows the activate message is not sent immediately when an item
@@ -136,6 +138,11 @@ class NativeMenuGtk : public MenuWrapper,
   // Vector of listeners to receive callbacks when the menu opens.
   std::vector<MenuListener*> listeners_;
 
+  // Nested dispatcher object that can outlive this object.
+  // This is to deal with the menu being deleted while the nested
+  // message loop is handled. see http://crosbug.com/7228 .
+  NestedDispatcherGtk* nested_dispatcher_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeMenuGtk);
 };
 
diff --git a/views/controls/menu/nested_dispatcher_gtk.cc b/views/controls/menu/nested_dispatcher_gtk.cc
new file mode 100644
index 0000000..856b0a8
--- /dev/null
+++ b/views/controls/menu/nested_dispatcher_gtk.cc
@@ -0,0 +1,39 @@
+// Copyright (c) 2010 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 "views/controls/menu/nested_dispatcher_gtk.h"
+
+namespace views {
+
+NestedDispatcherGtk::NestedDispatcherGtk(MessageLoopForUI::Dispatcher* creator,
+                                         bool allow_nested_task)
+  : creator_(creator),
+    allow_nested_task_(allow_nested_task) {
+}
+
+bool NestedDispatcherGtk::RunAndSelfDestruct() {
+  bool nestable = MessageLoopForUI::current()->NestableTasksAllowed();
+  if (allow_nested_task_)
+    MessageLoopForUI::current()->SetNestableTasksAllowed(true);
+  MessageLoopForUI::current()->Run(this);
+  if (allow_nested_task_)
+    MessageLoopForUI::current()->SetNestableTasksAllowed(nestable);
+  bool creator_is_deleted = creator_ == NULL;
+  delete this;
+  return creator_is_deleted;
+}
+
+void NestedDispatcherGtk::CreatorDestroyed() {
+  creator_ = NULL;
+}
+
+bool NestedDispatcherGtk::Dispatch(GdkEvent* event) {
+  if (creator_ != NULL) {
+    return creator_->Dispatch(event);
+  } else {
+    return false;
+  }
+}
+
+}  // namespace views
diff --git a/views/controls/menu/nested_dispatcher_gtk.h b/views/controls/menu/nested_dispatcher_gtk.h
new file mode 100644
index 0000000..06f54fa
--- /dev/null
+++ b/views/controls/menu/nested_dispatcher_gtk.h
@@ -0,0 +1,50 @@
+// Copyright (c) 2010 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.
+
+#ifndef VIEWS_CONTROLS_MENU_NESTED_DISPATCHER_GTK_H_
+#define VIEWS_CONTROLS_MENU_NATIVE_DISPATCHER_GTK_H_
+#pragma once
+
+#include "base/message_loop.h"
+
+namespace views {
+
+// A nested dispatcher that can out-live the creator of this
+// dispatcher. This is to deal with the scenario where a menu object
+// is deleted while it's handling the message loop.  Note that
+// |RunAndSelfDestruct| method always delete itself regardless of
+// whether or not the menu object is deleted, so a menu object should
+// create a new instance for each open request.
+// http://crosbug.com/7228, http://crosbug.com/7929
+class NestedDispatcherGtk : public MessageLoopForUI::Dispatcher {
+ public:
+  NestedDispatcherGtk(MessageLoopForUI::Dispatcher* creator,
+                      bool allow_nested_task);
+
+  // Run the messsage loop and returns if the menu has been
+  // deleted in the nested loop. Returns true if the menu is
+  // deleted, or false otherwise.
+  bool RunAndSelfDestruct();
+
+  // Tells the nested dispatcher that creator has been destroyed.
+  void CreatorDestroyed();
+
+ private:
+  virtual ~NestedDispatcherGtk() {}
+
+  // Overriden from MessageLoopForUI::Dispatcher:
+  virtual bool Dispatch(GdkEvent* event);
+
+  // Creator of the nested loop.
+  MessageLoopForUI::Dispatcher* creator_;
+
+  // True to allow nested task in the message loop.
+  bool allow_nested_task_;
+
+  DISALLOW_COPY_AND_ASSIGN(NestedDispatcherGtk);
+};
+
+}  // namespace views
+
+#endif  // VIEWS_CONTROLS_MENU_NESTED_DISPATCHER_GTK_H_
diff --git a/views/views.gyp b/views/views.gyp
index 14a6a5a..e011383 100644
--- a/views/views.gyp
+++ b/views/views.gyp
@@ -145,6 +145,8 @@
         'controls/menu/native_menu_gtk.h',
         'controls/menu/native_menu_win.cc',
         'controls/menu/native_menu_win.h',
+        'controls/menu/nested_dispatcher_gtk.cc',
+        'controls/menu/nested_dispatcher_gtk.h',
         'controls/menu/radio_button_image_gtk.cc',
         'controls/menu/radio_button_image_gtk.h',
         'controls/menu/submenu_view.cc',