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

Broken with the last change here:
https://codereview.chromium.org/1741093002

Since menus can be nested using the same MenuController I think it would be correct to move |hot_button_| and its accessors into MenuItemView. This way there can be multiple views in hot-tracked state in the same MenuController as long as only one hot-tracked view is in any given menu item.

BUG=592359

Review URL: https://codereview.chromium.org/1775533002

Cr-Commit-Position: refs/heads/master@{#383128}

3 files changed, 98 insertions, 9 deletions

diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index 23c2e1b..3e6ebe3 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -363,7 +363,8 @@ struct MenuController::SelectByCharDetails {
 // MenuController:State ------------------------------------------------------
 
 MenuController::State::State()
-    : item(NULL),
+    : item(nullptr),
+      hot_button(nullptr),
       submenu_open(false),
       anchor(MENU_ANCHOR_TOPLEFT),
       context_menu(false) {
@@ -424,6 +425,8 @@ MenuItemView* MenuController::Run(Widget* parent,
     // 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())));
@@ -558,8 +561,8 @@ bool MenuController::OnMousePressed(SubmenuView* source,
     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) {
@@ -834,9 +837,16 @@ void MenuController::ViewHierarchyChanged(
       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;
+      }
+    }
   }
 }
 
@@ -2549,6 +2559,7 @@ MenuItemView* MenuController::ExitMenuRun() {
     // 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.
@@ -2588,9 +2599,11 @@ MenuItemView* MenuController::ExitMenuRun() {
     }
   }
 
-  // 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);
 
   return result;
 }
diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
index 3741f76..dc21826 100644
--- a/ui/views/controls/menu/menu_controller.h
+++ b/ui/views/controls/menu/menu_controller.h
@@ -243,6 +243,10 @@ class VIEWS_EXPORT MenuController : public WidgetObserver {
     // The selected menu item.
     MenuItemView* item;
 
+    // Used to capture a hot tracked child button when a nested menu is opened
+    // and to restore the hot tracked state when exiting a nested menu.
+    CustomButton* hot_button;
+
     // If item has a submenu this indicates if the submenu is showing.
     bool submenu_open;
 
@@ -678,7 +682,7 @@ class VIEWS_EXPORT MenuController : public WidgetObserver {
   // screen coordinates). Otherwise this will be (0, 0).
   gfx::Point menu_start_mouse_press_loc_;
 
-  // Controls behviour differences between an asynchronous run, and other types
+  // Controls behaviour differences between an asynchronous run, and other types
   // of run (blocking, drag and drop).
   bool async_run_;
 
diff --git a/ui/views/controls/menu/menu_controller_unittest.cc b/ui/views/controls/menu/menu_controller_unittest.cc
index aa26ba9..ee26fb2 100644
--- a/ui/views/controls/menu/menu_controller_unittest.cc
+++ b/ui/views/controls/menu/menu_controller_unittest.cc
@@ -461,6 +461,19 @@ class MenuControllerTest : public ViewsTestBase {
     menu_item()->GetSubmenu()->ShowAt(owner(), menu_item()->bounds(), false);
   }
 
+  CustomButton* GetHotButton() {
+    return menu_controller_->hot_button_;
+  }
+
+  void SetHotTrackedButton(CustomButton* hot_button) {
+    menu_controller_->SetHotTrackedButton(hot_button);
+  }
+
+  void ExitMenuRun() {
+    menu_controller_->SetExitType(MenuController::ExitType::EXIT_OUTERMOST);
+    menu_controller_->ExitMenuRun();
+  }
+
  private:
   void DestroyMenuController() {
     if (!menu_controller_)
@@ -833,6 +846,65 @@ TEST_F(MenuControllerTest, DeleteChildButtonView) {
   EXPECT_FALSE(button3->IsHotTracked());
 }
 
+// Creates a menu with CustomButton child views, simulates running a nested
+// menu and tests that existing the nested run restores hot-tracked child view.
+TEST_F(MenuControllerTest, ChildButtonHotTrackedWhenNested) {
+  AddButtonMenuItems();
+
+  // Handle searching for 'f'; should find "Four".
+  SelectByChar('f');
+  EXPECT_EQ(4, pending_state_item()->GetCommand());
+
+  View* buttons_view = menu_item()->GetSubmenu()->child_at(4);
+  ASSERT_NE(nullptr, buttons_view);
+  CustomButton* button1 =
+      CustomButton::AsCustomButton(buttons_view->child_at(0));
+  ASSERT_NE(nullptr, button1);
+  CustomButton* button2 =
+      CustomButton::AsCustomButton(buttons_view->child_at(1));
+  ASSERT_NE(nullptr, button2);
+  CustomButton* button3 =
+      CustomButton::AsCustomButton(buttons_view->child_at(2));
+  ASSERT_NE(nullptr, button2);
+  EXPECT_FALSE(button1->IsHotTracked());
+  EXPECT_FALSE(button2->IsHotTracked());
+  EXPECT_FALSE(button3->IsHotTracked());
+
+  // Increment twice to move selection to |button2|.
+  IncrementSelection();
+  IncrementSelection();
+  EXPECT_EQ(5, pending_state_item()->GetCommand());
+  EXPECT_FALSE(button1->IsHotTracked());
+  EXPECT_TRUE(button2->IsHotTracked());
+  EXPECT_FALSE(button3->IsHotTracked());
+  EXPECT_EQ(button2, GetHotButton());
+
+  MenuController* controller = menu_controller();
+  controller->SetAsyncRun(true);
+  int mouse_event_flags = 0;
+  MenuItemView* run_result =
+      controller->Run(owner(), nullptr, menu_item(), gfx::Rect(),
+                      MENU_ANCHOR_TOPLEFT, false, false, &mouse_event_flags);
+  EXPECT_EQ(run_result, nullptr);
+
+  // |button2| should stay in hot-tracked state but menu controller should not
+  // track it anymore (preventing resetting hot-tracked state when changing
+  // selection while a nested run is active).
+  EXPECT_TRUE(button2->IsHotTracked());
+  EXPECT_EQ(nullptr, GetHotButton());
+
+  // Setting hot-tracked button while nested should get reverted when nested
+  // menu run ends.
+  SetHotTrackedButton(button1);
+  EXPECT_TRUE(button1->IsHotTracked());
+  EXPECT_EQ(button1, GetHotButton());
+
+  ExitMenuRun();
+  EXPECT_FALSE(button1->IsHotTracked());
+  EXPECT_TRUE(button2->IsHotTracked());
+  EXPECT_EQ(button2, GetHotButton());
+}
+
 // Tests that a menu opened asynchronously, will notify its
 // MenuControllerDelegate when Accept is called.
 TEST_F(MenuControllerTest, AsynchronousAccept) {
@@ -1038,7 +1110,7 @@ TEST_F(MenuControllerTest, AsynchronousTouchEventRepostEvent) {
   TestMenuControllerDelegate* delegate = menu_controller_delegate();
   controller->SetAsyncRun(true);
 
-  // Show a sub menu to targert with a touch event. However have the event occur
+  // Show a sub menu to target with a touch event. However have the event occur
   // outside of the bounds of the entire menu.
   MenuItemView* item = menu_item();
   SubmenuView* sub_menu = item->GetSubmenu();