Add support for desktopFileName provided by NETWinInfo

Summary:
KWindowSystem provides a KDE specific property for the desktop file
name. This allows KWin to take the icon from the desktop file. The
advantage from the desktop file is that KWin normally gets higher
resolution icons than provided through the xproperty based icons used
previously. If the desktop file does not provide an icon name, KWin
falls back to the previous implementation.

As on Wayland the icon is taken from the desktop file name already the
code for X11 and Wayland is merged in AbstractClient. Also to the
PlasmaWindowInterface the appId is taken from the new desktop file
instead of the resourceName. Due to that for Xwayland windows where KWin
knows the desktop file name it can be passed to PlasmaWindowInterface.
This allows e.g. the task manager to better map the windows to
applications and provide better icons. Also it means that icons do not
need to be passed as bitmap data to the clients.

Test Plan:
Verified that icon is taking from desktop file if provided and
from X property if not provided and that Wayland windows still have icon.

Reviewers: #kwin, #plasma_on_wayland, hein

Subscribers: plasma-devel, kwin

Tags: #plasma_on_wayland, #kwin

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

abstract_client.cpp

@@ -39,6 +39,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 #include <KDecoration2/Decoration>
 
+#include <KDesktopFile>
+
 #include <QMouseEvent>
 #include <QStyleHints>
 
@@ -674,7 +676,10 @@ void AbstractClient::setupWindowManagementInterface()
     w->setMinimizeable(isMinimizable());
     w->setFullscreenable(isFullScreenable());
     w->setIcon(icon());
-    w->setAppId(QString::fromUtf8(resourceName()));
+    auto updateAppId = [this, w] {
+        w->setAppId(QString::fromUtf8(m_desktopFileName.isEmpty() ? resourceName() : m_desktopFileName));
+    };
+    updateAppId();
     w->setSkipTaskbar(skipTaskbar());
     w->setShadeable(isShadeable());
     w->setShaded(isShade());
@@ -716,11 +721,8 @@ void AbstractClient::setupWindowManagementInterface()
             w->setIcon(icon());
         }
     );
-    connect(this, &AbstractClient::windowClassChanged, w,
-        [w, this] {
-            w->setAppId(QString::fromUtf8(resourceName()));
-        }
-    );
+    connect(this, &AbstractClient::windowClassChanged, w, updateAppId);
+    connect(this, &AbstractClient::desktopFileNameChanged, w, updateAppId);
     connect(this, &AbstractClient::shadeChanged, w, [w, this] { w->setShaded(isShade()); });
     connect(this, &AbstractClient::transientChanged, w,
         [w, this] {
@@ -1639,4 +1641,26 @@ bool AbstractClient::dockWantsInput() const
     return false;
 }
 
+void AbstractClient::setDesktopFileName(const QByteArray &name)
+{
+    if (name == m_desktopFileName) {
+        return;
+    }
+    m_desktopFileName = name;
+    emit desktopFileNameChanged();
+}
+
+QString AbstractClient::iconFromDesktopFile() const
+{
+    if (m_desktopFileName.isEmpty()) {
+        return QString();
+    }
+    QString desktopFile = QString::fromUtf8(m_desktopFileName);
+    if (!desktopFile.endsWith(QLatin1String(".desktop"))) {
+        desktopFile.append(QLatin1String(".desktop"));
+    }
+    KDesktopFile df(desktopFile);
+    return df.readIcon();
+}
+
 }

abstract_client.h

@@ -244,6 +244,17 @@ class KWIN_EXPORT AbstractClient : public Toplevel
      * Because of that there is no notify signal.
      **/
     Q_PROPERTY(bool resizeable READ isResizable)
+
+    /**
+     * The desktop file name of the application this AbstractClient belongs to.
+     *
+     * This is either the base name without full path and without file extension of the
+     * desktop file for the window's application (e.g. "org.kde.foo").
+     *
+     * The application's desktop file name can also be the full path to the desktop file
+     * (e.g. "/opt/kde/share/org.kde.foo.desktop") in case it's not in a standard location.
+     **/
+    Q_PROPERTY(QByteArray desktopFileName READ desktopFileName NOTIFY desktopFileNameChanged)
 public:
     virtual ~AbstractClient();
 
