Views: Expose an on_closed callback via the MenuRunner constructor.

ui/views/controls/menu/menu_runner.h

Index: ui/views/controls/menu/menu_runner.h
diff --git a/ui/views/controls/menu/menu_runner.h b/ui/views/controls/menu/menu_runner.h
index 1bbc905e2ba9670e3c66c721adf606320b0087a5..0be64f9bca928c1a637703276e5c19fe1eb05fa1 100644
--- a/ui/views/controls/menu/menu_runner.h
+++ b/ui/views/controls/menu/menu_runner.h
@@ -9,6 +9,7 @@
 
 #include <memory>
 
+#include "base/callback_forward.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "ui/base/ui_base_types.h"
@@ -105,9 +106,15 @@ class VIEWS_EXPORT MenuRunner {
     NORMAL_EXIT
   };
 
-  // Creates a new MenuRunner.
-  // |run_types| is a bitmask of RunTypes.
+  // Creates a new MenuRunner, which may use a native menu if available.
+  // |run_types| is a bitmask of RunTypes. If provided,
+  // |on_menu_closed_callback| is invoked when the menu is closed.
+  MenuRunner(ui::MenuModel* menu_model,
+             int32_t run_types,
+             const base::Closure& on_menu_closed_callback);
   MenuRunner(ui::MenuModel* menu_model, int32_t run_types);

[tapted, 2016/10/12 10:05:21]

Patchset 2 didn't keep this overload - there's a bit of fallout (and more if I
fixed those compile errors). But in a follow-up it might be neater to remove
this - to encourage providing a callback in _all_ cases, since clients who do
not reset() their |menu_runner_| instance effectively leak the menu until the
client itself is destroyed.

[jonross, 2016/10/12 14:35:18]

There are some clients which are transient. The views triggering the menus don't
last long. (Such as the Notifications bubble on Chromium OS.) So the closed
MenuRunner is not always around long.

However I wouldn't be against a forced closing step

+
+  // Creates a runner for a custom-created toolkit-views menu.
   MenuRunner(MenuItemView* menu, int32_t run_types);

[tapted, 2016/10/12 10:05:21]

I looked around for other things that might miss out on native menus. But there
aren't many callers of this overload left. Only 2 in the current Mac build:
 - scroll_bar (for a context menu that will probably get nerfed on Mac once
someone notices it exists at all).
 - RenderViewContextMenuBase's RenderViewContextMenuBase (since it does "live"
menu item updates) (but I think this is dead code on Mac right now)

Then, a few more in a mac_views_browser_build
 - toolbar_button.cc (since they allow middle-clicks on menu items)
 - The "App" menu (wrench/hotdog - since it has crazy customizations)
 - bookmarks (we would not use native menus on mac either, since we need drag &
drop)
 - toolbar_action_view.cc (it exposes a MenuItemView* for testing)

(but then a few more that are only used in ash)

But none of these scare me off wanting to use native menus more widely :). I.e.
I think there's a neat solution for things we want using native menus on Mac.

   ~MenuRunner();