WindowServer: Clean up some of the code around menu item hovering

We were writing to the currently hovered menu item index in a bunch
of places, which made it very confusing to follow how it changes.

Rename Menu::set_hovered_item() to set_hovered_index() and use it
in more places instead of manipulating m_hovered_item_index.

Userland/Services/WindowServer/Menu.cpp

@@ -300,10 +300,11 @@ MenuItem* Menu::hovered_item() const
 
 void Menu::update_for_new_hovered_item(bool make_input)
 {
-    if (hovered_item() && hovered_item()->is_submenu()) {
+    auto* hovered_item = this->hovered_item();
+    if (hovered_item && hovered_item->is_submenu()) {
         VERIFY(menu_window());
-        MenuManager::the().close_everyone_not_in_lineage(*hovered_item()->submenu());
+        MenuManager::the().close_everyone_not_in_lineage(*hovered_item->submenu());
-        hovered_item()->submenu()->do_popup(hovered_item()->rect().top_right().translated(menu_window()->rect().location()), make_input, true);
+        hovered_item->submenu()->do_popup(hovered_item->rect().top_right().translated(menu_window()->rect().location()), make_input, true);
     } else {
         MenuManager::the().close_everyone_not_in_lineage(*this);
         ensure_menu_window();
@@ -330,7 +331,7 @@ void Menu::descend_into_submenu_at_hovered_item()
     auto submenu = hovered_item()->submenu();
     VERIFY(submenu);
     MenuManager::the().open_menu(*submenu, false);
-    submenu->set_hovered_item(0);
+    submenu->set_hovered_index(0);
     VERIFY(submenu->hovered_item()->type() != MenuItem::Separator);
 }
 
@@ -353,12 +354,7 @@ void Menu::handle_mouse_move_event(const MouseEvent& mouse_event)
     }
 
     int index = item_index_at(mouse_event.position());
-    if (m_hovered_item_index == index)
+    set_hovered_index(index);
-        return;
-    m_hovered_item_index = index;
-
-    update_for_new_hovered_item();
-    return;
 }
 
 void Menu::event(Core::Event& event)
@@ -380,11 +376,7 @@ void Menu::event(Core::Event& event)
         m_scroll_offset = clamp(m_scroll_offset, 0, m_max_scroll_offset);
 
         int index = item_index_at(mouse_event.position());
-        if (m_hovered_item_index == index)
+        set_hovered_index(index);
-            return;
-
-        m_hovered_item_index = index;
-        update_for_new_hovered_item();
         return;
     }
 
@@ -402,62 +394,65 @@ void Menu::event(Core::Event& event)
             int counter = 0;
             for (const auto& item : m_items) {
                 if (item.type() != MenuItem::Separator && item.is_enabled()) {
-                    m_hovered_item_index = counter;
+                    set_hovered_index(counter, key == Key_Right);
                     break;
                 }
                 counter++;
             }
-            update_for_new_hovered_item(key == Key_Right);
             return;
         }
 
         if (key == Key_Up) {
-            VERIFY(m_items.at(0).type() != MenuItem::Separator);
+            VERIFY(item(0).type() != MenuItem::Separator);
 
             if (is_scrollable() && m_hovered_item_index == 0)
                 return;
 
             auto original_index = m_hovered_item_index;
+            auto new_index = original_index;
             do {
-                if (m_hovered_item_index == 0)
+                if (new_index == 0)
-                    m_hovered_item_index = m_items.size() - 1;
+                    new_index = m_items.size() - 1;
                 else
-                    --m_hovered_item_index;
+                    --new_index;
-                if (m_hovered_item_index == original_index)
+                if (new_index == original_index)
                     return;
-            } while (hovered_item()->type() == MenuItem::Separator || !hovered_item()->is_enabled());
+            } while (item(new_index).type() == MenuItem::Separator || !item(new_index).is_enabled());
 