@@ -599,6 +610,10 @@ public:
      **/
     virtual void showOnScreenEdge() = 0;
 
+    QByteArray desktopFileName() const {
+        return m_desktopFileName;
+    }
+
     // TODO: remove boolean trap
     static bool belongToSameApplication(const AbstractClient* c1, const AbstractClient* c2, bool active_hack = false);
 
@@ -640,6 +655,7 @@ Q_SIGNALS:
     void minimizeableChanged(bool);
     void shadeableChanged(bool);
     void maximizeableChanged(bool);
+    void desktopFileNameChanged();
 
 protected:
     AbstractClient();
@@ -918,6 +934,9 @@ protected:
     void startDecorationDoubleClickTimer();
     void invalidateDecorationDoubleClickTimer();
 
+    void setDesktopFileName(const QByteArray &name);
+    QString iconFromDesktopFile() const;
+
 private:
     void handlePaletteChange();
     QSharedPointer<TabBox::TabBoxClientImpl> m_tabBoxClient;
@@ -985,6 +1004,8 @@ private:
         QElapsedTimer doubleClickTimer;
     } m_decoration;
 
+    QByteArray m_desktopFileName;
+
 
     static bool s_haveResizeEffect;
 };

autotests/integration/data/example.desktop

@@ -0,0 +1,3 @@
+[Desktop Entry]
+Name=An example application
+Icon=kwin

autotests/integration/shell_client_test.cpp

@@ -62,6 +62,7 @@ private Q_SLOTS:
     void testWindowOpensLargerThanScreen();
     void testHidden_data();
     void testHidden();
+    void testDesktopFileName();
 };
 
 void TestShellClient::initTestCase()
@@ -583,5 +584,37 @@ void TestShellClient::testHidden()
     //QCOMPARE(workspace()->activeClient(), c);
 }
 
+void TestShellClient::testDesktopFileName()
+{
+    // this test verifies that desktop file name is passed correctly to the window
+    QScopedPointer<Surface> surface(Test::createSurface());
+    // only xdg-shell as ShellSurface misses the setter
+    QScopedPointer<XdgShellSurface> shellSurface(qobject_cast<XdgShellSurface*>(Test::createShellSurface(Test::ShellSurfaceType::XdgShellV5, surface.data())));
+    shellSurface->setAppId(QByteArrayLiteral("org.kde.foo"));
+    auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(c);
+    QCOMPARE(c->desktopFileName(), QByteArrayLiteral("org.kde.foo"));
+    // the desktop file does not exist, so icon should be generic Wayland
+    QCOMPARE(c->icon().name(), QStringLiteral("wayland"));
+
+    QSignalSpy desktopFileNameChangedSpy(c, &AbstractClient::desktopFileNameChanged);
+    QVERIFY(desktopFileNameChangedSpy.isValid());
+    QSignalSpy iconChangedSpy(c, &ShellClient::iconChanged);
+    QVERIFY(iconChangedSpy.isValid());
+    shellSurface->setAppId(QByteArrayLiteral("org.kde.bar"));
+    QVERIFY(desktopFileNameChangedSpy.wait());
+    QCOMPARE(c->desktopFileName(), QByteArrayLiteral("org.kde.bar"));
+    // icon should still be wayland
+    QCOMPARE(c->icon().name(), QStringLiteral("wayland"));
+    QVERIFY(iconChangedSpy.isEmpty());
+
+    const QString dfPath = QFINDTESTDATA("data/example.desktop");
+    shellSurface->setAppId(dfPath.toUtf8());
+    QVERIFY(desktopFileNameChangedSpy.wait());
+    QCOMPARE(iconChangedSpy.count(), 1);
+    QCOMPARE(QString::fromUtf8(c->desktopFileName()), dfPath);
+    QCOMPARE(c->icon().name(), QStringLiteral("kwin"));
+}
+
 WAYLANDTEST_MAIN(TestShellClient)
 #include "shell_client_test.moc"

client.cpp

