Views: Expose an on_closed callback via the MenuRunner constructor.

ui/views/controls/menu/menu_runner_impl_cocoa.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ui/views/controls/menu/menu_runner_impl_cocoa.h"
 
 #include "base/mac/sdk_forward_declarations.h"
 #import "ui/base/cocoa/menu_controller.h"
 #include "ui/base/models/menu_model.h"
 #include "ui/events/base_event_utils.h"
@@ skipping 66 matching lines @@
   [anchor_view setHidden:YES];
   [[window contentView] addSubview:anchor_view];
   return anchor_view;
 }
 
 }  // namespace
 
 // 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]);
 }
 
 bool MenuRunnerImplCocoa::IsRunning() const {
   return running_;
 }
 
 void MenuRunnerImplCocoa::Release() {
   if (IsRunning()) {
@@ skipping 37 matching lines @@
   }
 
   closing_event_time_ = ui::EventTimeForNow();
   running_ = false;
 
   if (delete_after_run_) {
     delete this;
     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;
 }
 
 void MenuRunnerImplCocoa::Cancel() {
   [menu_controller_ cancel];
 }
 
 base::TimeTicks MenuRunnerImplCocoa::GetClosingEventTime() const {
   return closing_event_time_;
 }
 
 MenuRunnerImplCocoa::~MenuRunnerImplCocoa() {
 }
 
 }  // namespace internal
 }  // namespace views