mac: Pass useDefaultAccelerator to getAcceleratorForCommandId

atom/browser/api/atom_api_menu.cc

@@ -53,11 +53,14 @@ bool Menu::IsCommandIdVisible(int command_id) const {
   return is_visible_.Run(command_id);
 }
 
-bool Menu::GetAcceleratorForCommandId(int command_id,
+bool Menu::GetAcceleratorForCommandIdWithParams(
-                                      ui::Accelerator* accelerator) {
+    int command_id,
+    bool use_default_accelerator,
+    ui::Accelerator* accelerator) const {
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
-  v8::Local<v8::Value> val = get_accelerator_.Run(command_id);
+  v8::Local<v8::Value> val = get_accelerator_.Run(
+      command_id, use_default_accelerator);
   return mate::ConvertFromV8(isolate(), val, accelerator);
 }
 

atom/browser/api/atom_api_menu.h

@@ -46,8 +46,10 @@ class Menu : public mate::TrackableObject<Menu>,
   bool IsCommandIdChecked(int command_id) const override;
   bool IsCommandIdEnabled(int command_id) const override;
   bool IsCommandIdVisible(int command_id) const override;
-  bool GetAcceleratorForCommandId(int command_id,
+  bool GetAcceleratorForCommandIdWithParams(
-                                  ui::Accelerator* accelerator) override;
+      int command_id,
+      bool use_default_accelerator,
+      ui::Accelerator* accelerator) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
@@ -89,7 +91,7 @@ class Menu : public mate::TrackableObject<Menu>,
   base::Callback<bool(int)> is_checked_;
   base::Callback<bool(int)> is_enabled_;
   base::Callback<bool(int)> is_visible_;
-  base::Callback<v8::Local<v8::Value>(int)> get_accelerator_;
+  base::Callback<v8::Local<v8::Value>(int, bool)> get_accelerator_;
   base::Callback<void(v8::Local<v8::Value>, int)> execute_command_;
   base::Callback<void()> menu_will_show_;
 

atom/browser/api/atom_api_menu_mac.mm

@@ -30,7 +30,8 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) {
     return;
 
   base::scoped_nsobject<AtomMenuController> menu_controller(
-      [[AtomMenuController alloc] initWithModel:model_.get()]);
+      [[AtomMenuController alloc] initWithModel:model_.get()
+                          useDefaultAccelerator:NO]);
   NSMenu* menu = [menu_controller menu];
   NSView* view = web_contents->GetView()->GetNativeView();
 
@@ -74,7 +75,8 @@ void MenuMac::PopupAt(Window* window, int x, int y, int positioning_item) {
 void Menu::SetApplicationMenu(Menu* base_menu) {
   MenuMac* menu = static_cast<MenuMac*>(base_menu);
   base::scoped_nsobject<AtomMenuController> menu_controller(
-      [[AtomMenuController alloc] initWithModel:menu->model_.get()]);
+      [[AtomMenuController alloc] initWithModel:menu->model_.get()
+                          useDefaultAccelerator:YES]);
   [NSApp setMainMenu:[menu_controller menu]];
 
   // Ensure the menu_controller_ is destroyed after main menu is set.

atom/browser/browser.h

@@ -25,16 +25,13 @@ class DictionaryValue;
 class FilePath;
 }
 
