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

Side by Side Diff: ui/views/controls/menu/menu_controller.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 (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 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_controller.h" 5 #include "ui/views/controls/menu/menu_controller.h"
6 6
7 #include "base/i18n/case_conversion.h" 7 #include "base/i18n/case_conversion.h"
8 #include "base/i18n/rtl.h" 8 #include "base/i18n/rtl.h"
9 #include "base/macros.h" 9 #include "base/macros.h"
10 #include "base/strings/utf_string_conversions.h" 10 #include "base/strings/utf_string_conversions.h"
(...skipping 415 matching lines...) Expand 10 before | Expand all | Expand 10 after
426 state_.hot_button = hot_button_; 426 state_.hot_button = hot_button_;
427 hot_button_ = nullptr; 427 hot_button_ = nullptr;
428 // We're already showing, push the current state. 428 // We're already showing, push the current state.
429 menu_stack_.push_back(std::make_pair(state_, std::move(pressed_lock_))); 429 menu_stack_.push_back(std::make_pair(state_, std::move(pressed_lock_)));
430 430
431 // The context menu should be owned by the same parent. 431 // The context menu should be owned by the same parent.
432 DCHECK_EQ(owner_, parent); 432 DCHECK_EQ(owner_, parent);
433 } else { 433 } else {
434 showing_ = true; 434 showing_ = true;
435 435
436 // If we are not showing we should be shutting down, this could lead to
437 // incorrect delegate states.
438 CHECK(!running_);
439
436 if (owner_) 440 if (owner_)
437 owner_->RemoveObserver(this); 441 owner_->RemoveObserver(this);
438 owner_ = parent; 442 owner_ = parent;
439 if (owner_) 443 if (owner_)
440 owner_->AddObserver(this); 444 owner_->AddObserver(this);
441 445
442 #if defined(USE_AURA) 446 #if defined(USE_AURA)
443 // Only create a MenuPreTargetHandler for non-nested menus. Nested menus 447 // Only create a MenuPreTargetHandler for non-nested menus. Nested menus
444 // will use the existing one. 448 // will use the existing one.
445 menu_pre_target_handler_.reset(new MenuPreTargetHandler(this, owner_)); 449 menu_pre_target_handler_.reset(new MenuPreTargetHandler(this, owner_));
(...skipping 17 matching lines...) Expand all
463 return NULL; 467 return NULL;
464 } 468 }
465 469
466 if (button) 470 if (button)
467 pressed_lock_.reset(new MenuButton::PressedLock(button)); 471 pressed_lock_.reset(new MenuButton::PressedLock(button));
468 472
469 // Make sure Chrome doesn't attempt to shut down while the menu is showing. 473 // Make sure Chrome doesn't attempt to shut down while the menu is showing.
470 if (ViewsDelegate::GetInstance()) 474 if (ViewsDelegate::GetInstance())
471 ViewsDelegate::GetInstance()->AddRef(); 475 ViewsDelegate::GetInstance()->AddRef();
472 476
477 // About to either exit and run async, or nest message loop.
478 running_ = true;
479
473 if (async_run_) 480 if (async_run_)
474 return nullptr; 481 return nullptr;
475 482
476 // We need to turn on nestable tasks as in some situations (pressing alt-f for 483 // We need to turn on nestable tasks as in some situations (pressing alt-f for
477 // one) the menus are run from a task. If we don't do this and are invoked 484 // one) the menus are run from a task. If we don't do this and are invoked
478 // from a task none of the tasks we schedule are processed and the menu 485 // from a task none of the tasks we schedule are processed and the menu
479 // appears totally broken. 486 // appears totally broken.
480 message_loop_depth_++; 487 message_loop_depth_++;
481 DCHECK_LE(message_loop_depth_, 2); 488 DCHECK_LE(message_loop_depth_, 2);
482 RunMessageLoop(); 489 RunMessageLoop();
(...skipping 28 matching lines...) Expand all
511 518
512 MenuItemView* selected = state_.item; 519 MenuItemView* selected = state_.item;
513 SetExitType(type); 520 SetExitType(type);
514 521
515 SendMouseCaptureLostToActiveView(); 522 SendMouseCaptureLostToActiveView();
516 523
517 // Hide windows immediately. 524 // Hide windows immediately.
518 SetSelection(NULL, SELECTION_UPDATE_IMMEDIATELY | SELECTION_EXIT); 525 SetSelection(NULL, SELECTION_UPDATE_IMMEDIATELY | SELECTION_EXIT);
519 526
520 if (!blocking_run_) { 527 if (!blocking_run_) {
528 bool nested = delegate_stack_.size() > 1;
529 CHECK(!nested);
530 base::WeakPtr<MenuController> this_ref = AsWeakPtr();
521 // If we didn't block the caller we need to notify the menu, which 531 // If we didn't block the caller we need to notify the menu, which
522 // triggers deleting us. 532 // triggers deleting us.
523 DCHECK(selected); 533 DCHECK(selected);
524 showing_ = false; 534 showing_ = false;
525 delegate_->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE, 535 delegate_->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE,
526 selected->GetRootMenuItem(), accept_event_flags_); 536 selected->GetRootMenuItem(), accept_event_flags_);
527 // WARNING: the call to MenuClosed deletes us. 537 // WARNING: the call to MenuClosed deletes us.
538 CHECK(!this_ref);
528 return; 539 return;
529 } 540 }
530 541
531 // On Windows and Linux the destruction of this menu's Widget leads to the 542 // On Windows and Linux the destruction of this menu's Widget leads to the
532 // teardown of the platform specific drag-and-drop Widget. Do not shutdown 543 // teardown of the platform specific drag-and-drop Widget. Do not shutdown
533 // while dragging, leave the Widget hidden until drag-and-drop has completed, 544 // while dragging, leave the Widget hidden until drag-and-drop has completed,
534 // at which point all menus will be destroyed. 545 // at which point all menus will be destroyed.
535 // 546 //
536 // If |type| is EXIT_ALL we update the state of the menu to not showing. So 547 // If |type| is EXIT_ALL we update the state of the menu to not showing. So
537 // that during the completion of a drag we are not incorrectly reporting the 548 // that during the completion of a drag we are not incorrectly reporting the
538 // visual state. 549 // visual state.
539 if (!drag_in_progress_) 550 if (!drag_in_progress_)
540 ExitAsyncRun(); 551 ExitAsyncRun();
541 else if (type == EXIT_ALL) 552 else if (type == EXIT_ALL)
542 showing_ = false; 553 showing_ = false;
543 } 554 }
544 555
545 void MenuController::AddNestedDelegate( 556 void MenuController::AddNestedDelegate(
546 internal::MenuControllerDelegate* delegate) { 557 internal::MenuControllerDelegate* delegate) {
558 for (auto delegates : delegate_stack_) {
559 // Having the same delegate in the stack could cause deletion order issues.
560 CHECK_NE(delegates.first, delegate);
561 }
562
547 delegate_stack_.push_back(std::make_pair(delegate, async_run_)); 563 delegate_stack_.push_back(std::make_pair(delegate, async_run_));
548 delegate_ = delegate; 564 delegate_ = delegate;
549 } 565 }
550 566
551 void MenuController::SetAsyncRun(bool is_async) { 567 void MenuController::SetAsyncRun(bool is_async) {
552 delegate_stack_.back().second = is_async; 568 delegate_stack_.back().second = is_async;
553 async_run_ = is_async; 569 async_run_ = is_async;
554 } 570 }
555 571
556 bool MenuController::OnMousePressed(SubmenuView* source, 572 bool MenuController::OnMousePressed(SubmenuView* source,
(...skipping 415 matching lines...) Expand 10 before | Expand all | Expand 10 after
972 988
973 // Set state such that we exit. 989 // Set state such that we exit.
974 showing_ = false; 990 showing_ = false;
975 SetExitType(EXIT_ALL); 991 SetExitType(EXIT_ALL);
976 992
977 // If over an empty menu item, drop occurs on the parent. 993 // If over an empty menu item, drop occurs on the parent.
978 if (drop_target->id() == MenuItemView::kEmptyMenuItemViewID) 994 if (drop_target->id() == MenuItemView::kEmptyMenuItemViewID)
979 drop_target = drop_target->GetParentMenuItem(); 995 drop_target = drop_target->GetParentMenuItem();
980 996
981 if (!IsBlockingRun()) { 997 if (!IsBlockingRun()) {
998 base::WeakPtr<MenuController> this_ref = AsWeakPtr();
982 delegate_->OnMenuClosed( 999 delegate_->OnMenuClosed(
983 internal::MenuControllerDelegate::DONT_NOTIFY_DELEGATE, 1000 internal::MenuControllerDelegate::DONT_NOTIFY_DELEGATE,
984 item->GetRootMenuItem(), accept_event_flags_); 1001 item->GetRootMenuItem(), accept_event_flags_);
1002 CHECK(!this_ref);
985 } 1003 }
986 1004
987 // WARNING: the call to MenuClosed deletes us. 1005 // WARNING: the call to MenuClosed deletes us.
988 1006
989 return drop_target->GetDelegate()->OnPerformDrop( 1007 return drop_target->GetDelegate()->OnPerformDrop(
990 drop_target, drop_position, event); 1008 drop_target, drop_position, event);
991 } 1009 }
992 1010
993 void MenuController::OnDragEnteredScrollButton(SubmenuView* source, 1011 void MenuController::OnDragEnteredScrollButton(SubmenuView* source,
994 bool is_up) { 1012 bool is_up) {
(...skipping 378 matching lines...) Expand 10 before | Expand all | Expand 10 after
1373 1391
1374 default: 1392 default:
1375 break; 1393 break;
1376 } 1394 }
1377 } 1395 }
1378 1396
1379 MenuController::MenuController(bool blocking, 1397 MenuController::MenuController(bool blocking,
1380 internal::MenuControllerDelegate* delegate) 1398 internal::MenuControllerDelegate* delegate)
1381 : blocking_run_(blocking), 1399 : blocking_run_(blocking),
1382 showing_(false), 1400 showing_(false),
1401 running_(false),
1383 exit_type_(EXIT_NONE), 1402 exit_type_(EXIT_NONE),
1384 did_capture_(false), 1403 did_capture_(false),
1385 result_(NULL), 1404 result_(NULL),
1386 accept_event_flags_(0), 1405 accept_event_flags_(0),
1387 drop_target_(NULL), 1406 drop_target_(NULL),
1388 drop_position_(MenuDelegate::DROP_UNKNOWN), 1407 drop_position_(MenuDelegate::DROP_UNKNOWN),
1389 owner_(NULL), 1408 owner_(NULL),
1390 possible_drag_(false), 1409 possible_drag_(false),
1391 drag_in_progress_(false), 1410 drag_in_progress_(false),
1392 did_initiate_drag_(false), 1411 did_initiate_drag_(false),
(...skipping 1258 matching lines...) Expand 10 before | Expand all | Expand 10 after
2651 delegate_ = delegate_stack_.back().first; 2670 delegate_ = delegate_stack_.back().first;
2652 async_run_ = delegate_stack_.back().second; 2671 async_run_ = delegate_stack_.back().second;
2653 } 2672 }
2654 } else { 2673 } else {
2655 #if defined(USE_AURA) 2674 #if defined(USE_AURA)
2656 menu_pre_target_handler_.reset(); 2675 menu_pre_target_handler_.reset();
2657 #endif 2676 #endif
2658 2677
2659 showing_ = false; 2678 showing_ = false;
2660 did_capture_ = false; 2679 did_capture_ = false;
2680 running_ = false;
2661 } 2681 }
2662 2682
2663 MenuItemView* result = result_; 2683 MenuItemView* result = result_;
2664 // In case we're nested, reset |result_|. 2684 // In case we're nested, reset |result_|.
2665 result_ = nullptr; 2685 result_ = nullptr;
2666 2686
2667 if (exit_type_ == EXIT_OUTERMOST) { 2687 if (exit_type_ == EXIT_OUTERMOST) {
2668 SetExitType(EXIT_NONE); 2688 SetExitType(EXIT_NONE);
2669 } else { 2689 } else {
2670 if (nested_menu && result) { 2690 if (nested_menu && result) {
(...skipping 75 matching lines...) Expand 10 before | Expand all | Expand 10 after
2746 if (hot_button_) 2766 if (hot_button_)
2747 hot_button_->SetHotTracked(false); 2767 hot_button_->SetHotTracked(false);
2748 hot_button_ = hot_button; 2768 hot_button_ = hot_button;
2749 if (hot_button) { 2769 if (hot_button) {
2750 hot_button->SetHotTracked(true); 2770 hot_button->SetHotTracked(true);
2751 hot_button->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true); 2771 hot_button->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
2752 } 2772 }
2753 } 2773 }
2754 2774
2755 } // namespace views 2775 } // namespace views
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698