[wayland] Don't honor the idle inhibitor object if the surface is not visible

Summary:
Currently, our implementation of zwp_idle_inhibitor_v1 is not fully
spec-compliant. If the associated surface is not visible, we still
honor the idle inhibitor object.

This change addresses those spec-compliance issues. If a surface is
minimized or it's not on the current virtual desktop, then the
associated inhibitor object won't be honored by KWin.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: graesslin, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D17343

autotests/integration/idle_inhibition_test.cpp

@@ -45,6 +45,10 @@ private Q_SLOTS:
 
     void testInhibit_data();
     void testInhibit();
+    void testDontInhibitWhenNotOnCurrentDesktop();
+    void testDontInhibitWhenMinimized();
+    void testDontInhibitWhenUnmapped();
+    void testDontInhibitWhenLeftCurrentDesktop();
 };
 
 void TestIdleInhibition::initTestCase()
@@ -70,6 +74,9 @@ void TestIdleInhibition::init()
 void TestIdleInhibition::cleanup()
 {
     Test::destroyWaylandConnection();
+
+    VirtualDesktopManager::self()->setCount(1);
+    QCOMPARE(VirtualDesktopManager::self()->count(), 1u);
 }
 
 void TestIdleInhibition::testInhibit_data()
@@ -125,5 +132,232 @@ void TestIdleInhibition::testInhibit()
     QCOMPARE(inhibitedSpy.count(), 4);
 }
 
