WindowServer+LibGUI: Allow arbitrary number of rects in messages.

To get truly atomic updates, add a mechanism for passing arbitrary amounts
of extra data along with WindowServer messages. This allows us to pass all
the rects in a single message.

LibGUI/GEventLoop.cpp

@@ -80,15 +80,19 @@ GEventLoop::~GEventLoop()
 {
 }
 
-void GEventLoop::handle_paint_event(const WSAPI_ServerMessage& event, GWindow& window)
+void GEventLoop::handle_paint_event(const WSAPI_ServerMessage& event, GWindow& window, const ByteBuffer& extra_data)
 {
 #ifdef GEVENTLOOP_DEBUG
     dbgprintf("WID=%x Paint [%d,%d %dx%d]\n", event.window_id, event.paint.rect.location.x, event.paint.rect.location.y, event.paint.rect.size.width, event.paint.rect.size.height);
 #endif
     Vector<Rect, 32> rects;
-    ASSERT(event.rect_count <= 32);
+    for (int i = 0; i < min(WSAPI_ServerMessage::max_inline_rect_count, event.rect_count); ++i)
-    for (int i = 0; i < event.rect_count; ++i)
         rects.append(event.rects[i]);
+    if (event.extra_size) {
+        auto* extra_rects = reinterpret_cast<const WSAPI_Rect*>(extra_data.data());
+        for (int i = 0; i < event.rect_count - WSAPI_ServerMessage::max_inline_rect_count; ++i)
+            rects.append(extra_rects[i]);
+    }
     post_event(window, make<GMultiPaintEvent>(rects, event.paint.window_size));
 }
 
@@ -198,14 +202,15 @@ void GEventLoop::handle_wm_event(const WSAPI_ServerMessage& event, GWindow& wind
     ASSERT_NOT_REACHED();
 }
 
