From 362a0d3efb9fe58351f5d8cf0896802ddb38fdfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivan=20=C4=8Cuki=C4=87?= <ivan.cukic@kde.org>
Date: Sun, 15 Feb 2015 17:00:27 +0100
Subject: [PATCH] Windows retain their assigned activities on kwin restart

The issue is essentially this - KActivities are now an asynchonous
library, while KWin tries to use them in the old manner.

When kwin restarts, it tries to validate the activity list of a window
against an invalid list of activities it thinks it gets from KAMD
because it does not check for the service status.

This patch disables the validation in the case of kwin restart/crash.
When starting kwin will see KActivities::Consumer::serviceStatus return
Unknown (before the class actually receives a response from the service
and starts syncing the data).

After kwin has started, the response will arrive and the service status
will change either to Running or NotRunning. The patch changes nothing
for this case.

BUG: 335967
REVIEW: 122577
---
 activities.cpp |  5 +++++
 activities.h   |  2 ++
 client.cpp     | 44 +++++++++++++++++++++++++++-----------------
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/activities.cpp b/activities.cpp
index 01a6329892..fef04d1db2 100644
--- a/activities.cpp
+++ b/activities.cpp
@@ -50,6 +50,11 @@ Activities::~Activities()
     s_self = NULL;
 }
 
+KActivities::Consumer::ServiceStatus Activities::serviceStatus() const
+{
+    return m_controller->serviceStatus();
+}
+
 void Activities::setCurrent(const QString &activity)
 {
     m_controller->setCurrentActivity(activity);
diff --git a/activities.h b/activities.h
index 0ec02b6348..6cfb213611 100644
--- a/activities.h
+++ b/activities.h
@@ -59,6 +59,8 @@ public:
 
     static QString nullUuid();
 
+    KActivities::Controller::ServiceStatus serviceStatus() const;
+
 Q_SIGNALS:
     /**
      * This signal is emitted when the global
diff --git a/client.cpp b/client.cpp
index 3e834f28c7..5b7c0b75ad 100644
--- a/client.cpp
+++ b/client.cpp
@@ -1415,9 +1415,11 @@ void Client::setOnActivities(QStringList newActivitiesList)
     newActivitiesList = joinedActivitiesList.split(QStringLiteral(","), QString::SkipEmptyParts);
 
     QStringList allActivities = Activities::self()->all();
-    if ( newActivitiesList.isEmpty() ||
-        (newActivitiesList.count() > 1 && newActivitiesList.count() == allActivities.count()) ||
-        (newActivitiesList.count() == 1 && newActivitiesList.at(0) == Activities::nullUuid())) {
+    if (// If we got the request to be on all activities explicitly
+        newActivitiesList.isEmpty() || joinedActivitiesList == Activities::nullUuid() ||
+        // If we got a list of activities that covers all activities
+        (newActivitiesList.count() > 1 && newActivitiesList.count() == allActivities.count())) {
+
         activityList.clear();
         const QByteArray nullUuid = Activities::nullUuid().toUtf8();
         m_client.changeProperty(atoms->activities, XCB_ATOM_STRING, 8, nullUuid.length(), nullUuid.constData());
@@ -2160,9 +2162,10 @@ void Client::readActivities(Xcb::StringProperty &property)
 {
 #ifdef KWIN_BUILD_ACTIVITIES
     QStringList newActivitiesList;
-    QByteArray prop = property;
+    QString prop = QString::fromUtf8(property);
     activitiesDefined = !prop.isEmpty();
-    if (QString::fromUtf8(prop) == Activities::nullUuid()) {
+
+    if (prop == Activities::nullUuid()) {
         //copied from setOnAllActivities to avoid a redundant XChangeProperty.
         if (!activityList.isEmpty()) {
             activityList.clear();
@@ -2179,22 +2182,29 @@ void Client::readActivities(Xcb::StringProperty &property)
         return;
     }
 
-    newActivitiesList = QString::fromUtf8(prop).split(QStringLiteral(","));
+    newActivitiesList = prop.split(QStringLiteral(","));
 
     if (newActivitiesList == activityList)
         return; //expected change, it's ok.
 
-    //otherwise, somebody else changed it. we need to validate before reacting
-    QStringList allActivities = Activities::self()->all();
-    if (allActivities.isEmpty()) {
-        qCDebug(KWIN_CORE) << "no activities!?!?";
-        //don't touch anything, there's probably something bad going on and we don't wanna make it worse
-        return;
-    }
-    for (int i = 0; i < newActivitiesList.size(); ++i) {
-        if (! allActivities.contains(newActivitiesList.at(i))) {
-            qCDebug(KWIN_CORE) << "invalid:" << newActivitiesList.at(i);
-            newActivitiesList.removeAt(i--);
+    //otherwise, somebody else changed it. we need to validate before reacting.
+    //if the activities are not synced, and there are existing clients with
+    //activities specified, somebody has restarted kwin. we can not validate
+    //activities in this case. we need to trust the old values.
+    if (Activities::self()->serviceStatus() != KActivities::Consumer::Unknown) {
+        QStringList allActivities = Activities::self()->all();
+        if (allActivities.isEmpty()) {
+            qCDebug(KWIN_CORE) << "no activities!?!?";
+            //don't touch anything, there's probably something bad going on and we don't wanna make it worse
+            return;
+        }
+
+
+        for (int i = 0; i < newActivitiesList.size(); ++i) {
+            if (! allActivities.contains(newActivitiesList.at(i))) {
+                qCDebug(KWIN_CORE) << "invalid:" << newActivitiesList.at(i);
+                newActivitiesList.removeAt(i--);
+            }
         }
     }
     setOnActivities(newActivitiesList);