Fix submenu leak in menu2.

 Added "destroy" signal handler to delete menu2 only when the native gtk widget
 gets destroyed first.

BUG=33475
TEST=chromeos valgrind tests passes without suppression.

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

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

2 files changed, 34 insertions, 10 deletions

diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index 1335b85..2bc1fbc 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -62,18 +62,23 @@ namespace views {
 ////////////////////////////////////////////////////////////////////////////////
 // NativeMenuGtk, public:
 
-NativeMenuGtk::NativeMenuGtk(menus::MenuModel* model)
+NativeMenuGtk::NativeMenuGtk(Menu2* menu)
     : parent_(NULL),
-      model_(model),
+      model_(menu->model()),
       menu_(NULL),
       menu_shown_(false),
       suppress_activate_signal_(false),
       activated_menu_(NULL),
-      activated_index_(-1) {
+      activated_index_(-1),
+      host_menu_(menu) {
 }
 
 NativeMenuGtk::~NativeMenuGtk() {
-  gtk_widget_destroy(menu_);
+  if (menu_) {
+    // Don't call MenuDestroyed because menu2 has already been destroyed.
+    g_signal_handler_disconnect(menu_, destroy_handler_id_);
+    gtk_widget_destroy(menu_);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -207,9 +212,6 @@ GtkWidget* NativeMenuGtk::AddMenuItemAt(int index,
   }
 
   if (type == menus::MenuModel::TYPE_SUBMENU) {
-    // TODO(beng): we're leaking these objects right now... consider some other
-    //             arrangement.
-    // See http://crbug.com/33475.
     Menu2* submenu = new Menu2(model_->GetSubmenuModelAt(index));
     static_cast<NativeMenuGtk*>(submenu->wrapper_.get())->set_parent(this);
     g_object_set_data(G_OBJECT(menu_item), "submenu", submenu);
@@ -232,9 +234,13 @@ GtkWidget* NativeMenuGtk::AddMenuItemAt(int index,
 }
 
 void NativeMenuGtk::ResetMenu() {
-  if (menu_)
+  if (menu_) {
+    g_signal_handler_disconnect(menu_, destroy_handler_id_);
     gtk_widget_destroy(menu_);
+  }
   menu_ = gtk_menu_new();
+  destroy_handler_id_ = g_signal_connect(
+      menu_, "destroy", G_CALLBACK(NativeMenuGtk::MenuDestroyed), host_menu_);
 }
 
 void NativeMenuGtk::UpdateMenuItemState(GtkWidget* menu_item) {
@@ -336,12 +342,21 @@ void NativeMenuGtk::Activate() {
   }
 }
 
+// static
+void NativeMenuGtk::MenuDestroyed(GtkWidget* widget, Menu2* menu2) {
+  NativeMenuGtk* native_menu =
+      static_cast<NativeMenuGtk*>(menu2->wrapper_.get());
+  // The native gtk widget has already been destroyed.
+  native_menu->menu_ = NULL;
+  delete menu2;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // MenuWrapper, public:
 
 // static
 MenuWrapper* MenuWrapper::CreateWrapper(Menu2* menu) {
-  return new NativeMenuGtk(menu->model());
+  return new NativeMenuGtk(menu);
 }
 
 }  // namespace views
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index b2aceff..af7e9b61 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -19,7 +19,7 @@ namespace views {
 // TODO(beng): rename to MenuGtk once the old class is dead.
 class NativeMenuGtk : public MenuWrapper {
  public:
-  explicit NativeMenuGtk(menus::MenuModel* model);
+  explicit NativeMenuGtk(Menu2* menu);
   virtual ~NativeMenuGtk();
 
   // Overridden from MenuWrapper:
@@ -61,6 +61,10 @@ class NativeMenuGtk : public MenuWrapper {
   // Notifies the model the user selected an item.
   void Activate();
 
+  // A callback to delete menu2 object when the native widget is
+  // destroyed first.
+  static void MenuDestroyed(GtkWidget* widget, Menu2* menu2);
+
   // If we're a submenu, this is the parent.
   NativeMenuGtk* parent_;
 
@@ -85,6 +89,11 @@ class NativeMenuGtk : public MenuWrapper {
   // user selects and not the root.
   int activated_index_;
 
+  // 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_;
+  gulong destroy_handler_id_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeMenuGtk);
 };