Merge pull request #6385 from electron/only-use-role-accelerator-in-app-menu

Only use default role accelerator in app menu

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.
@@ -138,7 +135,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

@@ -234,7 +234,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 {
-  return [menu_controller_ menu];
+  if (menu_controller_)
+    return [menu_controller_ menu];
+  else
+    return nil;
 }
 
 - (BOOL)application:(NSApplication*)sender

atom/browser/native_window.cc

@@ -290,7 +290,7 @@ bool NativeWindow::IsDocumentEdited() {
 void NativeWindow::SetFocusable(bool focusable) {
 }
 
-void NativeWindow::SetMenu(ui::MenuModel* menu) {
+void NativeWindow::SetMenu(AtomMenuModel* menu) {
 }
 
 bool NativeWindow::HasModalDialog() {

atom/browser/native_window.h

@@ -11,6 +11,7 @@
 
 #include "atom/browser/native_window_observer.h"
 #include "atom/browser/ui/accelerator_util.h"
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/cancelable_callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -43,10 +44,6 @@ namespace mate {
 class Dictionary;
 }
 
-namespace ui {
-class MenuModel;
-}
-
 namespace atom {
 
 struct DraggableRegion;
@@ -157,7 +154,7 @@ class NativeWindow : public base::SupportsUserData,
   virtual void SetIgnoreMouseEvents(bool ignore) = 0;
   virtual void SetContentProtection(bool enable) = 0;
   virtual void SetFocusable(bool focusable);
-  virtual void SetMenu(ui::MenuModel* menu);
+  virtual void SetMenu(AtomMenuModel* menu);
   virtual bool HasModalDialog();
   virtual void SetParentWindow(NativeWindow* parent);
   virtual gfx::NativeWindow GetNativeWindow() = 0;

atom/browser/native_window_views.cc

@@ -748,7 +748,7 @@ void NativeWindowViews::SetFocusable(bool focusable) {
 #endif
 }
 
-void NativeWindowViews::SetMenu(ui::MenuModel* menu_model) {
+void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) {
   if (menu_model == nullptr) {
     // Remove accelerators
     accelerator_table_.clear();
@@ -1182,7 +1182,7 @@ bool NativeWindowViews::AcceleratorPressed(const ui::Accelerator& accelerator) {
       &accelerator_table_, accelerator);
 }
 
-void NativeWindowViews::RegisterAccelerators(ui::MenuModel* menu_model) {
+void NativeWindowViews::RegisterAccelerators(AtomMenuModel* menu_model) {
   // Clear previous accelerators.
   views::FocusManager* focus_manager = GetFocusManager();
   accelerator_table_.clear();

atom/browser/native_window_views.h

@@ -98,7 +98,7 @@ class NativeWindowViews : public NativeWindow,
   void SetIgnoreMouseEvents(bool ignore) override;
   void SetContentProtection(bool enable) override;
   void SetFocusable(bool focusable) override;
-  void SetMenu(ui::MenuModel* menu_model) override;
+  void SetMenu(AtomMenuModel* menu_model) override;
   void SetParentWindow(NativeWindow* parent) override;
   gfx::NativeWindow GetNativeWindow() override;
   void SetOverlayIcon(const gfx::Image& overlay,
@@ -176,7 +176,7 @@ class NativeWindowViews : public NativeWindow,
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
 
   // Register accelerators supported by the menu model.
-  void RegisterAccelerators(ui::MenuModel* menu_model);
+  void RegisterAccelerators(AtomMenuModel* menu_model);
 
   // Returns the restore state for the window.
   ui::WindowShowState GetRestoredState();

atom/browser/ui/accelerator_util.cc

@@ -14,7 +14,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "ui/base/models/simple_menu_model.h"
 
 namespace accelerator_util {
 
@@ -69,16 +68,17 @@ bool StringToAccelerator(const std::string& shortcut,
   return true;
 }
 
-void GenerateAcceleratorTable(AcceleratorTable* table, ui::MenuModel* model) {
+void GenerateAcceleratorTable(AcceleratorTable* table,
+                              atom::AtomMenuModel* model) {
   int count = model->GetItemCount();
   for (int i = 0; i < count; ++i) {
-    ui::MenuModel::ItemType type = model->GetTypeAt(i);
+    atom::AtomMenuModel::ItemType type = model->GetTypeAt(i);
-    if (type == ui::MenuModel::TYPE_SUBMENU) {
+    if (type == atom::AtomMenuModel::TYPE_SUBMENU) {
-      ui::MenuModel* submodel = model->GetSubmenuModelAt(i);
+      auto submodel = model->GetSubmenuModelAt(i);
       GenerateAcceleratorTable(table, submodel);
     } else {
       ui::Accelerator accelerator;
-      if (model->GetAcceleratorAt(i, &accelerator)) {
+      if (model->GetAcceleratorAtWithParams(i, true, &accelerator)) {
         MenuItem item = { i, model };
         (*table)[accelerator] = item;
       }

atom/browser/ui/accelerator_util.h

@@ -8,15 +8,12 @@
 #include <map>
 #include <string>
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "ui/base/accelerators/accelerator.h"
 
-namespace ui {
-class MenuModel;
-}
-
 namespace accelerator_util {
 
-typedef struct { int position; ui::MenuModel* model; } MenuItem;
+typedef struct { int position; atom::AtomMenuModel* model; } MenuItem;
 typedef std::map<ui::Accelerator, MenuItem> AcceleratorTable;
 
 // Parse a string as an accelerator.
@@ -27,7 +24,8 @@ bool StringToAccelerator(const std::string& description,
 void SetPlatformAccelerator(ui::Accelerator* accelerator);
 
 // Generate a table that contains memu model's accelerators and command ids.
-void GenerateAcceleratorTable(AcceleratorTable* table, ui::MenuModel* model);
+void GenerateAcceleratorTable(AcceleratorTable* table,
+                              atom::AtomMenuModel* model);
 
 // Trigger command from the accelerators table.
 bool TriggerAcceleratorTableCommand(AcceleratorTable* table,

atom/browser/ui/atom_menu_model.cc

@@ -29,9 +29,25 @@ 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());
 }
 
+AtomMenuModel* AtomMenuModel::GetSubmenuModelAt(int index) {
+  return static_cast<AtomMenuModel*>(
+      ui::SimpleMenuModel::GetSubmenuModelAt(index));
+}
+
 }  // namespace atom

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, false, accelerator);
+    }
   };
 
   class Observer {
@@ -35,10 +48,16 @@ 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;
 
+  using SimpleMenuModel::GetSubmenuModelAt;
+  AtomMenuModel* GetSubmenuModelAt(int index);
+
  private:
   Delegate* delegate_;  // weak ref.
 

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()];
 }
 

atom/browser/ui/tray_icon_gtk.cc

@@ -50,7 +50,7 @@ void TrayIconGtk::SetToolTip(const std::string& tool_tip) {
   icon_->SetToolTip(base::UTF8ToUTF16(tool_tip));
 }
 
-void TrayIconGtk::SetContextMenu(ui::SimpleMenuModel* menu_model) {
+void TrayIconGtk::SetContextMenu(AtomMenuModel* menu_model) {
   icon_->UpdatePlatformContextMenu(menu_model);
 }
 

atom/browser/ui/tray_icon_gtk.h

@@ -25,7 +25,7 @@ class TrayIconGtk : public TrayIcon,
   // TrayIcon:
   void SetImage(const gfx::Image& image) override;
   void SetToolTip(const std::string& tool_tip) override;
-  void SetContextMenu(ui::SimpleMenuModel* menu_model) override;
+  void SetContextMenu(AtomMenuModel* menu_model) override;
 
  private:
   // views::StatusIconLinux::Delegate:

atom/browser/ui/views/global_menu_bar_x11.cc

@@ -16,6 +16,7 @@
 #include <glib-object.h>
 
 #include "atom/browser/native_window_views.h"
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -23,7 +24,6 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/accelerators/menu_label_accelerator_util_linux.h"
-#include "ui/base/models/menu_model.h"
 #include "ui/events/keycodes/keyboard_code_conversion_x.h"
 
 // libdbusmenu-glib types
@@ -141,8 +141,8 @@ void EnsureMethodsLoaded() {
       dlsym(dbusmenu_lib, "dbusmenu_server_set_root"));
 }
 
-ui::MenuModel* ModelForMenuItem(DbusmenuMenuitem* item) {
+AtomMenuModel* ModelForMenuItem(DbusmenuMenuitem* item) {
-  return reinterpret_cast<ui::MenuModel*>(
+  return reinterpret_cast<AtomMenuModel*>(
       g_object_get_data(G_OBJECT(item), "model"));
 }
 
@@ -188,7 +188,7 @@ std::string GlobalMenuBarX11::GetPathForWindow(gfx::AcceleratedWidget xid) {
   return base::StringPrintf("/com/canonical/menu/%lX", xid);
 }
 
-void GlobalMenuBarX11::SetMenu(ui::MenuModel* menu_model) {
+void GlobalMenuBarX11::SetMenu(AtomMenuModel* menu_model) {
   if (!IsServerStarted())
     return;
 
@@ -218,14 +218,14 @@ void GlobalMenuBarX11::OnWindowUnmapped() {
   GlobalMenuBarRegistrarX11::GetInstance()->OnWindowUnmapped(xid_);
 }
 
-void GlobalMenuBarX11::BuildMenuFromModel(ui::MenuModel* model,
+void GlobalMenuBarX11::BuildMenuFromModel(AtomMenuModel* model,
                                           DbusmenuMenuitem* parent) {
   for (int i = 0; i < model->GetItemCount(); ++i) {
     DbusmenuMenuitem* item = menuitem_new();
     menuitem_property_set_bool(item, kPropertyVisible, model->IsVisibleAt(i));
 
-    ui::MenuModel::ItemType type = model->GetTypeAt(i);
+    AtomMenuModel::ItemType type = model->GetTypeAt(i);
-    if (type == ui::MenuModel::TYPE_SEPARATOR) {
+    if (type == AtomMenuModel::TYPE_SEPARATOR) {
       menuitem_property_set(item, kPropertyType, kTypeSeparator);
     } else {
       std::string label = ui::ConvertAcceleratorsFromWindowsStyle(
@@ -236,22 +236,22 @@ void GlobalMenuBarX11::BuildMenuFromModel(ui::MenuModel* model,
       g_object_set_data(G_OBJECT(item), "model", model);
       SetMenuItemID(item, i);
 
-      if (type == ui::MenuModel::TYPE_SUBMENU) {
+      if (type == AtomMenuModel::TYPE_SUBMENU) {
         menuitem_property_set(item, kPropertyChildrenDisplay, kDisplaySubmenu);
         g_signal_connect(item, "about-to-show",
                          G_CALLBACK(OnSubMenuShowThunk), this);
       } else {
         ui::Accelerator accelerator;
-        if (model->GetAcceleratorAt(i, &accelerator))
+        if (model->GetAcceleratorAtWithParams(i, true, &accelerator))
           RegisterAccelerator(item, accelerator);
 
         g_signal_connect(item, "item-activated",
                          G_CALLBACK(OnItemActivatedThunk), this);
 
-        if (type == ui::MenuModel::TYPE_CHECK ||
+        if (type == AtomMenuModel::TYPE_CHECK ||
-            type == ui::MenuModel::TYPE_RADIO) {
+            type == AtomMenuModel::TYPE_RADIO) {
           menuitem_property_set(item, kPropertyToggleType,
-              type == ui::MenuModel::TYPE_CHECK ? kToggleCheck : kToggleRadio);
+              type == AtomMenuModel::TYPE_CHECK ? kToggleCheck : kToggleRadio);
           menuitem_property_set_int(item, kPropertyToggleState,
               model->IsItemCheckedAt(i));
         }
@@ -296,14 +296,14 @@ void GlobalMenuBarX11::RegisterAccelerator(DbusmenuMenuitem* item,
 void GlobalMenuBarX11::OnItemActivated(DbusmenuMenuitem* item,
                                        unsigned int timestamp) {
   int id;
-  ui::MenuModel* model = ModelForMenuItem(item);
+  AtomMenuModel* model = ModelForMenuItem(item);
   if (model && GetMenuItemID(item, &id))
     model->ActivatedAt(id, 0);
 }
 
 void GlobalMenuBarX11::OnSubMenuShow(DbusmenuMenuitem* item) {
   int id;
-  ui::MenuModel* model = ModelForMenuItem(item);
+  AtomMenuModel* model = ModelForMenuItem(item);
   if (!model || !GetMenuItemID(item, &id))
     return;
 

atom/browser/ui/views/global_menu_bar_x11.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/macros.h"
 #include "base/compiler_specific.h"
 #include "ui/base/glib/glib_signal.h"
@@ -17,7 +18,6 @@ typedef struct _DbusmenuServer   DbusmenuServer;
 
 namespace ui {
 class Accelerator;
-class MenuModel;
 }
 
 namespace atom {
@@ -43,7 +43,7 @@ class GlobalMenuBarX11 {
   // Creates the object path for DbusmenuServer which is attached to |xid|.
   static std::string GetPathForWindow(gfx::AcceleratedWidget xid);
 
-  void SetMenu(ui::MenuModel* menu_model);
+  void SetMenu(AtomMenuModel* menu_model);
   bool IsServerStarted() const;
 
   // Called by NativeWindow when it show/hides.
@@ -55,7 +55,7 @@ class GlobalMenuBarX11 {
   void InitServer(gfx::AcceleratedWidget xid);
 
   // Create a menu from menu model.
-  void BuildMenuFromModel(ui::MenuModel* model, DbusmenuMenuitem* parent);
+  void BuildMenuFromModel(AtomMenuModel* model, DbusmenuMenuitem* parent);
 
   // Sets the accelerator for |item|.
   void RegisterAccelerator(DbusmenuMenuitem* item,

atom/browser/ui/views/menu_bar.cc

@@ -58,7 +58,7 @@ MenuBar::MenuBar()
 MenuBar::~MenuBar() {
 }
 
-void MenuBar::SetMenu(ui::MenuModel* model) {
+void MenuBar::SetMenu(AtomMenuModel* model) {
   menu_model_ = model;
   RemoveAllChildViews(true);
 
@@ -105,7 +105,7 @@ int MenuBar::GetItemCount() const {
 }
 
 bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& point,
-                                           ui::MenuModel** menu_model,
+                                           AtomMenuModel** menu_model,
                                            views::MenuButton** button) {
   gfx::Point location(point);
   views::View::ConvertPointFromScreen(this, &location);
@@ -117,7 +117,7 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& point,
   for (int i = 0; i < child_count(); ++i) {
     views::View* view = child_at(i);
     if (view->bounds().Contains(location) &&
-        (menu_model_->GetTypeAt(i) == ui::MenuModel::TYPE_SUBMENU)) {
+        (menu_model_->GetTypeAt(i) == AtomMenuModel::TYPE_SUBMENU)) {
       *menu_model = menu_model_->GetSubmenuModelAt(i);
       *button = static_cast<views::MenuButton*>(view);
       return true;
@@ -144,8 +144,8 @@ void MenuBar::OnMenuButtonClicked(views::MenuButton* source,
     return;
 
   int id = source->tag();
-  ui::MenuModel::ItemType type = menu_model_->GetTypeAt(id);
+  AtomMenuModel::ItemType type = menu_model_->GetTypeAt(id);
-  if (type != ui::MenuModel::TYPE_SUBMENU) {
+  if (type != AtomMenuModel::TYPE_SUBMENU) {
     menu_model_->ActivatedAt(id, 0);
     return;
   }

atom/browser/ui/views/menu_bar.h

@@ -5,14 +5,11 @@
 #ifndef ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_
 #define ATOM_BROWSER_UI_VIEWS_MENU_BAR_H_
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 #include "ui/views/view.h"
 
-namespace ui {
-class MenuModel;
-}
-
 namespace views {
 class MenuButton;
 }
@@ -29,7 +26,7 @@ class MenuBar : public views::View,
   virtual ~MenuBar();
 
   // Replaces current menu with a new one.
-  void SetMenu(ui::MenuModel* menu_model);
+  void SetMenu(AtomMenuModel* menu_model);
 
   // Shows underline under accelerators.
   void SetAcceleratorVisibility(bool visible);
@@ -46,7 +43,7 @@ class MenuBar : public views::View,
 
   // Get the menu under specified screen point.
   bool GetMenuButtonFromScreenPoint(const gfx::Point& point,
-                                    ui::MenuModel** menu_model,
+                                    AtomMenuModel** menu_model,
                                     views::MenuButton** button);
 
  protected:
@@ -74,7 +71,7 @@ class MenuBar : public views::View,
   SkColor hover_color_;
 #endif
 
-  ui::MenuModel* menu_model_;
+  AtomMenuModel* menu_model_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuBar);
 };

atom/browser/ui/views/menu_delegate.cc

@@ -5,10 +5,10 @@
 #include "atom/browser/ui/views/menu_delegate.h"
 
 #include "atom/browser/ui/views/menu_bar.h"
+#include "atom/browser/ui/views/menu_model_adapter.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/menu/menu_item_view.h"
-#include "ui/views/controls/menu/menu_model_adapter.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/widget/widget.h"
 
@@ -22,7 +22,7 @@ MenuDelegate::MenuDelegate(MenuBar* menu_bar)
 MenuDelegate::~MenuDelegate() {
 }
 
-void MenuDelegate::RunMenu(ui::MenuModel* model, views::MenuButton* button) {
+void MenuDelegate::RunMenu(AtomMenuModel* model, views::MenuButton* button) {
   gfx::Point screen_loc;
   views::View::ConvertPointToScreen(button, &screen_loc);
   // Subtract 1 from the height to make the popup flush with the button border.
@@ -30,10 +30,10 @@ void MenuDelegate::RunMenu(ui::MenuModel* model, views::MenuButton* button) {
                    button->height() - 1);
 
   id_ = button->tag();
-  adapter_.reset(new views::MenuModelAdapter(model));
+  adapter_.reset(new MenuModelAdapter(model));
 
   views::MenuItemView* item = new views::MenuItemView(this);
-  static_cast<views::MenuModelAdapter*>(adapter_.get())->BuildMenu(item);
+  static_cast<MenuModelAdapter*>(adapter_.get())->BuildMenu(item);
 
   menu_runner_.reset(new views::MenuRunner(
       item,
@@ -102,7 +102,7 @@ views::MenuItemView* MenuDelegate::GetSiblingMenu(
     bool* has_mnemonics,
     views::MenuButton**) {
   views::MenuButton* button;
-  ui::MenuModel* model;
+  AtomMenuModel* model;
   if (menu_bar_->GetMenuButtonFromScreenPoint(screen_point, &model, &button) &&
       button->tag() != id_) {
     DCHECK(menu_runner_->IsRunning());

atom/browser/ui/views/menu_delegate.h

@@ -5,6 +5,7 @@
 #ifndef ATOM_BROWSER_UI_VIEWS_MENU_DELEGATE_H_
 #define ATOM_BROWSER_UI_VIEWS_MENU_DELEGATE_H_
 
+#include "atom/browser/ui/atom_menu_model.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/views/controls/menu/menu_delegate.h"
 
@@ -12,10 +13,6 @@ namespace views {
 class MenuRunner;
 }
 
-namespace ui {
-class MenuModel;
-}
-
 namespace atom {
 
 class MenuBar;
@@ -25,7 +22,7 @@ class MenuDelegate : public views::MenuDelegate {
   explicit MenuDelegate(MenuBar* menu_bar);
   virtual ~MenuDelegate();
 
-  void RunMenu(ui::MenuModel* model, views::MenuButton* button);
+  void RunMenu(AtomMenuModel* model, views::MenuButton* button);
 
  protected:
   // views::MenuDelegate:

atom/browser/ui/views/menu_model_adapter.cc

@@ -0,0 +1,28 @@
+// Copyright (c) 2016 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/ui/views/menu_model_adapter.h"
+
+namespace atom {
+
+MenuModelAdapter::MenuModelAdapter(AtomMenuModel* menu_model)
+  : views::MenuModelAdapter(menu_model),
+    menu_model_(menu_model) {
+}
+
+MenuModelAdapter::~MenuModelAdapter() {
+}
+
+bool MenuModelAdapter::GetAccelerator(int id,
+                                      ui::Accelerator* accelerator) const {
+  ui::MenuModel* model = menu_model_;
+  int index = 0;
+  if (ui::MenuModel::GetModelAndIndexForCommandId(id, &model, &index)) {
+    return static_cast<AtomMenuModel*>(model)->
+      GetAcceleratorAtWithParams(index, true, accelerator);
+  }
+  return false;
+}
+
+}  // namespace atom

atom/browser/ui/views/menu_model_adapter.h

@@ -0,0 +1,29 @@
+// Copyright (c) 2016 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_UI_VIEWS_MENU_MODEL_ADAPTER_H_
+#define ATOM_BROWSER_UI_VIEWS_MENU_MODEL_ADAPTER_H_
+
+#include "atom/browser/ui/atom_menu_model.h"
+#include "ui/views/controls/menu/menu_model_adapter.h"
+
+namespace atom {
+
+class MenuModelAdapter : public views::MenuModelAdapter {
+ public:
+  explicit MenuModelAdapter(AtomMenuModel* menu_model);
+  virtual ~MenuModelAdapter();
+
+ protected:
+  bool GetAccelerator(int id, ui::Accelerator* accelerator) const override;
+
+ private:
+  AtomMenuModel* menu_model_;
+
+  DISALLOW_COPY_AND_ASSIGN(MenuModelAdapter);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_UI_VIEWS_MENU_MODEL_ADAPTER_H_

atom/browser/ui/win/notify_icon.cc

@@ -132,7 +132,7 @@ void NotifyIcon::DisplayBalloon(HICON icon,
 }
 
 void NotifyIcon::PopUpContextMenu(const gfx::Point& pos,
-                                  ui::SimpleMenuModel* menu_model) {
+                                  AtomMenuModel* menu_model) {
   // Returns if context menu isn't set.
   if (menu_model == nullptr && menu_model_ == nullptr)
     return;
@@ -154,7 +154,7 @@ void NotifyIcon::PopUpContextMenu(const gfx::Point& pos,
       NULL, NULL, rect, views::MENU_ANCHOR_TOPLEFT, ui::MENU_SOURCE_MOUSE));
 }
 
-void NotifyIcon::SetContextMenu(ui::SimpleMenuModel* menu_model) {
+void NotifyIcon::SetContextMenu(AtomMenuModel* menu_model) {
   menu_model_ = menu_model;
 }
 

atom/browser/ui/win/notify_icon.h

@@ -52,8 +52,8 @@ class NotifyIcon : public TrayIcon {
                       const base::string16& title,
                       const base::string16& contents) 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;
 
  private:
@@ -75,7 +75,7 @@ class NotifyIcon : public TrayIcon {
   base::win::ScopedHICON icon_;
 
   // The context menu.
-  ui::SimpleMenuModel* menu_model_;
+  AtomMenuModel* menu_model_;
 
   DISALLOW_COPY_AND_ASSIGN(NotifyIcon);
 };

filenames.gypi

@@ -275,6 +275,8 @@
       'atom/browser/ui/views/menu_delegate.h',
       'atom/browser/ui/views/menu_layout.cc',
       'atom/browser/ui/views/menu_layout.h',
+      'atom/browser/ui/views/menu_model_adapter.cc',
+      'atom/browser/ui/views/menu_model_adapter.h',
       'atom/browser/ui/views/native_frame_view.cc',
       'atom/browser/ui/views/native_frame_view.h',
       'atom/browser/ui/views/submenu_button.cc',

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]

spec/api-menu-spec.js

@@ -400,27 +400,23 @@ describe('menu module', function () {
     it('includes a default label and accelerator', function () {
       var item = new MenuItem({role: 'close'})
       assert.equal(item.label, 'Close')
-      assert.equal(item.accelerator, 'CommandOrControl+W')
+      assert.equal(item.accelerator, undefined)
+      assert.equal(item.getDefaultRoleAccelerator(), 'CommandOrControl+W')
 
-      item = new MenuItem({role: 'close', label: 'Other'})
+      item = new MenuItem({role: 'close', label: 'Other', accelerator: 'D'})
       assert.equal(item.label, 'Other')
-      assert.equal(item.accelerator, 'CommandOrControl+W')
-
-      item = new MenuItem({role: 'close', accelerator: 'D'})
-      assert.equal(item.label, 'Close')
-      assert.equal(item.accelerator, 'D')
-
-      item = new MenuItem({role: 'close', label: 'C', accelerator: 'D'})
-      assert.equal(item.label, 'C')
       assert.equal(item.accelerator, 'D')
+      assert.equal(item.getDefaultRoleAccelerator(), 'CommandOrControl+W')
 
       item = new MenuItem({role: 'help'})
       assert.equal(item.label, 'Help')
       assert.equal(item.accelerator, undefined)
+      assert.equal(item.getDefaultRoleAccelerator(), undefined)
 
       item = new MenuItem({role: 'hide'})
       assert.equal(item.label, 'Hide Electron Test')
-      assert.equal(item.accelerator, 'Command+H')
+      assert.equal(item.accelerator, undefined)
+      assert.equal(item.getDefaultRoleAccelerator(), 'Command+H')
     })
   })
 })