mac: Create a static mapping of accelerators in the main menu.

chrome/browser/ui/cocoa/accelerators_cocoa_browsertest.mm

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Robert Sesek, 2014/02/11 16:19:48]

Happy new year x2

[erikchen, 2014/02/11 19:42:27]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/logging.h"
+#import "chrome/browser/ui/cocoa/accelerators_cocoa.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "ui/base/accelerators/platform_accelerator_cocoa.h"
+#import "ui/events/keycodes/keyboard_code_conversion_mac.h"
+#include "testing/gtest_mac.h"
+
+
+typedef InProcessBrowserTest AcceleratorsCocoaBrowsertest;

[Robert Sesek, 2014/02/11 16:19:48]

Browsertest -> BrowserTest

[erikchen, 2014/02/11 19:42:27]

I copied the casing from another browser_test in the same folder. I've made the
change you've suggested.

On 2014/02/11 16:19:48, rsesek wrote:

+
+namespace {
+
+// Adds all NSMenuItems with an accelerator to the array.
+void addAcceleratorItemsToArray(NSMenu* menu, NSMutableArray* array) {

[Robert Sesek, 2014/02/11 16:19:48]

naming: AddAccelerator

[erikchen, 2014/02/11 19:42:27]

Done.

[Robert Sesek, 2014/02/11 22:18:16]

Argh, sorry I should have been more clear. I was just pointing out
capitalization. The name AddAcceleratorItemsToArray is good, same with below.

[erikchen, 2014/02/11 23:16:28]

Done.

+  for (NSMenuItem* item in [menu itemArray]) {
+    NSMenu* submenu = [item submenu];
+    if (submenu)
+        addAcceleratorItemsToArray(submenu, array);

[Robert Sesek, 2014/02/11 16:19:48]

nit: over-indented

[erikchen, 2014/02/11 19:42:27]

Done.

+
+    if ([item keyEquivalent].length > 0)
+      [array addObject:item];
+  }
+}
+
+// Returns the NSMenuItem that has the given keyEquivalent and modifiers, or
+// nil.
+NSMenuItem* menuContainsAccelerator(NSMenu* menu,

[Robert Sesek, 2014/02/11 16:19:48]

naming: MenuContains

[erikchen, 2014/02/11 19:42:27]

Done.

+                                    NSString* keyEquivalent,
+                                    NSUInteger modifiers) {
+  for (NSMenuItem* item in [menu itemArray]) {
+    NSMenu* submenu = [item submenu];
+    if (submenu) {
+      NSMenuItem* result =
+          menuContainsAccelerator(submenu, keyEquivalent, modifiers);
+      if (result)
+        return result;
+    }
+
+    if ([[item keyEquivalent] isEqual:keyEquivalent]) {
+      BOOL maskEqual =
+          (modifiers == item.keyEquivalentModifierMask) ||
+          ((modifiers & (~NSShiftKeyMask)) == item.keyEquivalentModifierMask);

[Robert Sesek, 2014/02/11 16:19:48]

You use [square brackets] above for keyEquivalent but dot.notation here. Be
consistent.

[erikchen, 2014/02/11 19:42:27]

changed keyEquivalent, keyEquivalentModifierMask, and submenu to use dot
notation, since the interfaces match those of properties. I've kept itemArray as
a method.

On 2014/02/11 16:19:48, rsesek wrote:

+      if (maskEqual)
+        return item;
+    }
+  }
+  return nil;
+}
+
+}  // Anonymous namespace.

[Robert Sesek, 2014/02/11 16:19:48]

"// namespace"

[erikchen, 2014/02/11 19:42:27]

Done.

+
+// Checks that each NSMenuItem in the main menu has a corresponding accelerator,
+// and the keyEquivalent/modifiers match.
+IN_PROC_BROWSER_TEST_F(AcceleratorsCocoaBrowsertest,
+                       MainMenuAcceleratorsInMapping) {
+  NSMenu* menu = [NSApp mainMenu];
+  NSMutableArray* array = [NSMutableArray array];
+  addAcceleratorItemsToArray(menu, array);
+
+  for (NSMenuItem* item in array) {
+    int command_id = item.tag;
+    AcceleratorsCocoa* keymap = AcceleratorsCocoa::GetInstance();
+    const ui::Accelerator* accelerator;
+
+    // If the tag is zero, then the NSMenuItem must use a custom selector.
+    // Check that the accelerator is present as an un-mapped accelerator.
+    if (command_id == 0) {
+      accelerator = keymap->GetAcceleratorForHotKey(
+          item.keyEquivalent, item.keyEquivalentModifierMask);
+
+      EXPECT_TRUE(accelerator);
+      return;
+    }
+
+    // If the tag isn't zero, then it must correspond to an IDC_* command.
+    accelerator = keymap->GetAcceleratorForCommand(command_id);
+    EXPECT_TRUE(accelerator);
+    if (!accelerator)
+      continue;
+
+    // Get the Cocoa keyEquivalent associated with the accelerator.
+    const ui::PlatformAcceleratorCocoa* platform_accelerator =
+      static_cast<const ui::PlatformAcceleratorCocoa*>(
+          accelerator->platform_accelerator());
+    NSString* cocoaKeyEquivalent = platform_accelerator->characters();

[Robert Sesek, 2014/02/11 16:19:48]

naming: under_scores

[erikchen, 2014/02/11 19:42:27]

Done.

+
+    // Check that the menu item's keyEquivalent matches the one from the
+    // cocoa accelerator map.
+    EXPECT_NSEQ(cocoaKeyEquivalent, item.keyEquivalent);
+
+    // Check that the menu item's modifier mask matches the one stored in the
+    // accelerator. A mask that include NSShiftKeyMask may not include the
+    // relevant bit (the information is reflected in the keyEquivalent of the
+    // NSMenuItem).
+    NSUInteger mask = platform_accelerator->modifier_mask();
+    BOOL maskEqual =
+        (mask == item.keyEquivalentModifierMask) ||
+        ((mask & (~NSShiftKeyMask)) == item.keyEquivalentModifierMask);
+    EXPECT_TRUE(maskEqual);
+  }
+}
+
+// Check that each accelerator with a command_id has an associated NSMenuItem
+// in the main menu. If the selector is commandDispatch:, then the tag must
+// match the command_id.
+IN_PROC_BROWSER_TEST_F(AcceleratorsCocoaBrowsertest,
+                       MappingAcceleratorsInMainMenu) {
+  AcceleratorsCocoa* keymap = AcceleratorsCocoa::GetInstance();
+  for (AcceleratorsCocoa::AcceleratorMap::iterator it =
+           keymap->accelerators_.begin();
+       it != keymap->accelerators_.end();
+       ++it) {
+    const ui::PlatformAcceleratorCocoa* platform_accelerator =
+        static_cast<const ui::PlatformAcceleratorCocoa*>(
+            it->second.platform_accelerator());
+
+    // Check that there exists a corresponding NSMenuItem.
+    NSMenuItem* item =
+        menuContainsAccelerator([NSApp mainMenu],
+                                platform_accelerator->characters(),
+                                platform_accelerator->modifier_mask());
+    EXPECT_TRUE(item);
+
+    // If the menu uses a commandDispatch:, the tag must match the command id!
+    if (item.action == @selector(commandDispatch:))
+      EXPECT_EQ(item.tag, it->first);
+  }
+}
+

[Robert Sesek, 2014/02/11 16:19:48]

nit: extra blank line

[erikchen, 2014/02/11 19:42:27]

Done.