Fix the infamous 'dialogs show up on the Task Manager' bug once more.

- Initially set state (and type, and flags) in response to PlatformSurfaceCreated.
  We know reliably this will run before the window is mapped.

- Drop the comment about removing setState() form showEvent handler, as
  we need it to avoid state loss in this scenario:
  <mgraesslin> the window gets mapped first time: everything is fine
  <mgraesslin> window gets unmapped: kwin removes the state as per spec
  <mgraesslin> qt gets change event and removes the states it doesn't care about
  <mgraesslin> qt maps window again and sets states
  <mgraesslin> we lost the state
  <mgraesslin> which means we need to set the state again from our side before(!) Qt sets it
  <mgraesslin> and before Qt maps the window

autotests/CMakeLists.txt

@@ -40,6 +40,7 @@ ENDMACRO(PLASMA_UNIT_TESTS)
 
 PLASMA_UNIT_TESTS(
     dialogqmltest
+    dialogstatetest
     fallbackpackagetest
     packagestructuretest
     packageurlinterceptortest

autotests/dialogstatetest.cpp

@@ -0,0 +1,98 @@
+/********************************************************************************
+*   Copyright 2016 Eike Hein <hein@kde.org>                                  *
+*                                                                               *
+*   This library is free software; you can redistribute it and/or               *
+*   modify it under the terms of the GNU Library General Public                 *
+*   License as published by the Free Software Foundation; either                *
+*   version 2 of the License, or (at your option) any later version.            *
+*                                                                               *
+*   This library is distributed in the hope that it will be useful,             *
+*   but WITHOUT ANY WARRANTY; without even the implied warranty of              *
+*   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU            *
+*   Library General Public License for more details.                            *
+*                                                                               *
+*   You should have received a copy of the GNU Library General Public License   *
+*   along with this library; see the file COPYING.LIB.  If not, write to        *
+*   the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,        *
+*   Boston, MA 02110-1301, USA.                                                 *
+*********************************************************************************/
+
+#include "dialogstatetest.h"
+
+#include <KWindowSystem>
+
+void DialogStateTest::initTestCase()
+{
+    m_dialog = new PlasmaQuick::Dialog;
+    m_dialog->setLocation(Plasma::Types::TopEdge);
+    m_dialog->setGeometry(0, 0, 50, 50);
+
+    qRegisterMetaType<WId>("WId");
+}
+
+void DialogStateTest::cleanupTestCase()
+{
+    delete m_dialog;
+}
+
+void DialogStateTest::windowState()
+{
+    for (int i = 0; i <= 100; ++i) {
+        m_dialog->show();
+
+        QSignalSpy windowAddedSpy(KWindowSystem::self(), &KWindowSystem::windowAdded);
+        QVERIFY(windowAddedSpy.isValid());
+        QVERIFY(windowAddedSpy.wait());
+
+        bool windowAdded = false;
+
+        while (windowAddedSpy.count()) {
+            const QVariantList &arguments = windowAddedSpy.takeFirst();
+
+            if (arguments.at(0).value<WId>() == m_dialog->winId()) {
+                windowAdded = true;
+                break;
+            }
+        }
+
+        QVERIFY(windowAdded);
+
+        QVERIFY(verifyState(m_dialog));
+
+        m_dialog->hide();
+
+        QSignalSpy windowRemovedSpy(KWindowSystem::self(), &KWindowSystem::windowRemoved);
+        QVERIFY(windowRemovedSpy.isValid());
+        QVERIFY(windowRemovedSpy.wait());
+
+        bool windowRemoved = false;
+
+        while (windowRemovedSpy.count()) {
+            const QVariantList &arguments = windowRemovedSpy.takeFirst();
+
+            if (arguments.at(0).value<WId>() == m_dialog->winId()) {
+                windowRemoved = true;
+                break;
+            }
+        }
+
+        QVERIFY(windowRemoved);
+    }
+}
+
+bool DialogStateTest::verifyState(PlasmaQuick::Dialog *dialog) const
+{
+    KWindowInfo info(dialog->winId(), NET::WMState);
+
+    if (!(info.state() & NET::SkipTaskbar)) {
+        return false;
+    }
+
+    if (!info.hasState(NET::SkipPager)) {
+        return false;
+    }
+
+    return true;
+}
+
+QTEST_MAIN(DialogStateTest)

autotests/dialogstatetest.h

@@ -0,0 +1,44 @@
+/******************************************************************************
+*   Copyright 2016 Eike Hein <hein@kde.org>                                *
+*                                                                             *
+*   This library is free software; you can redistribute it and/or             *
+*   modify it under the terms of the GNU Library General Public               *
+*   License as published by the Free Software Foundation; either              *
+*   version 2 of the License, or (at your option) any later version.          *
+*                                                                             *
+*   This library is distributed in the hope that it will be useful,           *
+*   but WITHOUT ANY WARRANTY; without even the implied warranty of            *
+*   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU          *
+*   Library General Public License for more details.                          *
+*                                                                             *
+*   You should have received a copy of the GNU Library General Public License *
+*   along with this library; see the file COPYING.LIB.  If not, write to      *
+*   the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,      *
+*   Boston, MA 02110-1301, USA.                                               *
+*******************************************************************************/
+#ifndef DIALOGTEST_H
+#define DIALOGTEST_H
+
+#include <QtTest/QtTest>
+
+#include "plasmaquick/dialog.h"
+
+class DialogStateTest : public QObject
+{
+    Q_OBJECT
+
+public Q_SLOTS:
+    void initTestCase();
+    void cleanupTestCase();
+
+private Q_SLOTS:
+    void windowState();
+
+private:
+    bool verifyState(PlasmaQuick::Dialog *dialog) const;
+
+    PlasmaQuick::Dialog *m_dialog;
+};
+
+#endif
+

src/plasmaquick/dialog.cpp

@@ -32,6 +32,10 @@
 #include <QMenu>
 #include <QPointer>
 
+#if (QT_VERSION > QT_VERSION_CHECK(5, 5, 0))
+#include <QPlatformSurfaceEvent>
+#endif
+
 #include <kwindowsystem.h>
 #include <KWindowSystem/KWindowInfo>
 
@@ -317,6 +321,7 @@ void DialogPrivate::updateVisibility(bool visible)
         } else {
             q->setFlags(Qt::FramelessWindowHint | q->flags());
         }
