From 48451032e33e57cb6b01bfbfae026addbb48fd89 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 5 Jan 2016 10:22:42 +0800 Subject: [PATCH 1/3] Update brightray to fix menu not loading resources --- vendor/brightray | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/brightray b/vendor/brightray index d572361a4eee..f9c272ec86ee 160000 --- a/vendor/brightray +++ b/vendor/brightray @@ -1 +1 @@ -Subproject commit d572361a4eeeb3a2fe6d3f2de457fbecb5775c0a +Subproject commit f9c272ec86ee83915729cf2ecdfdd5aa418b77eb From 43bfce26a72b62128f2a4288e5f479c1f1dc8d58 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 5 Jan 2016 11:57:58 +0800 Subject: [PATCH 2/3] Do not behave like bookmarkbar menu --- atom/browser/ui/views/menu_bar.cc | 18 +++--- atom/browser/ui/views/menu_bar.h | 4 +- atom/browser/ui/views/menu_delegate.cc | 83 +++++++++++++------------- atom/browser/ui/views/menu_delegate.h | 20 ++----- 4 files changed, 60 insertions(+), 65 deletions(-) diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc index b7712929d024..54b6957fa52f 100644 --- a/atom/browser/ui/views/menu_bar.cc +++ b/atom/browser/ui/views/menu_bar.cc @@ -134,6 +134,16 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& point, return false; } +void MenuBar::RunMenu(views::MenuButton* button) { + int id = button->tag(); + ui::MenuModel::ItemType type = menu_model_->GetTypeAt(id); + if (type != ui::MenuModel::TYPE_SUBMENU) + return; + + MenuDelegate menu_delegate(this); + menu_delegate.RunMenu(menu_model_->GetSubmenuModelAt(id), button); +} + const char* MenuBar::GetClassName() const { return kViewClassName; } @@ -150,13 +160,7 @@ void MenuBar::OnMenuButtonClicked(views::View* source, return; views::MenuButton* button = static_cast(source); - int id = button->tag(); - ui::MenuModel::ItemType type = menu_model_->GetTypeAt(id); - if (type != ui::MenuModel::TYPE_SUBMENU) - return; - - menu_delegate_.reset(new MenuDelegate(this)); - menu_delegate_->RunMenu(menu_model_->GetSubmenuModelAt(id), button); + RunMenu(button); } } // namespace atom diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h index ac82711f8b9a..09d257c09732 100644 --- a/atom/browser/ui/views/menu_bar.h +++ b/atom/browser/ui/views/menu_bar.h @@ -49,6 +49,9 @@ class MenuBar : public views::View, ui::MenuModel** menu_model, views::MenuButton** button); + // Shows the menu with |button|. + void RunMenu(views::MenuButton* button); + protected: // views::View: const char* GetClassName() const override; @@ -71,7 +74,6 @@ class MenuBar : public views::View, #endif ui::MenuModel* menu_model_; - scoped_ptr menu_delegate_; DISALLOW_COPY_AND_ASSIGN(MenuBar); }; diff --git a/atom/browser/ui/views/menu_delegate.cc b/atom/browser/ui/views/menu_delegate.cc index 84c35d9cd14d..1c28cc10df39 100644 --- a/atom/browser/ui/views/menu_delegate.cc +++ b/atom/browser/ui/views/menu_delegate.cc @@ -5,7 +5,7 @@ #include "atom/browser/ui/views/menu_delegate.h" #include "atom/browser/ui/views/menu_bar.h" -#include "base/stl_util.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" @@ -16,13 +16,10 @@ namespace atom { MenuDelegate::MenuDelegate(MenuBar* menu_bar) : menu_bar_(menu_bar), - id_(-1), - items_(menu_bar_->GetItemCount()), - delegates_(menu_bar_->GetItemCount()) { + id_(-1) { } MenuDelegate::~MenuDelegate() { - STLDeleteElements(&delegates_); } void MenuDelegate::RunMenu(ui::MenuModel* model, views::MenuButton* button) { @@ -33,12 +30,15 @@ void MenuDelegate::RunMenu(ui::MenuModel* model, views::MenuButton* button) { button->height() - 1); id_ = button->tag(); - views::MenuItemView* item = BuildMenu(model); + adapter_.reset(new views::MenuModelAdapter(model)); - views::MenuRunner menu_runner( + views::MenuItemView* item = new views::MenuItemView(this); + static_cast(adapter_.get())->BuildMenu(item); + + menu_runner_.reset(new views::MenuRunner( item, - views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS); - ignore_result(menu_runner.RunMenuAt( + views::MenuRunner::CONTEXT_MENU | views::MenuRunner::HAS_MNEMONICS)); + ignore_result(menu_runner_->RunMenuAt( button->GetWidget()->GetTopLevelWidget(), button, bounds, @@ -46,68 +46,53 @@ void MenuDelegate::RunMenu(ui::MenuModel* model, views::MenuButton* button) { ui::MENU_SOURCE_MOUSE)); } -views::MenuItemView* MenuDelegate::BuildMenu(ui::MenuModel* model) { - DCHECK_GE(id_, 0); - - if (!items_[id_]) { - views::MenuModelAdapter* delegate = new views::MenuModelAdapter(model); - delegates_[id_] = delegate; - - views::MenuItemView* item = new views::MenuItemView(this); - delegate->BuildMenu(item); - items_[id_] = item; - } - - return items_[id_]; -} - void MenuDelegate::ExecuteCommand(int id) { - delegate()->ExecuteCommand(id); + adapter_->ExecuteCommand(id); } void MenuDelegate::ExecuteCommand(int id, int mouse_event_flags) { - delegate()->ExecuteCommand(id, mouse_event_flags); + adapter_->ExecuteCommand(id, mouse_event_flags); } bool MenuDelegate::IsTriggerableEvent(views::MenuItemView* source, const ui::Event& e) { - return delegate()->IsTriggerableEvent(source, e); + return adapter_->IsTriggerableEvent(source, e); } bool MenuDelegate::GetAccelerator(int id, ui::Accelerator* accelerator) const { - return delegate()->GetAccelerator(id, accelerator); + return adapter_->GetAccelerator(id, accelerator); } base::string16 MenuDelegate::GetLabel(int id) const { - return delegate()->GetLabel(id); + return adapter_->GetLabel(id); } const gfx::FontList* MenuDelegate::GetLabelFontList(int id) const { - return delegate()->GetLabelFontList(id); + return adapter_->GetLabelFontList(id); } bool MenuDelegate::IsCommandEnabled(int id) const { - return delegate()->IsCommandEnabled(id); + return adapter_->IsCommandEnabled(id); } bool MenuDelegate::IsCommandVisible(int id) const { - return delegate()->IsCommandVisible(id); + return adapter_->IsCommandVisible(id); } bool MenuDelegate::IsItemChecked(int id) const { - return delegate()->IsItemChecked(id); + return adapter_->IsItemChecked(id); } void MenuDelegate::SelectionChanged(views::MenuItemView* menu) { - delegate()->SelectionChanged(menu); + adapter_->SelectionChanged(menu); } void MenuDelegate::WillShowMenu(views::MenuItemView* menu) { - delegate()->WillShowMenu(menu); + adapter_->WillShowMenu(menu); } void MenuDelegate::WillHideMenu(views::MenuItemView* menu) { - delegate()->WillHideMenu(menu); + adapter_->WillHideMenu(menu); } views::MenuItemView* MenuDelegate::GetSiblingMenu( @@ -115,16 +100,28 @@ views::MenuItemView* MenuDelegate::GetSiblingMenu( const gfx::Point& screen_point, views::MenuAnchorPosition* anchor, bool* has_mnemonics, - views::MenuButton** button) { + views::MenuButton**) { + views::MenuButton* button; ui::MenuModel* model; - if (!menu_bar_->GetMenuButtonFromScreenPoint(screen_point, &model, button)) - return NULL; + if (menu_bar_->GetMenuButtonFromScreenPoint(screen_point, &model, &button) && + button->tag() != id_) { + // Switch to sibling menu on next tick, otherwise crash may happen. + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&MenuDelegate::SwitchToSiblingMenu, + base::Unretained(this), button)); + } - *anchor = views::MENU_ANCHOR_TOPLEFT; - *has_mnemonics = true; + return nullptr; +} - id_ = (*button)->tag(); - return BuildMenu(model); +void MenuDelegate::SwitchToSiblingMenu(views::MenuButton* button) { + menu_runner_->Cancel(); + // After canceling the menu, we need to wait until next tick so we are out of + // nested message loop. + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&MenuBar::RunMenu, base::Unretained(menu_bar_), button)); } } // namespace atom diff --git a/atom/browser/ui/views/menu_delegate.h b/atom/browser/ui/views/menu_delegate.h index a837e5d53eb2..f83e5896c606 100644 --- a/atom/browser/ui/views/menu_delegate.h +++ b/atom/browser/ui/views/menu_delegate.h @@ -5,12 +5,11 @@ #ifndef ATOM_BROWSER_UI_VIEWS_MENU_DELEGATE_H_ #define ATOM_BROWSER_UI_VIEWS_MENU_DELEGATE_H_ -#include - +#include "base/memory/scoped_ptr.h" #include "ui/views/controls/menu/menu_delegate.h" namespace views { -class MenuModelAdapter; +class MenuRunner; } namespace ui { @@ -51,20 +50,13 @@ class MenuDelegate : public views::MenuDelegate { views::MenuButton** button) override; private: - // Gets the cached menu item view from the model. - views::MenuItemView* BuildMenu(ui::MenuModel* model); - - // Returns delegate for current item. - views::MenuDelegate* delegate() const { return delegates_[id_]; } + // Close this menu and run the menu of |button|. + void SwitchToSiblingMenu(views::MenuButton* button); MenuBar* menu_bar_; - - // Current item's id. int id_; - // Cached menu items, managed by MenuRunner. - std::vector items_; - // Cached menu delegates for each menu item, managed by us. - std::vector delegates_; + scoped_ptr adapter_; + scoped_ptr menu_runner_; DISALLOW_COPY_AND_ASSIGN(MenuDelegate); }; From 698700716b44531e57035bcb24d83653b1c11dd8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 5 Jan 2016 12:05:27 +0800 Subject: [PATCH 3/3] Show menu by clicking the menu button --- atom/browser/ui/views/menu_bar.cc | 18 +++++++----------- atom/browser/ui/views/menu_bar.h | 3 --- atom/browser/ui/views/menu_delegate.cc | 3 ++- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc index 54b6957fa52f..ba0a542c1e8d 100644 --- a/atom/browser/ui/views/menu_bar.cc +++ b/atom/browser/ui/views/menu_bar.cc @@ -134,16 +134,6 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& point, return false; } -void MenuBar::RunMenu(views::MenuButton* button) { - int id = button->tag(); - ui::MenuModel::ItemType type = menu_model_->GetTypeAt(id); - if (type != ui::MenuModel::TYPE_SUBMENU) - return; - - MenuDelegate menu_delegate(this); - menu_delegate.RunMenu(menu_model_->GetSubmenuModelAt(id), button); -} - const char* MenuBar::GetClassName() const { return kViewClassName; } @@ -160,7 +150,13 @@ void MenuBar::OnMenuButtonClicked(views::View* source, return; views::MenuButton* button = static_cast(source); - RunMenu(button); + int id = button->tag(); + ui::MenuModel::ItemType type = menu_model_->GetTypeAt(id); + if (type != ui::MenuModel::TYPE_SUBMENU) + return; + + MenuDelegate menu_delegate(this); + menu_delegate.RunMenu(menu_model_->GetSubmenuModelAt(id), button); } } // namespace atom diff --git a/atom/browser/ui/views/menu_bar.h b/atom/browser/ui/views/menu_bar.h index 09d257c09732..9d77cfdf2a22 100644 --- a/atom/browser/ui/views/menu_bar.h +++ b/atom/browser/ui/views/menu_bar.h @@ -49,9 +49,6 @@ class MenuBar : public views::View, ui::MenuModel** menu_model, views::MenuButton** button); - // Shows the menu with |button|. - void RunMenu(views::MenuButton* button); - protected: // views::View: const char* GetClassName() const override; diff --git a/atom/browser/ui/views/menu_delegate.cc b/atom/browser/ui/views/menu_delegate.cc index 1c28cc10df39..8ef8bca72dee 100644 --- a/atom/browser/ui/views/menu_delegate.cc +++ b/atom/browser/ui/views/menu_delegate.cc @@ -121,7 +121,8 @@ void MenuDelegate::SwitchToSiblingMenu(views::MenuButton* button) { // nested message loop. content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, - base::Bind(&MenuBar::RunMenu, base::Unretained(menu_bar_), button)); + base::Bind(base::IgnoreResult(&views::MenuButton::Activate), + base::Unretained(button))); } } // namespace atom