Ensure that MenuGtks are destroyed before their accelerator group.

Ensure that MenuGtks are destroyed before their accelerator group.

When a MenuGtk is created, it is passed a GtkAccelGroup.  Previously the GtkMenu widgets were destroyed after the browser window, which was the owner of the GtkAccelGroup.  Make the toolbar own a reference to the GtkAccelGroup, and teardown all GtkMenus that reference it beforehand.

Some MenuGtk cleanup while debugging the problem.

BUG=8866

[Dean McNamee, 2009-03-17 13:56:17]

I am not actually 100% sure of this fix, I didn't write the code, and my
understanding of the relationships between menus, windows, and accelerator
groups are a bit muddy.  Seems to work, I will follow up if there are leaks, but
it is better than crashing.

[tony, 2009-03-17 17:02:49]

Adding Elliot as a reviewer since I think he wrote the accelerator code.

http://codereview.chromium.org/48065/diff/1004/1006
File chrome/browser/gtk/menu_gtk.cc (right):

http://codereview.chromium.org/48065/diff/1004/1006#newcode238
Line 238: void MenuGtk::SetMenuItemInfo(GtkWidget* widget, gpointer userdata) {
Nit: Can you just have the type be MenuGtk* and not need the static_cast below?

[Elliot Glaysher, 2009-03-17 17:08:40]

LGTM.

[Dean McNamee, 2009-03-17 17:12:44]

http://codereview.chromium.org/48065/diff/1004/1006
File chrome/browser/gtk/menu_gtk.cc (right):

http://codereview.chromium.org/48065/diff/1004/1006#newcode238
Line 238: void MenuGtk::SetMenuItemInfo(GtkWidget* widget, gpointer userdata) {
I prefer to have the prototypes actually match.  It should be a reinterpret_cast
though...

On 2009/03/17 17:02:49, tony wrote:
> Nit: Can you just have the type be MenuGtk* and not need the static_cast
below?

[Evan Stade, 2009-03-17 17:33:02]

ah! this is all because MenuGtk owns the gtk menu widgets, so they don't get
destroyed when the top level window gets destroyed. Good find. Too bad our
ownership model sacrifices GTK's (well tested) destruction order.

lgtm

As for G_CALLBACK prototypes, we've commonly had the signatures do the casting.
For the sake of consistency, can we pick one and stick to it? I think it's
cleaner not to have to cast.