Fixes incorrect clearing of hot-tracked state when context menu is opened from a menu item

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 345 matching lines @@
 
   // If there are multiple matches this is the index of the item after the
   // currently selected item whose mnemonic matches. This may remain -1 even
   // though there are matches.
   int next_match;
 };
 
 // MenuController:State ------------------------------------------------------
 
 MenuController::State::State()
-    : item(NULL),
+    : item(nullptr),
+      hot_button(nullptr),
       submenu_open(false),
       anchor(MENU_ANCHOR_TOPLEFT),
       context_menu(false) {
 }
 
 MenuController::State::State(const State& other) = default;
 
 MenuController::State::~State() {}
 
 // MenuController ------------------------------------------------------------
@@ skipping 40 matching lines @@
       }
     }
   }
 
   bool nested_menu = showing_;
   if (showing_) {
     // Only support nesting of blocking_run menus, nesting of
     // blocking/non-blocking shouldn't be needed.
     DCHECK(blocking_run_);
 
+    state_.hot_button = hot_button_;
+    hot_button_ = nullptr;
     // We're already showing, push the current state.
     menu_stack_.push_back(
         std::make_pair(state_, make_linked_ptr(pressed_lock_.release())));
 
     // The context menu should be owned by the same parent.
     DCHECK_EQ(owner_, parent);
   } else {
     showing_ = true;
 
 #if defined(USE_AURA)
@@ skipping 114 matching lines @@
 
   current_mouse_pressed_state_ |= event.changed_button_flags();
 
   if (forward_to_root) {
     ui::MouseEvent event_for_root(event);
     // Reset hot-tracking if a different view is getting a mouse press.
     ConvertLocatedEventForRootView(source, forward_to_root, &event_for_root);
     View* view =
         forward_to_root->GetEventHandlerForPoint(event_for_root.location());
     CustomButton* button = CustomButton::AsCustomButton(view);
-    if (hot_button_ && hot_button_ != button)
-      SetHotTrackedButton(nullptr);
+    if (hot_button_ != button)
+      SetHotTrackedButton(button);
 
     // Empty menu items are always handled by the menu controller.
     if (!view || view->id() != MenuItemView::kEmptyMenuItemViewID) {
       bool processed = forward_to_root->ProcessMousePressed(event_for_root);
       // If the event was processed, the root view becomes our current mouse
       // handler...
       if (processed && !current_mouse_event_target_) {
         current_mouse_event_target_ = forward_to_root;
       }
 
@@ skipping 254 matching lines @@
 
 void MenuController::ViewHierarchyChanged(
     SubmenuView* source,
     const View::ViewHierarchyChangedDetails& details) {
   if (!details.is_add) {
     // If the current mouse handler is removed, remove it as the handler.
     if (details.child == current_mouse_event_target_) {
       current_mouse_event_target_ = nullptr;
       current_mouse_pressed_state_ = 0;
     }
-    // Update |hot_button_| if it gets removed while a menu is up.
-    if (details.child == hot_button_)
+    // Update |hot_button_| (both in |this| and in |menu_stack_| if it gets
+    // removed while a menu is up.
+    if (details.child == hot_button_) {
       hot_button_ = nullptr;
+      for (auto nested_state : menu_stack_) {
+        State& state = nested_state.first;
+        if (details.child == state.hot_button)
+          state.hot_button = nullptr;
+      }
+    }
   }
 }
 
 bool MenuController::GetDropFormats(
       SubmenuView* source,
       int* formats,
       std::set<ui::Clipboard::FormatType>* format_types) {
   return source->GetMenuItem()->GetDelegate()->GetDropFormats(
       source->GetMenuItem(), formats, format_types);
 }
@@ skipping 1692 matching lines @@
 #endif
 
   linked_ptr<MenuButton::PressedLock> nested_pressed_lock;
   bool nested_menu = !menu_stack_.empty();
   if (nested_menu) {
     DCHECK(!menu_stack_.empty());
     // We're running from within a menu, restore the previous state.
     // The menus are already showing, so we don't have to show them.
     state_ = menu_stack_.back().first;
     pending_state_ = menu_stack_.back().first;
+    hot_button_ = state_.hot_button;
     nested_pressed_lock = menu_stack_.back().second;
     menu_stack_.pop_back();
     // Even though the menus are nested, there may not be nested delegates.
     if (delegate_stack_.size() > 1) {
       delegate_stack_.pop_back();
       delegate_ = delegate_stack_.back().first;
       async_run_ = delegate_stack_.back().second;
     }
   } else {
 #if defined(USE_AURA)
@@ skipping 19 matching lines @@
       SetSelection(nullptr, SELECTION_UPDATE_IMMEDIATELY | SELECTION_EXIT);
 
       // Set exit_all_, which makes sure all nested loops exit immediately.
       if (exit_type_ != EXIT_DESTROYED)
         SetExitType(EXIT_ALL);
     } else {
       TerminateNestedMessageLoopIfNecessary();
     }
   }
 
-  // Reset our pressed lock to the previous state's, if there was one.
-  // The lock handles the case if the button was destroyed.
+  // Reset our pressed lock and hot-tracked state to the previous state's, if
+  // they were active. The lock handles the case if the button was destroyed.
   pressed_lock_.reset(nested_pressed_lock.release());
+  if (hot_button_)
+    hot_button_->SetHotTracked(true);

[jonross, 2016/03/24 18:48:18]

Is it possible for this hot_button_ to no longer be valid if we have
CloseAllNestedMenus()? (line 2591)

[sky, 2016/03/24 19:12:56]

In looking at it, this method is confusingly named. It's really
HideAllNestedMenus. So, I believe you would still get a ViewHierarchyChanged()
if after CloseAllNestedMenus() the button is deleted.

 
   return result;
 }
 
 void MenuController::HandleMouseLocation(SubmenuView* source,
                                          const gfx::Point& mouse_location) {
   if (showing_submenu_)
     return;
 
   // Ignore mouse events if we're closing the menu.
@@ skipping 42 matching lines @@
     return;
   }
   if (hot_button_)
     hot_button_->SetHotTracked(false);
   hot_button_ = hot_button;
   if (hot_button)
     hot_button->SetHotTracked(true);
 }
 
 }  // namespace views