Do not behave like bookmarkbar menu

diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc
index b7712929d024708003e8f32c82fec1014b690568..54b6957fa52fb8dd3ccd788718ab362976b51a06 100644 (file)
--- 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<views::MenuButton*>(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 ac82711f8b9a060566196282583d7eeff27c22dd..09d257c0973299f78a27a354381999e30e19b0e3 100644 (file)
--- 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<MenuDelegate> 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 84c35d9cd14d3d9fc68d6c741dc1ee1e6ba84828..1c28cc10df3978fb64dd6e6574ba1a2d0f0a5ea0 100644 (file)
--- 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<views::MenuModelAdapter*>(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 a837e5d53eb2799eda5481f7245ac31c2daf6daf..f83e5896c606ba8c446fe09eae7a2a9e55c8107a 100644 (file)
--- 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 <vector>
-
+#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<views::MenuItemView*> items_;
-  // Cached menu delegates for each menu item, managed by us.
-  std::vector<views::MenuDelegate*> delegates_;
+  scoped_ptr<views::MenuDelegate> adapter_;
+  scoped_ptr<views::MenuRunner> menu_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuDelegate);
 };