LibGUI, WindowServer: Greatly simplify menubar logic

Currently, any number of menubars can be plugged in and out of a window.
This is unnecessary complexity, since we only need one menubar on a
window. This commit removes most of the logic for dynamically attaching
and detaching menubars and makes one menubar always available. The
menubar is only considered existent if it has at least a single menu in
it (in other words, an empty menubar will not be shown).

This commit additionally fixes a bug wherein menus added after a menubar
has been attached would not have their rects properly setup, and would
therefore appear glitched out on the top left corner of the menubar.

19 files changed, 152 insertions, 257 deletions

diff --git a/Userland/Libraries/LibGUI/CMakeLists.txt b/Userland/Libraries/LibGUI/CMakeLists.txt
index 62a0a42457..adb1fb7516 100644
--- a/Userland/Libraries/LibGUI/CMakeLists.txt
+++ b/Userland/Libraries/LibGUI/CMakeLists.txt
@@ -59,7 +59,6 @@ set(SOURCES
     LinkLabel.cpp
     ListView.cpp
     Menu.cpp
-    Menubar.cpp
     MenuItem.cpp
     MessageBox.cpp
     Model.cpp
diff --git a/Userland/Libraries/LibGUI/Menu.cpp b/Userland/Libraries/LibGUI/Menu.cpp
index 99034516e7..5d5d70d962 100644
--- a/Userland/Libraries/LibGUI/Menu.cpp
+++ b/Userland/Libraries/LibGUI/Menu.cpp
@@ -51,24 +51,35 @@ void Menu::set_icon(const Gfx::Bitmap* icon)
 
 void Menu::add_action(NonnullRefPtr<Action> action)
 {
-    m_items.append(make<MenuItem>(m_menu_id, move(action)));
+    auto item = make<MenuItem>(m_menu_id, move(action));
+    if (m_menu_id != -1)
+        realize_menu_item(*item, m_items.size());
+    m_items.append(move(item));
 }
 
 Menu& Menu::add_submenu(const String& name)
 {
     auto submenu = Menu::construct(name);
-    m_items.append(make<MenuItem>(m_menu_id, submenu));
+
+    auto item = make<MenuItem>(m_menu_id, submenu);
+    if (m_menu_id != -1)
+        realize_menu_item(*item, m_items.size());
+    m_items.append(move(item));
+
     return submenu;
 }
 
 void Menu::add_separator()
 {
-    m_items.append(make<MenuItem>(m_menu_id, MenuItem::Type::Separator));
+    auto item = make<MenuItem>(m_menu_id, MenuItem::Type::Separator);
+    if (m_menu_id != -1)
+        realize_menu_item(*item, m_items.size());
+    m_items.append(move(item));
 }
 
 void Menu::realize_if_needed(const RefPtr<Action>& default_action)
 {
-    if (m_menu_id == -1 || m_last_default_action.ptr() != default_action)
+    if (m_menu_id == -1 || m_current_default_action.ptr() != default_action)
         realize_menu(default_action);
 }
 
@@ -94,32 +105,13 @@ int Menu::realize_menu(RefPtr<Action> default_action)
 
     dbgln_if(MENU_DEBUG, "GUI::Menu::realize_menu(): New menu ID: {}", m_menu_id);
     VERIFY(m_menu_id > 0);
+    m_current_default_action = default_action;
+
     for (size_t i = 0; i < m_items.size(); ++i) {
-        auto& item = m_items[i];
-        item.set_menu_id({}, m_menu_id);
-        item.set_identifier({}, i);
-        if (item.type() == MenuItem::Type::Separator) {
-            WindowServerConnection::the().async_add_menu_separator(m_menu_id);
-            continue;
-        }
-        if (item.type() == MenuItem::Type::Submenu) {
-            auto& submenu = *item.submenu();
-            submenu.realize_if_needed(default_action);
-            auto icon = submenu.icon() ? submenu.icon()->to_shareable_bitmap() : Gfx::ShareableBitmap();
-            WindowServerConnection::the().async_add_menu_item(m_menu_id, i, submenu.menu_id(), submenu.name(), true, false, false, false, "", icon, false);
-            continue;
-        }
-        if (item.type() == MenuItem::Type::Action) {
-            auto& action = *item.action();
-            auto shortcut_text = action.shortcut().is_valid() ? action.shortcut().to_string() : String();
-            bool exclusive = action.group() && action.group()->is_exclusive() && action.is_checkable();
-            bool is_default = (default_action.ptr() == &action);
-            auto icon = action.icon() ? action.icon()->to_shareable_bitmap() : Gfx::ShareableBitmap();
-            WindowServerConnection::the().async_add_menu_item(m_menu_id, i, -1, action.text(), action.is_enabled(), action.is_checkable(), action.is_checkable() ? action.is_checked() : false, is_default, shortcut_text, icon, exclusive);
-        }
+        realize_menu_item(m_items[i], i);
     }
+
     all_menus().set(m_menu_id, this);
-    m_last_default_action = default_action;
     return m_menu_id;
 }
 
