Views: Expose an on_closed callback via the MenuRunner constructor.

ui/views/controls/menu/menu_runner_impl_cocoa.mm

Index: ui/views/controls/menu/menu_runner_impl_cocoa.mm
diff --git a/ui/views/controls/menu/menu_runner_impl_cocoa.mm b/ui/views/controls/menu/menu_runner_impl_cocoa.mm
index f04c2eb2137fe934a57d7680a1c38a9fc721d607..f4ac01b23dfaca81057923800d8aca9e8efcae6e 100644
--- a/ui/views/controls/menu/menu_runner_impl_cocoa.mm
+++ b/ui/views/controls/menu/menu_runner_impl_cocoa.mm
@@ -84,19 +84,23 @@ base::scoped_nsobject<NSView> CreateMenuAnchorView(
 // static
 MenuRunnerImplInterface* MenuRunnerImplInterface::Create(
     ui::MenuModel* menu_model,
-    int32_t run_types) {
+    int32_t run_types,
+    const base::Closure& on_menu_closed_callback) {
   if ((run_types & kNativeRunTypes) != 0 &&
       (run_types & MenuRunner::IS_NESTED) == 0) {
-    return new MenuRunnerImplCocoa(menu_model);
+    return new MenuRunnerImplCocoa(menu_model, on_menu_closed_callback);
   }
 
-  return new MenuRunnerImplAdapter(menu_model);
+  return new MenuRunnerImplAdapter(menu_model, on_menu_closed_callback);
 }
 
-MenuRunnerImplCocoa::MenuRunnerImplCocoa(ui::MenuModel* menu)
+MenuRunnerImplCocoa::MenuRunnerImplCocoa(
+    ui::MenuModel* menu,
+    const base::Closure& on_menu_closed_callback)
     : running_(false),
       delete_after_run_(false),
-      closing_event_time_(base::TimeTicks()) {
+      closing_event_time_(base::TimeTicks()),
+      on_menu_closed_callback_(on_menu_closed_callback) {
   menu_controller_.reset(
       [[MenuController alloc] initWithModel:menu useWithPopUpButtonCell:NO]);
 }
@@ -154,6 +158,11 @@ MenuRunner::RunResult MenuRunnerImplCocoa::RunMenuAt(Widget* parent,
     return MenuRunner::MENU_DELETED;
   }
 
+  // Don't invoke the callback if Release() was called, since that usually means
+  // the owning instance is being destroyed.
+  if (!on_menu_closed_callback_.is_null())
+    on_menu_closed_callback_.Run();

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

I just want to confirm, the beginning of this method perform the blocking calls.
Removing the calls into ui/views/controls/menu/menu_controller?

If so, that doesn't block the cleanup that we want to do there (remove the
synchronous code path)

One far off concern with having a set of menus blocking is related to the mus
window server, and how we ack events. As nested message loops remove guarantees
on receiving acks for a given event. However if we know the an isolated set of
cases, like this, we could post the acks for those.

[tapted, 2016/10/13 11:43:38]

Yup - exactly.
It's also possible we can get the mac-themed views menus to a point where
they're almost indistinguishable, so we can use that on Mus. For now they don't
integrate well with Mac screenreaders, and there are other subtle UI and UX
differences that that were making toolkit-views on mac less palatable :/

+
   return MenuRunner::NORMAL_EXIT;
 }