From 4f98a35bebdc7e39d71615aa57df33e7ebd77d20 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 13 Feb 2019 17:54:30 +0100 Subject: [PATCH] WindowServer: Begin refactoring towards a fully asynchronous protocol. In order to move the WindowServer to userspace, I have to eliminate its dependence on system call facilities. The communication channel with each client needs to be message-based in both directions. --- Kernel/GUIEventDevice.cpp | 5 +- Kernel/GUIEventDevice.h | 1 + Kernel/GUITypes.h | 20 ++++++++ Kernel/Process.h | 5 +- Kernel/ProcessGUI.cpp | 14 +----- Kernel/Syscall.cpp | 4 -- Kernel/Syscall.h | 2 - LibC/gui.cpp | 12 ----- LibC/gui.h | 2 - LibGUI/GEventLoop.cpp | 80 +++++++++++++++++++++++++------- LibGUI/GEventLoop.h | 11 ++++- LibGUI/GMenuBar.cpp | 31 +++++++++---- LibGUI/GMenuBar.h | 3 ++ WindowServer/WSMessage.h | 42 +++++++++++++++++ WindowServer/WSMessageLoop.cpp | 31 +++++++++++++ WindowServer/WSMessageLoop.h | 6 +++ WindowServer/WSWindowManager.cpp | 62 +++++++++++++++---------- WindowServer/WSWindowManager.h | 2 - 18 files changed, 242 insertions(+), 91 deletions(-) diff --git a/Kernel/GUIEventDevice.cpp b/Kernel/GUIEventDevice.cpp index 20ba6c917b3..a3914ee6358 100644 --- a/Kernel/GUIEventDevice.cpp +++ b/Kernel/GUIEventDevice.cpp @@ -2,6 +2,7 @@ #include #include #include +#include //#define GUIEVENTDEVICE_DEBUG @@ -32,7 +33,7 @@ ssize_t GUIEventDevice::read(Process& process, byte* buffer, size_t size) return size; } -ssize_t GUIEventDevice::write(Process&, const byte*, size_t) +ssize_t GUIEventDevice::write(Process& process, const byte* data, size_t size) { - return -EINVAL; + return WSMessageLoop::the().on_receive_from_client(process.gui_client_id(), data, size); } diff --git a/Kernel/GUIEventDevice.h b/Kernel/GUIEventDevice.h index d1f6b20b52f..f9ce7784941 100644 --- a/Kernel/GUIEventDevice.h +++ b/Kernel/GUIEventDevice.h @@ -1,6 +1,7 @@ #pragma once #include +#include class GUIEventDevice final : public CharacterDevice { public: diff --git a/Kernel/GUITypes.h b/Kernel/GUITypes.h index 263cca01450..426277649dd 100644 --- a/Kernel/GUITypes.h +++ b/Kernel/GUITypes.h @@ -68,6 +68,8 @@ struct GUI_Event { WindowDeactivated, WindowCloseRequest, MenuItemActivated, + DidCreateMenubar, + DidDestroyMenubar, }; Type type { Invalid }; int window_id { -1 }; @@ -90,12 +92,30 @@ struct GUI_Event { bool shift : 1; } key; struct { + int menubar_id; int menu_id; unsigned identifier; } menu; }; }; +struct GUI_ClientMessage { + enum Type : unsigned { + Invalid, + CreateMenubar, + DestroyMenubar, + }; + Type type { Invalid }; + int window_id { -1 }; + + union { + struct { + int menubar_id; + int menu_id; + } menu; + }; +}; + inline Rect::Rect(const GUI_Rect& r) : Rect(r.location, r.size) { } inline Point::Point(const GUI_Point& p) : Point(p.x, p.y) { } inline Size::Size(const GUI_Size& s) : Size(s.width, s.height) { } diff --git a/Kernel/Process.h b/Kernel/Process.h index e7dfebf6b4a..259f1770e3a 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -230,8 +230,6 @@ public: int gui$get_window_rect(int window_id, GUI_Rect*); int gui$set_window_rect(int window_id, const GUI_Rect*); int gui$set_global_cursor_tracking_enabled(int window_id, bool enabled); - int gui$menubar_create(); - int gui$menubar_destroy(int menubar_id); int gui$menubar_add_menu(int menubar_id, int menu_id); int gui$menu_create(const char* name); int gui$menu_destroy(int menu_id); @@ -313,6 +311,8 @@ public: bool has_used_fpu() const { return m_has_used_fpu; } void set_has_used_fpu(bool b) { m_has_used_fpu = b; } + int gui_client_id() const { return (int)this; } + private: friend class MemoryManager; friend class Scheduler; @@ -420,7 +420,6 @@ private: Vector m_gui_events; Lock m_gui_events_lock; int m_next_window_id { 1 }; - bool m_has_created_menus { false }; dword m_wakeup_requested { false }; bool m_has_used_fpu { false }; diff --git a/Kernel/ProcessGUI.cpp b/Kernel/ProcessGUI.cpp index cc32ffc7d24..9e8ff1e22a0 100644 --- a/Kernel/ProcessGUI.cpp +++ b/Kernel/ProcessGUI.cpp @@ -259,7 +259,7 @@ int Process::gui$set_global_cursor_tracking_enabled(int window_id, bool enabled) void Process::destroy_all_menus() { - if (!m_has_created_menus) + if (!WSMessageLoop::the().running()) return; WSWindowManager::the().destroy_all_menus(*this); } @@ -292,17 +292,6 @@ DisplayInfo Process::set_video_resolution(int width, int height) return info; } -int Process::gui$menubar_create() -{ - m_has_created_menus = true; - return WSWindowManager::the().api$menubar_create(); -} - -int Process::gui$menubar_destroy(int menubar_id) -{ - return WSWindowManager::the().api$menubar_destroy(menubar_id); -} - int Process::gui$menubar_add_menu(int menubar_id, int menu_id) { return WSWindowManager::the().api$menubar_add_menu(menubar_id, menu_id); @@ -312,7 +301,6 @@ int Process::gui$menu_create(const char* name) { if (!validate_read_str(name)) return -EFAULT; - m_has_created_menus = true; return WSWindowManager::the().api$menu_create(String(name)); } diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index 6f18495bfd7..f82bfaec74d 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -223,10 +223,6 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2, return current->sys$rmdir((const char*)arg1); case Syscall::SC_chmod: return current->sys$chmod((const char*)arg1, (mode_t)arg2); - case Syscall::SC_gui_menubar_create: - return current->gui$menubar_create(); - case Syscall::SC_gui_menubar_destroy: - return current->gui$menubar_destroy((int)arg1); case Syscall::SC_gui_menubar_add_menu: return current->gui$menubar_add_menu((int)arg1, (int)arg2); case Syscall::SC_gui_menu_create: diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h index 2e541278793..878ba916c1a 100644 --- a/Kernel/Syscall.h +++ b/Kernel/Syscall.h @@ -85,8 +85,6 @@ __ENUMERATE_SYSCALL(rmdir) \ __ENUMERATE_SYSCALL(chmod) \ __ENUMERATE_SYSCALL(usleep) \ - __ENUMERATE_SYSCALL(gui_menubar_create) \ - __ENUMERATE_SYSCALL(gui_menubar_destroy) \ __ENUMERATE_SYSCALL(gui_menubar_add_menu) \ __ENUMERATE_SYSCALL(gui_menu_create) \ __ENUMERATE_SYSCALL(gui_menu_destroy) \ diff --git a/LibC/gui.cpp b/LibC/gui.cpp index 0d9a07ed0a1..d5e355c5892 100644 --- a/LibC/gui.cpp +++ b/LibC/gui.cpp @@ -69,18 +69,6 @@ int gui_set_global_cursor_tracking_enabled(int window_id, bool enabled) __RETURN_WITH_ERRNO(rc, rc, -1); } -int gui_menubar_create() -{ - int rc = syscall(SC_gui_menubar_create); - __RETURN_WITH_ERRNO(rc, rc, -1); -} - -int gui_menubar_destroy(int menubar_id) -{ - int rc = syscall(SC_gui_menubar_destroy, menubar_id); - __RETURN_WITH_ERRNO(rc, rc, -1); -} - int gui_menubar_add_menu(int menubar_id, int menu_id) { int rc = syscall(SC_gui_menubar_add_menu, menubar_id, menu_id); diff --git a/LibC/gui.h b/LibC/gui.h index 5cf4ff6b960..91d8fa9c4a0 100644 --- a/LibC/gui.h +++ b/LibC/gui.h @@ -16,8 +16,6 @@ int gui_set_window_title(int window_id, const char*, size_t); int gui_get_window_rect(int window_id, GUI_Rect*); int gui_set_window_rect(int window_id, const GUI_Rect*); int gui_set_global_cursor_tracking_enabled(int window_id, bool); -int gui_menubar_create(); -int gui_menubar_destroy(int menubar_id); int gui_menubar_add_menu(int menubar_id, int menu_id); int gui_menu_create(const char* name); int gui_menu_destroy(int menu_id); diff --git a/LibGUI/GEventLoop.cpp b/LibGUI/GEventLoop.cpp index f77d41ef7a4..9e65597d847 100644 --- a/LibGUI/GEventLoop.cpp +++ b/LibGUI/GEventLoop.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include //#define GEVENTLOOP_DEBUG @@ -26,6 +28,12 @@ GEventLoop::GEventLoop() { if (!s_mainGEventLoop) s_mainGEventLoop = this; + + m_event_fd = open("/dev/gui_events", O_RDWR | O_NONBLOCK | O_CLOEXEC); + if (m_event_fd < 0) { + perror("GEventLoop(): open"); + exit(1); + } } GEventLoop::~GEventLoop() @@ -46,12 +54,6 @@ void GEventLoop::exit(int code) int GEventLoop::exec() { - m_event_fd = open("/dev/gui_events", O_RDONLY | O_NONBLOCK | O_CLOEXEC); - if (m_event_fd < 0) { - perror("GEventLoop::exec(): open"); - exit(1); - } - m_running = true; for (;;) { if (m_exit_requested) @@ -226,21 +228,17 @@ void GEventLoop::wait_for_event() if (!FD_ISSET(m_event_fd, &rfds)) return; - for (;;) { - GUI_Event event; - ssize_t nread = read(m_event_fd, &event, sizeof(GUI_Event)); - if (nread < 0) { - perror("read"); - exit(1); // FIXME: This should cause EventLoop::exec() to return 1. - } - if (nread == 0) - break; - assert(nread == sizeof(event)); + bool success = drain_events_from_server(); + ASSERT(success); + auto unprocessed_events = move(m_unprocessed_events); + for (auto& event : unprocessed_events) { switch (event.type) { case GUI_Event::MenuItemActivated: handle_menu_event(event); continue; + default: + break; } auto* window = GWindow::from_window_id(event.window_id); @@ -274,6 +272,23 @@ void GEventLoop::wait_for_event() } } +bool GEventLoop::drain_events_from_server() +{ + for (;;) { + GUI_Event event; + ssize_t nread = read(m_event_fd, &event, sizeof(GUI_Event)); + if (nread < 0) { + perror("read"); + exit(1); + return false; + } + if (nread == 0) + return true; + assert(nread == sizeof(event)); + m_unprocessed_events.append(move(event)); + } +} + bool GEventLoop::EventLoopTimer::has_expired() const { timeval now; @@ -333,3 +348,36 @@ void GEventLoop::unregister_notifier(Badge, GNotifier& notifier) { m_notifiers.remove(¬ifier); } + +bool GEventLoop::post_message_to_server(const GUI_ClientMessage& message) +{ + int nwritten = write(m_event_fd, &message, sizeof(GUI_ClientMessage)); + return nwritten == sizeof(GUI_ClientMessage); +} + +bool GEventLoop::wait_for_specific_event(GUI_Event::Type type, GUI_Event& event) +{ + for (;;) { + bool success = drain_events_from_server(); + if (!success) + return false; + for (size_t i = 0; i < m_unprocessed_events.size(); ++i) { + if (m_unprocessed_events[i].type == type) { + event = move(m_unprocessed_events[i]); + m_unprocessed_events.remove(i); + return true; + } + } + } +} + +GUI_Event GEventLoop::sync_request(const GUI_ClientMessage& request, GUI_Event::Type response_type) +{ + bool success = post_message_to_server(request); + ASSERT(success); + + GUI_Event response; + success = GEventLoop::main().wait_for_specific_event(response_type, response); + ASSERT(success); + return response; +} diff --git a/LibGUI/GEventLoop.h b/LibGUI/GEventLoop.h index e68df0dc10e..db28c3cea40 100644 --- a/LibGUI/GEventLoop.h +++ b/LibGUI/GEventLoop.h @@ -5,11 +5,11 @@ #include #include #include +#include class GObject; class GNotifier; class GWindow; -struct GUI_Event; class GEventLoop { public: @@ -34,15 +34,20 @@ public: void exit(int); + bool post_message_to_server(const GUI_ClientMessage&); + bool wait_for_specific_event(GUI_Event::Type, GUI_Event&); + + GUI_Event sync_request(const GUI_ClientMessage& request, GUI_Event::Type response_type); + private: void wait_for_event(); + bool drain_events_from_server(); void handle_paint_event(const GUI_Event&, GWindow&); void handle_mouse_event(const GUI_Event&, GWindow&); void handle_key_event(const GUI_Event&, GWindow&); void handle_window_activation_event(const GUI_Event&, GWindow&); void handle_window_close_request_event(const GUI_Event&, GWindow&); void handle_menu_event(const GUI_Event&); - void get_next_timer_expiration(timeval&); struct QueuedEvent { @@ -51,6 +56,8 @@ private: }; Vector m_queued_events; + Vector m_unprocessed_events; + int m_event_fd { -1 }; bool m_running { false }; bool m_exit_requested { false }; diff --git a/LibGUI/GMenuBar.cpp b/LibGUI/GMenuBar.cpp index 18c08f1a621..9a2467a8d29 100644 --- a/LibGUI/GMenuBar.cpp +++ b/LibGUI/GMenuBar.cpp @@ -1,4 +1,5 @@ #include +#include #include GMenuBar::GMenuBar() @@ -7,10 +8,7 @@ GMenuBar::GMenuBar() GMenuBar::~GMenuBar() { - if (m_menubar_id) { - gui_menubar_destroy(m_menubar_id); - m_menubar_id = 0; - } + unrealize_menubar(); } void GMenuBar::add_menu(OwnPtr&& menu) @@ -18,10 +16,29 @@ void GMenuBar::add_menu(OwnPtr&& menu) m_menus.append(move(menu)); } +int GMenuBar::realize_menubar() +{ + GUI_ClientMessage request; + request.type = GUI_ClientMessage::Type::CreateMenubar; + GUI_Event response = GEventLoop::main().sync_request(request, GUI_Event::Type::DidCreateMenubar); + return response.menu.menubar_id; +} + +void GMenuBar::unrealize_menubar() +{ + if (!m_menubar_id) + return; + GUI_ClientMessage request; + request.type = GUI_ClientMessage::Type::DestroyMenubar; + request.menu.menubar_id = m_menubar_id; + GEventLoop::main().sync_request(request, GUI_Event::Type::DidDestroyMenubar); + m_menubar_id = 0; +} + void GMenuBar::notify_added_to_application(Badge) { ASSERT(!m_menubar_id); - m_menubar_id = gui_menubar_create(); + m_menubar_id = realize_menubar(); ASSERT(m_menubar_id > 0); for (auto& menu : m_menus) { ASSERT(menu); @@ -35,7 +52,5 @@ void GMenuBar::notify_added_to_application(Badge) void GMenuBar::notify_removed_from_application(Badge) { - ASSERT(m_menubar_id); - gui_menubar_destroy(m_menubar_id); - m_menubar_id = 0; + unrealize_menubar(); } diff --git a/LibGUI/GMenuBar.h b/LibGUI/GMenuBar.h index 6d556da89d3..8a57cabb27b 100644 --- a/LibGUI/GMenuBar.h +++ b/LibGUI/GMenuBar.h @@ -18,6 +18,9 @@ public: void notify_removed_from_application(Badge); private: + int realize_menubar(); + void unrealize_menubar(); + int m_menubar_id { 0 }; Vector> m_menus; }; diff --git a/WindowServer/WSMessage.h b/WindowServer/WSMessage.h index f0c47897e2a..8e599d29108 100644 --- a/WindowServer/WSMessage.h +++ b/WindowServer/WSMessage.h @@ -23,6 +23,11 @@ public: WindowActivated, WindowDeactivated, WindowCloseRequest, + + __Begin_API_Client_Requests, + APICreateMenubarRequest, + APIDestroyMenubarRequest, + __End_API_Client_Requests, }; WSMessage() { } @@ -31,6 +36,7 @@ public: Type type() const { return m_type; } + bool is_client_request() const { return m_type > __Begin_API_Client_Requests && m_type < __End_API_Client_Requests; } bool is_mouse_event() const { return m_type == MouseMove || m_type == MouseDown || m_type == MouseUp; } bool is_key_event() const { return m_type == KeyUp || m_type == KeyDown; } @@ -38,6 +44,42 @@ private: Type m_type { Invalid }; }; +class WSAPIClientRequest : public WSMessage { +public: + WSAPIClientRequest(Type type, int client_id) + : WSMessage(type) + , m_client_id(client_id) + { + } + + int client_id() const { return m_client_id; } + +private: + int m_client_id { 0 }; +}; + +class WSAPICreateMenubarRequest : public WSAPIClientRequest { +public: + WSAPICreateMenubarRequest(int client_id) + : WSAPIClientRequest(WSMessage::APICreateMenubarRequest, client_id) + { + } +}; + +class WSAPIDestroyMenubarRequest : public WSAPIClientRequest { +public: + WSAPIDestroyMenubarRequest(int client_id, int menubar_id) + : WSAPIClientRequest(WSMessage::APIDestroyMenubarRequest, client_id) + , m_menubar_id(menubar_id) + { + } + + int menubar_id() const { return m_menubar_id; } + +private: + int m_menubar_id { 0 }; +}; + class WSClientFinishedPaintMessage final : public WSMessage { public: explicit WSClientFinishedPaintMessage(const Rect& rect = Rect()) diff --git a/WindowServer/WSMessageLoop.cpp b/WindowServer/WSMessageLoop.cpp index 71b3e6641f6..73f1ab9cc7f 100644 --- a/WindowServer/WSMessageLoop.cpp +++ b/WindowServer/WSMessageLoop.cpp @@ -67,6 +67,21 @@ int WSMessageLoop::exec() } } +Process* WSMessageLoop::process_from_client_id(int client_id) +{ + // FIXME: This shouldn't work this way lol. + return (Process*)client_id; +} + +void WSMessageLoop::post_message_to_client(int client_id, const GUI_Event& message) +{ + auto* process = process_from_client_id(client_id); + if (!process) + return; + LOCKER(process->gui_events_lock()); + process->gui_events().append(move(message)); +} + void WSMessageLoop::post_message(WSMessageReceiver* receiver, OwnPtr&& message) { LOCKER(m_lock); @@ -246,3 +261,19 @@ void WSMessageLoop::drain_keyboard() screen.on_receive_keyboard_data(event); } } + +ssize_t WSMessageLoop::on_receive_from_client(int client_id, const byte* data, size_t size) +{ + LOCKER(m_lock); + ASSERT(size == sizeof(GUI_ClientMessage)); + auto& message = *reinterpret_cast(data); + switch (message.type) { + case GUI_ClientMessage::Type::CreateMenubar: + post_message(&WSWindowManager::the(), make(client_id)); + break; + case GUI_ClientMessage::Type::DestroyMenubar: + post_message(&WSWindowManager::the(), make(client_id, message.menu.menubar_id)); + break; + } + return size; +} diff --git a/WindowServer/WSMessageLoop.h b/WindowServer/WSMessageLoop.h index a889e066e47..10667af932a 100644 --- a/WindowServer/WSMessageLoop.h +++ b/WindowServer/WSMessageLoop.h @@ -9,6 +9,7 @@ class WSMessageReceiver; class Process; +struct GUI_Event; class WSMessageLoop { public: @@ -29,6 +30,11 @@ public: int start_timer(int ms, Function&&); int stop_timer(int timer_id); + void post_message_to_client(int client_id, const GUI_Event&); + ssize_t on_receive_from_client(int client_id, const byte*, size_t); + + static Process* process_from_client_id(int client_id); + private: void wait_for_message(); void drain_mouse(); diff --git a/WindowServer/WSWindowManager.cpp b/WindowServer/WSWindowManager.cpp index 90c331184c6..bed78479647 100644 --- a/WindowServer/WSWindowManager.cpp +++ b/WindowServer/WSWindowManager.cpp @@ -733,12 +733,46 @@ void WSWindowManager::on_message(WSMessage& message) compose(); return; } + + if (message.is_client_request()) { + auto& request = static_cast(message); + switch (request.type()) { + case WSMessage::APICreateMenubarRequest: { + int menubar_id = m_next_menubar_id++; + auto menubar = make(menubar_id, *WSMessageLoop::process_from_client_id(request.client_id())); + m_menubars.set(menubar_id, move(menubar)); + GUI_Event event; + event.type = GUI_Event::Type::DidCreateMenubar; + event.menu.menubar_id = menubar_id; + WSMessageLoop::the().post_message_to_client(request.client_id(), event); + break; + } + case WSMessage::APIDestroyMenubarRequest: { + int menubar_id = static_cast(request).menubar_id(); + auto it = m_menubars.find(menubar_id); + if (it == m_menubars.end()) { + ASSERT_NOT_REACHED(); + // FIXME: Send an error. + return; + } + auto& menubar = *(*it).value; + if (&menubar == m_current_menubar) + set_current_menubar(nullptr); + m_menubars.remove(it); + GUI_Event event; + event.type = GUI_Event::Type::DidDestroyMenubar; + event.menu.menubar_id = menubar_id; + WSMessageLoop::the().post_message_to_client(request.client_id(), event); + } + default: + break; + } + } } void WSWindowManager::set_active_window(WSWindow* window) { LOCKER(m_lock); - dbgprintf("set_active_window %p\n", window); if (window == m_active_window.ptr()) return; @@ -751,6 +785,7 @@ void WSWindowManager::set_active_window(WSWindow* window) WSMessageLoop::the().post_message(m_active_window.ptr(), make(WSMessage::WindowActivated)); invalidate(*m_active_window); + ASSERT(window->process()); auto it = m_app_menubars.find(window->process()); if (it != m_app_menubars.end()) { auto* menubar = (*it).value; @@ -855,29 +890,6 @@ WSMenu& WSWindowManager::create_menu(String&& name) return *menu_ptr; } - -int WSWindowManager::api$menubar_create() -{ - LOCKER(m_lock); - int menubar_id = m_next_menubar_id++; - auto menubar = make(menubar_id, *current); - m_menubars.set(menubar_id, move(menubar)); - return menubar_id; -} - -int WSWindowManager::api$menubar_destroy(int menubar_id) -{ - LOCKER(m_lock); - auto it = m_menubars.find(menubar_id); - if (it == m_menubars.end()) - return -EBADMENUBAR; - auto& menubar = *(*it).value; - if (&menubar == m_current_menubar) - set_current_menubar(nullptr); - m_menubars.remove(it); - return 0; -} - int WSWindowManager::api$menubar_add_menu(int menubar_id, int menu_id) { LOCKER(m_lock); @@ -984,5 +996,5 @@ void WSWindowManager::destroy_all_menus(Process& process) set_current_menubar(nullptr); for (int menubar_id : menubar_ids) m_menubars.remove(menubar_id); - m_app_menubars.remove(current); + m_app_menubars.remove(&process); } diff --git a/WindowServer/WSWindowManager.h b/WindowServer/WSWindowManager.h index 10c7ceac4e0..5a4c315503e 100644 --- a/WindowServer/WSWindowManager.h +++ b/WindowServer/WSWindowManager.h @@ -59,8 +59,6 @@ public: Color menu_selection_color() const { return m_menu_selection_color; } int menubar_menu_margin() const; - int api$menubar_create(); - int api$menubar_destroy(int menubar_id); int api$menubar_add_menu(int menubar_id, int menu_id); int api$menu_create(String&&); int api$menu_destroy(int menu_id);