[Mac] Give the Wrench menu keyboard shortcuts.

[Mac] Give the Wrench menu keyboard shortcuts.

Note that these key equivalents are for display only and do not get fired. Commands are routed through the main menu/CommandUpdater.

BUG=45098
TEST=Open Wrench menu, see keyboard shortcuts.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50452

[Robert Sesek, 2010-06-21 15:23:57]

It'd be cool if this mapping of command_ids to key equivalents were globalized
in code somewhere and then attached to MainMenu.xib at runtime, but our current
architecture doesn't lend itself to that.

[pink (ping after 24hrs), 2010-06-21 15:47:17]

http://codereview.chromium.org/2800019/diff/1/3
File app/menus/accelerator_cocoa.h (right):

http://codereview.chromium.org/2800019/diff/1/3#newcode22
app/menus/accelerator_cocoa.h:22: : characters_([key_code copy]) {
as of 15 minutes ago, this will now warn that you're not explicitly calling the
base class ctor. In fact, you don't need to set key_code_ because it's getting
set (to the same value) by the default base class' ctor.

http://codereview.chromium.org/2800019/diff/1/3#newcode27
app/menus/accelerator_cocoa.h:27: AcceleratorCocoa(const AcceleratorCocoa&
accelerator) {
can you call through to the base class to copy its members rather than
enumerating them all here?

http://codereview.chromium.org/2800019/diff/1/3#newcode33
app/menus/accelerator_cocoa.h:33: AcceleratorCocoa() {
I don't understand why you need this, the base class does all this for you.

http://codereview.chromium.org/2800019/diff/1/3#newcode43
app/menus/accelerator_cocoa.h:43: modifiers_ = accelerator.modifiers_;
same as with copy ctor above.

http://codereview.chromium.org/2800019/diff/1/5
File chrome/browser/cocoa/toolbar_controller.mm (right):

http://codereview.chromium.org/2800019/diff/1/5#newcode115
chrome/browser/cocoa/toolbar_controller.mm:115: *accelerator =
menus::AcceleratorCocoa(@"t", NSCommandKeyMask);
can you put this into a table somewhere so it's at least table-driven and not a
huge switch statement? That seems like it would be easier to modify/maintain
moving forward.

[Robert Sesek, 2010-06-21 18:45:43]

All comments addressed.

[pink (ping after 24hrs), 2010-06-21 19:21:56]

LGTM with minor nits.

http://codereview.chromium.org/2800019/diff/6001/7002
File app/menus/accelerator_cocoa.h (right):

http://codereview.chromium.org/2800019/diff/6001/7002#newcode31
app/menus/accelerator_cocoa.h:31: AcceleratorCocoa() : Accelerator() {}
not sure this is necessary, but can't hurt.

http://codereview.chromium.org/2800019/diff/6001/7002#newcode42
app/menus/accelerator_cocoa.h:42: bool operator ==(const AcceleratorCocoa& rhs)
const {
no space after "operator"?

http://codereview.chromium.org/2800019/diff/6001/7003
File chrome/browser/cocoa/accelerators_cocoa.h (right):

http://codereview.chromium.org/2800019/diff/6001/7003#newcode35
chrome/browser/cocoa/accelerators_cocoa.h:35: AcceleratorCocoaMap accelerators_;
DISALLOW_COPY_AND_ASSIGN(AcceleratorsCocoa)?