From 09ab6cdfb9766ccf5a2631ca07d262af3ed6d1d5 Mon Sep 17 00:00:00 2001 From: Eike Hein Date: Wed, 4 Jan 2017 18:05:44 +0900 Subject: [PATCH] Fix "Pinned Chrome disappears when all Chrome windows are closed" Summary: It turns out that Chrome under certain conditions will change its window metadata as it quits, causing a race we sometimes lose, failing to reveal the associated launcher because we can no longer match it to the window at window closing time. Instead we are now forced to re-check all launchers after the window is gone. As a speed optimi- zation we only consider top-level windows (and startups) as being in a group implies matching siblings. In addition this refactoring eliminates a use of Qt::QueuedConnection that allowed for an unpredictable event loop spin inbetween things. BUG:365617 Reviewers: davidedmundson Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D3950 --- libtaskmanager/tasksmodel.cpp | 42 ++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/libtaskmanager/tasksmodel.cpp b/libtaskmanager/tasksmodel.cpp index 426018628..88688c337 100644 --- a/libtaskmanager/tasksmodel.cpp +++ b/libtaskmanager/tasksmodel.cpp @@ -70,6 +70,7 @@ public: bool launchInPlace = false; bool launchersEverSet = false; bool launcherSortingDirty = false; + bool launcherCheckNeeded = false; QList sortedPreFilterRows; QVector sortRowInsertQueue; QHash activityTaskCounts; @@ -366,21 +367,40 @@ void TasksModel::Private::initModels() } // When a window or startup task is removed, we have to trigger a re-filter of - // matching launchers to (possibly) pop them back in. - if (!launcherTasksModel - || !(sourceIndex.data(AbstractTasksModel::IsWindow).toBool() + // our launchers to (possibly) pop them back in. + // NOTE: An older revision of this code compared the window and startup tasks + // to the launchers to figure out which launchers should be re-filtered. This + // was fine until we discovered that certain applications (e.g. Google Chrome) + // change their window metadata specifically during tear-down, sometimes + // breaking TaskTools::appsMatch (it's a race) and causing the associated + // launcher to remain hidden. Therefore we now consider any top-level window or + // startup task removal a trigger to re-filter all launchers. We don't do this + // in response to the window metadata changes (even though it would be strictly + // more correct, as then-ending identity match-up was what caused the launcher + // to be hidden) because we don't want the launcher and window/startup task to + // briefly co-exist in the model. + if (!launcherCheckNeeded + && launcherTasksModel + && (sourceIndex.data(AbstractTasksModel::IsWindow).toBool() || sourceIndex.data(AbstractTasksModel::IsStartup).toBool())) { - continue; + launcherCheckNeeded = true; } + } + } + ); + + QObject::connect(filterProxyModel, &QAbstractItemModel::rowsRemoved, q, + [this](const QModelIndex &parent, int first, int last) { + Q_UNUSED(parent) + Q_UNUSED(first) + Q_UNUSED(last) - for (int j = 0; j < launcherTasksModel->rowCount(); ++j) { - const QModelIndex &launcherIndex = launcherTasksModel->index(j, 0); + if (launcherCheckNeeded) { + QMetaObject::invokeMethod(launcherTasksModel, "dataChanged", + Q_ARG(QModelIndex, launcherTasksModel->index(0, 0)), + Q_ARG(QModelIndex, launcherTasksModel->index(launcherTasksModel->rowCount() - 1, 0))); - if (appsMatch(sourceIndex, launcherIndex)) { - QMetaObject::invokeMethod(launcherTasksModel, "dataChanged", Qt::QueuedConnection, - Q_ARG(QModelIndex, launcherIndex), Q_ARG(QModelIndex, launcherIndex)); - } - } + launcherCheckNeeded = false; } } );