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

Side by Side Diff: ui/views/controls/menu/menu_runner_impl.cc

Issue 2656823008: New MenuController Checks (Closed)
Patch Set: Created 3 years, 10 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 unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "ui/views/controls/menu/menu_runner_impl.h" 5 #include "ui/views/controls/menu/menu_runner_impl.h"
6 6
7 #include "build/build_config.h" 7 #include "build/build_config.h"
8 #include "ui/native_theme/native_theme.h" 8 #include "ui/native_theme/native_theme.h"
9 #include "ui/views/controls/button/menu_button.h" 9 #include "ui/views/controls/button/menu_button.h"
10 #include "ui/views/controls/menu/menu_controller.h" 10 #include "ui/views/controls/menu/menu_controller.h"
(...skipping 51 matching lines...) Expand 10 before | Expand all | Expand 10 after
62 62
63 // Verify that the MenuController is still active. It may have been 63 // Verify that the MenuController is still active. It may have been
64 // destroyed out of order. 64 // destroyed out of order.
65 if (controller_) { 65 if (controller_) {
66 // Release is invoked when MenuRunner is destroyed. Assume this is 66 // Release is invoked when MenuRunner is destroyed. Assume this is
67 // happening because the object referencing the menu has been destroyed 67 // happening because the object referencing the menu has been destroyed
68 // and the menu button is no longer valid. 68 // and the menu button is no longer valid.
69 controller_->Cancel(MenuController::EXIT_DESTROYED); 69 controller_->Cancel(MenuController::EXIT_DESTROYED);
70 return; 70 return;
71 } 71 }
72
73 // Update for the ASAN stack trace to determine if we are in the running
74 // state during the incorrect destruction order.
75 delete this;
76 } else {
77 delete this;
72 } 78 }
73
74 delete this;
75 } 79 }
76 80
77 MenuRunner::RunResult MenuRunnerImpl::RunMenuAt(Widget* parent, 81 MenuRunner::RunResult MenuRunnerImpl::RunMenuAt(Widget* parent,
78 MenuButton* button, 82 MenuButton* button,
79 const gfx::Rect& bounds, 83 const gfx::Rect& bounds,
80 MenuAnchorPosition anchor, 84 MenuAnchorPosition anchor,
81 int32_t run_types) { 85 int32_t run_types) {
82 closing_event_time_ = base::TimeTicks(); 86 closing_event_time_ = base::TimeTicks();
83 if (running_) { 87 if (running_) {
84 // Ignore requests to show the menu while it's already showing. MenuItemView 88 // Ignore requests to show the menu while it's already showing. MenuItemView
85 // doesn't handle this very well (meaning it crashes). 89 // doesn't handle this very well (meaning it crashes).
86 return MenuRunner::NORMAL_EXIT; 90 return MenuRunner::NORMAL_EXIT;
87 } 91 }
88 92
93 // Verify that this was not a delegate previously used for a run, which was
94 // shutdown, but not deleted. Nesting the same delegate multiple times is
95 // dangerous.
96 CHECK(!controller_);
97
89 MenuController* controller = MenuController::GetActiveInstance(); 98 MenuController* controller = MenuController::GetActiveInstance();
90 if (controller) { 99 if (controller) {
91 if ((run_types & MenuRunner::IS_NESTED) != 0) { 100 if ((run_types & MenuRunner::IS_NESTED) != 0) {
92 if (!controller->IsBlockingRun()) { 101 if (!controller->IsBlockingRun()) {
93 controller->CancelAll(); 102 controller->CancelAll();
94 controller = NULL; 103 controller = NULL;
95 } else { 104 } else {
96 // Only nest the delegate when not cancelling drag-and-drop. When 105 // Only nest the delegate when not cancelling drag-and-drop. When
97 // cancelling this will become the root delegate of the new 106 // cancelling this will become the root delegate of the new
98 // MenuController 107 // MenuController
(...skipping 84 matching lines...) Expand 10 before | Expand all | Expand 10 after
183 MenuRunner::RunResult MenuRunnerImpl::MenuDone(NotifyType type, 192 MenuRunner::RunResult MenuRunnerImpl::MenuDone(NotifyType type,
184 MenuItemView* result, 193 MenuItemView* result,
185 int mouse_event_flags) { 194 int mouse_event_flags) {
186 menu_->RemoveEmptyMenus(); 195 menu_->RemoveEmptyMenus();
187 menu_->set_controller(nullptr); 196 menu_->set_controller(nullptr);
188 197
189 if (owns_controller_ && controller_) { 198 if (owns_controller_ && controller_) {
190 // We created the controller and need to delete it. 199 // We created the controller and need to delete it.
191 delete controller_.get(); 200 delete controller_.get();
192 owns_controller_ = false; 201 owns_controller_ = false;
202 controller_ = nullptr;
193 } 203 }
194 controller_ = nullptr; 204
195 // Make sure all the windows we created to show the menus have been 205 // Make sure all the windows we created to show the menus have been
196 // destroyed. 206 // destroyed.
197 menu_->DestroyAllMenuHosts(); 207 menu_->DestroyAllMenuHosts();
198 if (delete_after_run_) { 208 if (delete_after_run_) {
199 delete this; 209 delete this;
200 return MenuRunner::MENU_DELETED; 210 return MenuRunner::MENU_DELETED;
201 } 211 }
202 running_ = false; 212 running_ = false;
203 if (menu_->GetDelegate()) { 213 if (menu_->GetDelegate()) {
204 // Executing the command may also delete this. 214 // Executing the command may also delete this.
(...skipping 18 matching lines...) Expand all
223 #if defined(OS_WIN) 233 #if defined(OS_WIN)
224 // This is only needed on Windows. 234 // This is only needed on Windows.
225 if (!show_mnemonics) 235 if (!show_mnemonics)
226 show_mnemonics = ui::win::IsAltPressed(); 236 show_mnemonics = ui::win::IsAltPressed();
227 #endif 237 #endif
228 return show_mnemonics; 238 return show_mnemonics;
229 } 239 }
230 240
231 } // namespace internal 241 } // namespace internal
232 } // namespace views 242 } // namespace views
OLDNEW
« ui/views/controls/menu/menu_controller.h ('K') | « ui/views/controls/menu/menu_controller.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698