Linux: Fix a wide range of crashes in dynamic menu code on Ubuntu 12.04.

Linux: Fix a wide range of crashes resulting from a very bad interaction of
our dynamic menu-building code (mostly used by bookmarks, but also by some
extension menus) with libdbusmenu-gtk as used in Ubuntu 12.04 for global menu
support.

Basically, the problem is that it loads itself into our address space, and
shows and hides menus in ways that can't be triggered through normal user
interaction. It does this on purpose to get applications that build menus on
demand to do so; unfortunately, it doesn't work very well when the menus are
also torn down when hidden, especially the way we were doing it.

This CL changes how we tear down menus when hidden to avoid getting into loops
where menu changes are detected causing menus to be shown again, and is more
robust in the face of event orderings that don't normally occur.

BUG=131974, 124110, 128994
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=158316

[Mike Mammarella, 2012-09-22 21:39:27]

Actually, it turns out I figured out what the problem was and was able to come
up with a solution. So, please ignore the initial CL description.

[Evan Stade, 2012-09-24 08:49:09]

lgtm, but this doesn't seem to get rid of the create/destroy loop, it just
delays it by 2 seconds each round. Isn't that still wasting a lot of CPU cycles?
Worth filing a bug?

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
File chrome/browser/ui/gtk/menu_gtk.cc (right):

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
chrome/browser/ui/gtk/menu_gtk.cc:840: // TODO(mdm): Figure out why this can
sometimes be NULL. See bug 124110.
because you got two hides in a row? (seems possible considering you can get two
shows in a row, as you say above)

[Mike Mammarella, 2012-09-24 10:05:48]

Ah, it's not the 2 second delay that is avoiding the looping. That was something
I put in while tracking the problem and then left because I think it's
worthwhile. I've noted the cause of the looping below.

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
File chrome/browser/ui/gtk/menu_gtk.cc (left):

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
chrome/browser/ui/gtk/menu_gtk.cc:793: submenu = gtk_menu_new();
This is the part that was causing the looping - previously we created a new,
empty submenu and populated it on demand, throwing the old one away. Now we
clear out the old submenu and reuse it, which requires keeping track of whether
it's currently populated (the submenu-built data).

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
File chrome/browser/ui/gtk/menu_gtk.cc (right):

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
chrome/browser/ui/gtk/menu_gtk.cc:756: CHECK(menu_item);
This one is the same - originally I was adding more NULL checks here, and then I
figured out the problem, so now I've changed it to a CHECK.

https://codereview.chromium.org/10966043/diff/1003/chrome/browser/ui/gtk/menu...
chrome/browser/ui/gtk/menu_gtk.cc:840: // TODO(mdm): Figure out why this can
sometimes be NULL. See bug 124110.
On 2012/09/24 08:49:09, Evan Stade wrote:
> because you got two hides in a row? (seems possible considering you can get
two
> shows in a row, as you say above)

Well, we never clear this out on hide - it's only the submenu-model data that we
clear. I actually pretty strongly suspect that with this change, this will no
longer happen, which is why I've changed it to a CHECK rather than just bailing
out in this case.