+
         if (type == Dialog::Dock || type == Dialog::Notification || type == Dialog::OnScreenDisplay) {
             KWindowSystem::setOnAllDesktops(q->winId(), true);
         } else {
@@ -1067,7 +1072,6 @@ void Dialog::showEvent(QShowEvent *event)
         DialogShadows::self()->addWindow(this, d->frameSvgItem->enabledBorders());
     }
 
-    // TODO: Remove this call from other places once we can depend on Qt >= 5.6.1
     KWindowSystem::setState(winId(), NET::SkipTaskbar | NET::SkipPager);
 
     QQuickWindow::showEvent(event);
@@ -1076,7 +1080,17 @@ void Dialog::showEvent(QShowEvent *event)
 bool Dialog::event(QEvent *event)
 {
     if (event->type() == QEvent::Expose) {
+        // FIXME TODO: We can remove this once we depend on Qt 5.6.1+.
+        // See: https://bugreports.qt.io/browse/QTBUG-26978
         KWindowSystem::setState(winId(), NET::SkipTaskbar | NET::SkipPager);
+#if (QT_VERSION > QT_VERSION_CHECK(5, 5, 0))
+    } else if (event->type() == QEvent::PlatformSurface) {
+        const QPlatformSurfaceEvent *pSEvent = static_cast<QPlatformSurfaceEvent *>(event);
+
+        if (pSEvent->surfaceEventType() == QPlatformSurfaceEvent::SurfaceCreated) {
+            KWindowSystem::setState(winId(), NET::SkipTaskbar | NET::SkipPager);
+        }
+#endif
     } else if (event->type() == QEvent::Show) {
         d->updateVisibility(true);
     } else if (event->type() == QEvent::Hide) {
@@ -1193,9 +1207,10 @@ void Dialog::componentComplete()
     d->componentComplete = true;
     QQuickWindow::setVisible(d->visible);
     if (d->visible) {
+        // FIXME TODO: We can remove this once we depend on Qt 5.6.1+.
+        // See: https://bugreports.qt.io/browse/QTBUG-26978
         KWindowSystem::setState(winId(), NET::SkipTaskbar | NET::SkipPager);
     }
-
     d->updateTheme();
 
     if (d->mainItem) {
@@ -1247,6 +1262,8 @@ void Dialog::setVisible(bool visible)
         }
         QQuickWindow::setVisible(visible);
         if (visible) {
+            // FIXME TODO: We can remove this once we depend on Qt 5.6.1+.
+            // See: https://bugreports.qt.io/browse/QTBUG-26978
             KWindowSystem::setState(winId(), NET::SkipTaskbar | NET::SkipPager);
         }
         //signal will be emitted and proxied from the QQuickWindow code