WindowServer: Fix picking new active window after destroy

We need to mark windows as destroyed and not consider them
when picking a new active window. Fixes lost focus after
closing some windows.

4 files changed, 29 insertions, 7 deletions

diff --git a/Services/WindowServer/ClientConnection.cpp b/Services/WindowServer/ClientConnection.cpp
index 42a58c1037..9bbc8d6b7c 100644
--- a/Services/WindowServer/ClientConnection.cpp
+++ b/Services/WindowServer/ClientConnection.cpp
@@ -475,12 +475,19 @@ void ClientConnection::destroy_window(Window& window, Vector<i32>& destroyed_win
         destroy_window(*child_window, destroyed_window_ids);
     }
 
+    for (auto& accessory_window : window.accessory_windows()) {
+        if (!accessory_window)
+            continue;
+        ASSERT(accessory_window->window_id() != window.window_id());
+        destroy_window(*accessory_window, destroyed_window_ids);
+    }
+
     destroyed_window_ids.append(window.window_id());
 
     if (window.type() == WindowType::MenuApplet)
         AppletManager::the().remove_applet(window);
 
-    window.invalidate();
+    window.destroy();
     remove_child(window);
     m_windows.remove(window.window_id());
 }
diff --git a/Services/WindowServer/Window.cpp b/Services/WindowServer/Window.cpp
index 452b28e71f..c3bd001f50 100644
--- a/Services/WindowServer/Window.cpp
+++ b/Services/WindowServer/Window.cpp
@@ -126,6 +126,12 @@ Window::~Window()
     WindowManager::the().remove_window(*this);
 }
 
+void Window::destroy()
+{
+    m_destroyed = true;
+    invalidate();
+}
+
 void Window::set_title(const String& title)
 {
     if (m_title == title)
@@ -389,7 +395,7 @@ Window* Window::is_blocked_by_modal_window()
     // A window is blocked if any immediate child, or any child further
     // down the chain is modal
     for (auto& window: m_child_windows) {
-        if (window) {
+        if (window && !window->is_destroyed()) {
             if (window->is_modal())
                 return window;
             
diff --git a/Services/WindowServer/Window.h b/Services/WindowServer/Window.h
index 718d238c71..1a1696b452 100644
--- a/Services/WindowServer/Window.h
+++ b/Services/WindowServer/Window.h
@@ -256,6 +256,9 @@ public:
     int progress() const { return m_progress; }
     void set_progress(int);
 
+    bool is_destroyed() const { return m_destroyed; }
+    void destroy();
+
 private:
     void handle_mouse_event(const MouseEvent&);
     void update_menu_item_text(PopupMenuItem item);
@@ -289,6 +292,7 @@ private:
     bool m_maximized { false };
     bool m_fullscreen { false };
     bool m_accessory { false };
+    bool m_destroyed { false };
     WindowTileType m_tiled { WindowTileType::None };
     Gfx::IntRect m_untiled_rect;
     bool m_occluded { false };
diff --git a/Services/WindowServer/WindowManager.cpp b/Services/WindowServer/WindowManager.cpp
index aad228b0a7..e5175c2d99 100644
--- a/Services/WindowServer/WindowManager.cpp
+++ b/Services/WindowServer/WindowManager.cpp
@@ -260,7 +260,9 @@ void WindowManager::remove_window(Window& window)
 {
     window.invalidate();
     m_windows_in_order.remove(&window);
-    if (window.is_active())
+    auto* active = active_window();
+    auto* active_input = active_input_window();
+    if (active == &window || active_input == &window || (active && window.is_descendant_of(*active)) || (active_input && active_input != active && window.is_descendant_of(*active_input)))
         pick_new_active_window(&window);
     if (m_switcher.is_visible() && window.type() != WindowType::WindowSwitcher)
         m_switcher.refresh();
@@ -412,7 +414,10 @@ bool WindowManager::pick_new_active_window(Window* previous_active)
     bool new_window_picked = false;
     Window* first_candidate = nullptr;
     for_each_visible_window_of_type_from_front_to_back(WindowType::Normal, [&](Window& candidate) {
-        first_candidate = &candidate;
+        if (candidate.is_destroyed())
+            return IterationDecision::Continue;
+        if (previous_active != first_candidate)
+            first_candidate = &candidate;
         if ((!previous_active && !candidate.is_accessory()) || (previous_active && !candidate.is_accessory_of(*previous_active))) {
             set_active_window(&candidate);
             new_window_picked = true;
@@ -1167,10 +1172,10 @@ void WindowManager::set_active_window(Window* window, bool make_input)
             window = modal_window;
             make_input = true;
         }
-    }
 
-    if (window && !window_type_can_become_active(window->type()))
-        return;
+        if (!window_type_can_become_active(window->type()))
+            return;
+    }
 
     auto* new_active_input_window = window;
     if (window && window->is_accessory()) {