New MenuController Checks

ui/views/controls/menu/menu_runner_impl.cc

 // Copyright 2014 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_runner_impl.h"
 
 #include "build/build_config.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/menu/menu_controller.h"
@@ skipping 51 matching lines @@
 
     // Verify that the MenuController is still active. It may have been
     // destroyed out of order.
     if (controller_) {
       // Release is invoked when MenuRunner is destroyed. Assume this is
       // happening because the object referencing the menu has been destroyed
       // and the menu button is no longer valid.
       controller_->Cancel(MenuController::EXIT_DESTROYED);
       return;
     }
+
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
+    // Update for the ASAN stack trace to determine if we are in the running
+    // state during the incorrect destruction order.
+    delete this;
+  } else {
+    delete this;
   }
-
-  delete this;
 }
 
 MenuRunner::RunResult MenuRunnerImpl::RunMenuAt(Widget* parent,
                                                 MenuButton* button,
                                                 const gfx::Rect& bounds,
                                                 MenuAnchorPosition anchor,
                                                 int32_t run_types) {
   closing_event_time_ = base::TimeTicks();
   if (running_) {
     // Ignore requests to show the menu while it's already showing. MenuItemView
     // doesn't handle this very well (meaning it crashes).
     return MenuRunner::NORMAL_EXIT;
   }
 
+  // TODO(jonross): remove after tracking down the cause of (crbug.com/683087).
+  // Verify that this was not a delegate previously used for a run, which was
+  // shutdown, but not deleted. Nesting the same delegate multiple times is
+  // dangerous.
+  CHECK(!controller_);
+
   MenuController* controller = MenuController::GetActiveInstance();
   if (controller) {
     if ((run_types & MenuRunner::IS_NESTED) != 0) {
       if (!controller->IsBlockingRun()) {
         controller->CancelAll();
         controller = NULL;
       } else {
         // Only nest the delegate when not cancelling drag-and-drop. When
         // cancelling this will become the root delegate of the new
         // MenuController
@@ skipping 15 matching lines @@
   }
 
   running_ = true;
   async_ = (run_types & MenuRunner::ASYNC) != 0;
   for_drop_ = (run_types & MenuRunner::FOR_DROP) != 0;
   bool has_mnemonics = (run_types & MenuRunner::HAS_MNEMONICS) != 0;
   owns_controller_ = false;
   if (!controller) {
     // No menus are showing, show one.
     controller = new MenuController(!for_drop_, this);
+    // TODO(jonross): remove after tracking down the cause of
+    // (crbug.com/683087).
     owns_controller_ = true;
   }
   controller->SetAsyncRun(async_);
   controller->set_is_combobox((run_types & MenuRunner::COMBOBOX) != 0);
   controller_ = controller->AsWeakPtr();
   menu_->set_controller(controller_.get());
   menu_->PrepareForRun(owns_controller_,
                        has_mnemonics,
                        !for_drop_ && ShouldShowMnemonics(button));
 
@@ skipping 49 matching lines @@
 MenuRunner::RunResult MenuRunnerImpl::MenuDone(NotifyType type,
                                                MenuItemView* result,
                                                int mouse_event_flags) {
   menu_->RemoveEmptyMenus();
   menu_->set_controller(nullptr);
 
   if (owns_controller_ && controller_) {
     // We created the controller and need to delete it.
     delete controller_.get();
     owns_controller_ = false;
+    controller_ = nullptr;
   }
-  controller_ = nullptr;
+
   // Make sure all the windows we created to show the menus have been
   // destroyed.
   menu_->DestroyAllMenuHosts();
   if (delete_after_run_) {
     delete this;
     return MenuRunner::MENU_DELETED;
   }
   running_ = false;
   if (menu_->GetDelegate()) {
     // Executing the command may also delete this.
@@ skipping 18 matching lines @@
 #if defined(OS_WIN)
   // This is only needed on Windows.
   if (!show_mnemonics)
     show_mnemonics = ui::win::IsAltPressed();
 #endif
   return show_mnemonics;
 }
 
 }  // namespace internal
 }  // namespace views