@@ -154,4 +146,34 @@ void Menu::visibility_did_change(Badge<WindowServerConnection>, bool visible)
         on_visibility_change(visible);
 }
 
+void Menu::realize_menu_item(MenuItem& item, int item_id)
+{
+    item.set_menu_id({}, m_menu_id);
+    item.set_identifier({}, item_id);
+    switch (item.type()) {
+    case MenuItem::Type::Separator:
+        WindowServerConnection::the().async_add_menu_separator(m_menu_id);
+        break;
+    case MenuItem::Type::Action: {
+        auto& action = *item.action();
+        auto shortcut_text = action.shortcut().is_valid() ? action.shortcut().to_string() : String();
+        bool exclusive = action.group() && action.group()->is_exclusive() && action.is_checkable();
+        bool is_default = (m_current_default_action.ptr() == &action);
+        auto icon = action.icon() ? action.icon()->to_shareable_bitmap() : Gfx::ShareableBitmap();
+        WindowServerConnection::the().async_add_menu_item(m_menu_id, item_id, -1, action.text(), action.is_enabled(), action.is_checkable(), action.is_checkable() ? action.is_checked() : false, is_default, shortcut_text, icon, exclusive);
+        break;
+    }
+    case MenuItem::Type::Submenu: {
+        auto& submenu = *item.submenu();
+        submenu.realize_if_needed(m_current_default_action.strong_ref());
+        auto icon = submenu.icon() ? submenu.icon()->to_shareable_bitmap() : Gfx::ShareableBitmap();
+        WindowServerConnection::the().async_add_menu_item(m_menu_id, item_id, submenu.menu_id(), submenu.name(), true, false, false, false, "", icon, false);
+        break;
+    }
+    case MenuItem::Type::Invalid:
+    default:
+        VERIFY_NOT_REACHED();
+    }
+}
+
 }
diff --git a/Userland/Libraries/LibGUI/Menu.h b/Userland/Libraries/LibGUI/Menu.h
index 640cfe81f5..833c905aa1 100644
--- a/Userland/Libraries/LibGUI/Menu.h
+++ b/Userland/Libraries/LibGUI/Menu.h
@@ -10,6 +10,7 @@
 #include <AK/WeakPtr.h>
 #include <LibCore/Object.h>
 #include <LibGUI/Action.h>
+#include <LibGUI/Event.h>
 #include <LibGUI/Forward.h>
 #include <LibGfx/Forward.h>
 
@@ -53,11 +54,13 @@ private:
     void unrealize_menu();
     void realize_if_needed(const RefPtr<Action>& default_action);
 
+    void realize_menu_item(MenuItem&, int item_id);
+
     int m_menu_id { -1 };
     String m_name;
     RefPtr<Gfx::Bitmap> m_icon;
     NonnullOwnPtrVector<MenuItem> m_items;
-    WeakPtr<Action> m_last_default_action;
+    WeakPtr<Action> m_current_default_action;
     bool m_visible { false };
 };
 
diff --git a/Userland/Libraries/LibGUI/Menubar.cpp b/Userland/Libraries/LibGUI/Menubar.cpp
deleted file mode 100644
index 857f7acd25..0000000000
--- a/Userland/Libraries/LibGUI/Menubar.cpp
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <AK/Badge.h>
-#include <AK/IDAllocator.h>
-#include <LibGUI/Menu.h>
-#include <LibGUI/MenuItem.h>
-#include <LibGUI/Menubar.h>
-#include <LibGUI/WindowServerConnection.h>
-
-namespace GUI {
-
-static IDAllocator s_menubar_id_allocator;
-
-Menubar::Menubar()
-{
-}
-
-Menubar::~Menubar()
-{
-    unrealize_menubar();
-}
-
-Menu& Menubar::add_menu(String name)
-{
-    auto& menu = add<Menu>(move(name));
-    m_menus.append(menu);
-    return menu;
-}
-
-int Menubar::realize_menubar()
-{
-    auto menubar_id = s_menubar_id_allocator.allocate();
-    WindowServerConnection::the().async_create_menubar(menubar_id);
-    return menubar_id;
-}
-
-void Menubar::unrealize_menubar()
-{
-    if (m_menubar_id == -1)
-        return;
-    WindowServerConnection::the().async_destroy_menubar(m_menubar_id);
-    m_menubar_id = -1;
-}
-
-void Menubar::notify_added_to_window(Badge<Window>)
-{
-    VERIFY(m_menubar_id == -1);
-    m_menubar_id = realize_menubar();
-    VERIFY(m_menubar_id != -1);
-    for (auto& menu : m_menus) {
-        int menu_id = menu.realize_menu();
-        VERIFY(menu_id != -1);
-        WindowServerConnection::the().async_add_menu_to_menubar(m_menubar_id, menu_id);
-    }
-}
-
-void Menubar::notify_removed_from_window(Badge<Window>)
-{
-    unrealize_menubar();
-}
-
-}
diff --git a/Userland/Libraries/LibGUI/Menubar.h b/Userland/Libraries/LibGUI/Menubar.h
index 0f238a0982..42d97b7b91 100644
--- a/Userland/Libraries/LibGUI/Menubar.h
+++ b/Userland/Libraries/LibGUI/Menubar.h
@@ -1,15 +1,18 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
-#include <AK/Forward.h>
+#include <AK/Badge.h>
+#include <AK/IterationDecision.h>
 #include <AK/NonnullRefPtrVector.h>
 #include <LibCore/Object.h>
 #include <LibGUI/Forward.h>