-            VERIFY(m_hovered_item_index >= 0 && m_hovered_item_index <= static_cast<int>(m_items.size()) - 1);
+            VERIFY(new_index >= 0);
+            VERIFY(new_index <= static_cast<int>(m_items.size()) - 1);
 
-            if (is_scrollable() && m_hovered_item_index < m_scroll_offset)
+            if (is_scrollable() && new_index < m_scroll_offset)
                 --m_scroll_offset;
 
-            update_for_new_hovered_item();
+            set_hovered_index(new_index);
             return;
         }
 
         if (key == Key_Down) {
-            VERIFY(m_items.at(0).type() != MenuItem::Separator);
+            VERIFY(item(0).type() != MenuItem::Separator);
 
             if (is_scrollable() && m_hovered_item_index == static_cast<int>(m_items.size()) - 1)
                 return;
 
             auto original_index = m_hovered_item_index;
+            auto new_index = original_index;
             do {
-                if (m_hovered_item_index == static_cast<int>(m_items.size()) - 1)
+                if (new_index == static_cast<int>(m_items.size()) - 1)
-                    m_hovered_item_index = 0;
+                    new_index = 0;
                 else
-                    ++m_hovered_item_index;
+                    ++new_index;
-                if (m_hovered_item_index == original_index)
+                if (new_index == original_index)
                     return;
-            } while (hovered_item()->type() == MenuItem::Separator || !hovered_item()->is_enabled());
+            } while (item(new_index).type() == MenuItem::Separator || !item(new_index).is_enabled());
 
-            VERIFY(m_hovered_item_index >= 0 && m_hovered_item_index <= static_cast<int>(m_items.size()) - 1);
+            VERIFY(new_index >= 0);
+            VERIFY(new_index <= static_cast<int>(m_items.size()) - 1);
 
-            if (is_scrollable() && m_hovered_item_index >= (m_scroll_offset + visible_item_count()))
+            if (is_scrollable() && new_index >= (m_scroll_offset + visible_item_count()))
                 ++m_scroll_offset;
 
-            update_for_new_hovered_item();
+            set_hovered_index(new_index);
             return;
         }
     }

Userland/Services/WindowServer/Menu.h

@@ -101,10 +101,10 @@ public:
 
     MenuItem* hovered_item() const;
 
-    void set_hovered_item(int index)
+    void set_hovered_index(int index, bool make_input = false)
     {
         m_hovered_item_index = index;
-        update_for_new_hovered_item();
+        update_for_new_hovered_item(make_input);
     }
 
     void clear_hovered_item();

Userland/Services/WindowServer/MenuManager.cpp

@@ -96,7 +96,7 @@ void MenuManager::event(Core::Event& event)
                 //        with each keypress instead of activating immediately.
                 auto index = shortcut_item_indexes->at(0);
                 auto& item = m_current_menu->item(index);
-                m_current_menu->set_hovered_item(index);
+                m_current_menu->set_hovered_index(index);
                 if (item.is_submenu())
                     m_current_menu->descend_into_submenu_at_hovered_item();
                 else
@@ -117,7 +117,7 @@ void MenuManager::event(Core::Event& event)
                     set_current_menu(m_open_menu_stack.at(it.index() - 1));
                 else {
                     if (m_current_menu->hovered_item())
-                        m_current_menu->set_hovered_item(-1);
+                        m_current_menu->set_hovered_index(-1);
                     else {
                         auto* target_menu = previous_menu(m_current_menu);
                         if (target_menu) {

Userland/Services/WindowServer/Window.cpp

@@ -493,7 +493,7 @@ void Window::handle_keydown_event(const KeyEvent& event)
         if (menu_to_open) {
             frame().open_menubar_menu(*menu_to_open);
             if (!menu_to_open->is_empty())
-                menu_to_open->set_hovered_item(0);
+                menu_to_open->set_hovered_index(0);
             return;
         }
     }