-void GEventLoop::process_unprocessed_messages()
+void GEventLoop::process_unprocessed_bundles()
 {
     int coalesced_paints = 0;
     int coalesced_resizes = 0;
-    auto unprocessed_events = move(m_unprocessed_messages);
+    auto unprocessed_bundles = move(m_unprocessed_bundles);
 
     HashMap<int, Size> latest_size_for_window_id;
-    for (auto& event : unprocessed_events) {
+    for (auto& bundle : unprocessed_bundles) {
+        auto& event = bundle.message;
         if (event.type == WSAPI_ServerMessage::Type::WindowResized) {
             latest_size_for_window_id.set(event.window_id, event.window.rect.size);
         }
@@ -213,7 +218,8 @@ void GEventLoop::process_unprocessed_messages()
 
     int paint_count = 0;
     HashMap<int, Size> latest_paint_size_for_window_id;
-    for (auto& event : unprocessed_events) {
+    for (auto& bundle : unprocessed_bundles) {
+        auto& event = bundle.message;
         if (event.type == WSAPI_ServerMessage::Type::Paint) {
             ++paint_count;
 #ifdef COALESCING_DEBUG
@@ -226,7 +232,8 @@ void GEventLoop::process_unprocessed_messages()
     dbgprintf("paint_count: %d\n", paint_count);
 #endif
 
-    for (auto& event : unprocessed_events) {
+    for (auto& bundle : unprocessed_bundles) {
+        auto& event = bundle.message;
         if (event.type == WSAPI_ServerMessage::Type::Greeting) {
             s_server_pid = event.greeting.server_pid;
             GDesktop::the().did_receive_screen_rect(Badge<GEventLoop>(), event.greeting.screen_rect);
@@ -264,7 +271,7 @@ void GEventLoop::process_unprocessed_messages()
                 ++coalesced_paints;
                 break;
             }
-            handle_paint_event(event, *window);
+            handle_paint_event(event, *window, bundle.extra_data);
             break;
         case WSAPI_ServerMessage::Type::MouseDown:
         case WSAPI_ServerMessage::Type::MouseUp:
@@ -310,8 +317,8 @@ void GEventLoop::process_unprocessed_messages()
         dbgprintf("Coalesced %d resizes\n", coalesced_resizes);
 #endif
 
-    if (!m_unprocessed_messages.is_empty())
+    if (!m_unprocessed_bundles.is_empty())
-        process_unprocessed_messages();
+        process_unprocessed_bundles();
 }
 
 bool GEventLoop::drain_messages_from_server()
@@ -334,15 +341,30 @@ bool GEventLoop::drain_messages_from_server()
             return true;
         }
         assert(nread == sizeof(message));
-        m_unprocessed_messages.append(move(message));
+        ByteBuffer extra_data;
+        if (message.extra_size) {
+            extra_data = ByteBuffer::create_uninitialized(message.extra_size);
+            int extra_nread = read(s_event_fd, extra_data.data(), extra_data.size());
+            ASSERT(extra_nread == message.extra_size);
+        }
+        m_unprocessed_bundles.append({ move(message), move(extra_data) });
         is_first_pass = false;
     }
 }
 
-bool GEventLoop::post_message_to_server(const WSAPI_ClientMessage& message)
+bool GEventLoop::post_message_to_server(const WSAPI_ClientMessage& message, const ByteBuffer& extra_data)
 {
+    if (!extra_data.is_empty())
+        const_cast<WSAPI_ClientMessage&>(message).extra_size = extra_data.size();
+
     int nwritten = write(s_event_fd, &message, sizeof(WSAPI_ClientMessage));
-    return nwritten == sizeof(WSAPI_ClientMessage);
+    ASSERT(nwritten == sizeof(WSAPI_ClientMessage));
+    if (!extra_data.is_empty()) {
+        nwritten = write(s_event_fd, extra_data.data(), extra_data.size());
+        ASSERT(nwritten == extra_data.size());
+    }
+
+    return true;
 }
 
 bool GEventLoop::wait_for_specific_event(WSAPI_ServerMessage::Type type, WSAPI_ServerMessage& event)
@@ -357,10 +379,10 @@ bool GEventLoop::wait_for_specific_event(WSAPI_ServerMessage::Type type, WSAPI_S
         bool success = drain_messages_from_server();
         if (!success)
             return false;
-        for (ssize_t i = 0; i < m_unprocessed_messages.size(); ++i) {
+        for (ssize_t i = 0; i < m_unprocessed_bundles.size(); ++i) {
-            if (m_unprocessed_messages[i].type == type) {
+            if (m_unprocessed_bundles[i].message.type == type) {
-                event = move(m_unprocessed_messages[i]);
+                event = move(m_unprocessed_bundles[i].message);
-                m_unprocessed_messages.remove(i);
+                m_unprocessed_bundles.remove(i);
                 return true;
             }
         }

LibGUI/GEventLoop.h

@@ -16,7 +16,7 @@ public:
 
     static GEventLoop& current() { return static_cast<GEventLoop&>(CEventLoop::current()); }
 
-    static bool post_message_to_server(const WSAPI_ClientMessage&);
+    static bool post_message_to_server(const WSAPI_ClientMessage&, const ByteBuffer& extra_data = { });
     bool wait_for_specific_event(WSAPI_ServerMessage::Type, WSAPI_ServerMessage&);
     WSAPI_ServerMessage sync_request(const WSAPI_ClientMessage& request, WSAPI_ServerMessage::Type response_type);
 
@@ -25,7 +25,7 @@ public:
     virtual void take_pending_events_from(CEventLoop& other) override
     {
         CEventLoop::take_pending_events_from(other);
-        m_unprocessed_messages.append(move(static_cast<GEventLoop&>(other).m_unprocessed_messages));
+        m_unprocessed_bundles.append(move(static_cast<GEventLoop&>(other).m_unprocessed_bundles));
     }
 
 private:
@@ -43,13 +43,13 @@ private:
 
     virtual void do_processing() override
     {
-        process_unprocessed_messages();
+        process_unprocessed_bundles();
     }
 
     void wait_for_event();
     bool drain_messages_from_server();
-    void process_unprocessed_messages();
+    void process_unprocessed_bundles();
-    void handle_paint_event(const WSAPI_ServerMessage&, GWindow&);
+    void handle_paint_event(const WSAPI_ServerMessage&, GWindow&, const ByteBuffer& extra_data);
     void handle_resize_event(const WSAPI_ServerMessage&, GWindow&);
     void handle_mouse_event(const WSAPI_ServerMessage&, GWindow&);
     void handle_key_event(const WSAPI_ServerMessage&, GWindow&);
@@ -60,7 +60,12 @@ private:
     void handle_wm_event(const WSAPI_ServerMessage&, GWindow&);
     void connect_to_server();
 
-    Vector<WSAPI_ServerMessage, 64> m_unprocessed_messages;
+    struct IncomingWSMessageBundle {
+        WSAPI_ServerMessage message;
+        ByteBuffer extra_data;
+    };
+
+    Vector<IncomingWSMessageBundle, 64> m_unprocessed_bundles;
     static pid_t s_server_pid;
     static pid_t s_event_fd;
 };

LibGUI/GWindow.cpp

@@ -240,10 +240,13 @@ void GWindow::event(CEvent& event)
             WSAPI_ClientMessage message;
             message.type = WSAPI_ClientMessage::Type::DidFinishPainting;
             message.window_id = m_window_id;
-            message.rect_count = paint_event.rects().size();
+            message.rect_count = rects.size();
-            for (int i = 0; i < paint_event.rects().size(); ++i)
+            for (int i = 0; i < min(WSAPI_ClientMessage::max_inline_rect_count, rects.size()); ++i)
-                message.rects[i] = paint_event.rects()[i];
+                message.rects[i] = rects[i];
-            GEventLoop::current().post_message_to_server(message);
+            ByteBuffer extra_data;
+            if (rects.size() > WSAPI_ClientMessage::max_inline_rect_count)
+                extra_data = ByteBuffer::wrap((void*)&rects[WSAPI_ClientMessage::max_inline_rect_count], (rects.size() - WSAPI_ClientMessage::max_inline_rect_count) * sizeof(WSAPI_Rect));
+            GEventLoop::current().post_message_to_server(message, extra_data);
         }
         return;
     }
@@ -279,7 +282,10 @@ void GWindow::event(CEvent& event)
         auto new_size = static_cast<GResizeEvent&>(event).size();
         if (m_back_bitmap && m_back_bitmap->size() != new_size)
             m_back_bitmap = nullptr;
-        m_pending_paint_event_rects.clear();
+        if (!m_pending_paint_event_rects.is_empty()) {
+            m_pending_paint_event_rects.clear_with_capacity();
+            m_pending_paint_event_rects.append({ { }, new_size });
+        }
         m_rect_when_windowless = { { }, new_size };
         m_main_widget->set_relative_rect({ { }, new_size });
         return;
@@ -311,15 +317,16 @@ void GWindow::update(const Rect& a_rect)
 
     if (m_pending_paint_event_rects.is_empty()) {
         deferred_invoke([this] (auto&) {
-            // FIXME: Break it into multiple batches if needed.
-            ASSERT(m_pending_paint_event_rects.size() <= 32);
             WSAPI_ClientMessage request;
             request.type = WSAPI_ClientMessage::Type::InvalidateRect;
             request.window_id = m_window_id;
-            for (int i = 0; i < m_pending_paint_event_rects.size(); ++i)
+            for (int i = 0; i < min(WSAPI_ClientMessage::max_inline_rect_count, m_pending_paint_event_rects.size()); ++i)
                 request.rects[i] = m_pending_paint_event_rects[i];
+            ByteBuffer extra_data;
+            if (m_pending_paint_event_rects.size() > WSAPI_ClientMessage::max_inline_rect_count)
+                extra_data = ByteBuffer::wrap((void*)&m_pending_paint_event_rects[WSAPI_ClientMessage::max_inline_rect_count], (m_pending_paint_event_rects.size() - WSAPI_ClientMessage::max_inline_rect_count) * sizeof(WSAPI_Rect));
             request.rect_count = m_pending_paint_event_rects.size();
-            GEventLoop::current().post_message_to_server(request);
+            GEventLoop::current().post_message_to_server(request, extra_data);
             m_pending_paint_event_rects.clear_with_capacity();
         });
     }

LibGUI/GWindow.h

@@ -130,7 +130,7 @@ private:
     Rect m_rect_when_windowless;
     String m_title_when_windowless;
     String m_icon_path;
-    Vector<Rect> m_pending_paint_event_rects;
+    Vector<Rect, 32> m_pending_paint_event_rects;
     Size m_size_increment;
     Size m_base_size;
     Color m_background_color { Color::LightGray };

Servers/WindowServer/WSAPITypes.h

@@ -109,12 +109,14 @@ struct WSAPI_ServerMessage {
     };
     Type type { Invalid };
     int window_id { -1 };
+    unsigned extra_size { 0 };
 
     union {
         int text_length { 0 };
         int rect_count;
     };
 
+    static const int max_inline_rect_count = 1;
     union {
         char text[512];
         WSAPI_Rect rects[32];
@@ -214,13 +216,16 @@ struct WSAPI_ClientMessage {
     };
     Type type { Invalid };
     int window_id { -1 };
+    unsigned extra_size { 0 };
     union {
         int text_length { 0 };
         int rect_count;
     };
+
+    static const int max_inline_rect_count = 1;
     union {
         char text[512];
-        WSAPI_Rect rects[32];
+        WSAPI_Rect rects[max_inline_rect_count];
     };
     int value { 0 };
 

Servers/WindowServer/WSClientConnection.cpp

@@ -70,8 +70,11 @@ void WSClientConnection::post_error(const String& error_message)
     post_message(message);
 }
 
-void WSClientConnection::post_message(const WSAPI_ServerMessage& message)
+void WSClientConnection::post_message(const WSAPI_ServerMessage& message, const ByteBuffer& extra_data)
 {
+    if (!extra_data.is_empty())
+        const_cast<WSAPI_ServerMessage&>(message).extra_size = extra_data.size();
+
     int nwritten = write(m_fd, &message, sizeof(message));
     if (nwritten < 0) {
         if (errno == EPIPE) {
@@ -83,6 +86,20 @@ void WSClientConnection::post_message(const WSAPI_ServerMessage& message)
     }
 
     ASSERT(nwritten == sizeof(message));
+
+    if (!extra_data.is_empty()) {
+        nwritten = write(m_fd, extra_data.data(), extra_data.size());
+        if (nwritten < 0) {
+            if (errno == EPIPE) {
+                dbgprintf("WSClientConnection::post_message: Disconnected from peer during extra_data write.\n");
+                return;
+            }
+            perror("WSClientConnection::post_message write (extra_data)");
+            ASSERT_NOT_REACHED();
+        }
+
+        ASSERT(nwritten == extra_data.size());
+    }
 }
 
 void WSClientConnection::notify_about_new_screen_rect(const Rect& rect)
@@ -93,19 +110,28 @@ void WSClientConnection::notify_about_new_screen_rect(const Rect& rect)
     post_message(message);
 }
 
-void WSClientConnection::event(CEvent& message)
+void WSClientConnection::event(CEvent& event)
 {
-    if (static_cast<WSEvent&>(message).is_client_request()) {
+    if (static_cast<WSEvent&>(event).is_client_request()) {
-        on_request(static_cast<const WSAPIClientRequest&>(message));
+        on_request(static_cast<const WSAPIClientRequest&>(event));
         return;
     }
 
-    if (message.type() == WSEvent::WM_ClientDisconnected) {
+    if (event.type() == WSEvent::WM_ClientDisconnected) {
-        int client_id = static_cast<const WSClientDisconnectedNotification&>(message).client_id();
+        int client_id = static_cast<const WSClientDisconnectedNotification&>(event).client_id();
         dbgprintf("WSClientConnection: Client disconnected: %d\n", client_id);
         delete this;
         return;
     }
+
+    CObject::event(event);
+}
+
+void WSClientConnection::did_misbehave()
+{
+    dbgprintf("WSClientConnection{%p} (id=%d, pid=%d) misbehaved, disconnecting.\n", this, m_client_id, m_pid);
+    // FIXME: We should make sure we avoid processing any further messages from this client.
+    delete_later();
 }
 
 void WSClientConnection::handle_request(const WSAPICreateMenubarRequest&)
@@ -484,13 +510,14 @@ void WSClientConnection::post_paint_message(WSWindow& window)
     message.window_id = window.window_id();
     auto rect_set = window.take_pending_paint_rects();
     auto& rects = rect_set.rects();
-    // FIXME: Break it into multiple batches if needed.
-    ASSERT(rects.size() <= 32);
     message.rect_count = rects.size();
-    for (int i = 0; i < rects.size(); ++i)
+    for (int i = 0; i < min(WSAPI_ServerMessage::max_inline_rect_count, rects.size()); ++i)
         message.rects[i] = rects[i];
     message.paint.window_size = window.size();
-    post_message(message);
+    ByteBuffer extra_data;
+    if (rects.size() > WSAPI_ServerMessage::max_inline_rect_count)
+        extra_data = ByteBuffer::wrap((void*)&rects[WSAPI_ServerMessage::max_inline_rect_count], (rects.size() - WSAPI_ServerMessage::max_inline_rect_count) * sizeof(WSAPI_Rect));
+    post_message(message, extra_data);
 }
 
 void WSClientConnection::handle_request(const WSAPIInvalidateRectRequest& request)

Servers/WindowServer/WSClientConnection.h

@@ -21,7 +21,7 @@ public:
     static WSClientConnection* from_client_id(int client_id);
     static void for_each_client(Function<void(WSClientConnection&)>);
 
-    void post_message(const WSAPI_ServerMessage&);
+    void post_message(const WSAPI_ServerMessage&, const ByteBuffer& = { });
 
     int client_id() const { return m_client_id; }
     WSMenuBar* app_menubar() { return m_app_menubar.ptr(); }
@@ -39,6 +39,8 @@ public:
     void notify_about_new_screen_rect(const Rect&);
     void post_paint_message(WSWindow&);
 
+    void did_misbehave();
+
 private:
     virtual void event(CEvent&) override;
 

Servers/WindowServer/WSEventLoop.cpp

@@ -115,7 +115,23 @@ static WSWindowType from_api(WSAPI_WindowType api_type)
     }
 }
 
-void WSEventLoop::on_receive_from_client(int client_id, const WSAPI_ClientMessage& message)
+static Vector<Rect, 32> get_rects(const WSAPI_ClientMessage& message, const ByteBuffer& extra_data)
+{
+    Vector<Rect, 32> rects;
+    if (message.rect_count > (WSAPI_ClientMessage::max_inline_rect_count + extra_data.size() / sizeof(WSAPI_Rect))) {
+        return { };
+    }
+    for (int i = 0; i < min(WSAPI_ClientMessage::max_inline_rect_count, message.rect_count); ++i)
+        rects.append(message.rects[i]);
+    if (!extra_data.is_empty()) {
+        auto* extra_rects = reinterpret_cast<const WSAPI_Rect*>(extra_data.data());
+        for (int i = 0; i < (extra_data.size() / sizeof(WSAPI_Rect)); ++i)
+            rects.append(extra_rects[i]);
+    }
+    return rects;
+}
+
+bool WSEventLoop::on_receive_from_client(int client_id, const WSAPI_ClientMessage& message, ByteBuffer&& extra_data)
 {
     WSClientConnection& client = *WSClientConnection::from_client_id(client_id);
     switch (message.type) {
@@ -192,18 +208,20 @@ void WSEventLoop::on_receive_from_client(int client_id, const WSAPI_ClientMessag
         post_event(client, make<WSAPIGetClipboardContentsRequest>(client_id));
         break;
     case WSAPI_ClientMessage::Type::InvalidateRect: {
-        Vector<Rect, 32> rects;
+        auto rects = get_rects(message, extra_data);
-        ASSERT(message.rect_count <= 32);
+        if (rects.is_empty()) {
-        for (int i = 0; i < message.rect_count; ++i)
+            client.did_misbehave();
-            rects.append(message.rects[i]);
+            return false;
+        }
         post_event(client, make<WSAPIInvalidateRectRequest>(client_id, message.window_id, rects));
         break;
     }
     case WSAPI_ClientMessage::Type::DidFinishPainting: {
-        Vector<Rect, 32> rects;
+        auto rects = get_rects(message, extra_data);
-        ASSERT(message.rect_count <= 32);
+        if (rects.is_empty()) {
-        for (int i = 0; i < message.rect_count; ++i)
+            client.did_misbehave();
-            rects.append(message.rects[i]);
+            return false;
+        }
         post_event(client, make<WSAPIDidFinishPaintingNotification>(client_id, message.window_id, rects));
         break;
     }
@@ -232,6 +250,7 @@ void WSEventLoop::on_receive_from_client(int client_id, const WSAPI_ClientMessag
     default:
         break;
     }
+    return true;
 }
 
 void WSEventLoop::add_file_descriptors_for_select(fd_set& fds, int& max_fd_added)
@@ -279,7 +298,22 @@ void WSEventLoop::drain_client(WSClientConnection& client)
             perror("read");
             ASSERT_NOT_REACHED();
         }
-        on_receive_from_client(client.client_id(), message);
+        ByteBuffer extra_data;
+        if (message.extra_size) {
+            if (message.extra_size >= 32768) {
+                dbgprintf("message.extra_size is way too large\n");
+                return client.did_misbehave();
+            }
+
+            extra_data = ByteBuffer::create_uninitialized(message.extra_size);
+            int extra_nread = read(client.fd(), extra_data.data(), extra_data.size());
+            if (extra_nread != message.extra_size) {
+                dbgprintf("extra_nread(%d) != extra_size(%d)\n", extra_nread, extra_data.size());
+                return client.did_misbehave();
+            }
+        }
+        if (!on_receive_from_client(client.client_id(), message, move(extra_data)))
+            return;
         ++messages_received;
     }
 }

Servers/WindowServer/WSEventLoop.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <LibCore/CEventLoop.h>
+#include <AK/ByteBuffer.h>
 
 class WSClientConnection;
 struct WSAPI_ClientMessage;
@@ -20,7 +21,7 @@ private:
     void drain_mouse();
     void drain_keyboard();
     void drain_client(WSClientConnection&);
-    void on_receive_from_client(int client_id, const WSAPI_ClientMessage&);
+    bool on_receive_from_client(int client_id, const WSAPI_ClientMessage&, ByteBuffer&& extra_data);
 
     int m_keyboard_fd { -1 };
     int m_mouse_fd { -1 };

Servers/WindowServer/WSMenu.cpp

@@ -134,6 +134,8 @@ void WSMenu::event(CEvent& event)
         clear_hovered_item();
         return;
     }
+
+    CObject::event(event);
 }
 
 void WSMenu::clear_hovered_item()