@@ -1683,6 +1683,11 @@ void Client::getMotifHints()
 void Client::getIcons()
 {
     // First read icons from the window itself
+    const QString themedIconName = iconFromDesktopFile();
+    if (!themedIconName.isEmpty()) {
+        setIcon(QIcon::fromTheme(themedIconName));
+        return;
+    }
     QIcon icon;
     auto readIcon = [this, &icon](int size, bool scale = true) {
         const QPixmap pix = KWindowSystem::icon(window(), size, size, scale, KWindowSystem::NETWM | KWindowSystem::WMHints, info);

events.cpp

@@ -651,6 +651,9 @@ bool Client::windowEvent(xcb_generic_event_t *e)
         if (dirtyProperties2 & NET::WM2OpaqueRegion) {
             getWmOpaqueRegion();
         }
+        if (dirtyProperties2 & NET::WM2DesktopFileName) {
+            setDesktopFileName(QByteArray(info->desktopFileName()));
+        }
     }
 
     const uint8_t eventType = e->response_type & ~0x80;

manage.cpp

@@ -93,7 +93,8 @@ bool Client::manage(xcb_window_t w, bool isMapped)
         NET::WM2Protocols |
         NET::WM2InitialMappingState |
         NET::WM2IconPixmap |
-        NET::WM2OpaqueRegion;
+        NET::WM2OpaqueRegion |
+        NET::WM2DesktopFileName;
 
     auto wmClientLeaderCookie = fetchWmClientLeader();
     auto skipCloseAnimationCookie = fetchSkipCloseAnimation();
@@ -142,7 +143,10 @@ bool Client::manage(xcb_window_t w, bool isMapped)
 
     setModal((info->state() & NET::Modal) != 0);   // Needs to be valid before handling groups
     readTransientProperty(transientCookie);
+    setDesktopFileName(QByteArray(info->desktopFileName()));
     getIcons();
+    connect(this, &Client::desktopFileNameChanged, this, &Client::getIcons);
+
     m_geometryHints.read();
     getMotifHints();
     getWmOpaqueRegion();

shell_client.cpp

@@ -108,12 +108,13 @@ void ShellClient::initSurface(T *shellSurface)
             performMouseCommand(Options::MouseMove, Cursor::pos());
         }
     );
-    connect(shellSurface, &T::windowClassChanged, this, &ShellClient::updateIcon);
 
     setResourceClass(shellSurface->windowClass());
+    setDesktopFileName(shellSurface->windowClass());
     connect(shellSurface, &T::windowClassChanged, this,
         [this] (const QByteArray &windowClass) {
             setResourceClass(windowClass);
+            setDesktopFileName(windowClass);
         }
     );
     connect(shellSurface, &T::resizeRequested, this,
@@ -168,6 +169,7 @@ void ShellClient::initSurface(T *shellSurface)
 
 void ShellClient::init()
 {
+    connect(this, &ShellClient::desktopFileNameChanged, this, &ShellClient::updateIcon);
     findInternalWindow();
     createWindowId();
     setupCompositing();
@@ -233,7 +235,6 @@ void ShellClient::init()
     } else if (m_xdgShellPopup) {
         connect(m_xdgShellPopup, &XdgShellPopupInterface::destroyed, this, &ShellClient::destroyClient);
     }
-    updateIcon();
 
     // setup shadow integration
     getShadow();
@@ -1184,19 +1185,12 @@ bool ShellClient::hasStrut() const
 void ShellClient::updateIcon()
 {
     const QString waylandIconName = QStringLiteral("wayland");
-    QString desktopFile;
-    if (m_shellSurface) {
-        desktopFile = QString::fromUtf8(m_shellSurface->windowClass());
+    const QString dfIconName = iconFromDesktopFile();
+    const QString iconName = dfIconName.isEmpty() ? waylandIconName : dfIconName;
+    if (iconName == icon().name()) {
+        return;
     }
-    if (desktopFile.isEmpty()) {
-        setIcon(QIcon::fromTheme(waylandIconName));
-    }
-    if (!desktopFile.endsWith(QLatin1String(".desktop"))) {
-        desktopFile.append(QLatin1String(".desktop"));
-    }
-    KDesktopFile df(desktopFile);
-    const QString iconName = df.readIcon();
-    setIcon(QIcon::fromTheme(iconName.isEmpty() ? waylandIconName : iconName));
+    setIcon(QIcon::fromTheme(iconName));
 }
 
 bool ShellClient::isTransient() const