LibIPC+WindowServer+LibGUI: Detect and highlight unresponsive GUI apps

IPC::ClientConnection now tracks the time since the last time we got
a message from the client and calls a virtual function on itself after
3 seconds: may_have_become_unresponsive().

Subclasses of ClientConnection can then react to this if they like.

We use this mechanism in WindowServer to send out a friendly Ping
message to the client. If he doesn't Pong within 1 second, we mark
the client as "unresponsive" and recompose all of his windows with
a darkened appearance and amended title until he Pongs us.

This is a little on the aggressive side and we should figure out a way
to wake up less often. Perhaps this could only be done to windows the
user is currently interacting with, for example.

Anyways, this is pretty cool! :^)
This commit is contained in:
Andreas Kling 2020-06-11 22:46:49 +02:00
parent 940fbea3a7
commit 2ce2c4810a
9 changed files with 84 additions and 7 deletions

View file

@ -334,4 +334,9 @@ void WindowServerConnection::handle(const Messages::WindowClient::DisplayLinkNot
});
}
void WindowServerConnection::handle(const Messages::WindowClient::Ping&)
{
post_message(Messages::WindowServer::Pong());
}
}

View file

@ -74,6 +74,7 @@ private:
virtual void handle(const Messages::WindowClient::UpdateSystemTheme&) override;
virtual void handle(const Messages::WindowClient::WindowStateChanged&) override;
virtual void handle(const Messages::WindowClient::DisplayLinkNotification&) override;
virtual void handle(const Messages::WindowClient::Ping&) override;
bool m_display_link_notification_pending { false };
};

View file

@ -31,6 +31,7 @@
#include <LibCore/EventLoop.h>
#include <LibCore/LocalSocket.h>
#include <LibCore/Object.h>
#include <LibCore/Timer.h>
#include <LibIPC/Endpoint.h>
#include <LibIPC/Message.h>
#include <errno.h>
@ -91,12 +92,21 @@ public:
m_client_pid = creds.pid;
add_child(socket);
m_socket->on_ready_to_read = [this] { drain_messages_from_client(); };
m_responsiveness_timer = Core::Timer::construct();
m_responsiveness_timer->set_single_shot(true);
m_responsiveness_timer->set_interval(3000);
m_responsiveness_timer->on_timeout = [this] {
may_have_become_unresponsive();
};
}
virtual ~ClientConnection() override
{
}
virtual void may_have_become_unresponsive() {}
void post_message(const Message& message)
{
// NOTE: If this connection is being shut down, but has not yet been destroyed,
@ -151,6 +161,9 @@ public:
bytes.append(buffer, nread);
}
if (!bytes.is_empty())
m_responsiveness_timer->start();
size_t decoded_bytes = 0;
for (size_t index = 0; index < bytes.size(); index += decoded_bytes) {
auto remaining_bytes = ByteBuffer::wrap(bytes.data() + index, bytes.size() - index);
@ -205,6 +218,7 @@ protected:
private:
Endpoint& m_endpoint;
RefPtr<Core::LocalSocket> m_socket;
RefPtr<Core::Timer> m_responsiveness_timer;
int m_client_id { -1 };
int m_client_pid { -1 };
};

View file

@ -820,4 +820,28 @@ void ClientConnection::handle(const Messages::WindowServer::SetWindowProgress& m
it->value->set_progress(message.progress());
}
void ClientConnection::handle(const Messages::WindowServer::Pong&)
{
m_ping_timer = nullptr;
set_unresponsive(false);
}
void ClientConnection::set_unresponsive(bool unresponsive)
{
if (m_unresponsive == unresponsive)
return;
m_unresponsive = unresponsive;
for (auto& it : m_windows) {
it.value->invalidate();
}
}
void ClientConnection::may_have_become_unresponsive()
{
post_message(Messages::WindowClient::Ping());
m_ping_timer = Core::Timer::create_single_shot(1000, [this] {
set_unresponsive(true);
});
}
}

View file