-namespace ui {
-class MenuModel;
-}
-
 namespace gfx {
 class Image;
 }
 
 namespace atom {
 
+class AtomMenuModel;
 class LoginHandler;
 
 // This class is used for control application-wide operations.
@@ -130,7 +127,7 @@ class Browser : public WindowListObserver {
   void DockShow();
 
   // Set docks' menu.
-  void DockSetMenu(ui::MenuModel* model);
+  void DockSetMenu(AtomMenuModel* model);
 
   // Set docks' icon.
   void DockSetIcon(const gfx::Image& image);

atom/browser/browser_mac.mm

@@ -216,7 +216,7 @@ void Browser::DockShow() {
   }
 }
 
-void Browser::DockSetMenu(ui::MenuModel* model) {
+void Browser::DockSetMenu(AtomMenuModel* model) {
   AtomApplicationDelegate* delegate = (AtomApplicationDelegate*)[NSApp delegate];
   [delegate setApplicationDockMenu:model];
 }

atom/browser/mac/atom_application_delegate.h

@@ -11,9 +11,7 @@
   base::scoped_nsobject<AtomMenuController> menu_controller_;
 }
 
-- (id)init;
-
 // Sets the menu that will be returned in "applicationDockMenu:".
-- (void)setApplicationDockMenu:(ui::MenuModel*)model;
+- (void)setApplicationDockMenu:(atom::AtomMenuModel*)model;
 
 @end

atom/browser/mac/atom_application_delegate.mm

@@ -12,14 +12,9 @@
 
 @implementation AtomApplicationDelegate
 
-- (id)init {
+- (void)setApplicationDockMenu:(atom::AtomMenuModel*)model {
-  self = [super init];
+  menu_controller_.reset([[AtomMenuController alloc] initWithModel:model
-  menu_controller_.reset([[AtomMenuController alloc] init]);
+                                             useDefaultAccelerator:NO]);
-  return self;
-}
-
-- (void)setApplicationDockMenu:(ui::MenuModel*)model {
-  [menu_controller_ populateWithModel:model];
 }
 
 - (void)applicationWillFinishLaunching:(NSNotification*)notify {
@@ -34,7 +29,10 @@
 }
 
 - (NSMenu*)applicationDockMenu:(NSApplication*)sender {
+  if (menu_controller_)
     return [menu_controller_ menu];
+  else
+    return nil;
 }
 
 - (BOOL)application:(NSApplication*)sender

atom/browser/ui/atom_menu_model.cc

@@ -29,6 +29,17 @@ base::string16 AtomMenuModel::GetRoleAt(int index) {
     return base::string16();
 }
 
+bool AtomMenuModel::GetAcceleratorAtWithParams(
+    int index,
+    bool use_default_accelerator,
+    ui::Accelerator* accelerator) const {
+  if (delegate_) {
+    return delegate_->GetAcceleratorForCommandIdWithParams(
+        GetCommandIdAt(index), use_default_accelerator, accelerator);
+  }
+  return false;
+}
+
 void AtomMenuModel::MenuClosed() {
   ui::SimpleMenuModel::MenuClosed();
   FOR_EACH_OBSERVER(Observer, observers_, MenuClosed());

atom/browser/ui/atom_menu_model.h

@@ -17,6 +17,19 @@ class AtomMenuModel : public ui::SimpleMenuModel {
   class Delegate : public ui::SimpleMenuModel::Delegate {
    public:
     virtual ~Delegate() {}
+
+    virtual bool GetAcceleratorForCommandIdWithParams(
+        int command_id,
+        bool use_default_accelerator,
+        ui::Accelerator* accelerator) const = 0;
+
+   private:
+    // ui::SimpleMenuModel::Delegate:
+    bool GetAcceleratorForCommandId(int command_id,
+                                    ui::Accelerator* accelerator) {
+      return GetAcceleratorForCommandIdWithParams(
+          command_id, true, accelerator);
+    }
   };
 
   class Observer {
@@ -35,6 +48,9 @@ class AtomMenuModel : public ui::SimpleMenuModel {
 
   void SetRole(int index, const base::string16& role);
   base::string16 GetRoleAt(int index);
+  bool GetAcceleratorAtWithParams(int index,
+                                  bool use_default_accelerator,
+                                  ui::Accelerator* accelerator) const;
 
   // ui::SimpleMenuModel:
   void MenuClosed() override;

atom/browser/ui/cocoa/atom_menu_controller.h

@@ -11,8 +11,8 @@
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/string16.h"
 
-namespace ui {
+namespace atom {
-class MenuModel;
+class AtomMenuModel;
 }
 
 // A controller for the cross-platform menu model. The menu that's created
@@ -23,24 +23,20 @@ class MenuModel;
 // as it only maintains weak references.
 @interface AtomMenuController : NSObject<NSMenuDelegate> {
  @protected
-  ui::MenuModel* model_;  // weak
+  atom::AtomMenuModel* model_;  // weak
   base::scoped_nsobject<NSMenu> menu_;
   BOOL isMenuOpen_;
+  BOOL useDefaultAccelerator_;
 }
 
-@property(nonatomic, assign) ui::MenuModel* model;
+@property(nonatomic, assign) atom::AtomMenuModel* model;
-
-// NIB-based initializer. This does not create a menu. Clients can set the
-// properties of the object and the menu will be created upon the first call to
-// |-menu|. Note that the menu will be immutable after creation.
-- (id)init;
 
 // Builds a NSMenu from the pre-built model (must not be nil). Changes made
 // to the contents of the model after calling this will not be noticed.
-- (id)initWithModel:(ui::MenuModel*)model;
+- (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use;
 
 // Populate current NSMenu with |model|.
-- (void)populateWithModel:(ui::MenuModel*)model;
+- (void)populateWithModel:(atom::AtomMenuModel*)model;
 
 // Programmatically close the constructed menu.
 - (void)cancel;

atom/browser/ui/cocoa/atom_menu_controller.mm

@@ -48,15 +48,11 @@ Role kRolesMap[] = {
 
 @synthesize model = model_;
 
-- (id)init {
+- (id)initWithModel:(atom::AtomMenuModel*)model useDefaultAccelerator:(BOOL)use {
-  if ((self = [super init]))
-    [self menu];
-  return self;
-}
-
-- (id)initWithModel:(ui::MenuModel*)model {
   if ((self = [super init])) {
     model_ = model;
+    isMenuOpen_ = NO;
+    useDefaultAccelerator_ = use;
     [self menu];
   }
   return self;
@@ -73,7 +69,7 @@ Role kRolesMap[] = {
   [super dealloc];
 }
 
-- (void)populateWithModel:(ui::MenuModel*)model {
+- (void)populateWithModel:(atom::AtomMenuModel*)model {
   if (!menu_)
     return;
 
@@ -82,7 +78,7 @@ Role kRolesMap[] = {
 
   const int count = model->GetItemCount();
   for (int index = 0; index < count; index++) {
-    if (model->GetTypeAt(index) == ui::MenuModel::TYPE_SEPARATOR)
+    if (model->GetTypeAt(index) == atom::AtomMenuModel::TYPE_SEPARATOR)
       [self addSeparatorToMenu:menu_ atIndex:index];
     else
       [self addItemToMenu:menu_ atIndex:index fromModel:model];
@@ -99,12 +95,12 @@ Role kRolesMap[] = {
 
 // Creates a NSMenu from the given model. If the model has submenus, this can
 // be invoked recursively.
-- (NSMenu*)menuFromModel:(ui::MenuModel*)model {
+- (NSMenu*)menuFromModel:(atom::AtomMenuModel*)model {
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@""] autorelease];
 
   const int count = model->GetItemCount();
   for (int index = 0; index < count; index++) {
-    if (model->GetTypeAt(index) == ui::MenuModel::TYPE_SEPARATOR)
+    if (model->GetTypeAt(index) == atom::AtomMenuModel::TYPE_SEPARATOR)
       [self addSeparatorToMenu:menu atIndex:index];
     else
       [self addItemToMenu:menu atIndex:index fromModel:model];
@@ -126,9 +122,7 @@ Role kRolesMap[] = {
 // associated with the entry in the model identified by |modelIndex|.
 - (void)addItemToMenu:(NSMenu*)menu
               atIndex:(NSInteger)index
-            fromModel:(ui::MenuModel*)ui_model {
+            fromModel:(atom::AtomMenuModel*)model {
-  atom::AtomMenuModel* model = static_cast<atom::AtomMenuModel*>(ui_model);
-
   base::string16 label16 = model->GetLabelAt(index);
   NSString* label = l10n_util::FixUpWindowsStyleLabel(label16);
   base::scoped_nsobject<NSMenuItem> item(
@@ -141,12 +135,13 @@ Role kRolesMap[] = {
   if (model->GetIconAt(index, &icon) && !icon.IsEmpty())
     [item setImage:icon.ToNSImage()];
 
-  ui::MenuModel::ItemType type = model->GetTypeAt(index);
+  atom::AtomMenuModel::ItemType type = model->GetTypeAt(index);
-  if (type == ui::MenuModel::TYPE_SUBMENU) {
+  if (type == atom::AtomMenuModel::TYPE_SUBMENU) {
     // Recursively build a submenu from the sub-model at this index.
     [item setTarget:nil];
     [item setAction:nil];
-    ui::MenuModel* submenuModel = model->GetSubmenuModelAt(index);
+    atom::AtomMenuModel* submenuModel = static_cast<atom::AtomMenuModel*>(
+        model->GetSubmenuModelAt(index));
     NSMenu* submenu = [self menuFromModel:submenuModel];
     [submenu setTitle:[item title]];
     [item setSubmenu:submenu];
@@ -170,7 +165,8 @@ Role kRolesMap[] = {
     NSValue* modelObject = [NSValue valueWithPointer:model];
     [item setRepresentedObject:modelObject];  // Retains |modelObject|.
     ui::Accelerator accelerator;
-    if (model->GetAcceleratorAt(index, &accelerator)) {
+    if (model->GetAcceleratorAtWithParams(
+            index, useDefaultAccelerator_, &accelerator)) {
       const ui::PlatformAcceleratorCocoa* platformAccelerator =
           static_cast<const ui::PlatformAcceleratorCocoa*>(
               accelerator.platform_accelerator());
@@ -206,8 +202,8 @@ Role kRolesMap[] = {
     return NO;
 
   NSInteger modelIndex = [item tag];
-  ui::MenuModel* model =
+  atom::AtomMenuModel* model =
-      static_cast<ui::MenuModel*>(
+      static_cast<atom::AtomMenuModel*>(
           [[(id)item representedObject] pointerValue]);
   DCHECK(model);
   if (model) {
@@ -234,8 +230,8 @@ Role kRolesMap[] = {
 // item chosen.
 - (void)itemSelected:(id)sender {
   NSInteger modelIndex = [sender tag];
-  ui::MenuModel* model =
+  atom::AtomMenuModel* model =
-      static_cast<ui::MenuModel*>(
+      static_cast<atom::AtomMenuModel*>(
           [[sender representedObject] pointerValue]);
   DCHECK(model);
   if (model) {

atom/browser/ui/tray_icon.cc

@@ -27,7 +27,7 @@ void TrayIcon::DisplayBalloon(ImageType icon,
 }
 
 void TrayIcon::PopUpContextMenu(const gfx::Point& pos,
-                                ui::SimpleMenuModel* menu_model) {
+                                AtomMenuModel* menu_model) {
 }
 
 gfx::Rect TrayIcon::GetBounds() {

atom/browser/ui/tray_icon.h

@@ -8,9 +8,9 @@
 #include <string>
 #include <vector>
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "atom/browser/ui/tray_icon_observer.h"
 #include "base/observer_list.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace atom {
@@ -55,10 +55,10 @@ class TrayIcon {
 
   // Popups the menu.
   virtual void PopUpContextMenu(const gfx::Point& pos,
-                                ui::SimpleMenuModel* menu_model);
+                                AtomMenuModel* menu_model);
 
   // Set the context menu for this icon.
-  virtual void SetContextMenu(ui::SimpleMenuModel* menu_model) = 0;
+  virtual void SetContextMenu(AtomMenuModel* menu_model) = 0;
 
   // Returns the bounds of tray icon.
   virtual gfx::Rect GetBounds();

atom/browser/ui/tray_icon_cocoa.h

@@ -9,7 +9,6 @@
 
 #include <string>
 
-#include "atom/browser/ui/atom_menu_model.h"
 #include "atom/browser/ui/tray_icon.h"
 #include "base/mac/scoped_nsobject.h"
 
@@ -30,8 +29,8 @@ class TrayIconCocoa : public TrayIcon,
   void SetTitle(const std::string& title) override;
   void SetHighlightMode(bool highlight) override;
   void PopUpContextMenu(const gfx::Point& pos,
-                        ui::SimpleMenuModel* menu_model) override;
+                        AtomMenuModel* menu_model) override;
-  void SetContextMenu(ui::SimpleMenuModel* menu_model) override;
+  void SetContextMenu(AtomMenuModel* menu_model) override;
   gfx::Rect GetBounds() override;
 
  protected:

atom/browser/ui/tray_icon_cocoa.mm

@@ -249,11 +249,12 @@ const CGFloat kVerticalTitleMargin = 2;
   [self setNeedsDisplay:YES];
 }
 
-- (void)popUpContextMenu:(ui::SimpleMenuModel*)menu_model {
+- (void)popUpContextMenu:(atom::AtomMenuModel*)menu_model {
   // Show a custom menu.
   if (menu_model) {
     base::scoped_nsobject<AtomMenuController> menuController(
-        [[AtomMenuController alloc] initWithModel:menu_model]);
+        [[AtomMenuController alloc] initWithModel:menu_model
+                            useDefaultAccelerator:NO]);
     forceHighlight_ = YES;  // Should highlight when showing menu.
     [self setNeedsDisplay:YES];
     [statusItem_ popUpStatusItemMenu:[menuController menu]];
@@ -365,18 +366,19 @@ void TrayIconCocoa::SetHighlightMode(bool highlight) {
 }
 
 void TrayIconCocoa::PopUpContextMenu(const gfx::Point& pos,
-                                     ui::SimpleMenuModel* menu_model) {
+                                     AtomMenuModel* menu_model) {
   [status_item_view_ popUpContextMenu:menu_model];
 }
 
-void TrayIconCocoa::SetContextMenu(ui::SimpleMenuModel* menu_model) {
+void TrayIconCocoa::SetContextMenu(AtomMenuModel* menu_model) {
   // Substribe to MenuClosed event.
   if (menu_model_)
     menu_model_->RemoveObserver(this);
-  static_cast<AtomMenuModel*>(menu_model)->AddObserver(this);
+  menu_model->AddObserver(this);
 
   // Create native menu.
-  menu_.reset([[AtomMenuController alloc] initWithModel:menu_model]);
+  menu_.reset([[AtomMenuController alloc] initWithModel:menu_model
+                                  useDefaultAccelerator:NO]);
   [status_item_view_ setMenuController:menu_.get()];
 }
 

lib/browser/api/menu-item.js

@@ -31,7 +31,7 @@ const MenuItem = function (options) {
 
   this.overrideReadOnlyProperty('type', 'normal')
   this.overrideReadOnlyProperty('role')
-  this.overrideReadOnlyProperty('accelerator', roles.getDefaultAccelerator(this.role))
+  this.overrideReadOnlyProperty('accelerator')
   this.overrideReadOnlyProperty('icon')
   this.overrideReadOnlyProperty('submenu')
 
@@ -66,6 +66,10 @@ const MenuItem = function (options) {
 
 MenuItem.types = ['normal', 'separator', 'submenu', 'checkbox', 'radio']
 
+MenuItem.prototype.getDefaultRoleAccelerator = function () {
+  return roles.getDefaultAccelerator(this.role)
+}
+
 MenuItem.prototype.overrideProperty = function (name, defaultValue) {
   if (defaultValue == null) {
     defaultValue = null

lib/browser/api/menu.js

@@ -106,9 +106,11 @@ Menu.prototype._init = function () {
       var command = this.commandsMap[commandId]
       return command != null ? command.visible : undefined
     },
-    getAcceleratorForCommandId: (commandId) => {
+    getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => {
-      var command = this.commandsMap[commandId]
+      const command = this.commandsMap[commandId]
-      return command != null ? command.accelerator : undefined
+      if (command == null) return
+      if (command.accelerator != null) return command.accelerator
+      if (useDefaultAccelerator) return command.getDefaultRoleAccelerator()
     },
     getIconForCommandId: (commandId) => {
       var command = this.commandsMap[commandId]