Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(36)

Unified Diff: views/controls/menu/menu_item_view.cc

Issue 7720012: Moves ownership of MenuItemView to MenuRunner as well as responbility (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix unit test Created 9 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: views/controls/menu/menu_item_view.cc
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index 89fbfe6eb57b150bcbdf5a245d147a445a1ac993..86a97a8b2e4d5f278d0e6fc64ec60b25809c514f 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -19,10 +19,6 @@
#include "views/controls/menu/menu_separator.h"
#include "views/controls/menu/submenu_view.h"
-#if defined(OS_WIN)
-#include "base/win/win_util.h"
-#endif
-
namespace views {
namespace {
@@ -100,16 +96,6 @@ MenuItemView::MenuItemView(MenuDelegate* delegate)
Init(NULL, 0, SUBMENU, delegate);
}
-MenuItemView::~MenuItemView() {
- // TODO(sky): ownership is bit wrong now. In particular if a nested message
- // loop is running deletion can't be done, otherwise the stack gets
- // thoroughly screwed. The destructor should be made private, and
- // MenuController should be the only place handling deletion of the menu.
- // (57890).
- delete submenu_;
- STLDeleteElements(&removed_items_);
-}
-
void MenuItemView::ChildPreferredSizeChanged(View* child) {
pref_size_.SetSize(0, 0);
PreferredSizeChanged();
@@ -192,87 +178,6 @@ string16 MenuItemView::GetAccessibleNameForMenuItem(
return accessible_name;
}
-void MenuItemView::RunMenuAt(Widget* parent,
- MenuButton* button,
- const gfx::Rect& bounds,
- AnchorPosition anchor,
- bool has_mnemonics) {
- // Show mnemonics if the button has focus or alt is pressed.
- bool show_mnemonics = button ? button->HasFocus() : false;
-#if defined(OS_WIN)
- // We don't currently need this on gtk as showing the menu gives focus to the
- // button first.
- if (!show_mnemonics)
- show_mnemonics = base::win::IsAltPressed();
-#endif
- PrepareForRun(has_mnemonics, show_mnemonics);
- int mouse_event_flags;
-
- MenuController* controller = MenuController::GetActiveInstance();
- if (controller && !controller->IsBlockingRun()) {
- // A menu is already showing, but it isn't a blocking menu. Cancel it.
- // We can get here during drag and drop if the user right clicks on the
- // menu quickly after the drop.
- controller->Cancel(MenuController::EXIT_ALL);
- controller = NULL;
- }
- // TODO(sky): remove volatile, used in tracking 90860.
- volatile bool owns_controller = false;
- if (!controller) {
- // No menus are showing, show one.
- controller = new MenuController(true);
- MenuController::SetActiveInstance(controller);
- owns_controller = true;
- } else {
- // A menu is already showing, use the same controller.
-
- // Don't support blocking from within non-blocking.
- DCHECK(controller->IsBlockingRun());
- }
-
- controller_ = controller;
-
- // Run the loop.
- MenuItemView* result =
- controller->Run(parent, button, this, bounds, anchor, &mouse_event_flags);
-
- RemoveEmptyMenus();
-
- controller_ = NULL;
-
- if (owns_controller) {
- // We created the controller and need to delete it.
- if (MenuController::GetActiveInstance() == controller)
- MenuController::SetActiveInstance(NULL);
- delete controller;
- }
- // Make sure all the windows we created to show the menus have been
- // destroyed.
- DestroyAllMenuHosts();
- if (result && delegate_)
- delegate_->ExecuteCommand(result->GetCommand(), mouse_event_flags);
-}
-
-void MenuItemView::RunMenuForDropAt(Widget* parent,
- const gfx::Rect& bounds,
- AnchorPosition anchor) {
- PrepareForRun(false, false);
-
- // If there is a menu, hide it so that only one menu is shown during dnd.
- MenuController* current_controller = MenuController::GetActiveInstance();
- if (current_controller) {
- current_controller->Cancel(MenuController::EXIT_ALL);
- }
-
- // Always create a new controller for non-blocking.
- controller_ = new MenuController(false);
-
- // Set the instance, that way it can be canceled by another menu.
- MenuController::SetActiveInstance(controller_);
-
- controller_->Run(parent, NULL, this, bounds, anchor, NULL);
-}
-
void MenuItemView::Cancel() {
if (controller_ && !canceled_) {
canceled_ = true;
@@ -559,6 +464,11 @@ MenuItemView::MenuItemView(MenuItemView* parent,
Init(parent, command, type, NULL);
}
+MenuItemView::~MenuItemView() {
+ delete submenu_;
+ STLDeleteElements(&removed_items_);
+}
+
std::string MenuItemView::GetClassName() const {
return kViewClassName;
}
@@ -613,23 +523,6 @@ void MenuItemView::Init(MenuItemView* parent,
SetEnabled(root_delegate->IsCommandEnabled(command));
}
-void MenuItemView::DropMenuClosed(bool notify_delegate) {
- DCHECK(controller_);
- DCHECK(!controller_->IsBlockingRun());
- if (MenuController::GetActiveInstance() == controller_)
- MenuController::SetActiveInstance(NULL);
- delete controller_;
- controller_ = NULL;
-
- RemoveEmptyMenus();
-
- if (notify_delegate && delegate_) {
- // Our delegate is null when invoked from the destructor.
- delegate_->DropMenuClosed(this);
- }
- // WARNING: its possible the delegate deleted us at this point.
-}
-
void MenuItemView::PrepareForRun(bool has_mnemonics, bool show_mnemonics) {
// Currently we only support showing the root.
DCHECK(!parent_menu_item_);

Powered by Google App Engine
This is Rietveld 408576698