From 6df81c8a881b1defea57e4f490f9e09cbd753879 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 25 Jan 2020 10:25:49 +0100 Subject: [PATCH] WindowServer: Robustify WSWindow<->WSClientConnection link a bit WSWindow now detaches from WSClientConnection when being destroyed. Because of this, we can no longer rely on WSWindow::client() to tell us whether a window is internal to WindowServer or not. So instead we now have WSWindow::is_internal() which checks for client_id == -1. Note that WSWindow now also gets a copy of its client_id when constructed, since we can no longer rely on client() being valid after destruction has started. This allows various automatic mechanisms (e.g what happens in response to window removal) to skip over windows that are being destroyed. Fixes #387. --- Servers/WindowServer/WSClientConnection.cpp | 2 ++ Servers/WindowServer/WSWindow.cpp | 5 +++++ Servers/WindowServer/WSWindow.h | 6 ++++++ Servers/WindowServer/WSWindowManager.cpp | 17 +++++++++-------- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Servers/WindowServer/WSClientConnection.cpp b/Servers/WindowServer/WSClientConnection.cpp index 0b02575837..393a5de426 100644 --- a/Servers/WindowServer/WSClientConnection.cpp +++ b/Servers/WindowServer/WSClientConnection.cpp @@ -77,6 +77,8 @@ WSClientConnection::~WSClientConnection() { WSMenuManager::the().close_all_menus_from_client({}, *this); auto windows = move(m_windows); + for (auto& window : windows) + window.value->detach_client({}); } void WSClientConnection::die() diff --git a/Servers/WindowServer/WSWindow.cpp b/Servers/WindowServer/WSWindow.cpp index f3d168b056..a9980b9d94 100644 --- a/Servers/WindowServer/WSWindow.cpp +++ b/Servers/WindowServer/WSWindow.cpp @@ -63,6 +63,7 @@ WSWindow::WSWindow(WSClientConnection& client, WSWindowType window_type, int win , m_resizable(resizable) , m_fullscreen(fullscreen) , m_window_id(window_id) + , m_client_id(client.client_id()) , m_icon(default_window_icon()) , m_frame(*this) { @@ -76,6 +77,10 @@ WSWindow::WSWindow(WSClientConnection& client, WSWindowType window_type, int win WSWindow::~WSWindow() { + // Detach from client at the start of teardown since we don't want + // to confuse things by trying to send messages to it. + m_client = nullptr; + WSWindowManager::the().remove_window(*this); } diff --git a/Servers/WindowServer/WSWindow.h b/Servers/WindowServer/WSWindow.h index 91ab87e210..f77bd3ad30 100644 --- a/Servers/WindowServer/WSWindow.h +++ b/Servers/WindowServer/WSWindow.h @@ -114,6 +114,9 @@ public: WSWindowType type() const { return m_type; } int window_id() const { return m_window_id; } + bool is_internal() const { return m_client_id == -1; } + i32 client_id() const { return m_client_id; } + String title() const { return m_title; } void set_title(const String&); @@ -219,6 +222,8 @@ public: WSWindow* m_next { nullptr }; WSWindow* m_prev { nullptr }; + void detach_client(Badge) { m_client = nullptr; } + private: void handle_mouse_event(const WSMouseEvent&); void update_menu_item_text(PopupMenuItem item); @@ -248,6 +253,7 @@ private: RefPtr m_backing_store; RefPtr m_last_backing_store; int m_window_id { -1 }; + i32 m_client_id { -1 }; float m_opacity { 1 }; Size m_size_increment; Size m_base_size; diff --git a/Servers/WindowServer/WSWindowManager.cpp b/Servers/WindowServer/WSWindowManager.cpp index 67cc532314..2c1290a141 100644 --- a/Servers/WindowServer/WSWindowManager.cpp +++ b/Servers/WindowServer/WSWindowManager.cpp @@ -205,13 +205,14 @@ void WSWindowManager::remove_window(WSWindow& window) if (m_switcher.is_visible() && window.type() != WSWindowType::WindowSwitcher) m_switcher.refresh(); + recompute_occlusions(); for_each_window_listening_to_wm_events([&window](WSWindow& listener) { if (!(listener.wm_event_mask() & WSWMEventMask::WindowRemovals)) return IterationDecision::Continue; - if (window.client()) - listener.client()->post_message(WindowClient::WM_WindowRemoved(listener.window_id(), window.client()->client_id(), window.window_id())); + if (!window.is_internal()) + listener.client()->post_message(WindowClient::WM_WindowRemoved(listener.window_id(), window.client_id(), window.window_id())); return IterationDecision::Continue; }); } @@ -220,25 +221,25 @@ void WSWindowManager::tell_wm_listener_about_window(WSWindow& listener, WSWindow { if (!(listener.wm_event_mask() & WSWMEventMask::WindowStateChanges)) return; - if (!window.client()) + if (window.is_internal()) return; - listener.client()->post_message(WindowClient::WM_WindowStateChanged(listener.window_id(), window.client()->client_id(), window.window_id(), window.is_active(), window.is_minimized(), (i32)window.type(), window.title(), window.rect())); + listener.client()->post_message(WindowClient::WM_WindowStateChanged(listener.window_id(), window.client_id(), window.window_id(), window.is_active(), window.is_minimized(), (i32)window.type(), window.title(), window.rect())); } void WSWindowManager::tell_wm_listener_about_window_rect(WSWindow& listener, WSWindow& window) { if (!(listener.wm_event_mask() & WSWMEventMask::WindowRectChanges)) return; - if (!window.client()) + if (window.is_internal()) return; - listener.client()->post_message(WindowClient::WM_WindowRectChanged(listener.window_id(), window.client()->client_id(), window.window_id(), window.rect())); + listener.client()->post_message(WindowClient::WM_WindowRectChanged(listener.window_id(), window.client_id(), window.window_id(), window.rect())); } void WSWindowManager::tell_wm_listener_about_window_icon(WSWindow& listener, WSWindow& window) { if (!(listener.wm_event_mask() & WSWMEventMask::WindowIconChanges)) return; - if (!window.client()) + if (window.is_internal()) return; if (window.icon().shared_buffer_id() == -1) return; @@ -246,7 +247,7 @@ void WSWindowManager::tell_wm_listener_about_window_icon(WSWindow& listener, WSW if (share_buffer_with(window.icon().shared_buffer_id(), listener.client()->client_pid()) < 0) { ASSERT_NOT_REACHED(); } - listener.client()->post_message(WindowClient::WM_WindowIconBitmapChanged(listener.window_id(), window.client()->client_id(), window.window_id(), window.icon().shared_buffer_id(), window.icon().size())); + listener.client()->post_message(WindowClient::WM_WindowIconBitmapChanged(listener.window_id(), window.client_id(), window.window_id(), window.icon().shared_buffer_id(), window.icon().size())); } void WSWindowManager::tell_wm_listeners_window_state_changed(WSWindow& window)