Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Unified Diff: ui/base/cocoa/menu_controller.mm

Issue 2852233002: Mac[Views]: Make native menus more responsive by pumping private runloop modes. (Closed)
Patch Set: fix tests, respond to comments, selfnits Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..63876a36ee0ceed4044fdd333528ef807345ee19 100644
--- a/ui/base/cocoa/menu_controller.mm
+++ b/ui/base/cocoa/menu_controller.mm
@@ -4,8 +4,12 @@
#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/strings/sys_string_conversions.h"
+#include "base/threading/thread_task_runner_handle.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/platform_accelerator_cocoa.h"
#include "ui/base/l10n/l10n_util_mac.h"
@@ -15,6 +19,46 @@
#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.
+// |uiEventFlags| are the ui::EventFlags captured from the triggering NSEvent.
+- (void)itemSelected:(id)sender uiEventFlags:(int)uiEventFlags;
+@end
+
+namespace {
+
+// Object that calls -[MenuController itemSelected:uiEventFlags:] while a menu
+// is fading out.
+class PostedItemSelectedTask
Robert Sesek 2017/05/09 15:49:45 I think you can replace this class with a base::Ca
tapted 2017/05/10 03:49:06 Done (along with base::BindBlock to make the ObjC
+ : public base::SupportsWeakPtr<PostedItemSelectedTask> {
+ public:
+ // On construction, captures the flags from [NSApp currentEvent] and posts a
+ // task to Run() on the current thread's TaskRunner.
+ explicit PostedItemSelectedTask(NSMenuItem* item);
+
+ // Invokes -itemSelected:uiEventFlags: 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 ui_event_flags_;
+
+ DISALLOW_COPY_AND_ASSIGN(PostedItemSelectedTask);
+};
+
+} // namespace
+
NSString* const kMenuControllerMenuWillOpenNotification =
@"MenuControllerMenuWillOpen";
NSString* const kMenuControllerMenuDidCloseNotification =
@@ -25,10 +69,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 +165,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 +253,32 @@ 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:uiEventFlags:)]) {
+ postedItemSelectedTask_ = base::MakeUnique<PostedItemSelectedTask>(sender);
+ }
+}
+
- (void)itemSelected:(id)sender {
+ if (postedItemSelectedTask_) {
+ postedItemSelectedTask_->Run();
+ postedItemSelectedTask_.reset();
+ } else {
+ [self itemSelected:sender
+ uiEventFlags:ui::EventFlagsFromNative([NSApp currentEvent])];
+ }
+}
+
+- (void)itemSelected:(id)sender uiEventFlags:(int)uiEventFlags {
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, uiEventFlags);
}
- (NSMenu*)menu {
@@ -254,3 +321,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, in private run loop modes.
+- (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),
+ ui_event_flags_(ui::EventFlagsFromNative([NSApp currentEvent])) {
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE, base::Bind(&PostedItemSelectedTask::Run, AsWeakPtr()));
+}
+
+void PostedItemSelectedTask::Run() {
+ if (!item_)
+ return;
+
+ id target = [item_ target];
+ if ([target respondsToSelector:@selector(itemSelected:uiEventFlags:)])
+ [target itemSelected:item_.get() uiEventFlags:ui_event_flags_];
+ else
+ NOTREACHED();
+
+ item_.reset();
+}
+
+} // namespace

Powered by Google App Engine
This is Rietveld 408576698