reland "fixed multiple menu shortcuts issues"

Cong Liu · rogerwang · commit c800a50668d8 · 2017-02-20T18:38:02.000+08:00
This commit fixed following issues: * shortcuts was not unregistered when the item was removed or detached * shortcuts didn't work with inserted items fixed nwjs#5657
diff --git a/src/api/menu/menu.h b/src/api/menu/menu.h
@@ -104,6 +104,7 @@ class Menu : public Base {
 
 #if defined(OS_WIN) || defined(OS_LINUX)
   void UpdateKeys(views::FocusManager *focus_manager);
+  void RemoveKeys();
   ui::NwMenuModel* model() { return menu_model_.get(); }
   aura::Window* GetActiveNativeView(content::RenderFrameHost* rfh);
 #endif
diff --git a/src/api/menu/menu_views.cc b/src/api/menu/menu_views.cc
@@ -85,6 +85,8 @@ void Menu::Append(MenuItem* menu_item) {
 }
 
 void Menu::Insert(MenuItem* menu_item, int pos) {
+  if (pos < 0 || pos > (int)menu_items_.size()) return;
+
   if (menu_item->submenu_)
     menu_model_->InsertSubMenuAt(pos, menu_item->id(), menu_item->label_,
                                  menu_item->submenu_->menu_model_.get());
@@ -97,13 +99,17 @@ void Menu::Insert(MenuItem* menu_item, int pos) {
 
   is_menu_modified_ = true;
   menu_item->menu_ = this;
-
+  menu_items_.insert(menu_items_.begin() + pos, menu_item);
 }
 
 void Menu::Remove(MenuItem* menu_item, int pos) {
+  if (pos < 0 || pos >= (int)menu_items_.size()) return;
+
   menu_model_->RemoveItemAt(pos);
+  menu_items_.erase(menu_items_.begin() + pos);
   is_menu_modified_ = true;
   menu_item->menu_ = NULL;
+  menu_item->RemoveKeys();
 }
 
 void Menu::Popup(int x, int y, content::RenderFrameHost* rfh) {
@@ -142,14 +148,22 @@ void Menu::UpdateKeys(views::FocusManager *focus_manager){
     return ;
   } else {
     focus_manager_ = focus_manager;
-    std::vector<MenuItem*>::iterator it = menu_items_.begin();
-    while(it!=menu_items_.end()){
-      (*it)->UpdateKeys(focus_manager);
-      ++it;
+    for(auto item : menu_items_) {
+      item->UpdateKeys(focus_manager);
     }
   }
 }
 
+void Menu::RemoveKeys() {
+  if (!focus_manager_) return;
+
+  for(auto item: menu_items_) {
+    item->RemoveKeys();
+  }
+
+  focus_manager_ = NULL;
+}
+
 void Menu::UpdateStates() {
 }
 
diff --git a/src/api/menuitem/menuitem.h b/src/api/menuitem/menuitem.h
@@ -76,9 +76,10 @@ class MenuItem : public Base {
 #endif
 
 #if defined(OS_WIN) || defined(OS_LINUX)
-   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
-   bool CanHandleAccelerators() const override;
+  bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
+  bool CanHandleAccelerators() const override;
   void UpdateKeys(views::FocusManager *focus_manager);
+  void RemoveKeys();
 #endif
 
   void OnClick();
diff --git a/src/api/menuitem/menuitem_views.cc b/src/api/menuitem/menuitem_views.cc
@@ -168,6 +168,8 @@ void MenuItem::SetChecked(bool checked) {
 }
 
 void MenuItem::SetSubmenu(Menu* menu) {
+  if (submenu_) submenu_->RemoveKeys();
+
   submenu_ = menu;
 }
 
@@ -192,6 +194,18 @@ void MenuItem::UpdateKeys(views::FocusManager *focus_manager){
   }
 }
 
+void MenuItem::RemoveKeys() {
+  if (!focus_manager_) return;
+
+  if (enable_shortcut_) {
+    focus_manager_->UnregisterAccelerator(accelerator_, this);
+  }
+  if (submenu_) {
+    submenu_->RemoveKeys();
+  }
+  focus_manager_ = NULL;
+}
+
 #if defined(OS_WIN) || defined(OS_LINUX)
 bool MenuItem::AcceleratorPressed(const ui::Accelerator& accelerator) {
 #if defined(OS_WIN)
diff --git a/src/api/nw_window_api.cc b/src/api/nw_window_api.cc
@@ -339,6 +339,8 @@ bool NwCurrentWindowInternalClearMenuFunction::RunAsync() {
   browser_view_layout->set_menu_bar(NULL);
   native_app_window_views->layout_();
   native_app_window_views->SchedulePaint();
+  window->menu_->RemoveKeys();
+  window->menu_ = NULL;
 #endif
   return true;
 }
@@ -360,7 +362,12 @@ bool NwCurrentWindowInternalSetMenuFunction::RunNWSync(base::ListValue* response
   nw::ObjectManager* obj_manager = nw::ObjectManager::Get(browser_context());
   Menu* menu = (Menu*)obj_manager->GetApiObject(id);
 
+#if defined(OS_LINUX) || defined(OS_WIN)
+  Menu* old_menu = window->menu_;
+#endif
+
   window->menu_ = menu;
+  
 #if defined(OS_MACOSX)
   response->Append(NWChangeAppMenu(menu));
 #endif
@@ -376,6 +383,7 @@ bool NwCurrentWindowInternalSetMenuFunction::RunNWSync(base::ListValue* response
   menubar->UpdateMenu(menu->model());
   native_app_window_views->layout_();
   native_app_window_views->SchedulePaint();
+  if (old_menu) old_menu->RemoveKeys();
   menu->UpdateKeys( native_app_window_views->widget()->GetFocusManager() );
   response->Append(std::unique_ptr<base::ListValue>(new base::ListValue()));
 #endif

Original file line number	Diff line number	Diff line change
`@@ -168,6 +168,8 @@ void MenuItem::SetChecked(bool checked) {`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`void MenuItem::SetSubmenu(Menu* menu) {`
	`171`	`+ if (submenu_) submenu_->RemoveKeys();`
	`172`	`+`
`171`	`173`	`submenu_ = menu;`
`172`	`174`	`}`
`173`	`175`
`@@ -192,6 +194,18 @@ void MenuItem::UpdateKeys(views::FocusManager *focus_manager){`
`192`	`194`	`}`
`193`	`195`	`}`
`194`	`196`
	`197`	`+void MenuItem::RemoveKeys() {`
	`198`	`+ if (!focus_manager_) return;`
	`199`	`+`
	`200`	`+ if (enable_shortcut_) {`
	`201`	`+ focus_manager_->UnregisterAccelerator(accelerator_, this);`
	`202`	`+ }`
	`203`	`+ if (submenu_) {`
	`204`	`+ submenu_->RemoveKeys();`
	`205`	`+ }`
	`206`	`+ focus_manager_ = NULL;`
	`207`	`+}`
	`208`	`+`
`195`	`209`	`#if defined(OS_WIN) \|\| defined(OS_LINUX)`
`196`	`210`	`bool MenuItem::AcceleratorPressed(const ui::Accelerator& accelerator) {`
`197`	`211`	`#if defined(OS_WIN)`