+#include <LibGUI/Menu.h>
 
 namespace GUI {
 
@@ -17,22 +20,27 @@ class Menubar : public Core::Object {
     C_OBJECT(Menubar);
 
 public:
-    ~Menubar();
-
-    Menu& add_menu(String name);
-
-    void notify_added_to_window(Badge<Window>);
-    void notify_removed_from_window(Badge<Window>);
-
-    int menubar_id() const { return m_menubar_id; }
+    ~Menubar() { }
+
+    Menu& add_menu(Badge<Window>, String name)
+    {
+        auto& menu = add<Menu>(move(name));
+        m_menus.append(menu);
+        return menu;
+    }
+
+    void for_each_menu(Function<IterationDecision(Menu&)> callback)
+    {
+        for (auto& menu : m_menus) {
+            if (callback(menu) == IterationDecision::Break) {
+                return;
+            }
+        }
+    }
 
 private:
-    Menubar();
-
-    int realize_menubar();
-    void unrealize_menubar();
+    Menubar() { }
 
-    int m_menubar_id { -1 };
     NonnullRefPtrVector<Menu> m_menus;
 };
 
diff --git a/Userland/Libraries/LibGUI/Window.cpp b/Userland/Libraries/LibGUI/Window.cpp
index f7a6a26d9d..87a117c07f 100644
--- a/Userland/Libraries/LibGUI/Window.cpp
+++ b/Userland/Libraries/LibGUI/Window.cpp
@@ -66,6 +66,7 @@ Window* Window::from_window_id(int window_id)
 
 Window::Window(Core::Object* parent)
     : Core::Object(parent)
+    , m_menubar(Menubar::construct())
 {
     all_windows->set(this);
     m_rect_when_windowless = { -5000, -5000, 140, 140 };
@@ -92,8 +93,6 @@ Window::Window(Core::Object* parent)
 
 Window::~Window()
 {
-    if (m_menubar)
-        m_menubar->notify_removed_from_window({});
     all_windows->remove(this);
     hide();
 }
@@ -162,11 +161,11 @@ void Window::show()
 
     apply_icon();
 
-    if (m_menubar) {
-        // This little dance makes us create a server-side menubar.
-        auto menubar = move(m_menubar);
-        set_menubar(menubar);
-    }
+    m_menubar->for_each_menu([&](Menu& menu) {
+        menu.realize_menu_if_needed();
+        WindowServerConnection::the().async_add_menu(m_window_id, menu.menu_id());
+        return IterationDecision::Continue;
+    });
 
     reified_windows->set(m_window_id, this);
     Application::the()->did_create_window({});
@@ -1172,22 +1171,12 @@ Gfx::Bitmap* Window::back_bitmap()
 
 Menu& Window::add_menu(String name)
 {
-    if (!m_menubar)
-        set_menubar(GUI::Menubar::construct());
-    return m_menubar->add_menu(move(name));
-}
-
-void Window::set_menubar(RefPtr<Menubar> menubar)
-{
-    if (m_menubar == menubar)
-        return;
-    if (m_menubar)
-        m_menubar->notify_removed_from_window({});
-    m_menubar = move(menubar);
-    if (m_window_id && m_menubar) {
-        m_menubar->notify_added_to_window({});
-        WindowServerConnection::the().async_set_window_menubar(m_window_id, m_menubar->menubar_id());
+    Menu& menu = m_menubar->add_menu({}, move(name));
+    if (m_window_id) {
+        menu.realize_menu_if_needed();
+        WindowServerConnection::the().async_add_menu(m_window_id, menu.menu_id());
     }
+    return menu;
 }
 
 bool Window::is_modified() const
diff --git a/Userland/Libraries/LibGUI/Window.h b/Userland/Libraries/LibGUI/Window.h
index 0b597f90ed..18945fee26 100644
--- a/Userland/Libraries/LibGUI/Window.h
+++ b/Userland/Libraries/LibGUI/Window.h
@@ -201,7 +201,6 @@ public:
     void did_disable_focused_widget(Badge<Widget>);
 
     Menu& add_menu(String name);
-    void set_menubar(RefPtr<Menubar>);
 
 protected:
     Window(Core::Object* parent = nullptr);
@@ -242,7 +241,7 @@ private:
     OwnPtr<WindowBackingStore> m_front_store;
     OwnPtr<WindowBackingStore> m_back_store;
 
-    RefPtr<Menubar> m_menubar;
+    NonnullRefPtr<Menubar> m_menubar;
 
     RefPtr<Gfx::Bitmap> m_icon;
     RefPtr<Gfx::Bitmap> m_custom_cursor;
diff --git a/Userland/Services/WindowServer/CMakeLists.txt b/Userland/Services/WindowServer/CMakeLists.txt
index b0fd2f652e..b536a16ff5 100644
--- a/Userland/Services/WindowServer/CMakeLists.txt
+++ b/Userland/Services/WindowServer/CMakeLists.txt
@@ -18,8 +18,8 @@ set(SOURCES
     Cursor.cpp
     EventLoop.cpp
     main.cpp
-    Menubar.cpp
     Menu.cpp
+    Menubar.cpp
     MenuItem.cpp
     MenuManager.cpp
     MultiScaleBitmaps.cpp
diff --git a/Userland/Services/WindowServer/ClientConnection.cpp b/Userland/Services/WindowServer/ClientConnection.cpp
index 8b263586a9..2214b0f465 100644
--- a/Userland/Services/WindowServer/ClientConnection.cpp
+++ b/Userland/Services/WindowServer/ClientConnection.cpp
@@ -13,7 +13,6 @@
 #include <WindowServer/Compositor.h>
 #include <WindowServer/Menu.h>
 #include <WindowServer/MenuItem.h>
-#include <WindowServer/Menubar.h>
 #include <WindowServer/Screen.h>
 #include <WindowServer/Window.h>
 #include <WindowServer/WindowClientEndpoint.h>
@@ -91,22 +90,6 @@ void ClientConnection::notify_about_new_screen_rects()
     async_screen_rects_changed(Screen::rects(), Screen::main().index(), wm.window_stack_rows(), wm.window_stack_columns());
 }
 
-void ClientConnection::create_menubar(i32 menubar_id)
-{
-    auto menubar = Menubar::create(*this, menubar_id);
-    m_menubars.set(menubar_id, move(menubar));
-}
-
-void ClientConnection::destroy_menubar(i32 menubar_id)
-{
-    auto it = m_menubars.find(menubar_id);
-    if (it == m_menubars.end()) {
-        did_misbehave("DestroyMenubar: Bad menubar ID");
-        return;
-    }
-    m_menubars.remove(it);
-}
-
 void ClientConnection::create_menu(i32 menu_id, String const& menu_title)
 {
     auto menu = Menu::construct(this, menu_id, menu_title);
@@ -126,44 +109,21 @@ void ClientConnection::destroy_menu(i32 menu_id)
     remove_child(menu);
 }
 
-void ClientConnection::set_window_menubar(i32 window_id, i32 menubar_id)
-{
-    RefPtr<Window> window;
-    {
-        auto it = m_windows.find(window_id);
-        if (it == m_windows.end()) {
-            did_misbehave("SetWindowMenubar: Bad window ID");
-            return;
-        }
-        window = it->value;
-    }
-    RefPtr<Menubar> menubar;
-    if (menubar_id != -1) {
-        auto it = m_menubars.find(menubar_id);
-        if (it == m_menubars.end()) {
-            did_misbehave("SetWindowMenubar: Bad menubar ID");
-            return;
-        }
-        menubar = *(*it).value;
-    }
-    window->set_menubar(menubar);
-}
-
-void ClientConnection::add_menu_to_menubar(i32 menubar_id, i32 menu_id)
+void ClientConnection::add_menu(i32 window_id, i32 menu_id)
 {
-    auto it = m_menubars.find(menubar_id);
+    auto it = m_windows.find(window_id);
     auto jt = m_menus.find(menu_id);
-    if (it == m_menubars.end()) {
-        did_misbehave("AddMenuToMenubar: Bad menubar ID");
+    if (it == m_windows.end()) {
+        did_misbehave("AddMenu: Bad window ID");
         return;
     }
     if (jt == m_menus.end()) {
-        did_misbehave("AddMenuToMenubar: Bad menu ID");
+        did_misbehave("AddMenu: Bad menu ID");
         return;
     }
-    auto& menubar = *(*it).value;
+    auto& window = *(*it).value;
     auto& menu = *(*jt).value;
-    menubar.add_menu(menu);
+    window.add_menu(menu);
 }
 
 void ClientConnection::add_menu_item(i32 menu_id, i32 identifier, i32 submenu_id,
diff --git a/Userland/Services/WindowServer/ClientConnection.h b/Userland/Services/WindowServer/ClientConnection.h
index 74c0f29c32..d315ef1447 100644
--- a/Userland/Services/WindowServer/ClientConnection.h
+++ b/Userland/Services/WindowServer/ClientConnection.h
@@ -90,12 +90,9 @@ private:
     void set_unresponsive(bool);
     void destroy_window(Window&, Vector<i32>& destroyed_window_ids);
 
-    virtual void create_menubar(i32) override;
-    virtual void destroy_menubar(i32) override;
     virtual void create_menu(i32, String const&) override;
     virtual void destroy_menu(i32) override;
-    virtual void add_menu_to_menubar(i32, i32) override;
-    virtual void set_window_menubar(i32, i32) override;
+    virtual void add_menu(i32, i32) override;
     virtual void add_menu_item(i32, i32, i32, String const&, bool, bool, bool, bool, String const&, Gfx::ShareableBitmap const&, bool) override;
     virtual void add_menu_separator(i32) override;
     virtual void update_menu_item(i32, i32, i32, String const&, bool, bool, bool, bool, String const&) override;
@@ -173,7 +170,6 @@ private:
     Window* window_from_id(i32 window_id);
 
     HashMap<int, NonnullRefPtr<Window>> m_windows;
-    HashMap<int, NonnullRefPtr<Menubar>> m_menubars;
     HashMap<int, NonnullRefPtr<Menu>> m_menus;
 
     RefPtr<Core::Timer> m_ping_timer;
diff --git a/Userland/Services/WindowServer/Menu.h b/Userland/Services/WindowServer/Menu.h
index 49b46a5e5c..c3e4afa038 100644
--- a/Userland/Services/WindowServer/Menu.h
+++ b/Userland/Services/WindowServer/Menu.h
@@ -15,13 +15,14 @@
 #include <LibGfx/Forward.h>
 #include <LibGfx/Rect.h>
 #include <WindowServer/Cursor.h>
+#include <WindowServer/Event.h>
 #include <WindowServer/MenuItem.h>
-#include <WindowServer/Window.h>
 
 namespace WindowServer {
 
 class ClientConnection;
 class Menubar;
+class Window;
 
 class Menu final : public Core::Object {
     C_OBJECT(Menu);
diff --git a/Userland/Services/WindowServer/MenuManager.cpp b/Userland/Services/WindowServer/MenuManager.cpp
index c222a244d5..c1e9b88e8b 100644
--- a/Userland/Services/WindowServer/MenuManager.cpp
+++ b/Userland/Services/WindowServer/MenuManager.cpp
@@ -362,7 +362,7 @@ Menu* MenuManager::previous_menu(Menu* current)
         return nullptr;
     Menu* found = nullptr;
     Menu* previous = nullptr;
-    wm.window_with_active_menu()->menubar()->for_each_menu([&](Menu& menu) {
+    wm.window_with_active_menu()->menubar().for_each_menu([&](Menu& menu) {
         if (current == &menu) {
             found = previous;
             return IterationDecision::Break;
@@ -380,7 +380,7 @@ Menu* MenuManager::next_menu(Menu* current)
     auto& wm = WindowManager::the();
     if (!wm.window_with_active_menu())
         return nullptr;
-    wm.window_with_active_menu()->menubar()->for_each_menu([&](Menu& menu) {
+    wm.window_with_active_menu()->menubar().for_each_menu([&](Menu& menu) {
         if (is_next) {
             found = &menu;
             return IterationDecision::Break;
diff --git a/Userland/Services/WindowServer/Menubar.cpp b/Userland/Services/WindowServer/Menubar.cpp
index 3bd9ecea88..f0cd7c50e3 100644
--- a/Userland/Services/WindowServer/Menubar.cpp
+++ b/Userland/Services/WindowServer/Menubar.cpp
@@ -1,27 +1,26 @@
 /*
  * Copyright (c) 2018-2021, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include "Menubar.h"
-#include "Menu.h"
+#include "WindowManager.h"
 
 namespace WindowServer {
 
-Menubar::Menubar(ClientConnection& client, int menubar_id)
-    : m_client(client)
-    , m_menubar_id(menubar_id)
+void Menubar::layout_menu(Menu& menu, Gfx::IntRect window_rect)
 {
-}
+    // FIXME: Maybe move this to the theming system?
+    static constexpr auto menubar_menu_margin = 14;
 
-Menubar::~Menubar()
-{
-}
+    auto& wm = WindowManager::the();
+    auto menubar_rect = Gfx::WindowTheme::current().menubar_rect(Gfx::WindowTheme::WindowType::Normal, window_rect, wm.palette(), 1);
 
-void Menubar::add_menu(Menu& menu)
-{
-    m_menus.append(menu);
+    int text_width = wm.font().width(Gfx::parse_ampersand_string(menu.name()));
+    menu.set_rect_in_window_menubar({ m_next_menu_location.x(), 0, text_width + menubar_menu_margin, menubar_rect.height() });
+    m_next_menu_location.translate_by(menu.rect_in_window_menubar().width(), 0);
 }
 
 }
diff --git a/Userland/Services/WindowServer/Menubar.h b/Userland/Services/WindowServer/Menubar.h
index 838718252b..3463f8e5f5 100644
--- a/Userland/Services/WindowServer/Menubar.h
+++ b/Userland/Services/WindowServer/Menubar.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -7,26 +8,27 @@
 #pragma once
 
 #include "Menu.h"
+#include <AK/Function.h>
+#include <AK/IterationDecision.h>
 #include <AK/Vector.h>
-#include <AK/WeakPtr.h>
-#include <AK/Weakable.h>
 
 namespace WindowServer {
 
-class Menubar
-    : public RefCounted<Menubar>
-    , public Weakable<Menubar> {
+class Menubar {
 public:
-    static NonnullRefPtr<Menubar> create(ClientConnection& client, int menubar_id) { return adopt_ref(*new Menubar(client, menubar_id)); }
-    ~Menubar();
+    void add_menu(Menu& menu, Gfx::IntRect window_rect)
+    {
+        // FIXME: Check against duplicate menu additions.
+        m_menus.append(menu);
+        layout_menu(menu, window_rect);
+    }
 
-    ClientConnection& client() { return m_client; }
-    const ClientConnection& client() const { return m_client; }
-    int menubar_id() const { return m_menubar_id; }
-    void add_menu(Menu&);
+    bool has_menus()
+    {
+        return !m_menus.is_empty();
+    }
 
-    template<typename Callback>
-    void for_each_menu(Callback callback)
+    void for_each_menu(Function<IterationDecision(Menu&)> callback)
     {
         for (auto& menu : m_menus) {
             if (callback(menu) == IterationDecision::Break)
@@ -35,11 +37,13 @@ public:
     }
 
 private:
-    Menubar(ClientConnection&, int menubar_id);
+    void layout_menu(Menu&, Gfx::IntRect window_rect);
 
-    ClientConnection& m_client;
-    int m_menubar_id { 0 };
     Vector<Menu&> m_menus;
+
+    // FIXME: This doesn't support removing menus from a menubar or inserting a
+    //        menu in the middle.
+    Gfx::IntPoint m_next_menu_location { 0, 0 };
 };
 
 }
diff --git a/Userland/Services/WindowServer/Window.cpp b/Userland/Services/WindowServer/Window.cpp
index bec2c4b5be..2b56c56eb7 100644
--- a/Userland/Services/WindowServer/Window.cpp
+++ b/Userland/Services/WindowServer/Window.cpp
@@ -73,6 +73,7 @@ static Gfx::Bitmap& pin_icon()
         s_icon = Gfx::Bitmap::try_load_from_file("/res/icons/16x16/window-pin.png").leak_ref();
     return *s_icon;
 }
+
 Window::Window(Core::Object& parent, WindowType type)
     : Core::Object(&parent)
     , m_type(type)
@@ -576,15 +577,16 @@ void Window::handle_keydown_event(const KeyEvent& event)
         popup_window_menu(position, WindowMenuDefaultAction::Close);
         return;
     }
-    if (event.modifiers() == Mod_Alt && event.code_point() && menubar()) {
+    if (event.modifiers() == Mod_Alt && event.code_point() && m_menubar.has_menus()) {
         Menu* menu_to_open = nullptr;
-        menubar()->for_each_menu([&](Menu& menu) {
+        m_menubar.for_each_menu([&](Menu& menu) {
             if (to_ascii_lowercase(menu.alt_shortcut_character()) == to_ascii_lowercase(event.code_point())) {
                 menu_to_open = &menu;
                 return IterationDecision::Break;
             }
             return IterationDecision::Continue;
         });
+
         if (menu_to_open) {
             frame().open_menubar_menu(*menu_to_open);
             if (!menu_to_open->is_empty())
@@ -874,8 +876,8 @@ void Window::popup_window_menu(const Gfx::IntPoint& position, WindowMenuDefaultA
     m_window_menu_maximize_item->set_default(default_action == WindowMenuDefaultAction::Maximize || default_action == WindowMenuDefaultAction::Restore);
     m_window_menu_maximize_item->set_icon(m_maximized ? &restore_icon() : &maximize_icon());
     m_window_menu_close_item->set_default(default_action == WindowMenuDefaultAction::Close);
-    m_window_menu_menubar_visibility_item->set_enabled(menubar());
-    m_window_menu_menubar_visibility_item->set_checked(menubar() && m_should_show_menubar);
+    m_window_menu_menubar_visibility_item->set_enabled(m_menubar.has_menus());
+    m_window_menu_menubar_visibility_item->set_checked(m_menubar.has_menus() && m_should_show_menubar);
 
     m_window_menu->popup(position);
 }
@@ -1236,32 +1238,16 @@ Optional<HitTestResult> Window::hit_test(Gfx::IntPoint const& position, bool inc
     };
 }
 
-void Window::set_menubar(Menubar* menubar)
+void Window::add_menu(Menu& menu)
 {
-    if (m_menubar == menubar)
-        return;
-    m_menubar = menubar;
-    if (m_menubar) {
-        // FIXME: Maybe move this to the theming system?
-        static constexpr auto menubar_menu_margin = 14;
-
-        auto& wm = WindowManager::the();
-        Gfx::IntPoint next_menu_location { 0, 0 };
-        auto menubar_rect = Gfx::WindowTheme::current().menubar_rect(Gfx::WindowTheme::WindowType::Normal, rect(), wm.palette(), 1);
-        m_menubar->for_each_menu([&](Menu& menu) {
-            int text_width = wm.font().width(Gfx::parse_ampersand_string(menu.name()));
-            menu.set_rect_in_window_menubar({ next_menu_location.x(), 0, text_width + menubar_menu_margin, menubar_rect.height() });
-            next_menu_location.translate_by(menu.rect_in_window_menubar().width(), 0);
-            return IterationDecision::Continue;
-        });
-    }
+    m_menubar.add_menu(menu, rect());
     Compositor::the().invalidate_occlusions();
     frame().invalidate();
 }
 
 void Window::invalidate_menubar()
 {
-    if (!m_should_show_menubar || !menubar())
+    if (!m_should_show_menubar || !m_menubar.has_menus())
         return;
     frame().invalidate_menubar();
 }
diff --git a/Userland/Services/WindowServer/Window.h b/Userland/Services/WindowServer/Window.h
index 17f3baf5e8..4d5c7771be 100644
--- a/Userland/Services/WindowServer/Window.h
+++ b/Userland/Services/WindowServer/Window.h
@@ -14,6 +14,7 @@
 #include <LibGfx/DisjointRectSet.h>
 #include <LibGfx/Rect.h>
 #include <WindowServer/Cursor.h>
+#include <WindowServer/Menubar.h>
 #include <WindowServer/Screen.h>
 #include <WindowServer/WindowFrame.h>
 #include <WindowServer/WindowType.h>
@@ -25,7 +26,6 @@ class ClientConnection;
 class Cursor;
 class KeyEvent;
 class Menu;
-class Menubar;
 class MenuItem;
 class MouseEvent;
 class WindowStack;
@@ -338,9 +338,10 @@ public:
     // area needs to be rendered
     auto& affected_transparency_rects() { return m_affected_transparency_rects; }
 
-    Menubar* menubar() { return m_menubar; }
-    const Menubar* menubar() const { return m_menubar; }
-    void set_menubar(Menubar*);
+    Menubar& menubar() { return m_menubar; }
+    Menubar const& menubar() const { return m_menubar; }
+
+    void add_menu(Menu& menu);
 
     WindowStack& window_stack()
     {
@@ -395,7 +396,7 @@ private:
     Vector<WeakPtr<Window>> m_child_windows;
     Vector<WeakPtr<Window>> m_accessory_windows;
 
-    RefPtr<Menubar> m_menubar;
+    Menubar m_menubar;
 
     String m_title;
     Gfx::IntRect m_rect;
diff --git a/Userland/Services/WindowServer/WindowFrame.cpp b/Userland/Services/WindowServer/WindowFrame.cpp
index 4465e124f4..c2e38e33b3 100644
--- a/Userland/Services/WindowServer/WindowFrame.cpp
+++ b/Userland/Services/WindowServer/WindowFrame.cpp
@@ -56,7 +56,7 @@ static Gfx::IntRect frame_rect_for_window(Window& window, const Gfx::IntRect& re
 {
     if (window.is_frameless())
         return rect;
-    int menu_row_count = (window.menubar() && window.should_show_menubar()) ? 1 : 0;
+    int menu_row_count = (window.menubar().has_menus() && window.should_show_menubar()) ? 1 : 0;
     return Gfx::WindowTheme::current().frame_rect_for_window(to_theme_window_type(window.type()), rect, WindowManager::the().palette(), menu_row_count);
 }
 
@@ -199,7 +199,7 @@ void WindowFrame::did_set_maximized(Badge<Window>, bool maximized)
 
 Gfx::IntRect WindowFrame::menubar_rect() const
 {
-    if (!m_window.menubar() || !m_window.should_show_menubar())
+    if (!m_window.menubar().has_menus() || !m_window.should_show_menubar())
         return {};
     return Gfx::WindowTheme::current().menubar_rect(to_theme_window_type(m_window.type()), m_window.rect(), WindowManager::the().palette(), menu_row_count());
 }
@@ -261,7 +261,7 @@ void WindowFrame::paint_menubar(Gfx::Painter& painter)
     painter.add_clip_rect(menubar_rect);
     painter.translate(menubar_rect.location());
 
-    m_window.menubar()->for_each_menu([&](Menu& menu) {
+    m_window.menubar().for_each_menu([&](Menu& menu) {
         auto text_rect = menu.rect_in_window_menubar();
         Color text_color = palette.window_text();
         auto is_open = menu.is_open();
@@ -283,7 +283,7 @@ void WindowFrame::paint_normal_frame(Gfx::Painter& painter)
     auto leftmost_button_rect = m_buttons.is_empty() ? Gfx::IntRect() : m_buttons.last().relative_rect();
     Gfx::WindowTheme::current().paint_normal_frame(painter, window_state_for_theme(), m_window.rect(), m_window.computed_title(), m_window.icon(), palette, leftmost_button_rect, menu_row_count(), m_window.is_modified());
 
-    if (m_window.menubar() && m_window.should_show_menubar())
+    if (m_window.menubar().has_menus() && m_window.should_show_menubar())
         paint_menubar(painter);
 }
 
@@ -812,7 +812,7 @@ void WindowFrame::handle_menubar_mouse_event(const MouseEvent& event)
     Menu* hovered_menu = nullptr;
     auto menubar_rect = this->menubar_rect();
     auto adjusted_position = event.position().translated(-menubar_rect.location());
-    m_window.menubar()->for_each_menu([&](Menu& menu) {
+    m_window.menubar().for_each_menu([&](Menu& menu) {
         if (menu.rect_in_window_menubar().contains(adjusted_position)) {
             hovered_menu = &menu;
             handle_menu_mouse_event(menu, event);
@@ -968,7 +968,7 @@ int WindowFrame::menu_row_count() const
 {
     if (!m_window.should_show_menubar())
         return 0;
-    return m_window.menubar() ? 1 : 0;
+    return m_window.menubar().has_menus() ? 1 : 0;
 }
 
 }
diff --git a/Userland/Services/WindowServer/WindowManager.h b/Userland/Services/WindowServer/WindowManager.h
index 5cd81a2f43..1c2d077000 100644
--- a/Userland/Services/WindowServer/WindowManager.h
+++ b/Userland/Services/WindowServer/WindowManager.h
@@ -20,7 +20,6 @@
 #include <WindowServer/Cursor.h>
 #include <WindowServer/Event.h>
 #include <WindowServer/MenuManager.h>
-#include <WindowServer/Menubar.h>
 #include <WindowServer/ScreenLayout.h>
 #include <WindowServer/WMClientConnection.h>
 #include <WindowServer/WindowSwitcher.h>
diff --git a/Userland/Services/WindowServer/WindowServer.ipc b/Userland/Services/WindowServer/WindowServer.ipc
index 38bf959770..14e7e0bd99 100644
--- a/Userland/Services/WindowServer/WindowServer.ipc
+++ b/Userland/Services/WindowServer/WindowServer.ipc
@@ -3,13 +3,10 @@
 
 endpoint WindowServer
 {
-    create_menubar(i32 menubar_id) =|
-    destroy_menubar(i32 menubar_id) =|
-
     create_menu(i32 menu_id, [UTF8] String menu_title) =|
     destroy_menu(i32 menu_id) =|
 
-    add_menu_to_menubar(i32 menubar_id, i32 menu_id) =|
+    add_menu(i32 window_id, i32 menu_id) =|
 
     add_menu_item(
         i32 menu_id,
@@ -53,8 +50,6 @@ endpoint WindowServer
 
     destroy_window(i32 window_id) => (Vector<i32> destroyed_window_ids)
 
-    set_window_menubar(i32 window_id, i32 menubar_id) =|
-
     set_window_title(i32 window_id, [UTF8] String title) =|
     get_window_title(i32 window_id) => ([UTF8] String title)