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

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

Issue 2687753002: Revert 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
« no previous file with comments | « ui/views/controls/menu/menu_controller.cc ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 }
72 73
73 // TODO(jonross): remove after tracking down the cause of 74 delete this;
74 // (crbug.com/683087).
75 // Update for the ASAN stack trace to determine if we are in the running
76 // state during the incorrect destruction order.
77 delete this;
78 } else {
79 delete this;
80 }
81 } 75 }
82 76
83 MenuRunner::RunResult MenuRunnerImpl::RunMenuAt(Widget* parent, 77 MenuRunner::RunResult MenuRunnerImpl::RunMenuAt(Widget* parent,
84 MenuButton* button, 78 MenuButton* button,
85 const gfx::Rect& bounds, 79 const gfx::Rect& bounds,
86 MenuAnchorPosition anchor, 80 MenuAnchorPosition anchor,
87 int32_t run_types) { 81 int32_t run_types) {
88 closing_event_time_ = base::TimeTicks(); 82 closing_event_time_ = base::TimeTicks();
89 if (running_) { 83 if (running_) {
90 // Ignore requests to show the menu while it's already showing. MenuItemView 84 // Ignore requests to show the menu while it's already showing. MenuItemView
91 // doesn't handle this very well (meaning it crashes). 85 // doesn't handle this very well (meaning it crashes).
92 return MenuRunner::NORMAL_EXIT; 86 return MenuRunner::NORMAL_EXIT;
93 } 87 }
94 88
95 // TODO(jonross): remove after tracking down the cause of (crbug.com/683087).
96 // Verify that this was not a delegate previously used for a run, which was
97 // shutdown, but not deleted. Nesting the same delegate multiple times is
98 // dangerous.
99 CHECK(!controller_);
100
101 MenuController* controller = MenuController::GetActiveInstance(); 89 MenuController* controller = MenuController::GetActiveInstance();
102 if (controller) { 90 if (controller) {
103 if ((run_types & MenuRunner::IS_NESTED) != 0) { 91 if ((run_types & MenuRunner::IS_NESTED) != 0) {
104 if (!controller->IsBlockingRun()) { 92 if (!controller->IsBlockingRun()) {
105 controller->CancelAll(); 93 controller->CancelAll();
106 controller = NULL; 94 controller = NULL;
107 } else { 95 } else {
108 // Only nest the delegate when not cancelling drag-and-drop. When 96 // Only nest the delegate when not cancelling drag-and-drop. When
109 // cancelling this will become the root delegate of the new 97 // cancelling this will become the root delegate of the new
110 // MenuController 98 // MenuController
(...skipping 15 matching lines...) Expand all
126 } 114 }
127 115
128 running_ = true; 116 running_ = true;
129 async_ = (run_types & MenuRunner::ASYNC) != 0; 117 async_ = (run_types & MenuRunner::ASYNC) != 0;
130 for_drop_ = (run_types & MenuRunner::FOR_DROP) != 0; 118 for_drop_ = (run_types & MenuRunner::FOR_DROP) != 0;
131 bool has_mnemonics = (run_types & MenuRunner::HAS_MNEMONICS) != 0; 119 bool has_mnemonics = (run_types & MenuRunner::HAS_MNEMONICS) != 0;
132 owns_controller_ = false; 120 owns_controller_ = false;
133 if (!controller) { 121 if (!controller) {
134 // No menus are showing, show one. 122 // No menus are showing, show one.
135 controller = new MenuController(!for_drop_, this); 123 controller = new MenuController(!for_drop_, this);
136 // TODO(jonross): remove after tracking down the cause of
137 // (crbug.com/683087).
138 owns_controller_ = true; 124 owns_controller_ = true;
139 } 125 }
140 controller->SetAsyncRun(async_); 126 controller->SetAsyncRun(async_);
141 controller->set_is_combobox((run_types & MenuRunner::COMBOBOX) != 0); 127 controller->set_is_combobox((run_types & MenuRunner::COMBOBOX) != 0);
142 controller_ = controller->AsWeakPtr(); 128 controller_ = controller->AsWeakPtr();
143 menu_->set_controller(controller_.get()); 129 menu_->set_controller(controller_.get());
144 menu_->PrepareForRun(owns_controller_, 130 menu_->PrepareForRun(owns_controller_,
145 has_mnemonics, 131 has_mnemonics,
146 !for_drop_ && ShouldShowMnemonics(button)); 132 !for_drop_ && ShouldShowMnemonics(button));
147 133
(...skipping 49 matching lines...) Expand 10 before | Expand all | Expand 10 after
197 MenuRunner::RunResult MenuRunnerImpl::MenuDone(NotifyType type, 183 MenuRunner::RunResult MenuRunnerImpl::MenuDone(NotifyType type,
198 MenuItemView* result, 184 MenuItemView* result,
199 int mouse_event_flags) { 185 int mouse_event_flags) {
200 menu_->RemoveEmptyMenus(); 186 menu_->RemoveEmptyMenus();
201 menu_->set_controller(nullptr); 187 menu_->set_controller(nullptr);
202 188
203 if (owns_controller_ && controller_) { 189 if (owns_controller_ && controller_) {
204 // We created the controller and need to delete it. 190 // We created the controller and need to delete it.
205 delete controller_.get(); 191 delete controller_.get();
206 owns_controller_ = false; 192 owns_controller_ = false;
207 controller_ = nullptr;
208 } 193 }
209 194 controller_ = nullptr;
210 // Make sure all the windows we created to show the menus have been 195 // Make sure all the windows we created to show the menus have been
211 // destroyed. 196 // destroyed.
212 menu_->DestroyAllMenuHosts(); 197 menu_->DestroyAllMenuHosts();
213 if (delete_after_run_) { 198 if (delete_after_run_) {
214 delete this; 199 delete this;
215 return MenuRunner::MENU_DELETED; 200 return MenuRunner::MENU_DELETED;
216 } 201 }
217 running_ = false; 202 running_ = false;
218 if (menu_->GetDelegate()) { 203 if (menu_->GetDelegate()) {
219 // Executing the command may also delete this. 204 // Executing the command may also delete this.
(...skipping 18 matching lines...) Expand all
238 #if defined(OS_WIN) 223 #if defined(OS_WIN)
239 // This is only needed on Windows. 224 // This is only needed on Windows.
240 if (!show_mnemonics) 225 if (!show_mnemonics)
241 show_mnemonics = ui::win::IsAltPressed(); 226 show_mnemonics = ui::win::IsAltPressed();
242 #endif 227 #endif
243 return show_mnemonics; 228 return show_mnemonics;
244 } 229 }
245 230
246 } // namespace internal 231 } // namespace internal
247 } // namespace views 232 } // namespace views
OLDNEW
« no previous file with comments | « 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