New MenuController Checks

ui/views/controls/menu/menu_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/menu/menu_controller.h"
 
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 415 matching lines @@
     state_.hot_button = hot_button_;
     hot_button_ = nullptr;
     // We're already showing, push the current state.
     menu_stack_.push_back(std::make_pair(state_, std::move(pressed_lock_)));
 
     // The context menu should be owned by the same parent.
     DCHECK_EQ(owner_, parent);
   } else {
     showing_ = true;
 
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
+    // If we are not showing we should be shutting down, this could lead to
+    // incorrect delegate states.
+    CHECK(!running_);
+
     if (owner_)
       owner_->RemoveObserver(this);
     owner_ = parent;
     if (owner_)
       owner_->AddObserver(this);
 
 #if defined(USE_AURA)
     // Only create a MenuPreTargetHandler for non-nested menus. Nested menus
     // will use the existing one.
     menu_pre_target_handler_.reset(new MenuPreTargetHandler(this, owner_));
@@ skipping 17 matching lines @@
     return NULL;
   }
 
   if (button)
     pressed_lock_.reset(new MenuButton::PressedLock(button));
 
   // Make sure Chrome doesn't attempt to shut down while the menu is showing.
   if (ViewsDelegate::GetInstance())
     ViewsDelegate::GetInstance()->AddRef();
 
+  // TODO(jonross): remove after tracking down the cause of (crbug.com/683087).
+  // About to either exit and run async, or nest message loop.
+  running_ = true;
+
   if (async_run_)
     return nullptr;
 
   // We need to turn on nestable tasks as in some situations (pressing alt-f for
   // one) the menus are run from a task. If we don't do this and are invoked
   // from a task none of the tasks we schedule are processed and the menu
   // appears totally broken.
   message_loop_depth_++;
   DCHECK_LE(message_loop_depth_, 2);
   RunMessageLoop();
@@ skipping 28 matching lines @@
 
   MenuItemView* selected = state_.item;
   SetExitType(type);
 
   SendMouseCaptureLostToActiveView();
 
   // Hide windows immediately.
   SetSelection(NULL, SELECTION_UPDATE_IMMEDIATELY | SELECTION_EXIT);
 
   if (!blocking_run_) {
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
+    bool nested = delegate_stack_.size() > 1;
+    CHECK(!nested);
+    base::WeakPtr<MenuController> this_ref = AsWeakPtr();
     // If we didn't block the caller we need to notify the menu, which
     // triggers deleting us.
     DCHECK(selected);
     showing_ = false;
     delegate_->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE,
                             selected->GetRootMenuItem(), accept_event_flags_);
     // WARNING: the call to MenuClosed deletes us.
+    CHECK(!this_ref);
     return;
   }
 
   // On Windows and Linux the destruction of this menu's Widget leads to the
   // teardown of the platform specific drag-and-drop Widget. Do not shutdown
   // while dragging, leave the Widget hidden until drag-and-drop has completed,
   // at which point all menus will be destroyed.
   //
   // If |type| is EXIT_ALL we update the state of the menu to not showing. So
   // that during the completion of a drag we are not incorrectly reporting the
   // visual state.
   if (!drag_in_progress_)
     ExitAsyncRun();
   else if (type == EXIT_ALL)
     showing_ = false;
 }
 
 void MenuController::AddNestedDelegate(
     internal::MenuControllerDelegate* delegate) {
+  // TODO(jonross): remove after tracking down the cause of (crbug.com/683087).
+  for (auto delegates : delegate_stack_) {
+    // Having the same delegate in the stack could cause deletion order issues.
+    CHECK_NE(delegates.first, delegate);
+  }
+
   delegate_stack_.push_back(std::make_pair(delegate, async_run_));
   delegate_ = delegate;
 }
 
 void MenuController::SetAsyncRun(bool is_async) {
   delegate_stack_.back().second = is_async;
   async_run_ = is_async;
 }
 
 bool MenuController::OnMousePressed(SubmenuView* source,
@@ skipping 415 matching lines @@
 
   // Set state such that we exit.
   showing_ = false;
   SetExitType(EXIT_ALL);
 
   // If over an empty menu item, drop occurs on the parent.
   if (drop_target->id() == MenuItemView::kEmptyMenuItemViewID)
     drop_target = drop_target->GetParentMenuItem();
 
   if (!IsBlockingRun()) {
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
+    base::WeakPtr<MenuController> this_ref = AsWeakPtr();
     delegate_->OnMenuClosed(
         internal::MenuControllerDelegate::DONT_NOTIFY_DELEGATE,
         item->GetRootMenuItem(), accept_event_flags_);
+    CHECK(!this_ref);
   }
 
   // WARNING: the call to MenuClosed deletes us.
 
   return drop_target->GetDelegate()->OnPerformDrop(
       drop_target, drop_position, event);
 }
 
 void MenuController::OnDragEnteredScrollButton(SubmenuView* source,
                                                bool is_up) {
@@ skipping 378 matching lines @@
 
     default:
       break;
   }
 }
 
 MenuController::MenuController(bool blocking,
                                internal::MenuControllerDelegate* delegate)
     : blocking_run_(blocking),
       showing_(false),
+      running_(false),
       exit_type_(EXIT_NONE),
       did_capture_(false),
       result_(NULL),
       accept_event_flags_(0),
       drop_target_(NULL),
       drop_position_(MenuDelegate::DROP_UNKNOWN),
       owner_(NULL),
       possible_drag_(false),
       drag_in_progress_(false),
       did_initiate_drag_(false),
@@ skipping 1258 matching lines @@
       delegate_ = delegate_stack_.back().first;
       async_run_ = delegate_stack_.back().second;
     }
   } else {
 #if defined(USE_AURA)
     menu_pre_target_handler_.reset();
 #endif
 
     showing_ = false;
     did_capture_ = false;
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
+    running_ = false;
   }
 
   MenuItemView* result = result_;
   // In case we're nested, reset |result_|.
   result_ = nullptr;
 
   if (exit_type_ == EXIT_OUTERMOST) {
     SetExitType(EXIT_NONE);
   } else {
     if (nested_menu && result) {
@@ skipping 75 matching lines @@
   if (hot_button_)
     hot_button_->SetHotTracked(false);
   hot_button_ = hot_button;
   if (hot_button) {
     hot_button->SetHotTracked(true);
     hot_button->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
   }
 }
 
 }  // namespace views