From 0e3397aabe56941b86b8730ec0b17cb744819ba0 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 31 Oct 2021 23:38:04 +0100 Subject: [PATCH] WindowServer: Fix visibility of WindowSwitcher constructor Derivatives of Core::Object should be constructed through ClassName::construct(), to avoid handling ref-counted objects with refcount zero. Fixing the visibility means that misuses like this are more difficult. This commit is separate from the other Servives changes because it required additional adaption of the code. Note that the old code did precisely what these changes try to prevent: Create and handle a ref-counted object with a refcount of zero. --- Userland/Services/WindowServer/Compositor.cpp | 6 +-- .../Services/WindowServer/WindowManager.cpp | 41 ++++++++++--------- .../Services/WindowServer/WindowManager.h | 2 +- .../Services/WindowServer/WindowSwitcher.h | 3 +- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/Userland/Services/WindowServer/Compositor.cpp b/Userland/Services/WindowServer/Compositor.cpp index f66bda27f5..1e1d737d39 100644 --- a/Userland/Services/WindowServer/Compositor.cpp +++ b/Userland/Services/WindowServer/Compositor.cpp @@ -1084,10 +1084,10 @@ void Compositor::recompute_overlay_rects() void Compositor::recompute_occlusions() { auto& wm = WindowManager::the(); - bool is_switcher_visible = wm.m_switcher.is_visible(); + bool is_switcher_visible = wm.m_switcher->is_visible(); auto never_occlude = [&](WindowStack& window_stack) { if (is_switcher_visible) { - switch (wm.m_switcher.mode()) { + switch (wm.m_switcher->mode()) { case WindowSwitcher::Mode::ShowCurrentDesktop: // Any window on the currently rendered desktop should not be occluded, even if it's behind // another window entirely. @@ -1540,7 +1540,7 @@ void Compositor::finish_window_stack_switch() m_window_stack_transition_animation = nullptr; auto& wm = WindowManager::the(); - if (!wm.m_switcher.is_visible()) + if (!wm.m_switcher->is_visible()) previous_window_stack->set_all_occluded(true); wm.did_switch_window_stack({}, *previous_window_stack, *m_current_window_stack); diff --git a/Userland/Services/WindowServer/WindowManager.cpp b/Userland/Services/WindowServer/WindowManager.cpp index 810035d881..642d1fb15e 100644 --- a/Userland/Services/WindowServer/WindowManager.cpp +++ b/Userland/Services/WindowServer/WindowManager.cpp @@ -35,7 +35,8 @@ WindowManager& WindowManager::the() } WindowManager::WindowManager(Gfx::PaletteImpl const& palette) - : m_palette(palette) + : m_switcher(WindowSwitcher::construct()) + , m_palette(palette) { s_the = this; @@ -313,8 +314,8 @@ void WindowManager::add_window(Window& window) if (window.type() != WindowType::Desktop || is_first_window) set_active_window(&window); - if (m_switcher.is_visible() && window.type() != WindowType::WindowSwitcher) - m_switcher.refresh(); + if (m_switcher->is_visible() && window.type() != WindowType::WindowSwitcher) + m_switcher->refresh(); Compositor::the().invalidate_occlusions(); @@ -360,10 +361,10 @@ void WindowManager::do_move_to_front(Window& window, bool make_active, bool make if (make_active) set_active_window(&window, make_input); - if (m_switcher.is_visible()) { - m_switcher.refresh(); + if (m_switcher->is_visible()) { + m_switcher->refresh(); if (!window.is_accessory()) { - m_switcher.select_window(window); + m_switcher->select_window(window); set_highlight_window(&window); } } @@ -386,8 +387,8 @@ void WindowManager::remove_window(Window& window) window.invalidate_last_rendered_screen_rects_now(); - if (m_switcher.is_visible() && window.type() != WindowType::WindowSwitcher) - m_switcher.refresh(); + if (m_switcher->is_visible() && window.type() != WindowType::WindowSwitcher) + m_switcher->refresh(); Compositor::the().invalidate_occlusions(); @@ -543,8 +544,8 @@ static bool window_type_has_title(WindowType type) void WindowManager::notify_modified_changed(Window& window) { - if (m_switcher.is_visible()) - m_switcher.refresh(); + if (m_switcher->is_visible()) + m_switcher->refresh(); tell_wms_window_state_changed(window); } @@ -556,8 +557,8 @@ void WindowManager::notify_title_changed(Window& window) dbgln_if(WINDOWMANAGER_DEBUG, "[WM] Window({}) title set to '{}'", &window, window.title()); - if (m_switcher.is_visible()) - m_switcher.refresh(); + if (m_switcher->is_visible()) + m_switcher->refresh(); tell_wms_window_state_changed(window); } @@ -569,8 +570,8 @@ void WindowManager::notify_modal_unparented(Window& window) dbgln_if(WINDOWMANAGER_DEBUG, "[WM] Window({}) was unparented", &window); - if (m_switcher.is_visible()) - m_switcher.refresh(); + if (m_switcher->is_visible()) + m_switcher->refresh(); tell_wms_window_state_changed(window); } @@ -579,8 +580,8 @@ void WindowManager::notify_rect_changed(Window& window, Gfx::IntRect const& old_ { dbgln_if(RESIZE_DEBUG, "[WM] Window({}) rect changed {} -> {}", &window, old_rect, new_rect); - if (m_switcher.is_visible() && window.type() != WindowType::WindowSwitcher) - m_switcher.refresh(); + if (m_switcher->is_visible() && window.type() != WindowType::WindowSwitcher) + m_switcher->refresh(); tell_wms_window_rect_changed(window); @@ -1547,12 +1548,12 @@ void WindowManager::process_key_event(KeyEvent& event) if (event.type() == Event::KeyDown) { if ((event.modifiers() == Mod_Super && event.key() == Key_Tab) || (event.modifiers() == (Mod_Super | Mod_Shift) && event.key() == Key_Tab)) - m_switcher.show(WindowSwitcher::Mode::ShowAllWindows); + m_switcher->show(WindowSwitcher::Mode::ShowAllWindows); else if ((event.modifiers() == Mod_Alt && event.key() == Key_Tab) || (event.modifiers() == (Mod_Alt | Mod_Shift) && event.key() == Key_Tab)) - m_switcher.show(WindowSwitcher::Mode::ShowCurrentDesktop); + m_switcher->show(WindowSwitcher::Mode::ShowCurrentDesktop); } - if (m_switcher.is_visible()) { - m_switcher.on_key_event(event); + if (m_switcher->is_visible()) { + m_switcher->on_key_event(event); return; } diff --git a/Userland/Services/WindowServer/WindowManager.h b/Userland/Services/WindowServer/WindowManager.h index 4114343554..b0acb20a2b 100644 --- a/Userland/Services/WindowServer/WindowManager.h +++ b/Userland/Services/WindowServer/WindowManager.h @@ -432,7 +432,7 @@ private: u8 m_keyboard_modifiers { 0 }; - WindowSwitcher m_switcher; + NonnullRefPtr m_switcher; WeakPtr