Fix MenuRunner Releasing

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 1008 matching lines @@
   drag_in_progress_ = false;
   // During a drag, mouse events are processed directly by the widget, and not
   // sent to the MenuController. At drag completion, reset pressed state and
   // the event target.
   current_mouse_pressed_state_ = 0;
   current_mouse_event_target_ = nullptr;
 
   // Only attempt to close if the MenuHost said to.
   if (should_close) {
     if (showing_) {
-      // Close showing widgets.
+      // During a drag operation there are several ways in which this can be
+      // canceled and deleted. Verify that this is still active before closing
+      // the widgets.
       if (GetActiveInstance() == this) {
+        base::WeakPtr<MenuController> this_ref = AsWeakPtr();
         CloseAllNestedMenus();
         Cancel(EXIT_ALL);
+        // The above may have deleted us. If not perform a full shutdown.
+        if (!this_ref)
+          return;
+        ExitAsyncRun();
       }
-      // The above may have deleted us. If not perform a full shutdown.
-      if (GetActiveInstance() == this)
-        ExitAsyncRun();
     } else if (exit_type_ == EXIT_ALL) {
       // We may have been canceled during the drag. If so we still need to fully
       // shutdown.
       ExitAsyncRun();
     }
   }
 }
 
 ui::PostDispatchAction MenuController::OnWillDispatchKeyEvent(
     ui::KeyEvent* event) {
   if (exit_type() == EXIT_ALL || exit_type() == EXIT_DESTROYED) {
     // If the event has arrived after the menu's exit type had changed but
     // before its message loop terminated, the event will continue its normal
     // propagation for the following reason:
     // If the user accepts a menu item in a nested menu, the menu item action is
     // run after the base::RunLoop for the innermost menu has quit but before
     // the base::RunLoop for the outermost menu has quit. If the menu item
     // action starts a base::RunLoop, the outermost menu's base::RunLoop will
     // not quit till the action's base::RunLoop ends. IDC_BOOKMARK_BAR_OPEN_ALL
     // sometimes opens a modal dialog. The modal dialog starts a base::RunLoop
     // and keeps the base::RunLoop running for the duration of its lifetime.
     TerminateNestedMessageLoopIfNecessary();
     return ui::POST_DISPATCH_PERFORM_DEFAULT;
   }
 
   event->StopPropagation();
 
   if (event->type() == ui::ET_KEY_PRESSED) {
+    base::WeakPtr<MenuController> this_ref = AsWeakPtr();
     OnKeyDown(event->key_code());
-    // Menu controller might have been deleted.
-    if (!GetActiveInstance())
+    // Key events can lead to this being deleted.
+    if (!this_ref)
       return ui::POST_DISPATCH_NONE;
 
     // Do not check mnemonics if the Alt or Ctrl modifiers are pressed. For
     // example Ctrl+<T> is an accelerator, but <T> only is a mnemonic.
     const int kKeyFlagsMask = ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN;
     const int flags = event->flags();
     if (exit_type() == EXIT_NONE && (flags & kKeyFlagsMask) == 0) {
       base::char16 c = event->GetCharacter();
       SelectByChar(c);
-      // Menu controller might have been deleted.
-      if (!GetActiveInstance())
+      // SelectByChar can lead to this being deleted.
+      if (!this_ref)
         return ui::POST_DISPATCH_NONE;
     }
   }
 
   if (!TerminateNestedMessageLoopIfNecessary()) {
     ui::Accelerator accelerator(*event);
     ViewsDelegate::ProcessMenuAcceleratorResult result =
         ViewsDelegate::GetInstance()->ProcessAcceleratorWhileMenuShowing(
             accelerator);
     if (result == ViewsDelegate::ProcessMenuAcceleratorResult::CLOSE_MENU)
@@ skipping 16 matching lines @@
 void MenuController::OnWidgetDestroying(Widget* widget) {
   DCHECK_EQ(owner_, widget);
   owner_->RemoveObserver(this);
   owner_ = NULL;
 }
 
 bool MenuController::IsCancelAllTimerRunningForTest() {
   return cancel_all_timer_.IsRunning();
 }
 
+void MenuController::ClearStateForTest() {
+  state_ = State();
+  pending_state_ = State();
+}
+
 // static
 void MenuController::TurnOffMenuSelectionHoldForTest() {
   menu_selection_hold_time_ms = -1;
 }
 
 void MenuController::SetSelection(MenuItemView* menu_item,
                                   int selection_types) {
   size_t paths_differ_at = 0;
   std::vector<MenuItemView*> current_path;
   std::vector<MenuItemView*> new_path;
@@ skipping 117 matching lines @@
   item->PaintButton(canvas.get(), MenuItemView::PB_FOR_DRAG);
 
   OSExchangeData data;
   item->GetDelegate()->WriteDragData(item, &data);
   drag_utils::SetDragImageOnDataObject(*canvas,
                                        press_loc.OffsetFromOrigin(),
                                        &data);
   StopScrolling();
   int drag_ops = item->GetDelegate()->GetDragOperations(item);
   did_initiate_drag_ = true;
+  base::WeakPtr<MenuController> this_ref = AsWeakPtr();
   // TODO(varunjain): Properly determine and send DRAG_EVENT_SOURCE below.
   item->GetWidget()->RunShellDrag(NULL, data, widget_loc, drag_ops,
       ui::DragDropTypes::DRAG_EVENT_SOURCE_MOUSE);
-  // MenuController may have been deleted if |async_run_| so check for an active
-  // instance before accessing member variables.
-  if (GetActiveInstance() == this)
+  // MenuController may have been deleted if |async_run_| so check before
+  // accessing member variables.
+  if (this_ref)
     did_initiate_drag_ = false;
 }
 
 void MenuController::OnKeyDown(ui::KeyboardCode key_code) {
   // Do not process while performing drag-and-drop
   if (!blocking_run_)
     return;
 
   switch (key_code) {
     case ui::VKEY_UP:
@@ skipping 140 matching lines @@
 
 void MenuController::RunMessageLoop() {
   message_loop_->Run();
 }
 
 bool MenuController::SendAcceleratorToHotTrackedView() {
   CustomButton* hot_view = GetFirstHotTrackedView(pending_state_.item);
   if (!hot_view)
     return false;
 
+  base::WeakPtr<MenuController> this_ref = AsWeakPtr();
   ui::Accelerator accelerator(ui::VKEY_RETURN, ui::EF_NONE);
   hot_view->AcceleratorPressed(accelerator);
   // An accelerator may have canceled the menu after activation.
-  if (GetActiveInstance()) {
+  if (this_ref) {
     CustomButton* button = static_cast<CustomButton*>(hot_view);
     SetHotTrackedButton(button);
   }
   return true;
 }
 
 void MenuController::UpdateInitialLocation(const gfx::Rect& bounds,
                                            MenuAnchorPosition position,
                                            bool context_menu) {
   pending_state_.context_menu = context_menu;
@@ skipping 945 matching lines @@
   gfx::NativeView native_view = source->GetWidget()->GetNativeView();
   gfx::NativeWindow window = nullptr;
   if (native_view) {
     display::Screen* screen = display::Screen::GetScreen();
     window = screen->GetWindowAtScreenPoint(screen_loc);
   }
 #endif
 
 #if defined(OS_WIN)
   if (event->IsMouseEvent() || event->IsTouchEvent()) {
+    base::WeakPtr<MenuController> this_ref = AsWeakPtr();
     bool async_run = async_run_;
     if (state_.item) {
       state_.item->GetRootMenuItem()->GetSubmenu()->ReleaseCapture();
+      // We're going to close and we own the event capture. We need to repost
+      // the event, otherwise the window the user clicked on won't get the
+      // event.
       RepostEventImpl(event, screen_loc, native_view, window);
     } else {
       // We some times get an event after closing all the menus. Ignore it. Make
       // sure the menu is in fact not visible. If the menu is visible, then
       // we're in a bad state where we think the menu isn't visibile but it is.
       DCHECK(!source->GetWidget()->IsVisible());
     }
 
-    // We're going to close and we own the event capture. We need to repost the
-    // event, otherwise the window the user clicked on won't get the event.
-    //  RepostEvent(source, event, screen_loc, native_view, window);
-    // MenuController may have been deleted if |async_run_| so check for an
-    // active
-    // instance before accessing member variables.
-    if (!GetActiveInstance()) {
+    // Reposting the event may have deleted this, if so exit.
+    if (!this_ref) {
       DCHECK(async_run);
       return;
     }
   }
 #endif
 
   // Determine target to see if a complete or partial close of the menu should
   // occur.
   ExitType exit_type = EXIT_ALL;
   if (!menu_stack_.empty()) {
@@ skipping 157 matching lines @@
 void MenuController::ExitAsyncRun() {
   if (!async_run_)
     return;
   bool nested = delegate_stack_.size() > 1;
   // ExitMenuRun unwinds nested delegates
   internal::MenuControllerDelegate* delegate = delegate_;
   // MenuController may have been deleted when releasing ViewsDelegate ref.
   // However as |delegate| can outlive this, it must still be notified of the
   // menu closing so that it can perform teardown.
   int accept_event_flags = accept_event_flags_;
+  base::WeakPtr<MenuController> this_ref = AsWeakPtr();
   MenuItemView* result = ExitMenuRun();
   delegate->OnMenuClosed(internal::MenuControllerDelegate::NOTIFY_DELEGATE,
                          result, accept_event_flags);
-  // MenuController may have been deleted by |delegate|.
-  if (GetActiveInstance() && nested && exit_type_ == EXIT_ALL)
+  // |delegate| may have deleted this.
+  if (this_ref && nested && exit_type_ == EXIT_ALL)
     ExitAsyncRun();
 }
 
 MenuItemView* MenuController::ExitMenuRun() {
+  base::WeakPtr<MenuController> this_ref = AsWeakPtr();
+
   // Release the lock which prevents Chrome from shutting down while the menu is
   // showing.
   if (async_run_ && ViewsDelegate::GetInstance())
     ViewsDelegate::GetInstance()->ReleaseRef();
 
   // Releasing the lock can result in Chrome shutting down, deleting this.
-  if (!GetActiveInstance())
+  if (!this_ref)
     return nullptr;
 
   // Close any open menus.
   SetSelection(nullptr, SELECTION_UPDATE_IMMEDIATELY | SELECTION_EXIT);
 
 #if defined(OS_WIN)
   // On Windows, if we select the menu item by touch and if the window at the
   // location is another window on the same thread, that window gets a
   // WM_MOUSEACTIVATE message and ends up activating itself, which is not
   // correct. We workaround this by setting a property on the window at the
@@ skipping 124 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