WindowManager: Move more of the menu management logic to WSMenuManager

This patch moves a whole lot of the menu logic from WSWindowManager to
its proper home in WSMenuManager.

We also get rid of the "close_current_menu()" concept which was easily
confused in the presence of submenus. All operations should now be
aware of the menu stack instead. (The concept of a single, current menu
made a lot more sense when there were no nested menus.)

Servers/WindowServer/WSClientConnection.cpp

@@ -388,7 +388,7 @@ void WSClientConnection::handle_request(const WSAPIDestroyMenuRequest& request)
         return;
     }
     auto& menu = *(*it).value;
-    WSWindowManager::the().close_menu(menu);
+    menu.close();
     m_menus.remove(it);
     remove_child(menu);
     WSAPI_ServerMessage response;

Servers/WindowServer/WSMenu.cpp

@@ -198,30 +198,19 @@ void WSMenu::draw()
     }
 }
 
-void close_everyone_not_in_lineage(WSMenu& menu)
-{
-    for (auto& open_menu : WSWindowManager::the().menu_manager().open_menu_stack()) {
-        if (!open_menu)
-            continue;
-        if (&menu == open_menu.ptr() || open_menu->is_menu_ancestor_of(menu))
-            continue;
-        open_menu->menu_window()->set_visible(false);
-    }
-}
-
 void WSMenu::event(CEvent& event)
 {
     if (event.type() == WSEvent::MouseMove) {
         ASSERT(menu_window());
         auto* item = item_at(static_cast<const WSMouseEvent&>(event).position());
-        if (!item || m_hovered_item == item)
+        if (m_hovered_item == item)
             return;
         m_hovered_item = item;
-        if (m_hovered_item->is_submenu()) {
-            close_everyone_not_in_lineage(*m_hovered_item->submenu());
+        if (m_hovered_item && m_hovered_item->is_submenu()) {
+            WSWindowManager::the().menu_manager().close_everyone_not_in_lineage(*m_hovered_item->submenu());
             m_hovered_item->submenu()->popup(m_hovered_item->rect().top_right().translated(menu_window()->rect().location()), true);
         } else {
-            close_everyone_not_in_lineage(*this);
+            WSWindowManager::the().menu_manager().close_everyone_not_in_lineage(*this);
         }
         redraw();
         return;
@@ -253,7 +242,7 @@ void WSMenu::did_activate(WSMenuItem& item)
     if (on_item_activation)
         on_item_activation(item);
 
-    close();
+    WSWindowManager::the().menu_manager().close_everyone();
 
     WSAPI_ServerMessage message;
     message.type = WSAPI_ServerMessage::Type::MenuItemActivated;
@@ -284,9 +273,7 @@ WSMenuItem* WSMenu::item_at(const Point& position)
 
 void WSMenu::close()
 {
-    WSWindowManager::the().close_menu(*this);
-    if (menu_window())
-        menu_window()->set_visible(false);
+    WSWindowManager::the().menu_manager().close_menu_and_descendants(*this);
 }
 
 void WSMenu::popup(const Point& position, bool is_submenu)
@@ -305,7 +292,7 @@ void WSMenu::popup(const Point& position, bool is_submenu)
 
     window.move_to(adjusted_pos);
     window.set_visible(true);
-    WSWindowManager::the().set_current_menu(this, is_submenu);
+    WSWindowManager::the().menu_manager().set_current_menu(this, is_submenu);
 }
 
 bool WSMenu::is_menu_ancestor_of(const WSMenu& other) const

Servers/WindowServer/WSMenu.h

@@ -29,6 +29,7 @@ public:
 
     bool is_empty() const { return m_items.is_empty(); }
     int item_count() const { return m_items.size(); }
+    const WSMenuItem& item(int index) const { return m_items.at(index); }
 
     void add_item(NonnullOwnPtr<WSMenuItem>&& item) { m_items.append(move(item)); }
 

Servers/WindowServer/WSMenuManager.cpp

@@ -143,23 +143,24 @@ void WSMenuManager::handle_menu_mouse_event(WSMenu& menu, const WSMouseEvent& ev
         && !m_open_menu_stack.is_empty()
         && (m_open_menu_stack.first()->menubar() || m_open_menu_stack.first() == wm.system_menu());
     bool is_mousedown_with_left_button = event.type() == WSMouseEvent::MouseDown && event.button() == MouseButton::Left;
-    bool should_open_menu = &menu != wm.current_menu() && (is_hover_with_any_menu_open || is_mousedown_with_left_button);
+    bool should_open_menu = &menu != m_current_menu && (is_hover_with_any_menu_open || is_mousedown_with_left_button);
 
     if (should_open_menu) {
-        if (wm.current_menu() == &menu)
+        if (m_current_menu == &menu)
             return;
-        wm.close_current_menu();
+        close_everyone();
         if (!menu.is_empty()) {
             auto& menu_window = menu.ensure_menu_window();
             menu_window.move_to({ menu.rect_in_menubar().x(), menu.rect_in_menubar().bottom() + 2 });
             menu_window.set_visible(true);
         }
-        wm.set_current_menu(&menu);
+        set_current_menu(&menu);
         refresh();
         return;
     }
     if (event.type() == WSMouseEvent::MouseDown && event.button() == MouseButton::Left) {
-        wm.close_current_menu();
+        close_everyone();
+        set_current_menu(nullptr);
         return;
     }
 }
@@ -167,4 +168,75 @@ void WSMenuManager::handle_menu_mouse_event(WSMenu& menu, const WSMouseEvent& ev
 void WSMenuManager::set_needs_window_resize()
 {
     m_needs_window_resize = true;
-}
+}
+
+void WSMenuManager::close_everyone()
+{
+    for (auto& menu : m_open_menu_stack) {
+        if (menu && menu->menu_window())
+            menu->menu_window()->set_visible(false);
+    }
+    m_open_menu_stack.clear();
+    refresh();
+}
+
+void WSMenuManager::close_everyone_not_in_lineage(WSMenu& menu)
+{
+    Vector<WSMenu*> menus_to_close;
+    for (auto& open_menu : m_open_menu_stack) {
+        if (!open_menu)
+            continue;
+        if (&menu == open_menu.ptr() || open_menu->is_menu_ancestor_of(menu))
+            continue;
+        menus_to_close.append(open_menu);
+    }
+    close_menus(menus_to_close);
+}
+
+void WSMenuManager::close_menus(const Vector<WSMenu*>& menus)
+{
+    for (auto& menu : menus) {
+        if (menu == m_current_menu)
+            m_current_menu = nullptr;
+        if (menu->menu_window())
+            menu->menu_window()->set_visible(false);
+        m_open_menu_stack.remove_first_matching([&](auto& entry) {
+            return entry == menu;
+        });
+    }
+    refresh();
+}
+
+static void collect_menu_subtree(WSMenu& menu, Vector<WSMenu*>& menus)
+{
+    menus.append(&menu);
+    for (int i = 0; i < menu.item_count(); ++i) {
+        auto& item = menu.item(i);
+        if (!item.is_submenu())
+            continue;
+        collect_menu_subtree(*const_cast<WSMenuItem&>(item).submenu(), menus);
+    }
+}
+
+void WSMenuManager::close_menu_and_descendants(WSMenu& menu)
+{
+    Vector<WSMenu*> menus_to_close;
+    collect_menu_subtree(menu, menus_to_close);
+    close_menus(menus_to_close);
+}
+
+void WSMenuManager::set_current_menu(WSMenu* menu, bool is_submenu)
+{
+    if (!is_submenu && m_current_menu)
+        m_current_menu->close();
+    if (menu)
+        m_current_menu = menu->make_weak_ptr();
+
+    if (!is_submenu) {
+        close_everyone();
+        if (menu)
+            m_open_menu_stack.append(menu->make_weak_ptr());
+    } else {
+        m_open_menu_stack.append(menu->make_weak_ptr());
+    }
+}

Servers/WindowServer/WSMenuManager.h

@@ -23,7 +23,16 @@ public:
 
     void set_needs_window_resize();
 
+    WSMenu* current_menu() { return m_current_menu.ptr(); }
+    void set_current_menu(WSMenu*, bool is_submenu = false);
+
+    void close_everyone();
+    void close_everyone_not_in_lineage(WSMenu&);
+    void close_menu_and_descendants(WSMenu&);
+
 private:
+    void close_menus(const Vector<WSMenu*>&);
+
     WSWindow& window() { return *m_window; }
     const WSWindow& window() const { return *m_window; }
 
@@ -37,6 +46,7 @@ private:
     String m_username;
     RefPtr<CTimer> m_timer;
 
+    WeakPtr<WSMenu> m_current_menu;
     Vector<WeakPtr<WSMenu>> m_open_menu_stack;
 
     bool m_needs_window_resize;

Servers/WindowServer/WSWindowManager.cpp

@@ -45,15 +45,6 @@ WSWindowManager::WSWindowManager()
 
     reload_config(false);
 
-    struct AppMenuItem {
-        String binary_name;
-        String description;
-        String icon_path;
-        String category;
-    };
-
-    Vector<AppMenuItem> apps;
-
     CDirIterator dt("/res/apps", CDirIterator::SkipDots);
     while (dt.has_next()) {
         auto af_name = dt.next_path();
@@ -65,23 +56,23 @@ WSWindowManager::WSWindowManager()
         auto app_executable = af->read_entry("App", "Executable");
         auto app_category = af->read_entry("App", "Category");
         auto app_icon_path = af->read_entry("Icons", "16x16");
-        apps.append({ app_executable, app_name, app_icon_path, app_category });
+        m_apps.append({ app_executable, app_name, app_icon_path, app_category });
     }
 
     u8 system_menu_name[] = { 0xc3, 0xb8, 0 };
     m_system_menu = WSMenu::construct(nullptr, -1, String((const char*)system_menu_name));
 
     // First we construct all the necessary app category submenus.
-    for (const auto& app : apps) {
+    for (const auto& app : m_apps) {
         if (app.category.is_null())
             continue;
         if (m_app_category_menus.contains(app.category))
             continue;
         auto category_menu = WSMenu::construct(nullptr, 5000 + m_app_category_menus.size(), app.category);
-        category_menu->on_item_activation = [apps](auto& item) {
-            if (item.identifier() >= 1 && item.identifier() <= 1u + apps.size() - 1) {
+        category_menu->on_item_activation = [this](auto& item) {
+            if (item.identifier() >= 1 && item.identifier() <= 1u + m_apps.size() - 1) {
                 if (fork() == 0) {
-                    const auto& bin = apps[item.identifier() - 1].binary_name;
+                    const auto& bin = m_apps[item.identifier() - 1].executable;
                     execl(bin.characters(), bin.characters(), nullptr);
                     ASSERT_NOT_REACHED();
                 }
@@ -95,9 +86,9 @@ WSWindowManager::WSWindowManager()
 
     // Then we create and insert all the app menu items into the right place.
     int app_identifier = 1;
-    for (const auto& app : apps) {
+    for (const auto& app : m_apps) {
         auto parent_menu = m_app_category_menus.get(app.category).value_or(*m_system_menu);
-        parent_menu->add_item(make<WSMenuItem>(*m_system_menu, app_identifier++, app.description, String(), true, false, false, load_png(app.icon_path)));
+        parent_menu->add_item(make<WSMenuItem>(*m_system_menu, app_identifier++, app.name, String(), true, false, false, load_png(app.icon_path)));
     }
 
     m_system_menu->add_item(make<WSMenuItem>(*m_system_menu, WSMenuItem::Separator));
@@ -106,10 +97,10 @@ WSWindowManager::WSWindowManager()
     m_system_menu->add_item(make<WSMenuItem>(*m_system_menu, 200, "About...", String(), true, false, false, load_png("/res/icons/16x16/ladybug.png")));
     m_system_menu->add_item(make<WSMenuItem>(*m_system_menu, WSMenuItem::Separator));
     m_system_menu->add_item(make<WSMenuItem>(*m_system_menu, 300, "Shutdown..."));
-    m_system_menu->on_item_activation = [this, apps](WSMenuItem& item) {
-        if (item.identifier() >= 1 && item.identifier() <= 1u + apps.size() - 1) {
+    m_system_menu->on_item_activation = [this](WSMenuItem& item) {
+        if (item.identifier() >= 1 && item.identifier() <= 1u + m_apps.size() - 1) {
             if (fork() == 0) {
-                const auto& bin = apps[item.identifier() - 1].binary_name;
+                const auto& bin = m_apps[item.identifier() - 1].executable;
                 execl(bin.characters(), bin.characters(), nullptr);
                 ASSERT_NOT_REACHED();
             }
@@ -249,24 +240,6 @@ int WSWindowManager::menubar_menu_margin() const
     return 16;
 }
 
-void WSWindowManager::set_current_menu(WSMenu* menu, bool is_submenu)
-{
-    if (m_current_menu == menu)
-        return;
-    if (!is_submenu && m_current_menu)
-        m_current_menu->close();
-    if (menu)
-        m_current_menu = menu->make_weak_ptr();
-
-    if (!is_submenu) {
-        m_menu_manager.open_menu_stack().clear();
-        if (menu)
-            m_menu_manager.open_menu_stack().append(menu->make_weak_ptr());
-    } else {
-        m_menu_manager.open_menu_stack().append(menu->make_weak_ptr());
-    }
-}
-
 void WSWindowManager::set_current_menubar(WSMenuBar* menubar)
 {
     if (menubar)
@@ -434,19 +407,6 @@ void WSWindowManager::pick_new_active_window()
     });
 }
 
-void WSWindowManager::close_current_menu()
-{
-    if (m_current_menu && m_current_menu->menu_window())
-        m_current_menu->menu_window()->set_visible(false);
-    m_current_menu = nullptr;
-    for (auto& menu : m_menu_manager.open_menu_stack()) {
-        if (menu && menu->menu_window())
-            menu->menu_window()->set_visible(false);
-    }
-    m_menu_manager.open_menu_stack().clear();
-    m_menu_manager.refresh();
-}
-
 void WSWindowManager::start_window_drag(WSWindow& window, const WSMouseEvent& event)
 {
 #ifdef DRAG_DEBUG
@@ -761,14 +721,17 @@ void WSWindowManager::process_mouse_event(WSMouseEvent& event, WSWindow*& hovere
         return;
     }
 
-    if (m_current_menu && m_current_menu->menu_window()) {
-        auto& window = *m_current_menu->menu_window();
-        bool event_is_inside_current_menu = window.rect().contains(event.position());
+    if (!menu_manager().open_menu_stack().is_empty()) {
+        auto* topmost_menu = menu_manager().open_menu_stack().last().ptr();
+        ASSERT(topmost_menu);
+        auto* window = topmost_menu->menu_window();
+        ASSERT(window);
+        bool event_is_inside_current_menu = window->rect().contains(event.position());
         if (!event_is_inside_current_menu) {
-            if (m_current_menu->hovered_item())
-                m_current_menu->clear_hovered_item();
+            if (topmost_menu->hovered_item())
+                topmost_menu->clear_hovered_item();
             if (event.type() == WSEvent::MouseDown || event.type() == WSEvent::MouseUp)
-                close_current_menu();
+                m_menu_manager.close_everyone();
             if (event.type() == WSEvent::MouseMove) {
                 for (auto& menu : m_menu_manager.open_menu_stack()) {
                     if (!menu)
@@ -782,9 +745,9 @@ void WSWindowManager::process_mouse_event(WSMouseEvent& event, WSWindow*& hovere
                 }
             }
         } else {
-            hovered_window = &window;
-            auto translated_event = event.translated(-window.position());
-            deliver_mouse_event(window, translated_event);
+            hovered_window = window;
+            auto translated_event = event.translated(-window->position());
+            deliver_mouse_event(*window, translated_event);
         }
         return;
     }
@@ -1060,12 +1023,6 @@ void WSWindowManager::invalidate(const WSWindow& window, const Rect& rect)
     invalidate(inner_rect);
 }
 
-void WSWindowManager::close_menu(WSMenu& menu)
-{
-    if (current_menu() == &menu)
-        close_current_menu();
-}
-
 void WSWindowManager::close_menubar(WSMenuBar& menubar)
 {
     if (current_menubar() == &menubar)

Servers/WindowServer/WSWindowManager.h

@@ -84,8 +84,6 @@ public:
     Rect menubar_rect() const;
     WSMenuBar* current_menubar() { return m_current_menubar.ptr(); }
     void set_current_menubar(WSMenuBar*);
-    WSMenu* current_menu() { return m_current_menu.ptr(); }
-    void set_current_menu(WSMenu*, bool is_submenu = false);
     WSMenu* system_menu() { return m_system_menu.ptr(); }
 
     const WSCursor& active_cursor() const;
@@ -114,7 +112,6 @@ public:
     void close_menubar(WSMenuBar&);
     Color menu_selection_color() const { return m_menu_selection_color; }
     int menubar_menu_margin() const;
-    void close_current_menu();
 
     void set_resolution(int width, int height);
 
@@ -255,7 +252,6 @@ private:
     RefPtr<WSMenu> m_system_menu;
     Color m_menu_selection_color;
     WeakPtr<WSMenuBar> m_current_menubar;
-    WeakPtr<WSMenu> m_current_menu;
 
     WSWindowSwitcher m_switcher;
     WSMenuManager m_menu_manager;
@@ -265,6 +261,13 @@ private:
 
     RefPtr<CConfigFile> m_wm_config;
 
+    struct AppMetadata {
+        String executable;
+        String name;
+        String icon_path;
+        String category;
+    };
+    Vector<AppMetadata> m_apps;
     HashMap<String, NonnullRefPtr<WSMenu>> m_app_category_menus;
 };