+void TestIdleInhibition::testDontInhibitWhenNotOnCurrentDesktop()
+{
+    // This test verifies that the idle inhibitor object is not honored when
+    // the associated surface is not on the current virtual desktop.
+
+    VirtualDesktopManager::self()->setCount(2);
+    QCOMPARE(VirtualDesktopManager::self()->count(), 2u);
+
+    // Get reference to the idle interface.
+    auto idle = waylandServer()->display()->findChild<IdleInterface *>();
+    QVERIFY(idle);
+    QVERIFY(!idle->isInhibited());
+    QSignalSpy inhibitedSpy(idle, &IdleInterface::inhibitedChanged);
+    QVERIFY(inhibitedSpy.isValid());
+
+    // Create the test client.
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QVERIFY(!surface.isNull());
+    QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data()));
+    QVERIFY(!shellSurface.isNull());
+
+    // Create the inhibitor object.
+    QScopedPointer<IdleInhibitor> inhibitor(Test::waylandIdleInhibitManager()->createInhibitor(surface.data()));
+    QVERIFY(inhibitor->isValid());
+
+    // Render the client.
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+
+    // The test client should be only on the first virtual desktop.
+    QCOMPARE(c->desktops().count(), 1);
+    QCOMPARE(c->desktops().first(), VirtualDesktopManager::self()->desktops().first());
+
+    // This should inhibit our server object.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 1);
+
+    // Switch to the second virtual desktop.
+    VirtualDesktopManager::self()->setCurrent(2);
+
+    // The surface is no longer visible, so the compositor don't have to honor the
+    // idle inhibitor object.
+    QVERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 2);
+
+    // Switch back to the first virtual desktop.
+    VirtualDesktopManager::self()->setCurrent(1);
+
+    // The test client became visible again, so the compositor has to honor the idle
+    // inhibitor object back again.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 3);
+
+    // Destroy the test client.
+    shellSurface.reset();
+    QVERIFY(Test::waitForWindowDestroyed(c));
+    QTRY_VERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 4);
+}
+
+void TestIdleInhibition::testDontInhibitWhenMinimized()
+{
+    // This test verifies that the idle inhibitor object is not honored when the
+    // associated surface is minimized.
+
+     // Get reference to the idle interface.
+    auto idle = waylandServer()->display()->findChild<IdleInterface *>();
+    QVERIFY(idle);
+    QVERIFY(!idle->isInhibited());
+    QSignalSpy inhibitedSpy(idle, &IdleInterface::inhibitedChanged);
+    QVERIFY(inhibitedSpy.isValid());
+
+    // Create the test client.
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QVERIFY(!surface.isNull());
+    QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data()));
+    QVERIFY(!shellSurface.isNull());
+
+    // Create the inhibitor object.
+    QScopedPointer<IdleInhibitor> inhibitor(Test::waylandIdleInhibitManager()->createInhibitor(surface.data()));
+    QVERIFY(inhibitor->isValid());
+
+    // Render the client.
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+
+    // This should inhibit our server object.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 1);
+
+    // Minimize the client, the idle inhibitor object should not be honored.
+    c->minimize();
+    QVERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 2);
+
+    // Unminimize the client, the idle inhibitor object should be honored back again.
+    c->unminimize();
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 3);
+
+    // Destroy the test client.
+    shellSurface.reset();
+    QVERIFY(Test::waitForWindowDestroyed(c));
+    QTRY_VERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 4);
+}
+
+void TestIdleInhibition::testDontInhibitWhenUnmapped()
+{
+    // This test verifies that the idle inhibitor object is not honored by KWin
+    // when the associated client is unmapped.
+
+     // Get reference to the idle interface.
+    auto idle = waylandServer()->display()->findChild<IdleInterface *>();
+    QVERIFY(idle);
+    QVERIFY(!idle->isInhibited());
+    QSignalSpy inhibitedSpy(idle, &IdleInterface::inhibitedChanged);
+    QVERIFY(inhibitedSpy.isValid());
+
+    // Create the test client.
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QVERIFY(!surface.isNull());
+    QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data()));
+    QVERIFY(!shellSurface.isNull());
+
+    // Create the inhibitor object.
+    QScopedPointer<IdleInhibitor> inhibitor(Test::waylandIdleInhibitManager()->createInhibitor(surface.data()));
+    QVERIFY(inhibitor->isValid());
+
+    // Render the client.
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+
+    // This should inhibit our server object.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 1);
+
+    // Unmap the client.
+    QSignalSpy hiddenSpy(c, &ShellClient::windowHidden);
+    QVERIFY(hiddenSpy.isValid());
+    surface->attachBuffer(Buffer::Ptr());
+    surface->commit(Surface::CommitFlag::None);
+    QVERIFY(hiddenSpy.wait());
+
+    // The surface is no longer visible, so the compositor don't have to honor the
+    // idle inhibitor object.
+    QVERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 2);
+
+    // Map the client.
+    QSignalSpy windowShownSpy(c, &ShellClient::windowShown);
+    QVERIFY(windowShownSpy.isValid());
+    Test::render(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(windowShownSpy.wait());
+
+    // The test client became visible again, so the compositor has to honor the idle
+    // inhibitor object back again.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 3);
+
+    // Destroy the test client.
+    shellSurface.reset();
+    QVERIFY(Test::waitForWindowDestroyed(c));
+    QTRY_VERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 4);
+}
+
+void TestIdleInhibition::testDontInhibitWhenLeftCurrentDesktop()
+{
+    // This test verifies that the idle inhibitor object is not honored by KWin
+    // when the associated surface leaves the current virtual desktop.
+
+    VirtualDesktopManager::self()->setCount(2);
+    QCOMPARE(VirtualDesktopManager::self()->count(), 2u);
+
+    // Get reference to the idle interface.
+    auto idle = waylandServer()->display()->findChild<IdleInterface *>();
+    QVERIFY(idle);
+    QVERIFY(!idle->isInhibited());
+    QSignalSpy inhibitedSpy(idle, &IdleInterface::inhibitedChanged);
+    QVERIFY(inhibitedSpy.isValid());
+
+    // Create the test client.
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QVERIFY(!surface.isNull());
+    QScopedPointer<XdgShellSurface> shellSurface(Test::createXdgShellStableSurface(surface.data()));
+    QVERIFY(!shellSurface.isNull());
+
+    // Create the inhibitor object.
+    QScopedPointer<IdleInhibitor> inhibitor(Test::waylandIdleInhibitManager()->createInhibitor(surface.data()));
+    QVERIFY(inhibitor->isValid());
+
+    // Render the client.
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+
+    // The test client should be only on the first virtual desktop.
+    QCOMPARE(c->desktops().count(), 1);
+    QCOMPARE(c->desktops().first(), VirtualDesktopManager::self()->desktops().first());
+
+    // This should inhibit our server object.
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 1);
+
+    // Let the client enter the second virtual desktop.
+    c->enterDesktop(VirtualDesktopManager::self()->desktops().at(1));
+    QCOMPARE(inhibitedSpy.count(), 1);
+
+    // If the client leaves the first virtual desktop, then the associated idle
+    // inhibitor object should not be honored.
+    c->leaveDesktop(VirtualDesktopManager::self()->desktops().at(0));
+    QVERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 2);
+
+    // If the client enters the first desktop, then the associated idle inhibitor
+    // object should be honored back again.
+    c->enterDesktop(VirtualDesktopManager::self()->desktops().at(0));
+    QVERIFY(idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 3);
+
+    // Destroy the test client.
+    shellSurface.reset();
+    QVERIFY(Test::waitForWindowDestroyed(c));
+    QTRY_VERIFY(!idle->isInhibited());
+    QCOMPARE(inhibitedSpy.count(), 4);
+}
+
 WAYLANDTEST_MAIN(TestIdleInhibition)
 #include "idle_inhibition_test.moc"

idle_inhibition.cpp

@@ -3,6 +3,7 @@
  This file is part of the KDE project.
 
 Copyright (C) 2017 Martin Flöser <mgraesslin@kde.org>
+Copyright (C) 2018 Vlad Zagorodniy <vladzzag@gmail.com>
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -20,6 +21,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "idle_inhibition.h"
 #include "deleted.h"
 #include "shell_client.h"
+#include "workspace.h"
 
 #include <KWayland/Server/idle_interface.h>
 #include <KWayland/Server/surface_interface.h>
