Mac[Views]: Make native menus more responsive by pumping private runloop modes.

ui/base/cocoa/menu_controller.mm

Index: ui/base/cocoa/menu_controller.mm
diff --git a/ui/base/cocoa/menu_controller.mm b/ui/base/cocoa/menu_controller.mm
index 0c42fe7ba34504e04d011ebb24215a1f75ea9e2d..925dda3d709f734490626978bf115d56dad13365 100644
--- a/ui/base/cocoa/menu_controller.mm
+++ b/ui/base/cocoa/menu_controller.mm
@@ -4,7 +4,11 @@
 
 #import "ui/base/cocoa/menu_controller.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/platform_accelerator_cocoa.h"
@@ -15,6 +19,45 @@
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/text_elider.h"
 
+// Internal methods.
+@interface MenuController ()
+
+// Called via a private API hook shortly after the event that selects a menu
+// item arrives.
+- (void)itemWillBeSelected:(NSMenuItem*)sender;
+
+// Called when the user chooses a particular menu item. AppKit sends this only
+// after the menu has fully faded out. |sender| is the menu item chosen.
+- (void)itemSelected:(id)sender;
+
+// Called by the posted task to selected an item during menu fade out.

[Robert Sesek, 2017/05/08 22:42:10]

Make it clear that these are ui::EventFlags and not NSEventModifierFlags.

[tapted, 2017/05/09 04:57:42]

Done.

+- (void)itemSelected:(id)sender eventFlags:(int)eventFlags;
+@end
+
+namespace {
+
+// Object that calls -[MenuController itemSelected:eventFlags:] while a menu is
+// fading out.
+class PostedItemSelectedTask
+    : public base::SupportsWeakPtr<PostedItemSelectedTask> {
+ public:
+  // On construction, captures the flags from [NSApp currentEvent] and posts a
+  // task to Run() to MessageLoop::current().
+  explicit PostedItemSelectedTask(NSMenuItem* item);
+
+  // Invokes -itemSelected:eventFlags: on [item_ target], then clears |item_| so
+  // that later calls to Run() will do nothing.
+  void Run();
+
+ private:
+  base::scoped_nsobject<NSMenuItem> item_;
+  const int event_flags_;
+
+  DISALLOW_COPY_AND_ASSIGN(PostedItemSelectedTask);
+};
+
+}  // namespace
+
 NSString* const kMenuControllerMenuWillOpenNotification =
     @"MenuControllerMenuWillOpen";
 NSString* const kMenuControllerMenuDidCloseNotification =
@@ -25,10 +68,19 @@ NSString* const kMenuControllerMenuDidCloseNotification =
                    atIndex:(int)index;
 @end
 
-@implementation MenuController
+@interface ResponsiveNSMenuItem : NSMenuItem
+@end
+
+@implementation MenuController {
+  BOOL useWithPopUpButtonCell_;  // If YES, 0th item is blank
+  BOOL isMenuOpen_;
+  BOOL postItemSelectedAsTask_;
+  std::unique_ptr<PostedItemSelectedTask> postedItemSelectedTask_;
+}
 
 @synthesize model = model_;
 @synthesize useWithPopUpButtonCell = useWithPopUpButtonCell_;
+@synthesize postItemSelectedAsTask = postItemSelectedAsTask_;
 
 + (base::string16)elideMenuTitle:(const base::string16&)title
                          toWidth:(int)width {
@@ -112,10 +164,10 @@ NSString* const kMenuControllerMenuDidCloseNotification =
     label16 = [MenuController elideMenuTitle:label16 toWidth:maxWidth];
 
   NSString* label = l10n_util::FixUpWindowsStyleLabel(label16);
-  base::scoped_nsobject<NSMenuItem> item(
-      [[NSMenuItem alloc] initWithTitle:label
-                                 action:@selector(itemSelected:)
-                          keyEquivalent:@""]);
+  base::scoped_nsobject<NSMenuItem> item([[ResponsiveNSMenuItem alloc]
+      initWithTitle:label
+             action:@selector(itemSelected:)
+      keyEquivalent:@""]);
 
   // If the menu item has an icon, set it.
   gfx::Image icon;
@@ -200,18 +252,30 @@ NSString* const kMenuControllerMenuDidCloseNotification =
   return NO;
 }
 
-// Called when the user chooses a particular menu item. |sender| is the menu
-// item chosen.
+- (void)itemWillBeSelected:(NSMenuItem*)sender {
+  if (postItemSelectedAsTask_ && [sender action] == @selector(itemSelected:) &&
+      [[sender target] respondsToSelector:@selector(itemSelected:eventFlags:)])

[Robert Sesek, 2017/05/08 22:42:10]

nit: braces

[tapted, 2017/05/09 04:57:42]

Done.

+    postedItemSelectedTask_ = base::MakeUnique<PostedItemSelectedTask>(sender);
+}
+
 - (void)itemSelected:(id)sender {
+  if (postedItemSelectedTask_) {
+    postedItemSelectedTask_->Run();
+    postedItemSelectedTask_.reset();
+  } else {
+    [self itemSelected:sender
+            eventFlags:ui::EventFlagsFromNative([NSApp currentEvent])];
+  }
+}
+
+- (void)itemSelected:(id)sender eventFlags:(int)eventFlags {
   NSInteger modelIndex = [sender tag];
   ui::MenuModel* model =
       static_cast<ui::MenuModel*>(
           [[sender representedObject] pointerValue]);
   DCHECK(model);
-  if (model) {
-    int event_flags = ui::EventFlagsFromNative([NSApp currentEvent]);
-    model->ActivatedAt(modelIndex, event_flags);
-  }
+  if (model)
+    model->ActivatedAt(modelIndex, eventFlags);
 }
 
 - (NSMenu*)menu {
@@ -254,3 +318,42 @@ NSString* const kMenuControllerMenuDidCloseNotification =
 }
 
 @end
+
+@interface NSMenuItem (Private)
+// Private method which is invoked very soon after the event that activates a
+// menu item is received. AppKit then spends 300ms or so flashing the menu item,
+// and fading out the menu, blocking the UI thread the entire time.
+- (void)_sendItemSelectedNote;
+@end
+
+@implementation ResponsiveNSMenuItem
+- (void)_sendItemSelectedNote {
+  if ([[self target] respondsToSelector:@selector(itemWillBeSelected:)])
+    [[self target] itemWillBeSelected:self];
+  [super _sendItemSelectedNote];
+}
+@end
+
+namespace {
+
+PostedItemSelectedTask::PostedItemSelectedTask(NSMenuItem* item)
+    : item_(item, base::scoped_policy::RETAIN),
+      event_flags_(ui::EventFlagsFromNative([NSApp currentEvent])) {
+  base::MessageLoop::current()->task_runner()->PostTask(

[Robert Sesek, 2017/05/08 22:42:10]

The new thing, which I can never remember, is:

base::ThreadTaskRunnerHandle::Get()

[tapted, 2017/05/09 04:57:42]

Done.

(I wish there was just base::PostUITask(..) or something :/)

+      FROM_HERE, base::Bind(&PostedItemSelectedTask::Run, AsWeakPtr()));
+}
+
+void PostedItemSelectedTask::Run() {
+  if (!item_)
+    return;
+
+  id target = [item_ target];
+  if ([target respondsToSelector:@selector(itemSelected:eventFlags:)])
+    [target itemSelected:item_.get() eventFlags:event_flags_];
+  else
+    NOTREACHED();
+
+  item_.reset();
+}
+
+}  // namespace