Servers/WindowServer/WSWindowFrame.cpp

@@ -239,7 +239,7 @@ void WSWindowFrame::on_mouse_event(const WSMouseEvent& event)
         return;
     }
 
-    if (event.type() == WSEvent::MouseMove && event.buttons() == 0) {
+    if (m_window.is_resizable() && event.type() == WSEvent::MouseMove && event.buttons() == 0) {
         constexpr ResizeDirection direction_for_hot_area[3][3] = {
             { ResizeDirection::UpLeft, ResizeDirection::Up, ResizeDirection::UpRight },
             { ResizeDirection::Left, ResizeDirection::None, ResizeDirection::Right },
@@ -256,6 +256,6 @@ void WSWindowFrame::on_mouse_event(const WSMouseEvent& event)
         return;
     }
 
-    if (event.button() == MouseButton::Left)
+    if (m_window.is_resizable() && event.button() == MouseButton::Left)
         wm.start_window_resize(m_window, event.translated(rect().location()));
 }

Servers/WindowServer/WSWindowManager.cpp

@@ -681,7 +681,7 @@ void WSWindowManager::process_mouse_event(const WSMouseEvent& event, WSWindow*&
                 start_window_drag(window, event);
                 return IterationDecision::Abort;
             }
-            if (m_keyboard_modifiers == Mod_Logo && event.type() == WSEvent::MouseDown && event.button() == MouseButton::Right && !window.is_blocked_by_modal_window()) {
+            if (window.is_resizable() && m_keyboard_modifiers == Mod_Logo && event.type() == WSEvent::MouseDown && event.button() == MouseButton::Right && !window.is_blocked_by_modal_window()) {
                 start_window_resize(window, event);
                 return IterationDecision::Abort;
             }
@@ -983,6 +983,8 @@ void WSWindowManager::event(CEvent& event)
         compose();
         return;
     }
+
+    CObject::event(event);
 }
 
 void WSWindowManager::set_highlight_window(WSWindow* window)