@@ -35,21 +37,24 @@ IdleInhibition::IdleInhibition(IdleInterface *idle)
     : QObject(idle)
     , m_idle(idle)
 {
+    // Workspace is created after the wayland server is initialized.
+    connect(kwinApp(), &Application::workspaceCreated, this, &IdleInhibition::slotWorkspaceCreated);
 }
 
 IdleInhibition::~IdleInhibition() = default;
 
 void IdleInhibition::registerShellClient(ShellClient *client)
 {
-    auto inhibitsIdleChanged = [this, client] {
-        // TODO: only inhibit if the ShellClient is visible
-        if (client->surface()->inhibitsIdle()) {
-            inhibit(client);
-        } else {
-            uninhibit(client);
-        }
+    auto updateInhibit = [this, client] {
+        update(client);
     };
-    m_connections[client] = connect(client->surface(), &SurfaceInterface::inhibitsIdleChanged, this, inhibitsIdleChanged);
+
+    m_connections[client] = connect(client->surface(), &SurfaceInterface::inhibitsIdleChanged, this, updateInhibit);
+    connect(client, &ShellClient::desktopChanged, this, updateInhibit);
+    connect(client, &ShellClient::clientMinimized, this, updateInhibit);
+    connect(client, &ShellClient::clientUnminimized, this, updateInhibit);
+    connect(client, &ShellClient::windowHidden, this, updateInhibit);
+    connect(client, &ShellClient::windowShown, this, updateInhibit);
     connect(client, &ShellClient::windowClosed, this,
         [this, client] {
             uninhibit(client);
@@ -61,10 +66,10 @@ void IdleInhibition::registerShellClient(ShellClient *client)
         }
     );
 
-    inhibitsIdleChanged();
+    updateInhibit();
 }
 
-void IdleInhibition::inhibit(ShellClient *client)
+void IdleInhibition::inhibit(AbstractClient *client)
 {
     if (isInhibited(client)) {
         // already inhibited
@@ -75,7 +80,7 @@ void IdleInhibition::inhibit(ShellClient *client)
     // TODO: notify powerdevil?
 }
 
-void IdleInhibition::uninhibit(ShellClient *client)
+void IdleInhibition::uninhibit(AbstractClient *client)
 {
     auto it = std::find_if(m_idleInhibitors.begin(), m_idleInhibitors.end(), [client] (auto c) { return c == client; });
     if (it == m_idleInhibitors.end()) {
@@ -86,4 +91,26 @@ void IdleInhibition::uninhibit(ShellClient *client)
     m_idle->uninhibit();
 }
 
+void IdleInhibition::update(AbstractClient *client)
+{
+    // TODO: Don't honor the idle inhibitor object if the shell client is not
+    // on the current activity (currently, activities are not supported).
+    const bool visible = client->isShown(true) && client->isOnCurrentDesktop();
+    if (visible && client->surface()->inhibitsIdle()) {
+        inhibit(client);
+    } else {
+        uninhibit(client);
+    }
+}
+
+void IdleInhibition::slotWorkspaceCreated()
+{
+    connect(workspace(), &Workspace::currentDesktopChanged, this, &IdleInhibition::slotDesktopChanged);
+}
+
+void IdleInhibition::slotDesktopChanged()
+{
+    workspace()->forEachAbstractClient([this] (AbstractClient *c) { update(c); });
+}
+
 }

idle_inhibition.h

@@ -3,6 +3,7 @@
  This file is part of the KDE project.
 
 Copyright (C) 2017 Martin Flöser <mgraesslin@kde.org>
+Copyright (C) 2018 Vlad Zagorodniy <vladzzag@gmail.com>
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
@@ -37,6 +38,7 @@ using KWayland::Server::IdleInterface;
 
 namespace KWin
 {
+class AbstractClient;
 class ShellClient;
 
 class IdleInhibition : public QObject
@@ -51,16 +53,21 @@ public:
     bool isInhibited() const {
         return !m_idleInhibitors.isEmpty();
     }
-    bool isInhibited(ShellClient *client) const {
+    bool isInhibited(AbstractClient *client) const {
         return std::any_of(m_idleInhibitors.begin(), m_idleInhibitors.end(), [client] (auto c) { return c == client; });
     }
 
+private Q_SLOTS:
+    void slotWorkspaceCreated();
+    void slotDesktopChanged();
+
 private:
-    void inhibit(ShellClient *client);
-    void uninhibit(ShellClient *client);
+    void inhibit(AbstractClient *client);
+    void uninhibit(AbstractClient *client);
+    void update(AbstractClient *client);
 
     IdleInterface *m_idle;
-    QVector<ShellClient*> m_idleInhibitors;
-    QMap<ShellClient*, QMetaObject::Connection> m_connections;
+    QVector<AbstractClient *> m_idleInhibitors;
+    QMap<AbstractClient *, QMetaObject::Connection> m_connections;
 };
 }