@ -50,7 +50,8 @@ class ClientConnection final
C_OBJECT(ClientConnection)
public:
~ClientConnection() override;
virtual void die() override;
bool is_unresponsive() const { return m_unresponsive; }
void boost();
void deboost();
@ -85,6 +86,11 @@ public:
private:
explicit ClientConnection(Core::LocalSocket&, int client_id);
// ^ClientConnection
virtual void die() override;
virtual void may_have_become_unresponsive() override;
void set_unresponsive(bool);
void destroy_window(Window&, Vector<i32>& destroyed_window_ids);
virtual OwnPtr<Messages::WindowServer::GreetResponse> handle(const Messages::WindowServer::Greet&) override;
@ -134,6 +140,7 @@ private:
virtual void handle(const Messages::WindowServer::EnableDisplayLink&) override;
virtual void handle(const Messages::WindowServer::DisableDisplayLink&) override;
virtual void handle(const Messages::WindowServer::SetWindowProgress&) override;
virtual void handle(const Messages::WindowServer::Pong&) override;
Window* window_from_id(i32 window_id);
@ -142,11 +149,14 @@ private:
HashMap<int, NonnullRefPtr<Menu>> m_menus;
WeakPtr<MenuBar> m_app_menubar;
RefPtr<Core::Timer> m_ping_timer;
int m_next_menubar_id { 10000 };
int m_next_menu_id { 20000 };
int m_next_window_id { 1982 };
bool m_has_display_link { false };
bool m_unresponsive { false };
};
}

View file

@ -217,15 +217,22 @@ void Compositor::compose()
}
Gfx::IntRect dirty_rect_in_backing_coordinates = dirty_rect
.intersected(window.rect())
.intersected(backing_rect)
.translated(-backing_rect.location());
.intersected(window.rect())
.intersected(backing_rect)
.translated(-backing_rect.location());
if (dirty_rect_in_backing_coordinates.is_empty())
continue;
auto dst = backing_rect.location().translated(dirty_rect_in_backing_coordinates.location());
m_back_painter->blit(dst, *backing_store, dirty_rect_in_backing_coordinates, window.opacity());
if (window.client() && window.client()->is_unresponsive()) {
m_back_painter->blit_filtered(dst, *backing_store, dirty_rect_in_backing_coordinates, [](Color src) {
return src.to_grayscale().darkened(0.75f);
});
} else {
m_back_painter->blit(dst, *backing_store, dirty_rect_in_backing_coordinates, window.opacity());
}
for (auto background_rect : window.rect().shatter(backing_rect))
m_back_painter->fill_rect(background_rect, wm.palette().window());
}

View file

@ -35,4 +35,6 @@ endpoint WindowClient = 4
UpdateSystemTheme(i32 system_theme_buffer_id) =|
DisplayLinkNotification() =|
Ping() =|
}

View file

@ -24,6 +24,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include "ClientConnection.h"
#include <AK/Badge.h>
#include <LibGfx/CharacterBitmap.h>
#include <LibGfx/Font.h>
@ -244,12 +245,23 @@ void WindowFrame::paint_normal_frame(Gfx::Painter& painter)
}
}
String title_text;
if (window.client() && window.client()->is_unresponsive()) {
StringBuilder builder;
builder.append(window.title());
builder.append(" (Not responding)");
title_text = builder.to_string();
} else {
title_text = window.title();
}
auto clipped_title_rect = titlebar_title_rect;
clipped_title_rect.set_width(stripe_right - clipped_title_rect.x());
if (!clipped_title_rect.is_empty()) {
painter.draw_text(clipped_title_rect.translated(1, 2), window.title(), wm.window_title_font(), Gfx::TextAlignment::CenterLeft, border_color.darkened(0.4), Gfx::TextElision::Right);
painter.draw_text(clipped_title_rect.translated(1, 2), title_text, wm.window_title_font(), Gfx::TextAlignment::CenterLeft, border_color.darkened(0.4), Gfx::TextElision::Right);
// FIXME: The translated(0, 1) wouldn't be necessary if we could center text based on its baseline.
painter.draw_text(clipped_title_rect.translated(0, 1), window.title(), wm.window_title_font(), Gfx::TextAlignment::CenterLeft, title_color, Gfx::TextElision::Right);
painter.draw_text(clipped_title_rect.translated(0, 1), title_text, wm.window_title_font(), Gfx::TextAlignment::CenterLeft, title_color, Gfx::TextElision::Right);
}
painter.blit(titlebar_icon_rect.location(), window.icon(), window.icon().rect());

View file

@ -95,4 +95,6 @@ endpoint WindowServer = 2
EnableDisplayLink() =|
DisableDisplayLink() =|
Pong() =|
}