Commit 431f1131 authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

[Mac] Don't rebuild share menu every time a hotkey is pressed

Currently, the share menu controller's |menuNeedsUpdate:| is called
every time a hotkey is pressed.

Implementing |menuHasKeyEquivalent:forEvent:target:action:| disables
that path in the menu code, so this change implements that. Similarly
to the bookmark menu, we return NO unconditionally and let the hotkey
be handled by the menu item. To ensure the menu item exists to handle
it, we populate the menu on the first call to |menuHasKeyEquivalent:|
unless it's already been populated.

Bug: 787049
Change-Id: I321ed87dfc64f89f31c5738712472d33a923a55a
Reviewed-on: https://chromium-review.googlesource.com/801854Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520933}
parent e224543a
...@@ -65,6 +65,21 @@ NSString* const kRemindersSharingServiceName = ...@@ -65,6 +65,21 @@ NSString* const kRemindersSharingServiceName =
} }
// NSMenuDelegate // NSMenuDelegate
- (BOOL)menuHasKeyEquivalent:(NSMenu*)menu
forEvent:(NSEvent*)event
target:(id*)target
action:(SEL*)action {
// Load the menu if it hasn't loaded already.
if ([menu numberOfItems] == 0) {
[self menuNeedsUpdate:menu];
}
// Per tapted@'s comment in BookmarkMenuCocoaController, it's fine
// to return NO here if an item will handle this. This is why it's
// necessary to ensure the menu is loaded above.
return NO;
}
- (void)menuNeedsUpdate:(NSMenu*)menu { - (void)menuNeedsUpdate:(NSMenu*)menu {
[menu removeAllItems]; [menu removeAllItems];
[menu setAutoenablesItems:NO]; [menu setAutoenablesItems:NO];
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "net/base/mac/url_conversions.h" #include "net/base/mac/url_conversions.h"
#include "testing/gtest_mac.h" #include "testing/gtest_mac.h"
#include "ui/base/l10n/l10n_util_mac.h" #include "ui/base/l10n/l10n_util_mac.h"
#include "ui/events/test/cocoa_test_event_utils.h"
// Mock sharing service for sensing shared items. // Mock sharing service for sensing shared items.
@interface MockSharingService : NSSharingService @interface MockSharingService : NSSharingService
...@@ -213,3 +214,28 @@ IN_PROC_BROWSER_TEST_F(ShareMenuControllerTest, Histograms) { ...@@ -213,3 +214,28 @@ IN_PROC_BROWSER_TEST_F(ShareMenuControllerTest, Histograms) {
tester.ExpectTotalCount(histogram_name, 3); tester.ExpectTotalCount(histogram_name, 3);
tester.ExpectBucketCount(histogram_name, false, 1); tester.ExpectBucketCount(histogram_name, false, 1);
} }
IN_PROC_BROWSER_TEST_F(ShareMenuControllerTest, MenuHasKeyEquivalent) {
// If this method isn't implemented, |menuNeedsUpdate:| is called any time
// *any* hotkey is used
ASSERT_TRUE([controller_ respondsToSelector:@selector
(menuHasKeyEquivalent:forEvent:target:action:)]);
// Ensure that calling |menuHasKeyEquivalent:...| the first time populates the
// menu.
base::scoped_nsobject<NSMenu> menu([[NSMenu alloc] initWithTitle:@"Share"]);
EXPECT_EQ([menu numberOfItems], 0);
NSEvent* event = cocoa_test_event_utils::KeyEventWithKeyCode(
'i', 'i', NSKeyDown, NSCommandKeyMask | NSShiftKeyMask);
EXPECT_FALSE([controller_ menuHasKeyEquivalent:menu
forEvent:event
target:nil
action:nil]);
EXPECT_GT([menu numberOfItems], 0);
NSMenuItem* item = [menu itemAtIndex:0];
// |menuHasKeyEquivalent:....| shouldn't populate the menu after the first
// time.
[controller_ menuHasKeyEquivalent:menu forEvent:event target:nil action:nil];
EXPECT_EQ(item, [menu itemAtIndex:0]); // Pointer equality intended.
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment